RFR: 8346866: [ASAN] memoryReserver.cpp reported applying non-zero offset to non-null pointer produced null pointer [v9]

[Kim Barrett]

On Tue, 7 Jan 2025 15:38:58 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>> This PR add an extra loop condition check `p2u(attach_point) > stepsize` in function `HeapReserver::Instance::try_reserve_range` to make sure the loop will not wrap around, and make UndefinedBehaviorSanitizer silent. The change do not change the original logic, risk is low.
>> 
>> Additional testing:
>> 
>> - [ ]  jtreg tests(include tier1/2/3 etc., which include tests added by [PR22712](https://github.com/openjdk/jdk/pull/22712)) on linux-x64 with release build
>> - [ ]  jtreg tests(include tier1/2/3 etc., which include tests added by [PR22712](https://github.com/openjdk/jdk/pull/22712)) on linux-x64 with fastdebug build
>> - [ ]  jtreg tests(include tier1/2/3 etc., which include tests added by [PR22712](https://github.com/openjdk/jdk/pull/22712)) on linux-aarch64 with release build
>> - [ ]  jtreg tests(include tier1/2/3 etc., which include tests added by [PR22712](https://github.com/openjdk/jdk/pull/22712)) on linux-aarch64 with fastdebug build
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   rewrite function HeapReserver::Instance::try_reserve_range for avoid calculation addresses outside of the range

Changes requested by kbarrett (Reviewer).

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

> 433:   const uint64_t num_attempts_possible = (attach_range / attach_point_alignment) + 1;
> 434:   const uint64_t num_attempts_to_try   = MIN2((uint64_t)HeapSearchSteps, num_attempts_possible);
> 435:   const uint64_t num_intervals = num_attempts_to_try - 1;

pre-existing, but stands out more now.  Why the introduction of uint64_t in here?  It seems like this should
just be using size_t throughout.

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

> 437: 
> 438:   for (uint64_t i = 0; i < num_attempts_to_try; ++i) {
> 439:     char* const attach_point = highest_start - stepsize * i;

This is _much_ better.  (I didn't even look at the recent intermediate version that @stefank didn't like.  From the
description and his comments, I don't think I would have liked it either.)

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

PR Review: https://git.openjdk.org/jdk/pull/22897#pullrequestreview-2535406910
PR Review Comment: https://git.openjdk.org/jdk/pull/22897#discussion_r1906066653
PR Review Comment: https://git.openjdk.org/jdk/pull/22897#discussion_r1906064345