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

[Kim Barrett]

On Tue, 31 Dec 2024 05:43:12 GMT, Kim Barrett <kbarrett 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
>
> 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 && attach_point <= highest_start && p2u(attach_point) > stepsize;  // Avoid wrap around.
> 
> The `attach_point <= highest_start` part of the test should be removed.
> That's the old and incorrect attempt to avoid wrap around.

The proposed change is not correct.  Consider if attach_point, lowest_start,
and stepsize are all equal.  In such a case this would stop without trying
that case, even though it should try it.  Changing the comparison with
stepsize to >= would fix that.  But it still wouldn't be correct.  Consider if
attach_point == lowest_start and stepsize > lowest_start.  That should also be
tried.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22897#discussion_r1899927466