On Wed, 1 Feb 2023 15:26:27 GMT, Justin King <jck...@openjdk.org> wrote:
>> Adds initial LSan (LeakSanitizer) support to Hotspot. This setup has been
>> used to identify multiple leaks so far. It can run most of the test suite
>> except those that rely on testing compressed oops or compressed class
>> pointers. It is especially useful when combined with ASan, as LSan can use
>> poisoning information to determine what memory to scan or not to scan,
>> making leak detection more accurate and faster.
>>
>> **Suppressing:**
>> Currently the suppression list is only used to suppress JLI leaks that are
>> known, the rest are done in code. Suppressing needs to identify the source
>> of thet leak. Due to Hotspot's code organization, we would need to suppress
>> `os::malloc` and friends, which would suppress everything. Suppressing in
>> code has the added benefit of being explicit and surviving refactors if
>> methods change.
>>
>> **Caveats:**
>> - `UseCompressedOops` and `UseCompressedClassPointers` are forced to false
>> when LSan is enabled. This is necessary to ensure all pointers to memory
>> which could possible container pointers to malloc memory are property
>> aligned, which is an LSan requirement.
>> - By default ASan enables LSan, however we explicitly disable it unless
>> `--enable-lsan` is given. This is due to the other caveats. It is useful to
>> be able to use ASan without LSan. Using LSan by itself is less likely to be
>> useful and will probably not work, but its still possible currently.
>> - There are a series of tests that are upset due to the above flags being
>> forced false, as they rely on the arguments being supported. In the future
>> ideally these tests would be skipped nicely when LSan is enabled.
>
> Justin King has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Support CDS
>
> Signed-off-by: Justin King <jck...@google.com>
make/data/lsan/lsan_default_options.c line 49:
> 47: // extremely early during library loading, before main is called. We
> need to override the default
> 48: // options because LSan will perform leak checking at program exit.
> Unfortunately Hotspot does not
> 49: // shutdown cleaning at the moment and some leaks occur, we want to
> ignore these. Instead we
s/cleaning/cleanly/
src/hotspot/share/runtime/arguments.cpp line 3979:
> 3977: // LSAN relies on pointers to be natively aligned. Using compressed
> class pointers breaks this
> 3978: // expectation and results in nondeterministic leak reports.
> 3979: if (FLAG_SET_CMDLINE(UseCompressedOops, false) != JVMFlag::SUCCESS) {
This positioning doesn't look right. There are a number of inter-dependencies
on the compressed oops flags and I don't think you can just turn them off here.
Surely `Arguments::set_use_compressed_oops()` is where this needs to be done?
src/hotspot/share/runtime/init.cpp line 132:
> 130: VirtualSpaceSummary summary =
> Universe::heap()->create_heap_space_summary();
> 131: LSAN_REGISTER_ROOT_REGION(summary.start(), summary.reserved_size());
> 132: }
Why is this unconditional? I expected it to be in `#ifdef LEAK_SANITIZER`
src/hotspot/share/runtime/init.cpp line 198:
> 196: VirtualSpaceSummary summary =
> Universe::heap()->create_heap_space_summary();
> 197: LSAN_UNREGISTER_ROOT_REGION(summary.start(),
> summary.reserved_size());
> 198: }
Again Why is this unconditional? I expected it to be in `#ifdef LEAK_SANITIZER`
src/hotspot/share/runtime/java.cpp line 432:
> 430: static_cast<void>(result);
> 431: }
> 432:
Why is this unconditional? I expected it to be in `#ifdef LEAK_SANITIZER`
-------------
PR: https://git.openjdk.org/jdk/pull/12229
