Z: Restore Non Volatile GPRs in jitExitInterpreter0RestoreAll #22128

r30shah · 2025-06-23T16:41:34Z

When calling out jitMonitorEnter helper call, in case of the thread needs to yield when entering monitor in blocking mode and later when it is continued on other thread, return path back to JIT compiled code was missing preserving the Non Volatile GPRs and switching back to java stack - This commit adds the routine to switch stack and restore preserve registers.

r30shah · 2025-06-23T16:44:51Z

@gacholio / @TobiAjila can I get your review on the change ?

We have atleast seen 3 issues that can be potentially fixed by this [1][2][3], and I think [4] is fixed by this as well.

[1]. #21717
[2]. #21990
[3]. #22089
[4]. #22047

gacholio · 2025-06-23T17:54:34Z

runtime/oti/zhelpers.m4

@@ -729,6 +729,7 @@ ifelse(eval(ASM_JAVA_SPEC_VERSION >= 24), 1, {
 define({RESTORE_ALL_REGS},{
    RESTORE_C_VOLATILE_REGS($1)
    RESTORE_C_NONVOLATILE_REGS
+    RESTORE_PRESERVED_REGS_AND_SWITCH_TO_JAVA_STACK


Does this work if you simply switch to the java stack? The previous two lines have restored all of the registers already.

Note that the C interpreter has already executed RESTORE_PRESERVED_REGS_AND_SWITCH_TO_JAVA_STACK before jumping to the restore all target.

Can this be solved by storing the registers you don't want overwritten before doing the restores above?

We should arguably be restoring the hiword registers here as well, though we have no 31-bit builds any more.

RESTORE_C_VOLATILE_REGS($1) RESTORE_C_NONVOLATILE_REGS

Are no-ops on Z

Do we need to switch to java stack, dont we do that when we exit the interpreter in https://github.com/eclipse-openj9/openj9/blob/master/runtime/vm/zcinterp.m4#L85 ?

I'll check out the other platforms to make sure they aren't suffering from similar issues.

This code is somewhat confusing as the registers are stored in the monitor enter helper before calling the C support code, but they are not restored by the helper - we instead branch out to the yield helper which calls in to the C interpreter. The interpreter returns back to the JIT as if it were a method call once the monitor enter is complete.

Yes, that mix of different linkages is confusing.

I'm not sure if that's true - the C interpreter has only restored the registers that are preserved in the JIT private linkage before jumping to the helper.

I am looking at the JIT private linkage, on Z - we would GPR5 to GPR13 is considered preserved register - (GPR5 and GPR13 being J9SP and J9VMThread respectively), while C ABI on LoZ will consider GPR6 to GPR13 preserved). For Linux on Z, as private linkage and helper linkage are bit similar, it may be not that bad, but we would have issue on z/OS where C linkage bit different then the private linkage.

I think there's also the FPR/VR registers to be considered - none of the them are preserved in the JIT private linkage, so none will have been restored in the C interpteter transition code.

IIRC, we did not expect JIT helpers (The ones that were changed to use New Dual Mode helper code) to use Vector Registers or FPRs in the path. i.e, I do not see any FPRs/VRFs being preserved in Glue code when such C code helper is called. JIT dispatch sequence would preserve any C volatile register that is being used in the JIT compiled code.

Remember that the JIT code called jitMonitorEntry initially - the contract for this (and every other helper) is that all registers are preserved (with the possible exception of a return value, which does not exist in this case). When we resume after the yield, we effectively return from the monitor enter call, so all registers will need to be restored by the post-yield return point.

Yes, correct. So just to ensure I understand clearly and what I am seeing is correct, On Z, we have divided the preservation of all registers in two parts for Dual mode helpers such as jitMonitorEnter/jitMonitorExit. All the registers (GPR/FPR/VRF) which are considered volatile in C linkage on Z are preserved before JIT calls out to assembly glue code in 'znathelp.m4' [1]. If we do not need to enter slow path helper - we do not touch any of the register which are non-volatiles (Which C code should be preserving, and there is not interpreter / GC call there so no need to keep track of them).

If we need to call slow path, we execute the sequence in [2] that preserves the registers considered non volatile in the C ABI.

Now for JIT monitor entry, for thread to yield, the call is made at [3].

Given the exit point from JIT compiled code would preserve the registers considered volatile in C, all kinds of non volatile registers are needed to be restored right ?

[1]. https://github.com/eclipse-openj9/openj9/blob/0dcff38ad68b6336717667681c8217eab9e8b8a5/runtime/
codert_vm/znathelp.m4#L203-L266
[2].

openj9/runtime/codert_vm/znathelp.m4

Lines 215 to 216 in 0dcff38

SAVE_C_NONVOLATILE_GPRS

SAVE_C_NONVOLATILE_REGS

[3].

openj9/runtime/codert_vm/znathelp.m4

Lines 222 to 223 in 0dcff38

je LABEL_NAME(CONCAT(L_SLOW_,SYM_COUNT))

br CRINT

All the registers (GPR/FPR/VRF) which are considered volatile in C linkage on Z are preserved before JIT calls out to assembly glue code in 'znathelp.m4' .

This is incorrect - the contract is that all of the JIT helpers called from compiled code maintain and update ALL registers (in the case that a register holds an object pointer across a GC). This means FPR/VR as well.

In this case, the compiled code has called to jitMonitorEntry and is expecting a return from there with all registers intact/correct.

r30shah · 2025-06-23T23:58:57Z

I am converting this as draft - as going through the code, the interpreter loop and the code [1], which I think is the code being executed when returning from to the code that called jitMonitorEntry that was yielded.

So we should be restoring the register in the interpreter as @gacholio said as we would have executed the code in [2].

The problem specifically is with the GPR8 and GPR9 being used to hold the object as in past when I marked those registers as volatile in JIT dispatch, failure goes away. The problem is execution of [3] that happens after JIT restores preserved registers. This stub uses R8 and R9 to check if Vector registers are enabled or not to restore them, in process modifying the value of GPR8 and GPR9 which would have right objects.

I know how to fix this exactly. I will update the PR once I have made the change and tested it.

FYI @tajila @gacholio

[1].

openj9/runtime/vm/BytecodeInterpreter.hpp

Lines 5878 to 5885 in 0dcff38

    
           case J9VM_CONTINUATION_RETURN_FROM_JIT_MONITOR_ENTER: { 
        
           	rc = tryEnterBlockingMonitor(REGISTER_ARGS, syncObject, J9VM_CONTINUATION_RETURN_FROM_JIT_MONITOR_ENTER); 
        
           	if ((NULL != _currentThread->currentContinuation) && (EXECUTE_BYTECODE == rc)) { 
        
           		void *returnAddress = restoreJITResolveFrame(REGISTER_ARGS); 
        
           		VMStructHasBeenUpdated(REGISTER_ARGS); 
        
           		rc = promotedMethodOnTransitionFromJIT(REGISTER_ARGS, returnAddress, _vm->jitConfig->jitExitInterpreter0RestoreAll, true); 
        
           	} 
        
           	break;

[2].

openj9/runtime/vm/zcinterp.m4

Lines 73 to 86 in 0dcff38

    
           PLACE_LABEL(L_CINTERP) 
        
               L_GPR CARG2,J9TR_VMThread_javaVM(J9VMTHREAD) 
        
               L_GPR CARG2,J9TR_JavaVM_bytecodeLoop(CARG2) 
        
               LR_GPR CARG1,J9VMTHREAD 
        
               CALL_INDIRECT(CARG2) 
        
               LHI_GPR r0,J9TR_bcloop_exit_interpreter 
        
               CLR_GPR CRINT,r0 
        
               je LABEL_NAME(L_EXIT) 
        
               LHI_GPR r0,J9TR_bcloop_reenter_interpreter 
        
               CLR_GPR CRINT,r0 
        
               je LABEL_NAME(L_CINTERP) 
        
               RESTORE_LR 
        
               RESTORE_PRESERVED_REGS_AND_SWITCH_TO_JAVA_STACK 
        
               BRANCH_VIA_VMTHREAD(J9TR_VMThread_tempSlot)

[3].

openj9/runtime/oti/zhelpers.m4

Lines 713 to 721 in f91146b

    
           define({RESTORE_C_VOLATILE_REGS},{ 
        
               nr r9,r8 
        
               jz LABEL_NAME(CONCAT(L_RF,SYM_COUNT)) 
        
               RESTORE_C_VOLATILE_VRS 
        
               J LABEL_NAME(CONCAT(L_RV,SYM_COUNT)) 
        
           PLACE_LABEL(CONCAT(L_RF,SYM_COUNT)) 
        
               RESTORE_C_VOLATILE_FPRS 
        
           PLACE_LABEL(CONCAT(L_RV,SYM_COUNT)) 
        
           })

r30shah · 2025-06-26T18:25:12Z

@gacholio This is ready for review, in last changes, I have added routines to save and restore preserved FPRs and Vector registers as per s390x Linux and z/OS (31-bit/64-bit) in a4ab6ed.

In b10f8b5 I added routines for S390 linux.

I kept the routine exposed to only the NEW_DUAL_MODE_HELPER routines only and have tested out IBM SDK8 (build_info.php?build_id=97166).

When calling new dual mode helper such as jitMonitorEnter, it is possible that the thread will be unmounted from the career thread, upon returning, when entering the monitor in blocking mode, virtual thread can be mounted different thread. To ensure that all the non volatile registers are preserved across such transition, change in this PR adds assembly routines to save and restore non volatile FPRs and Vector Registers and updates the jitExitInterpreter0RestoreAll to restore them. Signed-off-by: Rahil Shah <rahil@ca.ibm.com>

r30shah · 2025-06-27T21:36:47Z

I have squashed commits.

@gacholio Can I request a review from you as well? In parallel, I am launching PR test as well.

r30shah · 2025-06-27T21:37:30Z

Jenkins test sanity zlinux jdk8,jdk21,jdk24

r30shah · 2025-07-02T13:25:49Z

@gacholio / @tajila All tests seems to have passed with this change. If you are ok with the changes, this is good to merge.

gacholio reviewed Jun 23, 2025

View reviewed changes

vij-singh mentioned this pull request Jun 23, 2025

Crash in walkStackFrames #21829

Open

r30shah marked this pull request as draft June 23, 2025 22:19

hzongaro mentioned this pull request Jun 24, 2025

Crash in Compiled_method=<...> or VM Access Assertion (vmThread)->publicFlags & J9_PUBLIC_FLAGS_VM_ACCESS #21824

Open

r30shah force-pushed the proto21717 branch 6 times, most recently from 8ae3fdc to b10f8b5 Compare June 26, 2025 14:43

r30shah marked this pull request as ready for review June 26, 2025 18:15

tajila approved these changes Jun 27, 2025

View reviewed changes

r30shah force-pushed the proto21717 branch from b10f8b5 to e9140a4 Compare June 27, 2025 21:34

r30shah requested a review from dsouzai as a code owner June 27, 2025 21:34

r30shah force-pushed the proto21717 branch from e9140a4 to bfead75 Compare June 27, 2025 21:35

gacholio approved these changes Jul 2, 2025

View reviewed changes

gacholio merged commit 2c544ac into eclipse-openj9:master Jul 2, 2025
12 checks passed

r30shah mentioned this pull request Jul 3, 2025

jdk_lang_j9_1 Java_java_lang_Thread_interruptImpl Segmentation error vmState=0x00000000 #21990

Open

This was referenced Jul 3, 2025

jdk_lang_0 java/lang/Thread/virtual/MiscMonitorTests.java#Xcomp-noTieredCompilation j9codertvm(j9ji.110 ASSERTION FAILED at runtime/codert_vm/jswalk.c:538: ((0 )) #22089

Open

JDK24: VirtualThread: Crash in threadstate() #22047

Open

Z: Restore Non Volatile GPRs in jitExitInterpreter0RestoreAll #22128

Z: Restore Non Volatile GPRs in jitExitInterpreter0RestoreAll #22128

Uh oh!

Conversation

r30shah commented Jun 23, 2025

Uh oh!

r30shah commented Jun 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

r30shah commented Jun 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

r30shah commented Jun 26, 2025

Uh oh!

r30shah commented Jun 27, 2025

Uh oh!

r30shah commented Jun 27, 2025

Uh oh!

r30shah commented Jul 2, 2025

Uh oh!

Uh oh!

Uh oh!

r30shah commented Jun 23, 2025 •

edited

Loading