From e80b95915024ecc8fd3dd30b9b4b82587b0bc9f6 Mon Sep 17 00:00:00 2001 From: Duncan Ogilvie Date: Wed, 15 Jul 2026 16:02:34 +0200 Subject: [PATCH 1/3] Keep per-thread stepping state Closes #21 Closes #20 --- TitanEngine/Global.Debugger.cpp | 5 ++-- TitanEngine/Global.Debugger.h | 16 ++++++++++--- TitanEngine/TitanEngine.Debugger.Control.cpp | 14 +++++++---- .../TitanEngine.Debugger.DebugLoop.cpp | 24 +++++++++++++------ 4 files changed, 41 insertions(+), 18 deletions(-) diff --git a/TitanEngine/Global.Debugger.cpp b/TitanEngine/Global.Debugger.cpp index f267967..11e4c5f 100644 --- a/TitanEngine/Global.Debugger.cpp +++ b/TitanEngine/Global.Debugger.cpp @@ -29,9 +29,7 @@ DWORD ProcessExitCode = 0; HANDLE DBGFileHandle; std::vector tlsCallBackList; std::vector hListProcess; -DWORD engineStepCount = 0; -LPVOID engineStepCallBack = NULL; -bool engineStepActive = false; +std::unordered_map engineStepThreads; bool engineProcessIsNowDetached = false; DWORD DBGCode = DBG_CONTINUE; bool engineFileIsBeingDebugged = false; @@ -90,6 +88,7 @@ void DebuggerReset() std::vector().swap(BreakPointBuffer); std::unordered_map().swap(MemoryBreakpointPages); recentlyDeletedBpx.clear(); + engineStepThreads.clear(); } void ClearProcessList() diff --git a/TitanEngine/Global.Debugger.h b/TitanEngine/Global.Debugger.h index 1861756..d0b8e11 100644 --- a/TitanEngine/Global.Debugger.h +++ b/TitanEngine/Global.Debugger.h @@ -2,6 +2,7 @@ #define _GLOBAL_DEBUGGER_H #include +#include #include extern HARDWARE_DATA DebugRegister[4]; @@ -29,9 +30,18 @@ extern DWORD ProcessExitCode; extern HANDLE DBGFileHandle; extern std::vector tlsCallBackList; extern std::vector hListProcess; -extern DWORD engineStepCount; -extern LPVOID engineStepCallBack; -extern bool engineStepActive; +// Per-thread pending single-step state. A single-step (user step or the internal +// step-over of a breakpoint) is always armed on a specific thread by setting that +// thread's trap flag, but the resulting STATUS_SINGLE_STEP event can be delivered +// interleaved with other threads' events. Keying the step state by thread id makes +// each thread consume only its own step, and allows several threads to be stepped +// independently (matching GleeBug). +struct EngineStepThreadState +{ + LPVOID callback; // step callback to fire when the step completes + DWORD count; // remaining repeats (0 = fire the callback on the next step) +}; +extern std::unordered_map engineStepThreads; extern bool engineProcessIsNowDetached; extern DWORD DBGCode; extern bool engineFileIsBeingDebugged; diff --git a/TitanEngine/TitanEngine.Debugger.Control.cpp b/TitanEngine/TitanEngine.Debugger.Control.cpp index 548f746..3984c0d 100644 --- a/TitanEngine/TitanEngine.Debugger.Control.cpp +++ b/TitanEngine/TitanEngine.Debugger.Control.cpp @@ -38,7 +38,9 @@ __declspec(dllexport) void TITCALL ForceClose() __declspec(dllexport) void TITCALL StepInto(LPVOID StepCallBack) { EnterCriticalSection(&engineStepActiveCr); - if(!engineStepActive) + // Arm a single-step for the current event thread only. State is per-thread, so a + // step already pending on another thread does not block this one. + if(engineStepThreads.find(DBGEvent.dwThreadId) == engineStepThreads.end()) { ULONG_PTR ueCurrentPosition = GetContextData(UE_CIP); unsigned char instr[16]; @@ -60,9 +62,7 @@ __declspec(dllexport) void TITCALL StepInto(LPVOID StepCallBack) myDBGContext.EFlags |= UE_TRAP_FLAG; SetThreadContext(hActiveThread, &myDBGContext); EngineCloseHandle(hActiveThread); - engineStepActive = true; - engineStepCallBack = StepCallBack; - engineStepCount = 0; + engineStepThreads[DBGEvent.dwThreadId] = { StepCallBack, 0 }; } } LeaveCriticalSection(&engineStepActiveCr); @@ -93,7 +93,11 @@ __declspec(dllexport) void TITCALL StepOut(LPVOID StepOut, bool StepFinal) __declspec(dllexport) void TITCALL SingleStep(DWORD StepCount, LPVOID StepCallBack) { StepInto(StepCallBack); - engineStepCount = StepCount - 1; //We already stepped once + EnterCriticalSection(&engineStepActiveCr); + auto it = engineStepThreads.find(DBGEvent.dwThreadId); + if(it != engineStepThreads.end()) + it->second.count = StepCount - 1; //We already stepped once + LeaveCriticalSection(&engineStepActiveCr); } __declspec(dllexport) void TITCALL SetNextDbgContinueStatus(DWORD SetDbgCode) diff --git a/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp b/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp index d11889f..4641709 100644 --- a/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp +++ b/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp @@ -16,22 +16,28 @@ static void engineStep() { EnterCriticalSection(&engineStepActiveCr); - if(engineStepActive) + // Only deliver the pending single-step to the thread it was armed for. State is + // per-thread, so an automatic breakpoint step-over on a different thread cannot + // consume this thread's step or fire its callback on the wrong thread. + auto it = engineStepThreads.find(DBGEvent.dwThreadId); + if(it != engineStepThreads.end()) { DBGCode = DBG_CONTINUE; - if(engineStepCount == 0) + if(it->second.count == 0) { typedef void(TITCALL * fCustomBreakPoint)(void); - auto cbStep = fCustomBreakPoint(engineStepCallBack); - engineStepActive = false; - engineStepCallBack = NULL; + auto cbStep = fCustomBreakPoint(it->second.callback); + engineStepThreads.erase(it); LeaveCriticalSection(&engineStepActiveCr); cbStep(); } else { - SingleStep(engineStepCount, engineStepCallBack); + auto count = it->second.count; + auto callback = it->second.callback; + engineStepThreads.erase(it); // SingleStep re-arms this thread's entry LeaveCriticalSection(&engineStepActiveCr); + SingleStep(count, callback); } } else @@ -1288,7 +1294,11 @@ __declspec(dllexport) void TITCALL DebugLoop() //general unhandled exception callback if(DBGCode == DBG_EXCEPTION_NOT_HANDLED) { - engineStepActive = false; + // The current thread's pending single-step (if any) did not complete + // normally; cancel it. Other threads' steps are unaffected. + EnterCriticalSection(&engineStepActiveCr); + engineStepThreads.erase(DBGEvent.dwThreadId); + LeaveCriticalSection(&engineStepActiveCr); if(DBGCustomHandler->chUnhandledException != NULL) { From ba2a3aa7c8cbc51f1b21f498592a6680dd6002aa Mon Sep 17 00:00:00 2001 From: Duncan Ogilvie Date: Wed, 15 Jul 2026 20:11:37 +0200 Subject: [PATCH 2/3] Complete orphaned breakpoint step-over reset when the stepped thread exits If a thread is terminated (e.g. by ExitProcess) while it is in the middle of a breakpoint step-over - its breakpoint temporarily removed and a restore pending on its next single-step - the EXIT_THREAD handler cleared ThreadBeingProcessed but left the reset state (ResetBPX/ResetHwBPX/ResetMemBPX) armed and the breakpoint removed. The next thread's event then consumed that orphaned reset and was misinterpreted as a step-over reset, producing a stray single-step that was passed to the debuggee unhandled and crashed it. This was most visible with hardware breakpoints under multi-thread contention. Complete the pending restore when the thread-being-processed exits. --- .../TitanEngine.Debugger.DebugLoop.cpp | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp b/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp index 4641709..942eb02 100644 --- a/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp +++ b/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp @@ -166,6 +166,36 @@ __declspec(dllexport) void TITCALL DebugLoop() SuspendedThreads.clear(); ThreadBeingProcessed = 0; + + // The thread was in the middle of a breakpoint step-over (its + // breakpoint was temporarily removed and a restore was pending on its + // next single-step) but it is exiting before that single-step arrives. + // Complete the restore now: otherwise the breakpoint is left removed + // and the next thread's event would consume this dead thread's orphaned + // reset state and be misinterpreted as a step-over reset, producing a + // stray single-step that is passed to (and crashes) the debuggee. + if(ResetBPX) + { + EnableBPX(ResetBPXAddressTo); + ResetBPXAddressTo = 0; + ResetBPX = false; + } + if(ResetHwBPX) + { + SetHardwareBreakPoint(DebugRegisterX.DrxBreakAddress, DebugRegisterXId, DebugRegisterX.DrxBreakPointType, DebugRegisterX.DrxBreakPointSize, (LPVOID)DebugRegisterX.DrxCallBack); + ResetHwBPX = false; + } + if(ResetMemBPX) + { + ResetMemBpxCallback(); + if(ResetMemBpxExtraCallback != nullptr) + { + ResetMemBpxExtraCallback(); + ResetMemBpxExtraCallback = nullptr; + } + ResetMemBPX = false; + } + PushfBPX = false; } } } From ed4bdd1a01de3054e011948bf4ae178ead81b213 Mon Sep 17 00:00:00 2001 From: Duncan Ogilvie Date: Thu, 16 Jul 2026 02:28:39 +0200 Subject: [PATCH 3/3] Address review: preserve other hardware breakpoints and per-thread step state on thread exit Two fixes from PR review of the thread-exit reset-recovery: - When restoring the hardware breakpoint after the stepped-over thread exits, rebuild each still-suspended thread's debug registers from the authoritative DebugRegister[] table (re-arming every enabled slot, as CREATE_THREAD does) instead of from the exiting thread's now-gone context. The old code rebuilt DR7 from GetContextData() (the dead thread), writing a DR7 with only the restored slot to every thread and silently disabling any other hardware breakpoint. The restore is also moved before the threads are resumed, since SetThreadContext does not reliably update the debug registers of a running thread. - Erase the per-thread single-step state in the EXIT_THREAD handler. A thread can exit after StepInto() armed its step but before the trap event arrives; leaving the entry behind means a reused thread id inherits it (StepInto declines to arm, or a later single-step fires the dead thread's callback). --- .../TitanEngine.Debugger.DebugLoop.cpp | 50 ++++++++++++++----- 1 file changed, 38 insertions(+), 12 deletions(-) diff --git a/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp b/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp index 942eb02..69ab90b 100644 --- a/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp +++ b/TitanEngine/TitanEngine.Debugger.DebugLoop.cpp @@ -160,20 +160,14 @@ __declspec(dllexport) void TITCALL DebugLoop() { if(ThreadBeingProcessed != 0 && DBGEvent.dwThreadId == ThreadBeingProcessed) { - // Resume the other threads since the thread being processed is exiting - for(auto & itr : SuspendedThreads) - ResumeThread(itr.second.hThread); - - SuspendedThreads.clear(); - ThreadBeingProcessed = 0; - // The thread was in the middle of a breakpoint step-over (its // breakpoint was temporarily removed and a restore was pending on its // next single-step) but it is exiting before that single-step arrives. - // Complete the restore now: otherwise the breakpoint is left removed - // and the next thread's event would consume this dead thread's orphaned - // reset state and be misinterpreted as a step-over reset, producing a - // stray single-step that is passed to (and crashes) the debuggee. + // Complete the restore now, while the other threads are still + // suspended - otherwise the breakpoint is left removed and the next + // thread's event would consume this dead thread's orphaned reset state + // and be misinterpreted as a step-over reset, producing a stray + // single-step that is passed to (and crashes) the debuggee. if(ResetBPX) { EnableBPX(ResetBPXAddressTo); @@ -182,7 +176,23 @@ __declspec(dllexport) void TITCALL DebugLoop() } if(ResetHwBPX) { - SetHardwareBreakPoint(DebugRegisterX.DrxBreakAddress, DebugRegisterXId, DebugRegisterX.DrxBreakPointType, DebugRegisterX.DrxBreakPointSize, (LPVOID)DebugRegisterX.DrxCallBack); + // Re-enable the stepped-over slot in the authoritative DebugRegister[] + // table (DeleteHardwareBreakPoint cleared it when the breakpoint was + // hit), then re-arm every enabled hardware breakpoint on each still- + // suspended thread from that table - exactly as CREATE_THREAD arms a + // new thread. Rebuilding from DebugRegister[] rather than from the + // exiting thread's (gone) context or a live thread's DR7 (which may + // have been cleared) is what keeps the OTHER hardware breakpoints from + // being disabled. This must run before the threads are resumed: + // SetThreadContext does not reliably update a running thread's debug + // registers. + int drIdx = (DebugRegisterXId == UE_DR0) ? 0 : (DebugRegisterXId == UE_DR1) ? 1 : (DebugRegisterXId == UE_DR2) ? 2 : 3; + DebugRegister[drIdx] = DebugRegisterX; + static const DWORD drIds[4] = { UE_DR0, UE_DR1, UE_DR2, UE_DR3 }; + for(auto & itr : SuspendedThreads) + for(int s = 0; s < 4; s++) + if(DebugRegister[s].DrxEnabled) + SetHardwareBreakPointEx(itr.second.hThread, DebugRegister[s].DrxBreakAddress, drIds[s], DebugRegister[s].DrxBreakPointType, DebugRegister[s].DrxBreakPointSize, (LPVOID)DebugRegister[s].DrxCallBack, NULL); ResetHwBPX = false; } if(ResetMemBPX) @@ -196,6 +206,13 @@ __declspec(dllexport) void TITCALL DebugLoop() ResetMemBPX = false; } PushfBPX = false; + + // Resume the other threads now that the breakpoint has been restored. + for(auto & itr : SuspendedThreads) + ResumeThread(itr.second.hThread); + + SuspendedThreads.clear(); + ThreadBeingProcessed = 0; } } } @@ -380,6 +397,15 @@ __declspec(dllexport) void TITCALL DebugLoop() break; } } + + // Drop any per-thread single-step state for the exiting thread. A thread can + // exit after StepInto() armed its step but before the trap event is delivered + // (e.g. it is terminated externally); without this, a reused thread id would + // inherit the stale entry - StepInto() would decline to arm the trap flag, or + // a later single-step could fire the dead thread's callback. + EnterCriticalSection(&engineStepActiveCr); + engineStepThreads.erase(DBGEvent.dwThreadId); + LeaveCriticalSection(&engineStepActiveCr); } break;