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

[Afshin Zafari]

On Tue, 25 Mar 2025 07:21:14 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> A problem we are facing here is that C++20 makes some integral operations
>> defined that were previously undefined. This followed what implementations were
>> actually doing. And yet, tools like ubsan (and constexpr-processing until
>> C++20) treat them as UB.
>> 
>> For example,
>> https://gcc.gnu.org/onlinedocs/gcc-14.2.0/gcc/Integers-implementation.html
>> "The results of some bitwise operations on signed integers (C90 6.3, C99 and C11 6.5).
>> ...
>> As an extension to the C language, GCC does not use the latitude given in C99
>> and C11 only to treat certain aspects of signed ‘<<’ as undefined. However,
>> -fsanitize=shift (and -fsanitize=undefined) will diagnose such cases. They are
>> also diagnosed where constant expressions are required."
>> 
>> Hence, I'm at least somewhat inclined to call a ubsan complaint about left
>> shift of a negative value a false positive.
>> 
>> The implementation-defined behavior of unsigned => signed conversions is
>> another thing that C++20 changed to be defined.
>
> Note that the discussion that led to the "weird-looking cast" in
> JAVA_INTEGER_OP significantly predates the standard committee's decision to
> enshrine two's-complement integers in C++20.  If we were to have that
> discussion today my opinion would be quite different from what it was at the
> time of that discussion.

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.

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

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