RFR: 8365163: [ubsan] left-shift issue in globalDefinitions.hpp

[Kim Barrett]

On Wed, 20 Aug 2025 10:28:05 GMT, Afshin Zafari <azafari at openjdk.org> wrote:

>> First, we do not care about left shift of negative value on its own
>> (JDK-8240259). Indicating this issue has anything to do with that is just
>> confusing matters.
>> 
>> What we care about is signed integer overflow, which is UB. And there is
>> indeed such overflow in this code. Having the JBS issue and the PR description
>> be more direct about that would have avoided confusion on my part.
>> 
>> (I'm somewhat surprised none of our compilers just breaks this obvious
>> overflow. Or maybe they do and we've not noticed, though that seems unlikely.)
>> 
>> As @theRealAph points out, the proposed change still contains overflows. For example
>> 
>> 1061:  *value &= (jlong)0xffffffff << 32;
>> 
>> 
>> But this can be vastly simplified to just
>> 
>> inline jlong jlong_from(jint h, jint l) {
>>   // First cast jint values to juint, so cast to julong will zero-extend.
>>   julong high = (julong)(juint)h << 32;
>>   julong low = (julong)(juint)l;
>>   return (jlong)(high | low);
>> }
>> 
>> and delete the now unused set_high and set_low functions. (There are set_high
>> and set_low functions in sharedRuntimeMath.hpp, but they deal with double
>> rather than jlong.)
>
> I am aware of not caring the issues of 'left-shift of negative values' since after C++20 they will not be UB anymore. I use UBSAN left-shift checks for finding other hidden/potential problems around left-shifts.
> 
> Sorry if missing enough explanation in title and/or description parts brought you confusion.
> 
> AFAIU, the code `(jlong)0xffffffff << 32` is not problematic since `jlong` is 64 bits and `0xffffffff` is `unsigned int` then casting to `jlong` makes it as `0x00000000ffffffff` and shifting it 32 bits to left results in `0xffffffff00000000` which means the the result is still representable with no loss. What am I missing here?
> 
> So, if my understanding above is correct, there will be no issue in these two functions since left-shift o f negative values are to be ignored for now. IOW, we can close this PR without integration.

`(jlong)0xffffffff << 32` is a (obvious) signed integer overflow. It's a
signed shift, and the sign bit changes.

However, there was a subtle but intentional change in C++14 that permits this
specific overflow case. A signed left shift that moves a 1 bit into the sign
bit position (but not beyond!) is permitted. The actual wording is

C++14 5.8/2
"[...] if E1 has a signed type and non-negative value, and E1×2E2 is
representable in the corresponding unsigned type of the result type, then that
value, converted to the result type, is the resulting value; [...]"

C++11 5.8/2
"[...] if E1 has a signed type and non-negative value, and E1×2E2 is representable
in the result type, then that is the resulting value; [...]"

So that code has an overflow that is UB in C++11 and earlier, but it's not UB
in C++14 and later.

That's all very complicated and subtle analysis. I think my proposed
alternative skips all, is very straightforward to understand, and insulates us
from any future explorations of the sort that led us here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26809#discussion_r2288334648