Skip to content

Commit 2db45f1

Browse files
tommcdontommcdon
authored
Implement out-of-process SetThreadContext support for debugger detach (#118849)
* Implement out-of-process detach support * Fix build break * Handle early process termination during detach in out-of-process debugging * Track active controller ref count for detach through DebuggerController creation and desctruction and DispatchPatchOrSingleStep * Continuously track active dispatched exceptions and flares sent * Revert "Continuously track active dispatched exceptions and flares sent" This reverts commit dd6d28c. * Revert "Track active controller ref count for detach through DebuggerController creation and desctruction and DispatchPatchOrSingleStep" This reverts commit 367c638. * Revert "Fix build break" This reverts commit 240b372. * Revert "Implement out-of-process detach support" This reverts commit d80b6c4. * Out-of-process detach support for CET Debugger events are tracked out of process to determine which threads have outstanding SetThreadContextNeeded flares * Remove unused enum * Break out of detach if timed out or process exited * Add ClearSetIP to SetThreadContextNeeded event, Refactor HandleSetThreadContextNeeded for improved readability * Handle case when out of process setthreadcontext is oped-out but available on the platform * PR feedback on code comment
1 parent 532169b commit 2db45f1

File tree

8 files changed

+579
-192
lines changed

8 files changed

+579
-192
lines changed

‎src/coreclr/debug/di/process.cpp‎

Lines changed: 482 additions & 167 deletions
Large diffs are not rendered by default.

‎src/coreclr/debug/di/rspriv.h‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2920,10 +2920,14 @@ struct DbgAssertAppDomainDeletedData
29202920
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
29212921
class UnmanagedThreadTracker
29222922
{
2923-
DWORD m_dwThreadId = (DWORD)-1;
2924-
HANDLE m_hThread = INVALID_HANDLE_VALUE;
2925-
CORDB_ADDRESS_TYPE *m_pPatchSkipAddress = NULL;
2926-
DWORD m_dwSuspendCount = 0;
2923+
DWORD m_dwThreadId = (DWORD)-1; // The OS thread ID of the unmanaged thread we are tracking
2924+
HANDLE m_hThread = INVALID_HANDLE_VALUE; // Handle to the unmanaged thread, used for suspending and resuming the thread
2925+
CORDB_ADDRESS_TYPE *m_pPatchSkipAddress = NULL; // If non-NULL, this is the address to which we should set the IP when resuming the thread.
2926+
DWORD m_dwSuspendCount = 0; // The suspend count of the thread when we last checked it.
2927+
bool m_pendingSetIP = false; // Set to true if there is a breakpoint or outstanding single-step operation on target thread
2928+
#ifdef _DEBUG
2929+
bool m_fIsDebuggerPatchSkip = false; // Set to true if the thread is currently in a debugger patch skip
2930+
#endif
29272931

29282932
public:
29292933
UnmanagedThreadTracker(DWORD wThreadId, HANDLE hThread) : m_dwThreadId(wThreadId), m_hThread(hThread) {}
@@ -2937,6 +2941,14 @@ class UnmanagedThreadTracker
29372941
void Suspend();
29382942
void Resume();
29392943
void Close();
2944+
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
2945+
void SetPendingSetIP(bool fPendingSetIP) { m_pendingSetIP = fPendingSetIP; }
2946+
bool HasPendingSetIP() const { return m_pendingSetIP; }
2947+
#ifdef _DEBUG
2948+
void SetIsDebuggerPatchSkip(bool fIsDebuggerPatchSkip) { m_fIsDebuggerPatchSkip = fIsDebuggerPatchSkip; }
2949+
bool IsDebuggerPatchSkip() const { return m_fIsDebuggerPatchSkip; }
2950+
#endif
2951+
#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
29402952
};
29412953

29422954
class EMPTY_BASES_DECL CUnmanagedThreadSHashTraits : public DefaultSHashTraits<UnmanagedThreadTracker*>
@@ -3316,8 +3328,9 @@ class CordbProcess :
33163328
}
33173329

33183330
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
3319-
void HandleSetThreadContextNeeded(DWORD dwThreadId);
3331+
bool HandleSetThreadContextNeeded(DWORD dwThreadId);
33203332
bool HandleInPlaceSingleStep(DWORD dwThreadId, PVOID pExceptionAddress);
3333+
bool SetPendingSetIP(DWORD dwThreadId);
33213334
#endif
33223335

33233336
//
@@ -4151,8 +4164,15 @@ class CordbProcess :
41514164
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
41524165
CUnmanagedThreadHashTableImpl m_unmanagedThreadHashTable;
41534166
DWORD m_dwOutOfProcessStepping;
4167+
bool m_fOutOfProcessSetThreadContextEventReceived;
4168+
HRESULT EnableInPlaceSingleStepping(UnmanagedThreadTracker * pCurThread, CORDB_ADDRESS_TYPE *patchSkipAddr, PRD_TYPE opcode);
41544169
public:
41554170
void HandleDebugEventForInPlaceStepping(const DEBUG_EVENT * pEvent);
4171+
bool CanDetach(); // Must only be called on the Win32ET, determines if it is safe to detach. Only used by W32ETA_CAN_DETACH
4172+
void TryDetach(); // Sets detach state to TryDetach, starting the detach evacuation counter.
4173+
bool IsOutOfProcessStepping() { return m_dwOutOfProcessStepping != 0; }
4174+
private:
4175+
HANDLE m_detachSetThreadContextNeededEvent;
41564176
#endif // OUT_OF_PROCESS_SETTHREADCONTEXT
41574177

41584178
};
@@ -10160,6 +10180,10 @@ class CordbWin32EventThread
1016010180

1016110181
HRESULT SendDetachProcessEvent(CordbProcess *pProcess);
1016210182

10183+
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
10184+
HRESULT SendCanDetach();
10185+
#endif
10186+
1016310187
#ifdef FEATURE_INTEROP_DEBUGGING
1016410188
HRESULT SendUnmanagedContinue(CordbProcess *pProcess,
1016510189
EUMContinueType eContType);
@@ -10211,6 +10235,10 @@ class CordbWin32EventThread
1021110235

1021210236
void ExitProcess(bool fDetach);
1021310237

10238+
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
10239+
void HandleCanDetach();
10240+
#endif
10241+
1021410242
private:
1021510243
RSSmartPtr<Cordb> m_cordb;
1021610244

‎src/coreclr/debug/ee/controller.cpp‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,8 +1153,10 @@ void DebuggerController::DeleteAllControllers()
11531153
while (pDebuggerController != NULL)
11541154
{
11551155
pNextDebuggerController = pDebuggerController->m_next;
1156-
pDebuggerController->DebuggerDetachClean();
1157-
pDebuggerController->Delete();
1156+
if (pDebuggerController->DebuggerDetachClean())
1157+
{
1158+
pDebuggerController->Delete();
1159+
}
11581160
pDebuggerController = pNextDebuggerController;
11591161
}
11601162
}
@@ -1216,9 +1218,10 @@ void DebuggerController::Delete()
12161218
}
12171219
}
12181220

1219-
void DebuggerController::DebuggerDetachClean()
1221+
bool DebuggerController::DebuggerDetachClean()
12201222
{
12211223
//do nothing here
1224+
return true;
12221225
}
12231226

12241227
//static
@@ -4466,9 +4469,9 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
44664469
// so remember the attach state now.
44674470
#ifdef _DEBUG
44684471
bool fWasAttached = false;
4469-
#ifdef DEBUGGING_SUPPORTED
4472+
#if defined(DEBUGGING_SUPPORTED) && !defined(OUT_OF_PROCESS_SETTHREADCONTEXT)
44704473
fWasAttached = (CORDebuggerAttached() != 0);
4471-
#endif //DEBUGGING_SUPPORTED
4474+
#endif //DEBUGGING_SUPPORTED && !OUT_OF_PROCESS_SETTHREADCONTEXT
44724475
#endif //_DEBUG
44734476

44744477
{
@@ -4574,12 +4577,14 @@ bool DebuggerController::DispatchNativeException(EXCEPTION_RECORD *pException,
45744577
);
45754578
LOG((LF_CORDB, LL_EVERYTHING, "DC::DNE DispatchPatch call returned\n"));
45764579

4580+
#ifndef OUT_OF_PROCESS_SETTHREADCONTEXT
45774581
// If we detached, we should remove all our breakpoints. So if we try
45784582
// to handle this breakpoint, make sure that we're attached.
45794583
if (IsInUsedAction(result) == true)
45804584
{
45814585
_ASSERTE(fWasAttached);
45824586
}
4587+
#endif
45834588
break;
45844589

45854590
case EXCEPTION_SINGLE_STEP:
@@ -4773,7 +4778,7 @@ DebuggerPatchSkip::~DebuggerPatchSkip()
47734778
#endif // !FEATURE_EMULATE_SINGLESTEP
47744779
}
47754780

