[jdk21u-dev] RFR: 8358334: C2/Shenandoah: incorrect execution with Unsafe

[Roland Westrelin]

On Tue, 1 Jul 2025 17:20:57 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> Patch doesn't apply cleanly:
>> 
>> - jdk21u still has IU barriers: context for some of the changes is
>>   different.
>>   
>> - Because of IU barriers, there's an extra call to `fix_ctrl()` for
>>   which the renaming to `nodes_above_barriers` must be applied.
>>   
>> - The initial patch makes a subtle change that doesn't affect jdk
>>   25/26 in the code that was
>>   refactored. `ShenandoahBarrierC2Support::push_data_inputs_at_control()`
>>   is introduced with logic:
>> 
>>     if (in != nullptr && phase->has_ctrl(in) && phase->get_ctrl(in) == ctrl) {
>> 
>> 
>>   which replaces the same logic in
>>   `ShenandoahBarrierC2Support::fix_ctrl()` but also a slightly
>>   different logic in
>>   `ShenandoahBarrierC2Support::is_dominator_same_ctrl()`:
>>   
>> 
>>     if (m->in(i) != nullptr && phase->ctrl_or_self(m->in(i)) == c) {
>> 
>> 
>>   that is, it uses `ctrl_or_self()` which works for both data nodes
>>   and control nodes but the new method uses `has_ctrl(in) &&
>>   get_ctrl(in)` which can only be true for data nodes. That change
>>   causes failures in jdk 21 again because of IU barriers that produce
>>   a new memory state when expanded and need the logic from
>>   `MemoryGraphFixer`. What I propose in this backport to be on the
>>   safe side is to leave
>>   `ShenandoahBarrierC2Support::is_dominator_same_ctrl()` alone (not
>>   apply that part of the refactoring).
>>   
>> Tested with hotspot_gc_shenandoah + tier1 with -XX:+UseShenandoahGC
>
> Ah yes, apologies, it slipped off my radar. I'll take a look.

Thanks for the review @shipilev 
I pushed a new commit with what you suggested. I think that works.

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

PR Comment: https://git.openjdk.org/jdk21u-dev/pull/1901#issuecomment-3043892322