On Tue, 7 Feb 2023 15:40:34 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 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 one additional > commit since the last revision: > > Revert changes to JDK > > Signed-off-by: Justin King <jck...@google.com> I apologize, the fault lies entirely with me. Justin should have no blame in this -- he is not a committer and is not expected to fully know all rules for integration. That responsibility lies with the sponsor, in this case, me. I read @dholmes-ora's comment: > Updates look good - glad to see the flag changes go away! > I suggest factoring out the change to > test/jdk/jni/nullCaller/exeNullCallerTest.cpp as it is a JDK test and not > part of hotspot. Thanks. as an approval from Hotspot, given that the JDK test was removed, which a later commit did indeed remove. And, like Justin, I interpreted Thomas comments as questions that were now answered, rather than an ongoing discussion about the I did not realize there were still an ongoing discussion, and was too eager to sponsor this PR. @tstuefe @dholmes-ora Do you want me to revert this change? Or should we continue the discussion in a follow-up bug that can address the remaining problems? ------------- PR: https://git.openjdk.org/jdk/pull/12229
