EE Suspension on x86 should use RtlRestoreContext when available (#65878)

* RestoreContextSimulated

* probe for RtlRestoreContext

* ntdll.dll

* restore self-trap sequence

* PR feedback

* Clarify CopyContext in RedirectedHandledJITCaseExceptionFilter

* simpler indentation.

* restore last error on the legacy path.

* Update src/coreclr/vm/threads.h

Co-authored-by: Dan Moseley <danmose@microsoft.com>

Author: VSadov

src/coreclr/vm/threads.h

@@ -3076,6 +3076,14 @@ class Thread
     static void __stdcall RedirectedHandledJITCaseForGCStress();
 #endif // defined(HAVE_GCCOVER) && USE_REDIRECT_FOR_GCSTRESS
 
+#ifdef TARGET_X86
+    // RtlRestoreContext is available on x86, but relatively recently.
+    // RestoreContextSimulated uses SEH machinery for a similar result on legacy OS-es.
+    // This function should not be used on new OS-es as the pattern is not
+    // guaranteed to continue working in the future.
+    static void RestoreContextSimulated(Thread* pThread, CONTEXT* pCtx, void* pFrame, DWORD dwLastError);
+#endif
+
     friend void CPFH_AdjustContextForThreadSuspensionRace(T_CONTEXT *pContext, Thread *pThread);
 #endif // FEATURE_HIJACK && !TARGET_UNIX
 

src/coreclr/vm/threadsuspend.cpp

@@ -1948,6 +1948,9 @@ typedef BOOL(WINAPI* PINITIALIZECONTEXT2)(PVOID Buffer, DWORD ContextFlags, PCON
 PINITIALIZECONTEXT2 pfnInitializeContext2 = NULL;
 
 #ifdef TARGET_X86
+typedef VOID(__cdecl* PRTLRESTORECONTEXT)(PCONTEXT ContextRecord, struct _EXCEPTION_RECORD* ExceptionRecord);
+PRTLRESTORECONTEXT pfnRtlRestoreContext = NULL;
+
 #define CONTEXT_COMPLETE (CONTEXT_FULL | CONTEXT_FLOATING_POINT |       \
                           CONTEXT_DEBUG_REGISTERS | CONTEXT_EXTENDED_REGISTERS | CONTEXT_EXCEPTION_REQUEST)
 #else
@@ -1967,6 +1970,14 @@ CONTEXT* AllocateOSContextHelper(BYTE** contextBuffer)
         pfnInitializeContext2 = (PINITIALIZECONTEXT2)GetProcAddress(hm, "InitializeContext2");
     }
 
+#ifdef TARGET_X86
+    if (pfnRtlRestoreContext == NULL)
+    {
+        HMODULE hm = GetModuleHandleW(_T("ntdll.dll"));
+        pfnRtlRestoreContext = (PRTLRESTORECONTEXT)GetProcAddress(hm, "RtlRestoreContext");
+    }
+#endif //TARGET_X86
+
     // Determine if the processor supports AVX so we could
     // retrieve extended registers
     DWORD64 FeatureMask = GetEnabledXStateFeatures();
