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

[Johan Sjölen]

On Fri, 5 Sep 2025 11:02:32 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> The minimum acceptable value was 0 where using it as address was problematic according to UBSAN.
>> The acceptable value is changed to 64K.
>> 
>> Tests:
>> linux-x64 tier1
>
> Afshin Zafari has updated the pull request incrementally with one additional commit since the last revision:
> 
>   lowest can be equal to highest

src/hotspot/share/gc/shared/jvmFlagConstraintsGC.cpp line 288:

> 286:   // If an overflow happened in Arguments::set_heap_size(), MaxHeapSize will have too large a value.
> 287:   // Check for this by ensuring that MaxHeapSize plus the requested min base address still fit within max_uintx.
> 288:   if (value + MaxHeapSize < MaxHeapSize) {// overflow

Can we perform the overflow check via subtraction from the max value instead? I know that unsigned types do wrap around, but at a first glance this will look wrong and make the reader stop and think.

Style: Space between { and //


if (std::numeric_limits<size_t>::max() - value < MaxHeapSize) { // overflow

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

> 558:   if (!FLAG_IS_DEFAULT(HeapBaseMinAddress)) {
> 559:     reserved = try_reserve_memory(size + noaccess_prefix, alignment, page_size, (char *)aligned_heap_base_min_address);
> 560:     if (reserved.base() != (char *)aligned_heap_base_min_address) { // Enforce this exact address.

Style: Please hug the * to the char for the casts

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

> 582:       // Calc address range within we try to attach (range of possible start addresses).
> 583:       char* const highest_start = align_down((char *)UnscaledOopHeapMax - size, attach_point_alignment);
> 584:       char* const lowest_start  = align_up((char *)aligned_heap_base_min_address, attach_point_alignment);

Keep these as uintptr_t instead and only cast when trying to reserve

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

> 593: 
> 594:     // Give it several tries from top of range to bottom.
> 595:     if (aligned_heap_base_min_address + size <= (uintptr_t)zerobased_max && // Zerobased theoretical possible.

Do the opposite: Have zerobased_max be uintptr_t and only cast it when it needs to become a char*

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

> 610:       }
> 611:       lowest_start = align_up(lowest_start, attach_point_alignment);
> 612:       assert(lowest_start <= highest_start, "lowest: " INTPTR_FORMAT " highest: " INTPTR_FORMAT,

Keep these as uintptr_t and cast to char* only when reserving

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2343523999
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2343578098
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2343582330
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2343588323
PR Review Comment: https://git.openjdk.org/jdk/pull/26955#discussion_r2343591444