RFR: 8286876: NMT.test_unaliged_block_address_vm_assert fails if using clang toolchain

[Thomas Stuefe]

On Fri, 2 Dec 2022 15:47:15 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> This fixes a bug by removing some UB which Clang used in order to make a test fail. Specifically, Clang optimized the `is_aligned` call in ` MallocHeader::assert_block_integrity` to `true`, because an unaligned `this` is  UB. This is fixed by making a static method, essentially. The new code liberally uses `reinterpret_cast<uintptr_t>()`, this is a specifically mentioned as legal use on https://en.cppreference.com/w/cpp/language/reinterpret_cast .
>> 
>> **The idea is essentially:** Make sure the pointer looks OK before casting it to a `MallocHeader*`.
>> 
>> I also changed the definition of `malloc_header`:
>> 
>> ```c++
>> // old
>>     return (MallocHeader*)((char*)memblock - sizeof(MallocHeader));
>> // new
>>     return &(((MallocHeader*)memblock)[-1]);
>> 
>> 
>> The previous definition was UB, because you shouldn't cast from a pointer with smaller alignment to one with larger alignment requirements.
>> 
>> I also made `MallocHeader::print_block_on_error` static, as the `this` argument was always equal to the `bad_address`.
>
> src/hotspot/share/services/mallocHeader.hpp line 139:
> 
>> 137:   // If block is broken, print out a report to tty (optionally with
>> 138:   // hex dump surrounding the broken block), then trigger a fatal error
>> 139:   inline static MallocHeader* assert_block_integrity(const void* memblock);
> 
> Proposal: rename to `malloc_header_with_checks()`, and possibly move the existing `malloc_header` here?

Also, I think this should, like the MemTracker::malloc_header variants, exist in two forms. One that takes and gives out const pointers, one that does non-const. I would not cast away the constness.

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

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