[release/9.0-staging] Fix crash during Async Break when APC and CET are enabled (#114932)

* backport 111408

* Fix compilation failure

* Fix compilation

* trying to fix compilation

* Trying to fix compilation

Author: thaystg

src/coreclr/debug/ee/controller.cpp

@@ -4312,7 +4312,19 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
     }
 #endif
 
-
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+     if (pCurThread->m_State & Thread::TS_SSToExitApcCall)
+     {
+         if (!CheckActivationSafePoint(GetIP(pContext)))
+         {
+             return FALSE;
+         }
+         pCurThread->SetThreadState(Thread::TS_SSToExitApcCallDone);
+         pCurThread->ResetThreadState(Thread::TS_SSToExitApcCall);        
+         DebuggerController::UnapplyTraceFlag(pCurThread);
+         pCurThread->MarkForSuspensionAndWait(Thread::TS_DebugSuspendPending);
+     }
+#endif
 
     // Must restore the filter context. After the filter context is gone, we're
     // unprotected again and unsafe for a GC.

src/coreclr/debug/ee/debugger.cpp

@@ -15075,6 +15075,14 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo,
         return CORDBG_E_ILLEGAL_IN_STACK_OVERFLOW;
     }
 
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    if (pThread->m_hasPendingActivation)
+    {
+        _ASSERTE(!"Should never get here with a pending activation. (Debugger::FuncEvalSetup)");
+        return CORDBG_E_ILLEGAL_IN_NATIVE_CODE;
+    }
+#endif  
+
     bool fInException = pEvalInfo->evalDuringException;
 
     // The thread has to be at a GC safe place for now, just in case the func eval causes a collection. Processing an
@@ -16732,7 +16740,6 @@ Debugger::EnumMemoryRegionsIfFuncEvalFrame(CLRDataEnumMemoryFlags flags, Frame *
         }
     }
 }
-
 #endif // #ifdef DACCESS_COMPILE
 
 #ifndef DACCESS_COMPILE
@@ -16820,7 +16827,6 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo
     LOG((LF_CORDB, LL_INFO10000, "D::SSTCN SetThreadContextNeededFlare returned\n"));
     _ASSERTE(!"We failed to SetThreadContext from out of process!");
 }
-
 BOOL Debugger::IsOutOfProcessSetContextEnabled()
 {
     return m_fOutOfProcessSetContextEnabled;
@@ -16837,6 +16843,16 @@ BOOL Debugger::IsOutOfProcessSetContextEnabled()
 }
 #endif // OUT_OF_PROCESS_SETTHREADCONTEXT
 #endif // DACCESS_COMPILE
-
+#ifndef DACCESS_COMPILE
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+void Debugger::SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext)
+{
+    pThread->SetThreadState(Thread::TS_SSToExitApcCall);
+    g_pEEInterface->SetThreadFilterContext(pThread, interruptedContext);
+    DebuggerController::EnableSingleStep(pThread);
+    g_pEEInterface->SetThreadFilterContext(pThread, NULL);
+}
+#endif //FEATURE_SPECIAL_USER_MODE_APC
+#endif // DACCESS_COMPILE
 #endif //DEBUGGING_SUPPORTED
 

src/coreclr/debug/ee/debugger.h

@@ -3098,6 +3098,11 @@ class Debugger : public DebugInterface
     // Used by Debugger::FirstChanceNativeException to update the context from out of process
     void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL);
     BOOL IsOutOfProcessSetContextEnabled();
+#ifndef DACCESS_COMPILE
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    void SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext);
+#endif // FEATURE_SPECIAL_USER_MODE_APC
+#endif
 };
 
 

src/coreclr/vm/dbginterface.h

@@ -414,6 +414,9 @@ class DebugInterface
 #ifndef DACCESS_COMPILE
     virtual HRESULT DeoptimizeMethod(Module* pModule, mdMethodDef methodDef) = 0;
     virtual HRESULT IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BOOL *pResult) = 0;
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    virtual void SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext) = 0;
+#endif // FEATURE_SPECIAL_USER_MODE_APC     
 #endif //DACCESS_COMPILE
 };
 

src/coreclr/vm/threads.h

@@ -489,6 +489,7 @@ class Thread
     friend void STDCALL OnHijackWorker(HijackArgs * pArgs);
 #ifdef FEATURE_THREAD_ACTIVATION
     friend void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext);
+    friend void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool suspendForDebugger);    
     friend BOOL CheckActivationSafePoint(SIZE_T ip);
 #endif // FEATURE_THREAD_ACTIVATION
 
@@ -557,7 +558,7 @@ class Thread
         TS_Hijacked               = 0x00000080,    // Return address has been hijacked
 #endif // FEATURE_HIJACK
 
