RFR: 8332865: ubsan: os::attempt_reserve_memory_between reports overflow

[Kim Barrett]

On Tue, 4 Jun 2024 15:19:11 GMT, Matthias Baesken <mbaesken at openjdk.org> wrote:

> When running by ubsan-enabled binaries on Linux x86_64, os::attempt_reserve_memory_between reports overflows.
> 
> "runtime error: pointer index expression with base 0x000000001000 overflowed to 0xfffffffffffff000"
> 
> This coding triggers the ubsan issue
> 
> 
>   char* const hi_att = align_down(MIN2(max, absolute_max) - bytes, alignment_adjusted);
>   if (hi_att > max) {
>     return nullptr; // overflow
>   }
> 
> 
> However the function already contains overflow handling, so probably it is sufficient to add an attribute to the function os::attempt_reserve_memory_between to disable ubsan checks for this function.

Are you sure about the triggering code?  I don't see anything in that snippet
that might overflow.  The error message in JBS says the failure in on line
1928, but that doesn't look right either.

The calculation of lo_att seems like it could potentially overflow, and the
check is commented as being for overflow.

src/hotspot/share/runtime/os.cpp line 1894:

> 1892: #if defined(__clang__) || defined(__GNUC__)
> 1893: __attribute__((no_sanitize("undefined")))
> 1894: #endif

I don't think this attribute addition should be made. I agree with @stefank that we should be eliminating the
potential overflow, since the compiler (without ubsan) is within its rights to discard a pointer overflow check,
since pointer overflow is UB.

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

Changes requested by kbarrett (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19543#pullrequestreview-2098016305
PR Review Comment: https://git.openjdk.org/jdk/pull/19543#discussion_r1626927254