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

[Kim Barrett]

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

>> Note we seem to do this a lot e.g.
>> 
>>> ./share/oops/klass.cpp:  `int  tag   =  isobj ? _lh_array_tag_obj_value : _lh_array_tag_type_value;`
>> 
>> `_lh_array_tag_type_value` is unsigned. So this call sequence goes from unsigned -> signed -> unsigned -> signed
>
> 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.

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

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