-        // unused                 = 0x00000100,
+        TS_SSToExitApcCall        = 0x00000100,    // Enable SS and resume the thread to exit an APC Call and keep the thread in suspend state
         TS_Background             = 0x00000200,    // Thread is a background thread
         TS_Unstarted              = 0x00000400,    // Thread has never been started
         TS_Dead                   = 0x00000800,    // Thread is dead
@@ -574,7 +575,7 @@ class Thread
         TS_ReportDead             = 0x00010000,    // in WaitForOtherThreads()
         TS_FullyInitialized       = 0x00020000,    // Thread is fully initialized and we are ready to broadcast its existence to external clients
 
-        // unused                 = 0x00040000,
+        TS_SSToExitApcCallDone    = 0x00040000,    // The thread exited an APC Call and it is already resumed and paused on SS
 
         TS_SyncSuspended          = 0x00080000,    // Suspended via WaitSuspendEvent
         TS_DebugWillSync          = 0x00100000,    // Debugger will wait for this thread to sync
@@ -2568,6 +2569,9 @@ class Thread
     // Waiting & Synchronization
     //-------------------------------------------------------------
 
+friend class DebuggerController;
+protected:
+    void MarkForSuspensionAndWait(ULONG bit);
     // For suspends.  The thread waits on this event.  A client sets the event to cause
     // the thread to resume.
     void    WaitSuspendEvents();

src/coreclr/vm/threadsuspend.cpp

@@ -4238,6 +4238,18 @@ bool Thread::SysSweepThreadsForDebug(bool forceSync)
         if ((thread->m_State & TS_DebugWillSync) == 0)
             continue;
 
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+        if (thread->m_State & Thread::TS_SSToExitApcCallDone)
+        {
+            thread->ResetThreadState(Thread::TS_SSToExitApcCallDone);
+            goto Label_MarkThreadAsSynced;
+        }
+        if (thread->m_State & Thread::TS_SSToExitApcCall)
+        {
+            continue;
+        }
+ #endif
+
         if (!UseContextBasedThreadRedirection())
         {
             // On platforms that do not support safe thread suspension we either
@@ -5353,6 +5365,19 @@ BOOL Thread::HandledJITCase()
 #endif // FEATURE_HIJACK
 
 // Some simple helpers to keep track of the threads we are waiting for
+void Thread::MarkForSuspensionAndWait(ULONG bit)
+{
+    CONTRACTL {
+        NOTHROW;
+        GC_NOTRIGGER;
+    }
+    CONTRACTL_END;
+    m_DebugSuspendEvent.Reset();
+    InterlockedOr((LONG*)&m_State, bit);
+    ThreadStore::IncrementTrapReturningThreads();
+    m_DebugSuspendEvent.Wait(INFINITE,FALSE);
+}
+
 void Thread::MarkForSuspension(ULONG bit)
 {
     CONTRACTL {
@@ -5775,7 +5800,7 @@ BOOL CheckActivationSafePoint(SIZE_T ip)
 //       address to take the thread to the appropriate stub (based on the return
 //       type of the method) which will then handle preparing the thread for GC.
 //
-void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
+void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool suspendForDebugger)
 {
     struct AutoClearPendingThreadActivation
     {
@@ -5811,6 +5836,18 @@ void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
     if (!codeInfo.IsValid())
         return;
 
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    // It's not allowed to change the IP while paused in an APC Callback for security reasons if CET is turned on
+    // So we enable the single step in the thread that is running the APC Callback
+    // and then it will be paused using single step exception after exiting the APC callback
+    // this will allow the debugger to setIp to execute FuncEvalHijack.
+    if (suspendForDebugger)
+    {
+        g_pDebugInterface->SingleStepToExitApcCall(pThread, interruptedContext);
+        return;
+    }
+ #endif    
+
     DWORD addrOffset = codeInfo.GetRelOffset();
 
     ICodeManager *pEECM = codeInfo.GetCodeManager();
@@ -5886,6 +5923,11 @@ void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
     }
 }
 
+void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
+{
+    HandleSuspensionForInterruptedThread(interruptedContext, false);
+}
+
 #ifdef FEATURE_SPECIAL_USER_MODE_APC
 void Thread::ApcActivationCallback(ULONG_PTR Parameter)
 {
@@ -5915,7 +5957,7 @@ void Thread::ApcActivationCallback(ULONG_PTR Parameter)
         case ActivationReason::SuspendForGC:
         case ActivationReason::SuspendForDebugger:
         case ActivationReason::ThreadAbort:
-            HandleSuspensionForInterruptedThread(pContext);
+            HandleSuspensionForInterruptedThread(pContext, reason == ActivationReason::SuspendForDebugger);
             break;
 
         default: