RFR: 8352140: UBSAN: fix the left shift of negative value in klass.hpp, array_layout_helper()

[Afshin Zafari]

On Tue, 25 Mar 2025 15:36:20 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> For my own learning: 
>> When developers use left-shift for doubling a value, then a negative operand may changed to a positive since the sign-bit may change. For example in
>> 
>>     signed short int x = -32768;
>>     signed short int y = x << 1;
>> ``` 
>> the value of `y` would be `0`. So, when the left-shift is used as an arithmetic op, both the sign and size of the result/operand should be carefully considered. And, this is not dependent on C++xx.
>> So, left-shift of negative value is UB, until the developer explicitly decides on the type of the operand or the result.
>
> signed short int x = -32768;
>     signed short int y = x << 1;
> 
> 
> That does seem like an interestingly weird case. Unless I'm missing something,
> there's no UB-overflow in that. The shift expression promotes `short x` to
> `int x`, sign extending it. The `int`-typed shift is fine (since C++20, and
> effectively so prior to that in non-constexpr-required contexts - see below).
> And the implicit conversion to `short y` is implementation-defined (before
> C++20, though gcc may warn (-Woverflow)) or fine (since C++20).
> 
> gcc warns about x being negative in C++11 to C++17 modes
> (-Wshift-negative-value enabled by default), but doesn't treat it as UB.
> Before C++20 gcc errors (warns if -fpermissive) if it's in a
> required-constexpr-context, even if -Wshift-negative-value is disabled.
> That all seems consistent.

I had to emphasize that the case shown in the example may happen at run-time where compiler has no chance to warn/avoid/address it. 
My concern is that developers should not rely on the compiler to check the validation of left-shift op. They should be aware of the `signed` <-> `unsigned` conversions during the left-shift.
To find invalid cases of left-shift, UBSAN instruments them with assertions to catch them at run-time. If the assertion raised, good we found the problem. However, if no assertion raised for some left-shift ops, it doesn't mean that they are valid.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24184#discussion_r2013721972