Skip to content

Commit c7e6e39

Browse files
authored
Revert "[release/9.0-staging] Fix crash during Async Break when APC and CET a…" (#116015)
This reverts commit 753e467.
1 parent 55a7a99 commit c7e6e39

File tree

6 files changed

+8
-90
lines changed

6 files changed

+8
-90
lines changed

src/coreclr/debug/ee/controller.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4312,19 +4312,7 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
43124312
}
43134313
#endif
43144314

4315-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
4316-
if (pCurThread->m_State & Thread::TS_SSToExitApcCall)
4317-
{
4318-
if (!CheckActivationSafePoint(GetIP(pContext)))
4319-
{
4320-
return FALSE;
4321-
}
4322-
pCurThread->SetThreadState(Thread::TS_SSToExitApcCallDone);
4323-
pCurThread->ResetThreadState(Thread::TS_SSToExitApcCall);
4324-
DebuggerController::UnapplyTraceFlag(pCurThread);
4325-
pCurThread->MarkForSuspensionAndWait(Thread::TS_DebugSuspendPending);
4326-
}
4327-
#endif
4315+
43284316

43294317
// Must restore the filter context. After the filter context is gone, we're
43304318
// unprotected again and unsafe for a GC.

src/coreclr/debug/ee/debugger.cpp

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15075,14 +15075,6 @@ HRESULT Debugger::FuncEvalSetup(DebuggerIPCE_FuncEvalInfo *pEvalInfo,
1507515075
return CORDBG_E_ILLEGAL_IN_STACK_OVERFLOW;
1507615076
}
1507715077

15078-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
15079-
if (pThread->m_hasPendingActivation)
15080-
{
15081-
_ASSERTE(!"Should never get here with a pending activation. (Debugger::FuncEvalSetup)");
15082-
return CORDBG_E_ILLEGAL_IN_NATIVE_CODE;
15083-
}
15084-
#endif
15085-
1508615078
bool fInException = pEvalInfo->evalDuringException;
1508715079

1508815080
// The thread has to be at a GC safe place for now, just in case the func eval causes a collection. Processing an
@@ -16740,6 +16732,7 @@ Debugger::EnumMemoryRegionsIfFuncEvalFrame(CLRDataEnumMemoryFlags flags, Frame *
1674016732
}
1674116733
}
1674216734
}
16735+
1674316736
#endif // #ifdef DACCESS_COMPILE
1674416737

1674516738
#ifndef DACCESS_COMPILE
@@ -16827,6 +16820,7 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo
1682716820
LOG((LF_CORDB, LL_INFO10000, "D::SSTCN SetThreadContextNeededFlare returned\n"));
1682816821
_ASSERTE(!"We failed to SetThreadContext from out of process!");
1682916822
}
16823+
1683016824
BOOL Debugger::IsOutOfProcessSetContextEnabled()
1683116825
{
1683216826
return m_fOutOfProcessSetContextEnabled;
@@ -16843,16 +16837,6 @@ BOOL Debugger::IsOutOfProcessSetContextEnabled()
1684316837
}
1684416838
#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
1684516839
#endif // DACCESS_COMPILE
16846-
#ifndef DACCESS_COMPILE
16847-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
16848-
void Debugger::SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext)
16849-
{
16850-
pThread->SetThreadState(Thread::TS_SSToExitApcCall);
16851-
g_pEEInterface->SetThreadFilterContext(pThread, interruptedContext);
16852-
DebuggerController::EnableSingleStep(pThread);
16853-
g_pEEInterface->SetThreadFilterContext(pThread, NULL);
16854-
}
16855-
#endif //FEATURE_SPECIAL_USER_MODE_APC
16856-
#endif // DACCESS_COMPILE
16840+
1685716841
#endif //DEBUGGING_SUPPORTED
1685816842

src/coreclr/debug/ee/debugger.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3098,11 +3098,6 @@ class Debugger : public DebugInterface
30983098
// Used by Debugger::FirstChanceNativeException to update the context from out of process
30993099
void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL);
31003100
BOOL IsOutOfProcessSetContextEnabled();
3101-
#ifndef DACCESS_COMPILE
3102-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
3103-
void SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext);
3104-
#endif // FEATURE_SPECIAL_USER_MODE_APC
3105-
#endif
31063101
};
31073102

31083103

src/coreclr/vm/dbginterface.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -414,9 +414,6 @@ class DebugInterface
414414
#ifndef DACCESS_COMPILE
415415
virtual HRESULT DeoptimizeMethod(Module* pModule, mdMethodDef methodDef) = 0;
416416
virtual HRESULT IsMethodDeoptimized(Module *pModule, mdMethodDef methodDef, BOOL *pResult) = 0;
417-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
418-
virtual void SingleStepToExitApcCall(Thread* pThread, CONTEXT *interruptedContext) = 0;
419-
#endif // FEATURE_SPECIAL_USER_MODE_APC
420417
#endif //DACCESS_COMPILE
421418
};
422419

src/coreclr/vm/threads.h

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -489,7 +489,6 @@ class Thread
489489
friend void STDCALL OnHijackWorker(HijackArgs * pArgs);
490490
#ifdef FEATURE_THREAD_ACTIVATION
491491
friend void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext);
492-
friend void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool suspendForDebugger);
493492
friend BOOL CheckActivationSafePoint(SIZE_T ip);
494493
#endif // FEATURE_THREAD_ACTIVATION
495494

@@ -558,7 +557,7 @@ class Thread
558557
TS_Hijacked = 0x00000080, // Return address has been hijacked
559558
#endif // FEATURE_HIJACK
560559

561-
TS_SSToExitApcCall = 0x00000100, // Enable SS and resume the thread to exit an APC Call and keep the thread in suspend state
560+
// unused = 0x00000100,
562561
TS_Background = 0x00000200, // Thread is a background thread
563562
TS_Unstarted = 0x00000400, // Thread has never been started
564563
TS_Dead = 0x00000800, // Thread is dead
@@ -575,7 +574,7 @@ class Thread
575574
TS_ReportDead = 0x00010000, // in WaitForOtherThreads()
576575
TS_FullyInitialized = 0x00020000, // Thread is fully initialized and we are ready to broadcast its existence to external clients
577576

578-
TS_SSToExitApcCallDone = 0x00040000, // The thread exited an APC Call and it is already resumed and paused on SS
577+
// unused = 0x00040000,
579578

580579
TS_SyncSuspended = 0x00080000, // Suspended via WaitSuspendEvent
581580
TS_DebugWillSync = 0x00100000, // Debugger will wait for this thread to sync
@@ -2569,9 +2568,6 @@ class Thread
25692568
// Waiting & Synchronization
25702569
//-------------------------------------------------------------
25712570

2572-
friend class DebuggerController;
2573-
protected:
2574-
void MarkForSuspensionAndWait(ULONG bit);
25752571
// For suspends. The thread waits on this event. A client sets the event to cause
25762572
// the thread to resume.
25772573
void WaitSuspendEvents();

src/coreclr/vm/threadsuspend.cpp

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -4238,18 +4238,6 @@ bool Thread::SysSweepThreadsForDebug(bool forceSync)
42384238
if ((thread->m_State & TS_DebugWillSync) == 0)
42394239
continue;
42404240

4241-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
4242-
if (thread->m_State & Thread::TS_SSToExitApcCallDone)
4243-
{
4244-
thread->ResetThreadState(Thread::TS_SSToExitApcCallDone);
4245-
goto Label_MarkThreadAsSynced;
4246-
}
4247-
if (thread->m_State & Thread::TS_SSToExitApcCall)
4248-
{
4249-
continue;
4250-
}
4251-
#endif
4252-
42534241
if (!UseContextBasedThreadRedirection())
42544242
{
42554243
// On platforms that do not support safe thread suspension we either
@@ -5365,19 +5353,6 @@ BOOL Thread::HandledJITCase()
53655353
#endif // FEATURE_HIJACK
53665354

53675355
// Some simple helpers to keep track of the threads we are waiting for
5368-
void Thread::MarkForSuspensionAndWait(ULONG bit)
5369-
{
5370-
CONTRACTL {
5371-
NOTHROW;
5372-
GC_NOTRIGGER;
5373-
}
5374-
CONTRACTL_END;
5375-
m_DebugSuspendEvent.Reset();
5376-
InterlockedOr((LONG*)&m_State, bit);
5377-
ThreadStore::IncrementTrapReturningThreads();
5378-
m_DebugSuspendEvent.Wait(INFINITE,FALSE);
5379-
}
5380-
53815356
void Thread::MarkForSuspension(ULONG bit)
53825357
{
53835358
CONTRACTL {
@@ -5800,7 +5775,7 @@ BOOL CheckActivationSafePoint(SIZE_T ip)
58005775
// address to take the thread to the appropriate stub (based on the return
58015776
// type of the method) which will then handle preparing the thread for GC.
58025777
//
5803-
void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool suspendForDebugger)
5778+
void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
58045779
{
58055780
struct AutoClearPendingThreadActivation
58065781
{
@@ -5836,18 +5811,6 @@ void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool susp
58365811
if (!codeInfo.IsValid())
58375812
return;
58385813

5839-
#ifdef FEATURE_SPECIAL_USER_MODE_APC
5840-
// It's not allowed to change the IP while paused in an APC Callback for security reasons if CET is turned on
5841-
// So we enable the single step in the thread that is running the APC Callback
5842-
// and then it will be paused using single step exception after exiting the APC callback
5843-
// this will allow the debugger to setIp to execute FuncEvalHijack.
5844-
if (suspendForDebugger)
5845-
{
5846-
g_pDebugInterface->SingleStepToExitApcCall(pThread, interruptedContext);
5847-
return;
5848-
}
5849-
#endif
5850-
58515814
DWORD addrOffset = codeInfo.GetRelOffset();
58525815

58535816
ICodeManager *pEECM = codeInfo.GetCodeManager();
@@ -5923,11 +5886,6 @@ void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext, bool susp
59235886
}
59245887
}
59255888

5926-
void HandleSuspensionForInterruptedThread(CONTEXT *interruptedContext)
5927-
{
5928-
HandleSuspensionForInterruptedThread(interruptedContext, false);
5929-
}
5930-
59315889
#ifdef FEATURE_SPECIAL_USER_MODE_APC
59325890
void Thread::ApcActivationCallback(ULONG_PTR Parameter)
59335891
{
@@ -5957,7 +5915,7 @@ void Thread::ApcActivationCallback(ULONG_PTR Parameter)
59575915
case ActivationReason::SuspendForGC:
59585916
case ActivationReason::SuspendForDebugger:
59595917
case ActivationReason::ThreadAbort:
5960-
HandleSuspensionForInterruptedThread(pContext, reason == ActivationReason::SuspendForDebugger);
5918+
HandleSuspensionForInterruptedThread(pContext);
59615919
break;
59625920

59635921
default:

0 commit comments

Comments
 (0)