RFR: JDK-8298445: Add LeakSanitizer support in HotSpot [v3]

Magnus Ihse Bursie ihse at openjdk.org
Tue Feb 7 13:52:50 UTC 2023


On Thu, 2 Feb 2023 15:52:01 GMT, Justin King <jcking at 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 also fail with ASan, which is being looked at separately. 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:**
>> - By default ASan enables LSan, however we explicitly disable it unless `--enable-lsan` is given. 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.
>
> Justin King has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Fix grammatical error
>    
>    Signed-off-by: Justin King <jcking at google.com>
>  - Update based on review and remove restriction on compressed oops and class pointers
>    
>    Signed-off-by: Justin King <jcking at google.com>

Marked as reviewed by ihse (Reviewer).

make/data/asan/asan_default_options.c line 53:

> 51: ATTRIBUTE_DEFAULT_VISIBILITY ATTRIBUTE_USED const char* __asan_default_options() {
> 52:   return
> 53: #ifndef LEAK_SANITIZER

I'm trying to understand this part. The test for `LEAK_SANITIZER` was put in place in this file, before it was ever generated by the build system..? Was this in anticipation for this PR, or intended for the user to add an extra `-DLEAK_SANITIZER` to the compiler flags manually?

Also, this patch changes the values for asan, regardless of the state of lsan. Is this required to get the lsan patch working, or is this additional, separate fixes to asan being "smuggled" into this PR? (I don't mind a certain amount of "smuggling", since sometimes it is too tedious to open a separate JBS issue for minor, separate fixes one found out during development), but it is nice to be explicit about it.

And finally, now that the if statement also has an else, maybe reverse it to an `#ifdef` instead of `#ifndef`..?

Apart from this, the build changes look good.

-------------

PR: https://git.openjdk.org/jdk/pull/12229



More information about the build-dev mailing list