RFR: 8369393: NMT: poison the malloc header and footer under ASAN build [v2]

[Johan Sjölen]

On Wed, 17 Dec 2025 08:58:42 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> Under ASAN builds and when NMT is enabled, the header and footer of MallocHeader are poisoned/unpoisoned at malloc/realloc/free times.
>> Checking the integrity of memory blocks is off under ASAN builds.
>
> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
> 
>  - Merge remote-tracking branch 'origin/master' into asan_poison_malloc_hdr_ftr_v2
>  - removed extra newlines.
>  - fixes.
>  - inlining
>  - review comments applied.
>  - revision
>  - jtreg test excluded when ASAN is enabled.
>  - 8369393: NMT: poison the malloc header and footer under ASAN build

src/hotspot/share/nmt/mallocHeader.cpp line 80:

> 78: }
> 79: 
> 80: MallocHeader* MallocHeader::kill_block(void* memblock) {

Put these functions in the inline file instead, seems unnecessary to not get inlining of these calls.

src/hotspot/share/nmt/mallocHeader.cpp line 83:

> 81:   MallocHeader* header = (MallocHeader*)memblock - 1;
> 82:   ASAN_UNPOISON_MEMORY_REGION(header, sizeof(MallocHeader));
> 83:   ASAN_UNPOISON_MEMORY_REGION(header->footer_address(), sizeof(uint16_t));

We spread around the knowledge of the size of the footer being 16 bytes by adding this. We should have one place which tells us the size of the footer (maybe a `constexpr size_t footer_size;` stmt in the class header?) and use that only.

src/hotspot/share/nmt/mallocHeader.cpp line 95:

> 93:   ASAN_POISON_MEMORY_REGION(header, sizeof(MallocHeader));
> 94:   return header;
> 95: }

Here you missed that we shouldn't return the header as it's invalid.

src/hotspot/share/runtime/os.cpp line 716:

> 714:     assert(mem_tag == header->mem_tag(), "weird NMT type mismatch (new:\"%s\" != old:\"%s\")\n",
> 715:            NMTUtil::tag_to_name(mem_tag), NMTUtil::tag_to_name(header->mem_tag()));
> 716:     bool success = false;

Here's an alternative variant where `success` is assigned before branches and the conditions are given names (instead of having comments).
```c++
    // Perform integrity checks on and mark the old block as dead *before* calling the real realloc(3) since it
    // may invalidate the old block, including its header.
    assert(mem_tag == header->mem_tag(), "weird NMT type mismatch (new:"%s" != old:"%s")\n",
           NMTUtil::tag_to_name(mem_tag), NMTUtil::tag_to_name(header->mem_tag()));
    bool within_malloc_limit = !((size > old_size) && MemTracker::check_exceeds_limit(size - old_size, mem_tag));
    bool success = within_malloc_limit;
    if (success) {
      void* const new_outer_ptr = permit_forbidden_function::realloc(header, new_outer_size);
      bool realloc_succeeded = new_outer_ptr != nullptr;
      success = realloc_succeeded;
      if (success) {
        // realloc(3) succeeded, variable header now points to invalid memory and we need to deaccount the old block.
        MemTracker::deaccount(free_info);
        // After a successful realloc(3), we account the resized block with its new size
        // to NMT.
        void* const new_inner_ptr = MemTracker::record_malloc(new_outer_ptr, size, mem_tag, stack);

#ifdef ASSERT
        if (ZapCHeap && old_size < size) {
          // We also zap the newly extended region.
          ::memset((char*)new_inner_ptr + old_size, uninitBlockPad, size - old_size);
        }
#endif

        rc = new_inner_ptr;
      }
    }

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28503#discussion_r2580015707
PR Review Comment: https://git.openjdk.org/jdk/pull/28503#discussion_r2580013449
PR Review Comment: https://git.openjdk.org/jdk/pull/28503#discussion_r2580007813
PR Review Comment: https://git.openjdk.org/jdk/pull/28503#discussion_r2580006613