RFR: 8369393: NMT: poison the canaries of malloc header under ASAN build [v20]

[Afshin Zafari]

On Mon, 10 Nov 2025 12:20:35 GMT, Paul Hübner <phubner at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   order of includes corrected.
>
> src/hotspot/share/nmt/mallocHeader.hpp line 120:
> 
>> 118:   void set_footer(uint16_t v) {
>> 119:     AsanPoisoningHelper aph(reinterpret_cast<uint16_t*>(footer_address()));
>> 120:     footer_address()[0] = (uint8_t)(v >> 8); footer_address()[1] = (uint8_t)v;
> 
> Nit: can we please separate these onto two separate lines, it's cumbersome to read.

Done.

> src/hotspot/share/sanitizers/address.hpp line 86:
> 
>> 84:   AsanPoisoningHelper(const T* addr) : _memory_region(addr) {
>> 85:     #if INCLUDE_ASAN
>> 86:       ASAN_UNPOISON_MEMORY_REGION(_memory_region, sizeof(T));
> 
> So if I understand correctly, when we do e.g.:
> 
> AsanPoisoningHelper aph(reinterpret_cast<uint16_t*>(footer_address()));
> footer_address()[0] = (uint8_t)(v >> 8); footer_address()[1] = (uint8_t)v;
> 
> We are first unpoisoning, doing our memory modification, and then re-poisoning once the helper falls out of scope. Wouldn't it make that an `Asan*Un*poisoningHelper`? Maybe you could introduce a quick comment on what this class is/what you would use it for, because it's a bit counter intuitive at least to me.

Done.

> test/hotspot/gtest/nmt/test_nmt_buffer_overflow_detection.cpp line 325:
> 
>> 323:     a = 2;
>> 324:     EXPECT_EQ(a, 2);
>> 325:   }
> 
> Maybe you could add another assert once the helper is out of scope to make sure that there is a death?

Done.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2511804900
PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2511806186
PR Review Comment: https://git.openjdk.org/jdk/pull/27685#discussion_r2511806606