[lworld] RFR: 8366717: [lworld] Cleanup defensive fixing of JDK-8365996 [v3]

[Tobias Hartmann]

On Wed, 7 Jan 2026 10:18:32 GMT, Marc Chevalier <mchevalier at openjdk.org> wrote:

>> Let's proceed piece by piece.
>> 
>> # `G1BarrierSetAssembler::g1_write_barrier_pre` in `g1BarrierSetAssembler_aarch64.cpp`
>> 
>> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/gc/g1/g1BarrierSetAssembler_aarch64.cpp#L216-L220
>> 
>> `push_call_clobbered_registers`/`pop_call_clobbered_registers` should be enough around a runtime call, that's what clobbered registers are.
>> 
>> But let's dig a bit, that will be useful later!
>> 
>> 
>>    push_call_clobbered_registers()
>> => push_call_clobbered_registers_except(exclude = empty set)
>> => push(call_clobbered_gp_registers() - exclude, sp)  // with exclude = empty set
>> 
>> So, we save at least `call_clobbered_gp_registers` which is defined as:
>> 
>> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L3614-L3620
>> 
>> So, we save r0-r7 and r10-r17
>> 
>> # `CardTableBarrierSetAssembler::oop_store_at` in `cardTableBarrierSetAssembler_aarch64.cpp` and `cardTableBarrierSetAssembler_x86.cpp`
>> 
>> https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/aarch64/gc/shared/cardTableBarrierSetAssembler_aarch64.cpp#L116-L124
>> 
>> Digging the history, it seems it was added to fix a corruption of `dst.base()`. Yet, it might not be always necessary. It's rather nicer to save where it matter that the register is not corrupted than all the way down. So, there are a few such places:
>> 
>> - `gen_c2i_adapter_helper` in `sharedRuntime_x86_64.cpp`
>>   `gen_c2i_adapter_helper` calls `store_heap_oop` that can corrupts `to.base()`. `gen_c2i_adapter_helper` is called, for instance from `gen_c2i_adapter`:
>>   https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L1069-L1070
>>   where we can see that `to.base()` is `r14`, which is set
>>   https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L1026
>>   before the loop enclosing the call to `gen_c2i_adapter_helper`
>>   https://github.com/openjdk/valhalla/blob/1077e4f9f06336af30d95fc28cbab4d31c9f2a44/src/hotspot/cpu/x86/sharedRuntime_x86_64.cpp#L1036
>>   If we do more than one iteration of this loop when entering the `store_heap_oop` branch, `r14` is corrupted at the second call and we get a segfault.
>> - `generate_buffered_inline_type_adapter` in `sharedRuntime_x86_64.cpp`: sam...
>
> Marc Chevalier has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 18 commits:
> 
>  - Update copyright
>  - Merge remote-tracking branch 'origin/lworld' into JDK-8366717.testing
>  - Fixing aarch64
>  - Save in more places
>  - Lift saving rax
>  - Pick the right comment again
>  - Merge remote-tracking branch 'origin/lworld' into JDK-8366717.investigate
>  - oop_store_at
>  - assert in oop_store_at
>  - Cleanup
>  - ... and 8 more: https://git.openjdk.org/valhalla/compare/90dfb439...05dd65e4

Nice! Thanks for the thorough investigation. Looks good to me.

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

Marked as reviewed by thartmann (Committer).

PR Review: https://git.openjdk.org/valhalla/pull/1824#pullrequestreview-3635605860