4776-
void DebuggerPatchSkip::DebuggerDetachClean()
4781+
bool DebuggerPatchSkip::DebuggerDetachClean()
47774782
{
47784783
// Since for ARM/ARM64 SharedPatchBypassBuffer isn't existed, we don't have to anything here.
47794784
#ifndef FEATURE_EMULATE_SINGLESTEP
@@ -4792,6 +4797,15 @@ void DebuggerPatchSkip::DebuggerDetachClean()
47924797
// 2. Create a "stack walking" implementation for native code and use it to get the current IP and
47934798
// set the IP to the right place.
47944799

4800+
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
4801+
// during detach we need to ensure the context is only being updated out of process
4802+
bool bUsingOutOfProcEvents = g_pDebugInterface->IsOutOfProcessSetContextEnabled();
4803+
if (bUsingOutOfProcEvents)
4804+
{
4805+
return false;
4806+
}
4807+
#endif
4808+
47954809
Thread *thread = GetThreadNULLOk();
47964810
if (thread != NULL)
47974811
{
@@ -4806,6 +4820,8 @@ void DebuggerPatchSkip::DebuggerDetachClean()
48064820
}
48074821
}
48084822
#endif // !FEATURE_EMULATE_SINGLESTEP
4823+
4824+
return true;
48094825
}
48104826

48114827
TP_RESULT DebuggerPatchSkip::TriggerPatch(DebuggerControllerPatch *patch,

‎src/coreclr/debug/ee/controller.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1223,7 +1223,7 @@ class DebuggerController
12231223
static void ApplyTraceFlag(Thread *thread);
12241224
static void UnapplyTraceFlag(Thread *thread);
12251225

1226-
virtual void DebuggerDetachClean();
1226+
virtual bool DebuggerDetachClean();
12271227

12281228
public:
12291229
static const BYTE *GetILPrestubDestination(const BYTE *prestub);
@@ -1525,7 +1525,7 @@ class DebuggerPatchSkip : public DebuggerController
15251525

15261526
void DecodeInstruction(CORDB_ADDRESS_TYPE *code);
15271527

1528-
void DebuggerDetachClean();
1528+
bool DebuggerDetachClean();
15291529

15301530
CORDB_ADDRESS_TYPE *m_address;
15311531
int m_iOrigDisp; // the original displacement of a relative call or jump

‎src/coreclr/debug/ee/debugger.cpp‎

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5451,6 +5451,14 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception,
54515451
bool retVal;
54525452
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
54535453
DebuggerSteppingInfo debuggerSteppingInfo;
5454+
if ((void*)GetIP(context) == (void*)SetThreadContextNeededFlare)
5455+
{
5456+
// The out-of-proc debugger is ignoring the SetThreadContextNeeded flare
5457+
// SSP will be pointing to the instruction after the breakpoint, so advance the IP to that instruction and continue
5458+
PCODE ip = ::GetIP(context);
5459+
::SetIP(context, ip + CORDbg_BREAK_INSTRUCTION_SIZE);
5460+
return true;
5461+
}
54545462
#endif
54555463

54565464
{
@@ -5473,11 +5481,15 @@ bool Debugger::FirstChanceNativeException(EXCEPTION_RECORD *exception,
54735481
}
54745482

54755483
#if defined(OUT_OF_PROCESS_SETTHREADCONTEXT) && !defined(DACCESS_COMPILE)
5476-
if (retVal && fIsVEH)
5484+
// NOTE: During detach, the right-side will wait until all SendSetThreadContextNeeded flares have been sent
5485+
// We assume that any breakpoint debug event caught on the right-side will result in a SendSetThreadContextNeeded flare
5486+
// If there is an active patch skip on the thread, then we assume there will be an additional
5487+
// SendSetThreadContextNeeded flare to move the instruction pointer out of the patch buffer
5488+
if ((retVal || code == EXCEPTION_BREAKPOINT) && fIsVEH)
54775489
{
5478-
// This does not return. Out-of-proc debugger will update the thread context
5479-
// within this call.
5480-
SendSetThreadContextNeeded(context, &debuggerSteppingInfo);
5490+
// If retVal is true, the out-of-proc debugger will update the thread context within this call.
5491+
// Otherwise we are sending a ClearSetIP request, and in which case the out-of-proc debugger will ignore the SetThreadContextNeeded request and simply clear the PendingSetIP flag
5492+
SendSetThreadContextNeeded(context, &debuggerSteppingInfo, thread->HasActivePatchSkip(), !retVal);
54815493
}
54825494
#endif
54835495
return retVal;
@@ -16168,7 +16180,7 @@ void Debugger::StartCanaryThread()
1616816180

1616916181
#ifndef DACCESS_COMPILE
1617016182
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
16171-
void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo)
16183+
void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo, bool fHasDebuggerPatchSkip, bool fClearSetIP)
1617216184
{
1617316185
STATIC_CONTRACT_NOTHROW;
1617416186
STATIC_CONTRACT_GC_NOTRIGGER;
@@ -16223,10 +16235,20 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo
1622316235
PRD_TYPE opcode = pDebuggerSteppingInfo != NULL ? pDebuggerSteppingInfo->GetOpcode() : CORDbg_BREAK_INSTRUCTION;
1622416236

1622516237
// send the context to the right side
16226-
LOG((LF_CORDB, LL_INFO10000, "D::SSTCN ContextFlags=0x%X contextSize=%d fIsInPlaceSingleStep=%d opcode=%x\n", contextFlags, contextSize, fIsInPlaceSingleStep, opcode));
16238+
LOG((LF_CORDB, LL_INFO10000, "D::SSTCN ContextFlags=0x%X contextSize=%d fIsInPlaceSingleStep=%d opcode=%x fHasDebuggerPatchSkip=%d\n", contextFlags, contextSize, fIsInPlaceSingleStep, opcode, fHasDebuggerPatchSkip));
1622716239
EX_TRY
1622816240
{
16229-
SetThreadContextNeededFlare((TADDR)pContext, contextSize, fIsInPlaceSingleStep, opcode);
16241+
DWORD flag = fIsInPlaceSingleStep ? 0x1 : 0;
16242+
if (fHasDebuggerPatchSkip)
16243+
{
16244+
flag |= 0x2;
16245+
}
16246+
if (fClearSetIP)
16247+
{
16248+
_ASSERTE(!fIsInPlaceSingleStep && !fHasDebuggerPatchSkip);
16249+
flag |= 0x4;
16250+
}
16251+
SetThreadContextNeededFlare((TADDR)pContext, contextSize, flag, opcode);
1623016252
}
1623116253
EX_CATCH
1623216254
{
@@ -16237,15 +16259,15 @@ void Debugger::SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo
1623716259
#endif
1623816260

1623916261
LOG((LF_CORDB, LL_INFO10000, "D::SSTCN SetThreadContextNeededFlare returned\n"));
16240-
_ASSERTE(!"We failed to SetThreadContext from out of process!");
16262+
_ASSERTE(fClearSetIP);
1624116263
}
1624216264

1624316265
BOOL Debugger::IsOutOfProcessSetContextEnabled()
1624416266
{
1624516267
return m_fOutOfProcessSetContextEnabled;
1624616268
}
1624716269
#else
16248-
void Debugger::SendSetThreadContextNeeded(CONTEXT* context, DebuggerSteppingInfo *pDebuggerSteppingInfo)
16270+
void Debugger::SendSetThreadContextNeeded(CONTEXT* context, DebuggerSteppingInfo *pDebuggerSteppingInfo, bool fHasDebuggerPatchSkip, bool fClearSetIP)
1624916271
{
1625016272
_ASSERTE(!"SendSetThreadContextNeeded is not enabled on this platform");
1625116273
}

‎src/coreclr/debug/ee/debugger.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,7 +1864,7 @@ extern "C" void __stdcall NotifyRightSideOfSyncCompleteFlare(void);
18641864
extern "C" void __stdcall NotifySecondChanceReadyForDataFlare(void);
18651865
#ifdef OUT_OF_PROCESS_SETTHREADCONTEXT
18661866
#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64)
1867-
extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size, bool fIsInPlaceSingleStep, PRD_TYPE opcode);
1867+
extern "C" void __stdcall SetThreadContextNeededFlare(TADDR pContext, DWORD size, DWORD64 flag, PRD_TYPE opcode);
18681868
#else
18691869
#error Platform not supported
18701870
#endif
@@ -3025,7 +3025,7 @@ class Debugger : public DebugInterface
30253025
BOOL m_fOutOfProcessSetContextEnabled;
30263026
public:
30273027
// Used by Debugger::FirstChanceNativeException to update the context from out of process
3028-
void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL);
3028+
void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = NULL, bool fHasActivePatchSkip = false, bool fClearSetIP = false);
30293029
BOOL IsOutOfProcessSetContextEnabled();
30303030
};
30313031

‎src/coreclr/vm/dbginterface.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ class DebugInterface
197197

198198

199199
// Used by EditAndContinueModule::FixContextAndResume
200-
virtual void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = nullptr) = 0;
200+
virtual void SendSetThreadContextNeeded(CONTEXT *context, DebuggerSteppingInfo *pDebuggerSteppingInfo = nullptr, bool HasActivePatchSkip = false, bool fClearSetIP = false) = 0;
201201
virtual BOOL IsOutOfProcessSetContextEnabled() = 0;
202202
#endif // FEATURE_METADATA_UPDATER
203203

‎src/coreclr/vm/threads.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3561,6 +3561,12 @@ class Thread
35613561
_ASSERTE(!m_debuggerActivePatchSkipper.Load());
35623562
}
35633563

3564+
bool HasActivePatchSkip() const
3565+
{
3566+
LIMITED_METHOD_DAC_CONTRACT;
3567+
return m_debuggerActivePatchSkipper.Load() != NULL;
3568+
}
3569+
35643570
private:
35653571

35663572
static BOOL EnterWorkingOnThreadContext(Thread *pThread)

0 commit comments

Comments
 (0)