On Tue, 28 Mar 2023 15:05:41 GMT, Justin King <[email protected]> wrote:
> This change fixes or skips some NMT tests when running under ASan, as well as
> fixing a leak.
>
> - `allocator_may_return_null=1` is added as the default is `0`, meaning ASan
> will never return `nullptr` and will instead crash. NMT tests check that
> large allocations return `nullptr` so those do not work under ASan currently.
> - We skip tests that intentionally access memory outside the allocated range
> or do things that ASan will catch, causing the tests to fail.
> - Fix a memory leak in `test_nmtpreinit.cpp`.
> - Enable coredumps to work on some platforms with ASan, some other tests rely
> on this working.
>
> All of this is related to ASan and NMT, so I opted to do it in a single
> change.
Mostly good.
test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp line 46:
> 44: #define DEFINE_TEST(test_function, expected_assertion_message)
> \
> 45: TEST_VM_FATAL_ERROR_MSG_IF(ASAN_NOT_ENABLED(), NMT, test_function,
> \
> 46: ".*" expected_assertion_message ".*") {
> \
I'd prefer a simple ifndef ADDRESS_SANITIZER instead of xxx_IF. And a comment
why this does not work with asan.
-------------
PR Review: https://git.openjdk.org/jdk/pull/13208#pullrequestreview-1373023156
PR Review Comment: https://git.openjdk.org/jdk/pull/13208#discussion_r1158590940
