[NativeAOT] Some cleanup of assembly helpers in hijack area. (#72542)

* remove RhpReversePInvokeAttachOrTrapThread (dead code) * remove RhpWaitForGC * removing some dead code * remove unused extraStack parameter to PUSH_PROBE_FRAME * make all working variants of RhpGcProbeHijack to have the same shape * fix Unix build, tweak some comments * check the trap flag in RhpGcProbeHijack * add a stub for RhpGcStressHijack on Unix * save flags for windows x64 should not have rdx * not saving scratch registers on win-arm64 * PUSH_PROBE_FRAME on arm64 * couple comments * make RhpGcProbeHijack responsible for setting PTFF_SAVE_ bits * revert `RhpReversePInvokeAttachOrTrapThread2` change * fix indentation * made 32bit RhpGcStressHijack similar to 64bit counterparts (as much as can be done without trying to compile and test) * Renamed `RhpGcProbe` --> `RhpWaitForGC`
dotnet · Jul 22, 2022 · 41def7c · 41def7c
1 parent 8c71651
commit 41def7c
Show file tree

Hide file tree

Showing 19 changed files with 269 additions and 1,848 deletions.
diff --git a/src/coreclr/nativeaot/Runtime/ICodeManager.h b/src/coreclr/nativeaot/Runtime/ICodeManager.h
@@ -53,10 +53,9 @@ C_ASSERT(PTFF_X1_IS_BYREF == ((uint64_t)GCRK_Scalar_Byref << 32));
 
 inline uint64_t ReturnKindToTransitionFrameFlags(GCRefKind returnKind)
 {
-    if (returnKind == GCRK_Scalar)
-        return 0;
-
-    return PTFF_SAVE_X0 | PTFF_SAVE_X1 | ((uint64_t)returnKind << 32);
+    // just need to report gc ref bits here.
+    // appropriate PTFF_SAVE_ bits will be added by the frame building routine.
+    return ((uint64_t)returnKind << 32);
 }
 
 inline GCRefKind TransitionFrameFlagsToReturnKind(uint64_t transFrameFlags)
@@ -76,10 +75,9 @@ C_ASSERT(PTFF_RDX_IS_BYREF == ((uint64_t)GCRK_Scalar_Byref << 16));
 
 inline uint64_t ReturnKindToTransitionFrameFlags(GCRefKind returnKind)
 {
-    if (returnKind == GCRK_Scalar)
-        return 0;
-
-    return PTFF_SAVE_RAX | PTFF_SAVE_RDX | ((uint64_t)returnKind << 16);
+    // just need to report gc ref bits here.
+    // appropriate PTFF_SAVE_ bits will be added by the frame building routine.
+    return ((uint64_t)returnKind << 16);
 }
 
 inline GCRefKind TransitionFrameFlagsToReturnKind(uint64_t transFrameFlags)

diff --git a/src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc b/src/coreclr/nativeaot/Runtime/amd64/AsmMacros.inc
@@ -396,7 +396,6 @@ EXTERN RhpGcAlloc                               : PROC
 EXTERN RhpValidateExInfoPop                     : PROC
 EXTERN RhDebugBreak                             : PROC
 EXTERN RhpWaitForGC2                            : PROC
-EXTERN RhpReversePInvokeAttachOrTrapThread2     : PROC
 EXTERN RhExceptionHandling_FailedAllocation     : PROC
 EXTERN RhThrowHwEx                              : PROC
 EXTERN RhThrowEx                                : PROC

diff --git a/src/coreclr/nativeaot/Runtime/amd64/GcProbe.S b/src/coreclr/nativeaot/Runtime/amd64/GcProbe.S
@@ -67,24 +67,6 @@
     pop         rdx
 .endm
 
-//
-// Macro to clear the hijack state. This is safe to do because the suspension code will not Unhijack this
-// thread if it finds it at an IP that isn`t managed code.
-//
-// Register state on entry:
-//  R11: thread pointer
-//
-// Register state on exit:
-//  R9: trashed
-//
-.macro ClearHijackState
-        xor         r9, r9
-        mov         [r11 + OFFSETOF__Thread__m_ppvHijackedReturnAddressLocation], r9
-        mov         [r11 + OFFSETOF__Thread__m_pvHijackedReturnAddress], r9
-        mov         [r11 + OFFSETOF__Thread__m_uHijackedReturnValueFlags], r9
-.endm
-
-
 //
 // The prolog for all GC suspension hijacks (normal and stress). Fixes up the hijacked return address, and
 // clears the hijack state.
@@ -98,7 +80,7 @@
 //  RAX, RDX preserved, other volatile regs trashed
 //
 .macro FixupHijackedCallstack
-        // preserve RAX, RDX as they may contain retuvalues
+        // preserve RAX, RDX as they may contain return values
         push rax
         push rdx
 
@@ -109,88 +91,43 @@
         pop rdx
         pop rax
 
-        //
         // Fix the stack by pushing the original return address
-        //
         mov         rcx, [r11 + OFFSETOF__Thread__m_pvHijackedReturnAddress]
         push        rcx
 
+        // Fetch the return address flags
         mov         rcx, [r11 + OFFSETOF__Thread__m_uHijackedReturnValueFlags]
 
-        ClearHijackState
-.endm
-
-//////////////////////////////////////////////////////////////////////////////////////////////////////////////
-//
-// RhpWaitForGCNoAbort -- rare path for WaitForGCCompletion
-//
-//
-// INPUT: RDI: transition frame
-//
-// TRASHES: RCX, RDI, R8, R9, R10, R11
-//
-//////////////////////////////////////////////////////////////////////////////////////////////////////////////
-NESTED_ENTRY RhpWaitForGCNoAbort, _TEXT, NoHandler
-        END_PROLOGUE
-
-        mov         rdx, [rdi + OFFSETOF__PInvokeTransitionFrame__m_pThread]
-
-        test        dword ptr [rdx + OFFSETOF__Thread__m_ThreadStateFlags], TSF_DoNotTriggerGc
-        jnz         Done
-
-        // passing transition frame pointer in rdi
-        call        C_FUNC(RhpWaitForGC2)
-
-Done:
-        ret
-
-NESTED_END RhpWaitForGCNoAbort, _TEXT
-
-//
-// Set the Thread state and wait for a GC to complete.
-//
-// Register state on entry:
-//  RBX: thread pointer
-//
-// Register state on exit:
-//  RBX: thread pointer
-//  All other registers trashed
-//
-
-.macro WaitForGCCompletion
-        test        dword ptr [rbx + OFFSETOF__Thread__m_ThreadStateFlags], TSF_SuppressGcStress + TSF_DoNotTriggerGc
-        jnz         LOCAL_LABEL(NoWait)
-
-        mov         rdi, [rbx + OFFSETOF__Thread__m_pDeferredTransitionFrame]
-        call        C_FUNC(RhpWaitForGCNoAbort)
-LOCAL_LABEL(NoWait):
-
+        // Clear hijack state
+        xor         r9, r9
+        mov         [r11 + OFFSETOF__Thread__m_ppvHijackedReturnAddressLocation], r9
+        mov         [r11 + OFFSETOF__Thread__m_pvHijackedReturnAddress], r9
+        mov         [r11 + OFFSETOF__Thread__m_uHijackedReturnValueFlags], r9
 .endm
 
-//
-//
 //
 // GC Probe Hijack target
 //
-//
-
 NESTED_ENTRY RhpGcProbeHijack, _TEXT, NoHandler
         END_PROLOGUE
         FixupHijackedCallstack
-        or          ecx, DEFAULT_FRAME_SAVE_FLAGS  + PTFF_SAVE_RAX + PTFF_SAVE_RDX
-        jmp         C_FUNC(RhpGcProbe)
-NESTED_END RhpGcProbeHijack, _TEXT
 
-NESTED_ENTRY RhpGcProbe, _TEXT, NoHandler
         test        dword ptr [C_VAR(RhpTrapThreads)], TrapThreadsFlags_TrapThreads
-        jnz         LOCAL_LABEL(RhpGcProbe_Trap)
+        jnz         LOCAL_LABEL(WaitForGC)
         ret
-LOCAL_LABEL(RhpGcProbe_Trap):
+
+LOCAL_LABEL(WaitForGC):
+        or          ecx, DEFAULT_FRAME_SAVE_FLAGS  + PTFF_SAVE_RAX + PTFF_SAVE_RDX
+        jmp         C_FUNC(RhpWaitForGC)
+NESTED_END RhpGcProbeHijack, _TEXT
+
+NESTED_ENTRY RhpWaitForGC, _TEXT, NoHandler
         PUSH_PROBE_FRAME r11, rax, rcx
         END_PROLOGUE
 
         mov         rbx, r11
-        WaitForGCCompletion
+        mov         rdi, [rbx + OFFSETOF__Thread__m_pDeferredTransitionFrame]
+        call        C_FUNC(RhpWaitForGC2)
 
         mov         rax, [rbx + OFFSETOF__Thread__m_pDeferredTransitionFrame]
         test        dword ptr [rax + OFFSETOF__PInvokeTransitionFrame__m_Flags], PTFF_THREAD_ABORT
@@ -203,7 +140,7 @@ LOCAL_LABEL(Abort):
         pop         rdx         // return address as exception RIP
         jmp         C_FUNC(RhpThrowHwEx) // Throw the ThreadAbortException as a special kind of hardware exception
 
-NESTED_END RhpGcProbe, _TEXT
+NESTED_END RhpWaitForGC, _TEXT
 
 
 LEAF_ENTRY RhpGcPoll, _TEXT
@@ -212,7 +149,6 @@ LEAF_ENTRY RhpGcPoll, _TEXT
         ret
 LOCAL_LABEL(RhpGcPoll_RarePath):
         jmp C_FUNC(RhpGcPollRare)
-
 LEAF_END RhpGcPoll, _TEXT
 
 NESTED_ENTRY RhpGcPollRare, _TEXT, NoHandler