Merge pull request #29 from 3rdit/fix/breakpoint-deletion-race

Fix multi-thread breakpoint deletion race condition

TitanEngine/Global.Breakpoints.cpp

@@ -4,6 +4,7 @@
 
 std::vector<BreakPointDetail> BreakPointBuffer;
 std::unordered_map<ULONG_PTR, MemoryBreakpointPageDetail> MemoryBreakpointPages;
+std::unordered_set<ULONG_PTR> recentlyDeletedBpx;
 
 ULONG_PTR dr7uint(DR7* dr7)
 {

TitanEngine/Global.Breakpoints.h

@@ -3,6 +3,7 @@
 
 #include <vector>
 #include <unordered_map>
+#include <unordered_set>
 
 #include "Global.Engine.Threading.h"
 #include "Global.Engine.h"
@@ -11,6 +12,7 @@
 
 extern std::vector<BreakPointDetail> BreakPointBuffer;
 extern std::unordered_map<ULONG_PTR, MemoryBreakpointPageDetail> MemoryBreakpointPages;
+extern std::unordered_set<ULONG_PTR> recentlyDeletedBpx;
 
 void uintdr7(ULONG_PTR dr7, DR7* ret);
 ULONG_PTR dr7uint(DR7* dr7);

TitanEngine/Global.Debugger.cpp

@@ -89,6 +89,7 @@ void DebuggerReset()
     }
     std::vector<BreakPointDetail>().swap(BreakPointBuffer);
     std::unordered_map<ULONG_PTR, MemoryBreakpointPageDetail>().swap(MemoryBreakpointPages);
+    recentlyDeletedBpx.clear();
 }
 
 void ClearProcessList()

TitanEngine/TitanEngine.Breakpoints.cpp

@@ -303,6 +303,7 @@ __declspec(dllexport) bool TITCALL DeleteBPX(ULONG_PTR bpxAddress)
     FlushInstructionCache(dbgProcessInformation.hProcess, NULL, 0);
     VirtualProtectEx(dbgProcessInformation.hProcess, (LPVOID)bpxAddress, BreakPointBuffer.at(found).BreakPointSize, OldProtect, &OldProtect);
     BreakPointBuffer.erase(BreakPointBuffer.begin() + found);
+    recentlyDeletedBpx.insert(bpxAddress);
     return true;
 }
 

TitanEngine/TitanEngine.Debugger.DebugLoop.cpp

@@ -101,6 +101,8 @@ __declspec(dllexport) void TITCALL DebugLoop()
     memset(&DLLDebugFileName, 0, sizeof(DLLDebugFileName));
     engineFileIsBeingDebugged = true;
 
+    uint32_t consecutiveTimeouts = 0;
+
     while(!BreakDBG) //actual debug loop
     {
         bool synchronizedStep = false;
@@ -124,10 +126,17 @@ __declspec(dllexport) void TITCALL DebugLoop()
             else
             {
                 // Regular timeout, wait again
+                // After 2 consecutive timeouts, clear recently deleted breakpoints
+                consecutiveTimeouts++;
+                if(consecutiveTimeouts >= 2)
+                    recentlyDeletedBpx.clear();
                 continue;
             }
         }
 
+        // Event received, reset timeout counter
+        consecutiveTimeouts = 0;
+
         if(IsDbgReplyLaterSupported)
         {
             if(DBGEvent.dwDebugEventCode == EXCEPTION_DEBUG_EVENT)
@@ -589,6 +598,30 @@ __declspec(dllexport) void TITCALL DebugLoop()
                 {
                     if(DebugAttachedToProcess || !FirstBPX) //program generated a breakpoint exception
                     {
+                        ULONG_PTR exceptionAddress = (ULONG_PTR)DBGEvent.u.Exception.ExceptionRecord.ExceptionAddress;
+
+                        if(recentlyDeletedBpx.find(exceptionAddress) != recentlyDeletedBpx.end())
+                        {
+                            //breakpoint was recently deleted - handle the stale event gracefully
+                            hActiveThread = EngineOpenThread(THREAD_GETSETSUSPEND, false, DBGEvent.dwThreadId);
+                            if(hActiveThread != NULL)
+                            {
+                                CONTEXT myDBGContext;
+                                myDBGContext.ContextFlags = ContextControlFlags;
+                                GetThreadContext(hActiveThread, &myDBGContext);
+#if defined(_WIN64)
+                                myDBGContext.Rip = exceptionAddress;
+#else
+                                myDBGContext.Eip = (DWORD)exceptionAddress;
+#endif
+                                SetThreadContext(hActiveThread, &myDBGContext);
+                                EngineCloseHandle(hActiveThread);
+                                DBGCode = DBG_CONTINUE;
+                            }
+                        }
+                        else
+                        {
+                            //not a recently deleted breakpoint - pass to debuggee
                             DBGCode = DBG_EXCEPTION_NOT_HANDLED;
                             if(DBGCustomHandler->chBreakPoint != NULL)
                             {
@@ -596,6 +629,7 @@ __declspec(dllexport) void TITCALL DebugLoop()
                                 myCustomHandler(&DBGEvent.u.Exception.ExceptionRecord);
                             }
                         }
+                    }
                     else //system breakpoint
                     {
                         FirstBPX = false;