[debugger] Fix crash during Async Break when APC and CET are enabled

src/coreclr/debug/ee/controller.cpp

@@ -4469,7 +4469,15 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
         ThisFunctionMayHaveTriggerAGC();
     }
 #endif
-
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    if (pCurThread->m_State & Thread::TS_SSToExitApcCall)
+    {
+        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

src/coreclr/debug/ee/debugger.cpp

@@ -15026,6 +15026,14 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo,
         return CORDBG_E_ILLEGAL_IN_STACK_OVERFLOW;
     }
 
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    if (pThread->m_hasPendingActivation)
+    {
+        //It should never get here
+        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
@@ -16797,6 +16805,15 @@ void Debugger::ExternalMethodFixupNextStep(PCODE address)
 {
     DebuggerController::DispatchExternalMethodFixup(address);
 }
+#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

@@ -3063,6 +3063,9 @@ 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();
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    void SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext);
+#endif // FEATURE_SPECIAL_USER_MODE_APC        
 };
 
 

src/coreclr/vm/dbginterface.h

@@ -412,6 +412,9 @@ class DebugInterface
     virtual HRESULT IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BOOL *pResult) = 0;
     virtual void MulticastTraceNextStep(DELEGATEREF pbDel, INT32 count) = 0;
     virtual void ExternalMethodFixupNextStep(PCODE address) = 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

@@ -484,7 +484,9 @@ class Thread
     friend void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext);
     friend BOOL CheckActivationSafePoint(SIZE_T ip);
 #endif // FEATURE_THREAD_ACTIVATION
-
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+    friend void HandleSuspensionForInterruptedThreadForDebugger(CONTEXT *interruptedContext);
+#endif // FEATURE_SPECIAL_USER_MODE_APC
 #endif // FEATURE_HIJACK
 
     friend void         InitThreadManager();
@@ -550,7 +552,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
@@ -567,7 +569,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
@@ -2575,7 +2577,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

@@ -4224,6 +4224,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
@@ -5337,6 +5349,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 {
@@ -5744,6 +5769,58 @@ BOOL CheckActivationSafePoint(SIZE_T ip)
     return isActivationSafePoint;
 }
 
+// This function is called when thread suspension is pending for debugger if APC is enabled.
+// As it's not allowed to change the IP while paused in an APC Callback for security reasons 
+// if CET is enabled, the goal of this function is to enable single step in the paused thread
+// and then pause using single step exception after exit the APC callback
+// this will allow the debugger to setIp to execute FuncEvalHijack.
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+void HandleSuspensionForInterruptedThreadForDebugger(CONTEXT *interruptedContext)
+{
+    struct AutoClearPendingThreadActivation
+    {
+        ~AutoClearPendingThreadActivation()
+        {
+            GetThread()->m_hasPendingActivation = false;
+        }
+    } autoClearPendingThreadActivation;
+
+    Thread *pThread = GetThread();
+
+    if (pThread->PreemptiveGCDisabled() != TRUE)
+        return;
+
+    PCODE ip = GetIP(interruptedContext);
+
+    // This function can only be called when the interrupted thread is in
+    // an activation safe point.
+    _ASSERTE(CheckActivationSafePoint(ip));
+
+    Thread::WorkingOnThreadContextHolder workingOnThreadContext(pThread);
+    if (!workingOnThreadContext.Acquired())
+        return;
+
+#if defined(DEBUGGING_SUPPORTED) && defined(TARGET_WINDOWS)
+    // If we are running under the control of a managed debugger that may have placed breakpoints in the code stream,
+    // then there is a special case that we need to check. See the comments in debugger.cpp for more information.
+    if (CORDebuggerAttached() && g_pDebugInterface->IsThreadContextInvalid(pThread, interruptedContext))
+        return;
+#endif // DEBUGGING_SUPPORTED && TARGET_WINDOWS
+
+    EECodeInfo codeInfo(ip);
+    if (!codeInfo.IsValid())
+        return;
+
+    DWORD addrOffset = codeInfo.GetRelOffset();
+
+    ICodeManager *pEECM = codeInfo.GetCodeManager();
+    _ASSERTE(pEECM != NULL);
+
+    printf("olha o context no HandleSuspensionForInterruptedThreadForDebugger - %p\n", interruptedContext);
+    g_pDebugInterface->SingleStepToExitApcCall(pThread, interruptedContext);
+}
+#endif
+
 // This function is called when thread suspension is pending. It tries to ensure that the current
 // thread is taken to a GC-safe place as quickly as possible. It does this by doing
 // one of the following:
@@ -5895,8 +5972,12 @@ void Thread::ApcActivationCallback(ULONG_PTR Parameter)
 
     switch (reason)
     {
-        case ActivationReason::SuspendForGC:
         case ActivationReason::SuspendForDebugger:
+#ifdef FEATURE_SPECIAL_USER_MODE_APC
+            HandleSuspensionForInterruptedThreadForDebugger(pContext);
+            break;
+#endif
+        case ActivationReason::SuspendForGC:
         case ActivationReason::ThreadAbort:
             HandleSuspensionForInterruptedThread(pContext);
             break;