Revert "[debugger] Fix crash during Async Break when APC and CET are enabled (#111408)" (#116025)

This reverts commit f6ecb6e.

Author: tommcdon

src/coreclr/debug/ee/controller.cpp

@@ -4469,19 +4469,7 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
         ThisFunctionMayHaveTriggerAGC();
     }
 #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

src/coreclr/debug/ee/debugger.cpp

@@ -15001,14 +15001,6 @@ 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
@@ -16780,15 +16772,6 @@ 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
 
 unsigned FuncEvalFrame::GetFrameAttribs_Impl(void)

src/coreclr/debug/ee/debugger.h

@@ -3061,9 +3061,6 @@ 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,9 +412,6 @@ 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

@@ -482,7 +482,6 @@ 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
 
@@ -550,7 +549,7 @@ class Thread
         TS_Hijacked               = 0x00000080,    // Return address has been hijacked
 #endif // FEATURE_HIJACK
 
-        TS_SSToExitApcCall        = 0x00000100,    // Enable SS and resume the thread to exit an APC Call and keep the thread in suspend state
+        // unused                 = 0x00000100,
         TS_Background             = 0x00000200,    // Thread is a background thread
         TS_Unstarted              = 0x00000400,    // Thread has never been started
         TS_Dead                   = 0x00000800,    // Thread is dead
@@ -567,7 +566,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
 
-        TS_SSToExitApcCallDone    = 0x00040000,    // The thread exited an APC Call and it is already resumed and paused on SS
+        // unused                 = 0x00040000,
 
         TS_SyncSuspended          = 0x00080000,    // Suspended via WaitSuspendEvent
         TS_DebugWillSync          = 0x00100000,    // Debugger will wait for this thread to sync
@@ -2571,9 +2570,7 @@ 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

@@ -4210,18 +4210,6 @@ 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
@@ -5346,19 +5334,6 @@ 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 {
@@ -5781,8 +5756,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, bool suspendForDebugger)
+void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
 {
     struct AutoClearPendingThreadActivation
     {
@@ -5818,18 +5792,6 @@ void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool susp
     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();
@@ -5905,11 +5867,6 @@ void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool susp
     }
 }
 
-void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
-{
-    HandleSuspensionForInterruptedThread(interruptedContext, false);
-}
-
 #ifdef FEATURE_SPECIAL_USER_MODE_APC
 void Thread::ApcActivationCallback(ULONG_PTR Parameter)
 {
@@ -5936,10 +5893,10 @@ void Thread::ApcActivationCallback(ULONG_PTR Parameter)
 
     switch (reason)
     {
-        case ActivationReason::SuspendForDebugger:
         case ActivationReason::SuspendForGC:
+        case ActivationReason::SuspendForDebugger:
         case ActivationReason::ThreadAbort:
-            HandleSuspensionForInterruptedThread(pContext, reason == ActivationReason::SuspendForDebugger);
+            HandleSuspensionForInterruptedThread(pContext);
             break;
 
         default: