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

Stefan Karlsson stefank at openjdk.org
Fri Jan 3 08:59:46 UTC 2025


On Fri, 3 Jan 2025 02:50:13 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 three additional commits since the last revision:
> 
>  - remove ultra empty line
>  - Use offsets in the loop logic instead
>  - Use offsets in the loop logic instead

This looks good to me. Let's see if Kim agrees or not.

While rewriting this to use offsets I think it becomes clear that the code above the change mixes offsets and pointers.

const size_t stepsize = (attach_range == 0) ? // Only one try.
    (size_t) highest_start : align_up(attach_range / num_attempts_to_try, attach_point_alignment);


I think the `(size_t) highest_start` should be something that isn't a pointer. Maybe simply `1` if we now change this to calculate using offsets. I don't know if we should squeeze that into this fix or deal with that as a small, separate change.

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

Marked as reviewed by stefank (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22897#pullrequestreview-2528738534


More information about the hotspot-runtime-dev mailing list