Skip to content

backport 111408 #114932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: release/9.0-staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/coreclr/debug/ee/controller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Preview

Copilot AI Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verify that the ordering of state flag updates in the APC exit block is consistent with expected behavior and that resetting TS_SSToExitApcCall occurs only after setting TS_SSToExitApcCallDone.

Copilot uses AI. Check for mistakes.

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.
Expand Down
22 changes: 19 additions & 3 deletions src/coreclr/debug/ee/debugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -16732,7 +16740,6 @@ Debugger::EnumMemoryRegionsIfFuncEvalFrame(CLRDataEnumMemoryFlags flags, Frame *
}
}
}

#endif // #ifdef DACCESS_COMPILE

#ifndef DACCESS_COMPILE
Expand Down Expand Up @@ -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;
Expand All @@ -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

5 changes: 5 additions & 0 deletions src/coreclr/debug/ee/debugger.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};


Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/vm/dbginterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
};

Expand Down
8 changes: 6 additions & 2 deletions src/coreclr/vm/threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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();
Expand Down
46 changes: 44 additions & 2 deletions src/coreclr/vm/threadsuspend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
{
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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:
Expand Down
Loading