@@ -2515,18 +2526,18 @@ void RedirectedThreadFrame::ExceptionUnwind()
 int RedirectedHandledJITCaseExceptionFilter(
     PEXCEPTION_POINTERS pExcepPtrs,     // Exception data
     RedirectedThreadFrame *pFrame,      // Frame on stack
-    BOOL fDone,                         // Whether redirect completed without exception
-    CONTEXT *pCtx)                      // Saved context
+    CONTEXT *pCtx,                      // Saved context
+    DWORD dwLastError)                  // saved last error
 {
     // !!! Do not use a non-static contract here.
     // !!! Contract may insert an exception handling record.
     // !!! This function assumes that GetCurrentSEHRecord() returns the exception record set up in
-    // !!! Thread::RedirectedHandledJITCase
+    // !!! Thread::RestoreContextSimulated
     //
     // !!! Do not use an object with dtor, since it injects a fs:0 entry.
     STATIC_CONTRACT_NOTHROW;
     STATIC_CONTRACT_GC_TRIGGERS;
-    STATIC_CONTRACT_MODE_ANY;
+    STATIC_CONTRACT_MODE_COOPERATIVE;
 
     if (pExcepPtrs->ExceptionRecord->ExceptionCode == STATUS_STACK_OVERFLOW)
     {
@@ -2535,48 +2546,18 @@ int RedirectedHandledJITCaseExceptionFilter(
 
     // Get the thread handle
     Thread *pThread = GetThread();
-
-    STRESS_LOG2(LF_SYNC, LL_INFO100, "In RedirectedHandledJITCaseExceptionFilter fDone = %d pFrame = %p\n", fDone, pFrame);
-
-    // If we get here via COM+ exception, gc-mode is unknown.  We need it to
-    // be cooperative for this function.
-    GCX_COOP_NO_DTOR();
-
-    // If the exception was due to the called client, then we need to figure out if it
-    // is an exception that can be eaten or if it needs to be handled elsewhere.
-    if (!fDone)
-    {
-        if (pExcepPtrs->ExceptionRecord->ExceptionFlags & EXCEPTION_NONCONTINUABLE)
-        {
-            return (EXCEPTION_CONTINUE_SEARCH);
-        }
-
-        // Get the latest thrown object
-        OBJECTREF throwable = CLRException::GetThrowableFromExceptionRecord(pExcepPtrs->ExceptionRecord);
-
-        // If this is an uncatchable exception, then let the exception be handled elsewhere
-        if (IsUncatchable(&throwable))
-        {
-            pThread->EnablePreemptiveGC();
-            return (EXCEPTION_CONTINUE_SEARCH);
-        }
-    }
-#ifdef _DEBUG
-    else
-    {
-        _ASSERTE(pExcepPtrs->ExceptionRecord->ExceptionCode == EXCEPTION_HIJACK);
-    }
-#endif
+    STRESS_LOG1(LF_SYNC, LL_INFO100, "In RedirectedHandledJITCaseExceptionFilter pFrame = %p\n", pFrame);
+    _ASSERTE(pExcepPtrs->ExceptionRecord->ExceptionCode == EXCEPTION_HIJACK);
 
     // Unlink the frame in preparation for resuming in managed code
     pFrame->Pop();
 
-    // Copy the saved context record into the EH context;
-    // NB: cannot use ReplaceExceptionContextRecord here.
-    //     these contexts may contain extended registers and may have different format
-    //     for reasons such as alignment or context compaction
+    // Copy everything in the saved context record into the EH context.
+    // Historically the EH context has enough space for every enabled context feature.
+    // That may not hold for the future features beyond AVX, but this codepath is
+    // supposed to be used only on OSes that do not have RtlRestoreContext.
     CONTEXT* pTarget = pExcepPtrs->ContextRecord;
-    if (!CopyContext(pTarget, pTarget->ContextFlags, pCtx))
+    if (!CopyContext(pTarget, pCtx->ContextFlags, pCtx))
     {
         STRESS_LOG1(LF_SYNC, LL_ERROR, "ERROR: Could not set context record, lastError = 0x%x\n", GetLastError());
         EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE);
@@ -2610,6 +2591,9 @@ int RedirectedHandledJITCaseExceptionFilter(
     // Register the special OS handler as the top handler with the OS
     SetCurrentSEHRecord(pCurSEH);
 
+    // restore last error
+    SetLastError(dwLastError);
+
     // Resume execution at point where thread was originally redirected
     return (EXCEPTION_CONTINUE_EXECUTION);
 }
@@ -2642,6 +2626,38 @@ extern "C" PCONTEXT __stdcall GetCurrentSavedRedirectContext()
     return pContext;
 }
 
+#ifdef TARGET_X86
+
+void Thread::RestoreContextSimulated(Thread* pThread, CONTEXT* pCtx, void* pFrame, DWORD dwLastError)
+{
+    pThread->HandleThreadAbort();        // Might throw an exception.
+
+    // A counter to avoid a nasty case where an
+    // up-stack filter throws another exception
+    // causing our filter to be run again for
+    // some unrelated exception.
+    int filter_count = 0;
+
+    __try
+    {
+        // Save the instruction pointer where we redirected last.  This does not race with the check
+        // against this variable in HandledJitCase because the GC will not attempt to redirect the
+        // thread until the instruction pointer of this thread is back in managed code.
+        pThread->m_LastRedirectIP = GetIP(pCtx);
+        pThread->m_SpinCount = 0;
+
+        RaiseException(EXCEPTION_HIJACK, 0, 0, NULL);
+    }
+    __except (++filter_count == 1
+            ? RedirectedHandledJITCaseExceptionFilter(GetExceptionInformation(), (RedirectedThreadFrame*)pFrame, pCtx, dwLastError)
+            : EXCEPTION_CONTINUE_SEARCH)
+    {
+        _ASSERTE(!"Reached body of __except in Thread::RedirectedHandledJITCase");
+    }
+}
+
+#endif // TARGET_X86
+
 void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason)
 {
     STATIC_CONTRACT_THROWS;
@@ -2665,140 +2681,106 @@ void __stdcall Thread::RedirectedHandledJITCase(RedirectReason reason)
 
     STRESS_LOG5(LF_SYNC, LL_INFO1000, "In RedirectedHandledJITcase reason 0x%x pFrame = %p pc = %p sp = %p fp = %p", reason, &frame, GetIP(pCtx), GetSP(pCtx), GetFP(pCtx));
 
-#ifdef TARGET_X86
-    // This will indicate to the exception filter whether or not the exception is caused
-    // by us or the client.
-    BOOL fDone = FALSE;
-    int filter_count = 0;       // A counter to avoid a nasty case where an
-                                // up-stack filter throws another exception
-                                // causing our filter to be run again for
-                                // some unrelated exception.
-
-    __try
-#endif // TARGET_X86
-    {
-        // Make sure this thread doesn't reuse the context memory.
-        pThread->MarkRedirectContextInUse(pCtx);
+    // Make sure this thread doesn't reuse the context memory.
+    pThread->MarkRedirectContextInUse(pCtx);
 
-        // Link in the frame
-        frame.Push();
+    // Link in the frame
+    frame.Push();
 
 #if defined(HAVE_GCCOVER) && defined(USE_REDIRECT_FOR_GCSTRESS) // GCCOVER
-        if (reason == RedirectReason_GCStress)
-        {
-            _ASSERTE(pThread->PreemptiveGCDisabledOther());
-            DoGcStress(frame.GetContext(), NULL);
-        }
-        else
+    if (reason == RedirectReason_GCStress)
+    {
+        _ASSERTE(pThread->PreemptiveGCDisabledOther());
+        DoGcStress(frame.GetContext(), NULL);
+    }
+    else
 #endif // HAVE_GCCOVER && USE_REDIRECT_FOR_GCSTRESS
-        {
-            // Enable PGC before calling out to the client to allow runtime suspend to finish
-            GCX_PREEMP_NO_DTOR();
+    {
+        _ASSERTE(reason == RedirectReason_GCSuspension ||
+                    reason == RedirectReason_DebugSuspension ||
+                    reason == RedirectReason_UserSuspension);
 
-            // Notify the interface of the pending suspension
-            switch (reason) {
-            case RedirectReason_GCSuspension:
-                break;
-            case RedirectReason_DebugSuspension:
-                break;
-            case RedirectReason_UserSuspension:
-                // Do nothing;
-                break;
-            default:
-                _ASSERTE(!"Invalid redirect reason");
-                break;
-            }
+        // Actual self-suspension.
+        // Leave and reenter COOP mode to be trapped on the way back.
+        GCX_PREEMP_NO_DTOR();
+        GCX_PREEMP_NO_DTOR_END();
+    }
 
-            // Disable preemptive GC so we can unlink the frame
-            GCX_PREEMP_NO_DTOR_END();
-        }
+    // Once we get here the suspension is over!
+    // We will restore the state as it was at the point of redirection
+    // and continue normal execution.
 
 #ifdef TARGET_X86
-        pThread->HandleThreadAbort();        // Might throw an exception.
-
-        // Indicate that the call to the service went without an exception, and that
-        // we're raising our own exception to resume the thread to where it was
-        // redirected from
-        fDone = TRUE;
-
-        // Save the instruction pointer where we redirected last.  This does not race with the check
-        // against this variable in HandledJitCase because the GC will not attempt to redirect the
-        // thread until the instruction pointer of this thread is back in managed code.
-        pThread->m_LastRedirectIP = GetIP(pCtx);
-        pThread->m_SpinCount = 0;
-
-        RaiseException(EXCEPTION_HIJACK, 0, 0, NULL);
+    if (!pfnRtlRestoreContext)
+    {
+        RestoreContextSimulated(pThread, pCtx, &frame, dwLastError);
 
-#else // TARGET_X86
+        // we never return to the caller.
+        __UNREACHABLE();
+    }
+#endif // TARGET_X86
 
 #if defined(HAVE_GCCOVER) && defined(USE_REDIRECT_FOR_GCSTRESS) // GCCOVER
-        //
-        // If GCStress interrupts an IL stub or inlined p/invoke while it's running in preemptive mode, it switches the mode to
-        // cooperative - but we will resume to preemptive below.  We should not trigger an abort in that case, as it will fail
-        // due to the GC mode.
-        //
-        if (!pThread->m_fPreemptiveGCDisabledForGCStress)
+    //
+    // If GCStress interrupts an IL stub or inlined p/invoke while it's running in preemptive mode, it switches the mode to
+    // cooperative - but we will resume to preemptive below.  We should not trigger an abort in that case, as it will fail
+    // due to the GC mode.
+    //
+    if (!pThread->m_fPreemptiveGCDisabledForGCStress)
 #endif
-        {
+    {
 
-            UINT_PTR uAbortAddr;
-            UINT_PTR uResumePC = (UINT_PTR)GetIP(pCtx);
-            CopyOSContext(pThread->m_OSContext, pCtx);
-            uAbortAddr = (UINT_PTR)COMPlusCheckForAbort();
-            if (uAbortAddr)
-            {
-                LOG((LF_EH, LL_INFO100, "thread abort in progress, resuming thread under control... (handled jit case)\n"));
+        UINT_PTR uAbortAddr;
+        UINT_PTR uResumePC = (UINT_PTR)GetIP(pCtx);
+        CopyOSContext(pThread->m_OSContext, pCtx);
+        uAbortAddr = (UINT_PTR)COMPlusCheckForAbort();
+        if (uAbortAddr)
+        {
+            LOG((LF_EH, LL_INFO100, "thread abort in progress, resuming thread under control... (handled jit case)\n"));
 
-                CONSISTENCY_CHECK(CheckPointer(pCtx));
+            CONSISTENCY_CHECK(CheckPointer(pCtx));
 
-                STRESS_LOG1(LF_EH, LL_INFO10, "resume under control: ip: %p (handled jit case)\n", uResumePC);
+            STRESS_LOG1(LF_EH, LL_INFO10, "resume under control: ip: %p (handled jit case)\n", uResumePC);
 
-                SetIP(pThread->m_OSContext, uResumePC);
+            SetIP(pThread->m_OSContext, uResumePC);
 
 #if defined(TARGET_ARM)
-                // Save the original resume PC in Lr
-                pCtx->Lr = uResumePC;
+            // Save the original resume PC in Lr
+            pCtx->Lr = uResumePC;
 
-                // Since we have set a new IP, we have to clear conditional execution flags too.
-                ClearITState(pThread->m_OSContext);
+            // Since we have set a new IP, we have to clear conditional execution flags too.
+            ClearITState(pThread->m_OSContext);
 #endif // TARGET_ARM
 
-                SetIP(pCtx, uAbortAddr);
-            }
+            SetIP(pCtx, uAbortAddr);
         }
+    }
 
-        // Unlink the frame in preparation for resuming in managed code
-        frame.Pop();
+    // Unlink the frame in preparation for resuming in managed code
+    frame.Pop();
 
-        {
-            // Allow future use of the context
-            pThread->UnmarkRedirectContextInUse(pCtx);
+    // Allow future use of the context
+    pThread->UnmarkRedirectContextInUse(pCtx);
 
 #if defined(HAVE_GCCOVER) && defined(USE_REDIRECT_FOR_GCSTRESS) // GCCOVER
-            if (pThread->m_fPreemptiveGCDisabledForGCStress)
-            {
-                pThread->EnablePreemptiveGC();
-                pThread->m_fPreemptiveGCDisabledForGCStress = false;
-            }
+    if (pThread->m_fPreemptiveGCDisabledForGCStress)
+    {
+        pThread->EnablePreemptiveGC();
+        pThread->m_fPreemptiveGCDisabledForGCStress = false;
+    }
 #endif
 
-            LOG((LF_SYNC, LL_INFO1000, "Resuming execution with RtlRestoreContext\n"));
-
-            SetLastError(dwLastError); // END_PRESERVE_LAST_ERROR
+    LOG((LF_SYNC, LL_INFO1000, "Resuming execution with RtlRestoreContext\n"));
+    SetLastError(dwLastError); // END_PRESERVE_LAST_ERROR
 
-            RtlRestoreContext(pCtx, NULL);
-        }
-#endif // TARGET_X86
-    }
 #ifdef TARGET_X86
-    __except (++filter_count == 1
-        ? RedirectedHandledJITCaseExceptionFilter(GetExceptionInformation(), &frame, fDone, pCtx)
-        : EXCEPTION_CONTINUE_SEARCH)
-    {
-        _ASSERTE(!"Reached body of __except in Thread::RedirectedHandledJITCase");
-    }
+    pfnRtlRestoreContext(pCtx, NULL);
+#else
+    RtlRestoreContext(pCtx, NULL);
+#endif
 
-#endif // TARGET_X86
+    // we never return to the caller.
+    __UNREACHABLE();
 }
 
 //****************************************************************************************