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

Kim Barrett kbarrett at openjdk.org
Sun Jun 8 18:19:01 UTC 2025


On Wed, 23 Apr 2025 10:22:34 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> Afshin Zafari has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - Merge remote-tracking branch 'origin/master' into _8352140_lshift_klass_hpp
>>  - minimum change.
>>  - Merge remote-tracking branch 'origin/master' into _8352140_lshift_klass_hpp
>>  - 8352140: UBSAN: fix the left shift of negative value in klass.hpp, array_layout_helper()
>
> Changes requested by kbarrett (Reviewer).

> I made the `int->uint32` change at the minimum level, because the layout_helper constants are `public` in the `Kalss` class and are used all around Hotspot code, particularly in architecture dependent codes. Changing them to unsigned breaks the build (at SYMBOL generation phase). In addition, the two most-significant bits of the layout are used for `array` types and `lh < 0` (or more precisely `lh < _lh_neutral_value`) is used in C++ and assembly language of arch-dep code. @kimbarrett , can we limit the change here to `array_layout_helper` or should we proceed to use `uint32_t` instead?

I'm inclined against uglifying the code for a ubsan issue just to keep the
change small. I'd rather we did the "right" fix instead, even though there's
some fannout. I don't know how urgent we consider fixing the ubsan issue
though.

But if we're going to do a minimal change, I like the @dean-long suggestion here:
https://github.com/openjdk/jdk/pull/24184/files#r2090835514

The latest proposal also doesn't do anything about the issue I mentioned at
the end of this comment:
https://github.com/openjdk/jdk/pull/24184#discussion_r2055739014
where there's a related problem with layout_helper_boolean_diffbit.
That could be treated as a separate bug, in which case just file it.

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

PR Comment: https://git.openjdk.org/jdk/pull/24184#issuecomment-2954209375


More information about the hotspot-dev mailing list