luzer: fix memory leak in FDP

[ligurio]

Fixes #52
Fixes #65

[Buristan]

Sergey,
Thanks for the patch!
I suppose the next commit may be squashed into this one as well.

Please consider my comment below.

[Buristan]

I suppose we have several smoking guns here:

• Since Lua (and therefore LuaJIT) has the GC-based memory management model, it may accumulate memory between iterations of state mutations in the TestOneInput. Libfuzzer checks the possible leaks during fuzzing between state mutations. Hence, we should add -detect_leaks=0 (see options) to the fuzzer options to be sure that we don't fail valid runs due to these false positives. This option disables only intermediate checks. The shutdown check is still enabled (but it may be ignored if you specify the environment variable LSAN_OPTIONS). Adding of lua_gc(), may not be so helpful if the object is allocated once at the first call (loading library via require) and cleared on the exit.
• Even with the aforementioned option the lua_close() isn't called since the process is finished by the atexit handler, which is set in the FuzzerDriver(). So, we will observe leaks of the FDP object allocated via new.

[ligurio]

Also https://github.com/google/atheris/blob/master/native_extension_fuzzing.md#leak-detection

Leak detection
Python is known to leak certain data, such as at interpreter initialization time. You should disable leak detection, for example with -detect_leaks=0.

[ligurio]

Since Lua (and therefore LuaJIT) has the GC-based memory management model,

Honestly, I don't like the proposed solution (always disable -detect_leaks=0 for everyone). Sooner or later, this option will lead to an OOM, and we'll have to figure out why the memory leak occurred. I prefer the scenario where the test crashes at the first sign of symptoms.

I propose another solution - leave the patch as is and add a section to a documentation (done).

Even with the aforementioned option

atexit() handler helps for another case - LLVMFuzzerRunDriver() returns due to error

[Buristan]

Honestly, I don't like the proposed solution (always disable -detect_leaks=0 for everyone). Sooner or later, this option will lead to an OOM, and we'll have to figure out why the memory leak occurred. I prefer the scenario where the test crashes at the first sign of symptoms.

The generated input (a too large string, for example) may also lead to OOM in tests as well. This is not the part of our concern. OTOH, false-positive leaks, which are possible for user code as well, remain a problem. I'll repeat it again: -detect_leaks=0 doesn't fully disable leak detection but instead postpones it until the end of the fuzzing process, so all true leaks will be reported to the user.

Without this flag, we just see more false-positives, and have no protection against OOM in generated input as well. These reports are meaningless for the GC-based VMs.

[ligurio]

The second patch was squashed, the commit message was updated.