RFR: 8360775: Fix Shenandoah GC test failures when APX is enabled [v2]

Srinivas Vamsi Parasa sparasa at openjdk.org
Mon Jun 30 22:19:18 UTC 2025


On Mon, 30 Jun 2025 17:07:04 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

>> src/hotspot/cpu/x86/assembler_x86.cpp line 15675:
>> 
>>> 15673: void Assembler::pusha_uncached() { // 64bit
>>> 15674:   if (UseAPX) {
>>> 15675:     // Data being pushed by PUSH2 must be 16B-aligned on the stack, for this push rax upfront
>> 
>> Hi @vamsi-parasa ,
>> 
>> PUSHA / POPA assembler is agnostic to the use of hardcoded registers in calling context, e.g. in following line of code
>> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/gc/shenandoah/shenandoahBarrierSetAssembler_x86.cpp#L495
>> 
>> If dst and tmp1 are RAX then we endup currpting it since RAX is used as a scratch register for stack alignment, and in case RAX holds an oop pointer then we may see random crashes. Such idioms are limited to GC barreirs currently, and we have recently fixed one such issue in https://github.com/openjdk/jdk/pull/25351
>> 
>> While the instruction sequence of PUSHA/ POPA with PPX hints is correct, Do you think for the time being we should limit the scope of this fix to save_machine_state and restor_machine_state routines rather than making generic fix in pusha/popa ?
>> 
>> I have tried it and it's working.
>
> @jatin-bhateja Pusha is not expected to change any registers. The inadvertent change of registers is very hard to debug. So in my thoughts it is better to have a conservative implementation currently which doesn't change RAX register.

Please see the updated code which fixes the issue by restoring the contents of RAX. The tests are passing with this update.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26009#discussion_r2176059486


More information about the shenandoah-dev mailing list