RFR: 8370405: C2: mismatched store from MergeStores wrongly scalarized in allocation elimination

[Manuel Hässig]

On Mon, 27 Oct 2025 10:40:18 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> Note: @oliviermattmann found this bug with his whitebox fuzzer. See also https://github.com/openjdk/jdk/pull/27991
> 
> **Analysis**
> We run Escape Analysis, and see that a local array allocation could possibly be removed, we only have matching `StoreI` to the `int[]`. But there is one `StoreI` that is still in a loop, and so we wait with the actual allocation removal until later, hoping it may go away, or drop out of the loop.
> During loop opts, the `StoreI` drops out of the loop, now there should be nothing in the way of allocation removal.
> But now we run `MergeStores`, and merge two of the `StoreI` into a mismatched `StoreL`.
> 
> Then, we eventually remove the allocation, but don't check again if any new mismatched store has appeared.
> Instead of a `ConI`, we receive a `ConL`, for the first of the two merged `StoreI`. The second merged `StoreI` instead captures the state before the `StoreL`, and that is wrong.
> 
> **Solution**
> We should have some assert, that checks that the captured `field_val` corresponds to the expected `field_type`.
> 
> But the real fix was suggested by @merykitty : apparently he just had a similar issue in Valhalla:
> https://github.com/openjdk/valhalla/blame/60af17ff5995cfa5de075332355f7f475c163865/src/hotspot/share/opto/macro.cpp#L709-L713
> (the idea is to bail out of the elimination if any of the found stores are mismatched.)
> 
> **Details**
> 
> How the bad sequence develops, and which components are involved.
> 
> 1) The `SafePoint` contains a `ConL` and 3 `ConI`. (Correct would have been 4 `ConI`)
> 
>   6  ConI  === 23  [[ 4 ]]  #int:16777216
>   7  ConI  === 23  [[ 4 ]]  #int:256
>   8  ConI  === 23  [[ 4 ]]  #int:1048576
>   9  ConL  === 23  [[ 4 ]]  #long:68719476737
>  54  DefinitionSpillCopy  === _ 27  [[ 16 12 4 ]]
>   4  CallStaticJavaDirect  === 47 29 30 26 32 33 0 34 0 54 9 8 7 6  [[ 5 3 52 ]] Static wrapper for: uncommon_trap(reason='unstable_if' action='reinterpret' debug_id='0') # void ( int ) C=0.000100 Test::test @ bci:38 (line 21) reexecute !jvms: Test::test @ bci:38 (line 21)
> 
> 
> 2) This is then encoded into an `ObjectValue`. A `Type::Long` / `ConL` is converted into a `[int=0, long=ConL]` pair, see:
> https://github.com/openjdk/jdk/blob/da7121aff9eccb046b82a75093034f1cdbd9b9e4/src/hotspot/share/opto/output.cpp#L920-L925
> If I understand it right, there zero is just a placeholder.
> 
> And so we get:
> 
> (rr) p sv->print_fields_on(tty)
>         Fields: 0, 68719476737, 1048576, 256, 16777216
> 
> We can see the `zero`, followed by the `ConL`, and then 3 `ConI`.
> 
> This se...

Thank you for fixing this and providing such a detailed explanation in the reproducer! 

If I understand correctly, this fix prevents mismatched accesses to occur when rematerializing rather than fixing rematerialization to handle mismatched accesses and the assert is to find cases that still trigger the incorrect corner case of rematerialization?
This begs the naive question: How much harder is it to fix rematerialization to handle mismatched accesses?

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

PR Review: https://git.openjdk.org/jdk/pull/27997#pullrequestreview-3392451330