RFR: 8286876: NMT.test_unaliged_block_address_vm_assert fails if using clang toolchain [v6]

[Thomas Stuefe]

On Fri, 13 Jan 2023 15:38:51 GMT, Gerard Ziemski <gziemski at openjdk.org> wrote:

>> Johan Sjölen has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Style
>>  - Style
>
> Just FYI I'm taking a look at this...

Hi @gerard-ziemski ,

> I was a bit concerned over the non-trivial change this fix turned out to be, so I investigated a little.

thank you for your thoughts. Skimming patches down is always good.
> 
> The smallest fix I can come up with is to change:
> 
> ```
> if (!is_aligned(this, sizeof(uint64_t))) {
> ```
> 
> to
> 
> ```
>   volatile uintptr_t ptr = (uintptr_t)this;
>   if (!is_aligned(ptr, sizeof(uint64_t))) {
> ```

I'd personally be fine with it but still would be UB and other compilers may complain again.

> 
> which works, since using volatile turns any compiler optimizations off, so we end up with the needed pointer (? that's how I understand it, but not 100% sure)
> 
> Another way I can see this fixed is to pass the `memblock` into `MallocHeader::assert_block_integrity()` keeping it as `void*`, which we can then pass right onto `check_block_integrity()`, which we need to adapt slightly to receive the pointer.
> 

So, make `MallocHeader::assert_block_integrity()` a static function, and pass the void* payload pointer in? That could work.

> I am OK with the proposed fix, though I personally would prefer either one of the ones I presented.

A small patch would be easier to backport and clearer with respect to what is actually the fix.

But Johans patch also has some cleanups I like:
- moving the header resolving functions out of memTracker.hpp into mallocHeader.hpp and making them static members of MallocHeader
- the solution to the cons/nonconst duplication via templates is neat
- -1 instead of sizeof to resolve the header
- and other small things.

So possibly it would be fine to have a minimal patch, and then some cleanups afterwards?

I leave this up to @jdksjolen. He has put a lot of work into it already, and if he wants to be done with it and ship it that totally fine for me.

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

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