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

[Kim Barrett]

On Tue, 31 Dec 2024 09:44:18 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:
> 
>   Add "attach_point <= stepsize" check at the end of for loop, to make sanitizer silent and avoid warp around.

Changes requested by kbarrett (Reviewer).

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

> 438:   // Try attach points from top to bottom.
> 439:   for (char* attach_point = highest_start;
> 440:        attach_point >= lowest_start;  // Avoid wrap around.

The comment is no longer true or relevant.  That comment was about the clause that is being removed.

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

> 453: 
> 454:     if (p2u(attach_point) <= stepsize)
> 455:       break;  // Make sanizier silent and avoid warp around.

Sanitizer isn't interesting.  The point is this avoids pointer underflow.  Also, braces are required for multiline
`if` statements - see style guide.

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

PR Review: https://git.openjdk.org/jdk/pull/22897#pullrequestreview-2527183712
PR Review Comment: https://git.openjdk.org/jdk/pull/22897#discussion_r1900640694
PR Review Comment: https://git.openjdk.org/jdk/pull/22897#discussion_r1900642209