RFR: 8351334: [ubsan] memoryReserver.cpp:552:60: runtime error: applying non-zero offset 1073741824 to null pointer [v11]

[David Holmes]

On Thu, 30 Oct 2025 12:06:00 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> The issue happens when the HeapMinBaseAddress option gets 0 as input value. Since this option is used as an address, then using 0 in pointer arithmetics is UB.
>> The fix is using `unitptr_t` instead of `address`/`char*`, etc.  In doing that, it is found that an overflow check does not work in all cases due to checking more conditions. That overflow check is changed too. We also need to check overflow after aligning addresses and sizes of memory regions in this context. Assertions are added to check these cases.
>> 
>> Tests:
>> linux-x64 tier1
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fix arguments.cpp for HeapMinBaseAddress type.

Sorry but I am finding that the actual fix here is getting lost in a lot of not-obviously-needed changes to variable type declarations.

src/hotspot/share/memory/memoryReserver.cpp line 549:

> 547:   const size_t attach_point_alignment = lcm(alignment, os_attach_point_alignment);
> 548: 
> 549:   uintptr_t aligned_heap_base_min_address = align_up(MAX2(HeapBaseMinAddress, alignment), alignment);

Just to be clear, this is the crux of the fix, where we ensure the min-address is now never zero - right?

src/hotspot/share/memory/memoryReserver.cpp line 586:

> 584:                                           lowest_start, highest_start);
> 585:       reserved = try_reserve_range((char*)highest_start, (char*)lowest_start, attach_point_alignment,
> 586:                                    (char*)aligned_heap_base_min_address, (char*)UnscaledOopHeapMax, size, alignment, page_size);

Not obvious to me this actually improves anything - what is it fixing?

src/hotspot/share/memory/memoryReserver.cpp line 590:

> 588: 
> 589:     // zerobased: Attempt to allocate in the lower 32G.
> 590:     size_t zerobased_max = OopEncodingHeapMax;

Again not obvious what this improves. We obviously have very inconsistent use of types here in that we loosely use `char*`, `uint64_t` and `size_t` to all mean a 64-bit unsigned value, ansd no matter what types we use in the declarations we have to cast something somewhere.

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

PR Review: https://git.openjdk.org/jdk/pull/26955#pullrequestreview-3470648246
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2532394079
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2532395457
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2532399134