Merge pull request #3913 from x64dbg/snow-investigation

Improve stale software breakpoint handling

src/tests/CMakeLists.txt

@@ -181,6 +181,7 @@ set(X64DBG_STANDARD_TESTS
     script_run_exit
     scriptcmd_call
     scriptcmd_call_threads
+    swbp_stale
     trace_cmd_condition
 )
 

src/tests/swbp_stale/README.md

@@ -0,0 +1,7 @@
+# Stale software-breakpoint events
+
+This test starts 64 threads on the same short `INT3` breakpoint and disables it after the first hit.
+
+On x64, the restored instruction is a blocking `syscall`. Its internal single-step takes 250 ms, exceeding the old two-timeout lifetime of the recently-deleted-breakpoint cache while other breakpoint events are deferred by safe stepping.
+
+The run and step variants fail if a deferred breakpoint event reaches x64dbg's generic exception handler or the debuggee's VEH. The x86 build exercises the same multithreaded deletion path with a fast instruction.

src/tests/swbp_stale/target.cpp

@@ -0,0 +1,153 @@
+#include <windows.h>
+#include <cstdint>
+#include <cstring>
+#include <intrin.h>
+
+extern "C" __declspec(dllexport) void* gBreakpointAddress = nullptr;
+extern "C" __declspec(dllexport) volatile LONG gEscapedBreakpoints = 0;
+
+static volatile LONG gReady = 0;
+static volatile LONG gGo = 0;
+static volatile LONG gPhase = 0;
+
+#ifdef _WIN64
+using TestInstruction = LONG(NTAPI*)(BOOLEAN alertable, PLARGE_INTEGER delay);
+static TestInstruction gTestInstruction = nullptr;
+#else
+using TestInstruction = void(*)();
+static TestInstruction gTestInstruction = nullptr;
+#endif
+
+static LONG CALLBACK BreakpointHandler(EXCEPTION_POINTERS* pointers)
+{
+    if(pointers->ExceptionRecord->ExceptionCode != EXCEPTION_BREAKPOINT ||
+            pointers->ExceptionRecord->ExceptionAddress != gBreakpointAddress)
+        return EXCEPTION_CONTINUE_SEARCH;
+
+    InterlockedIncrement(&gEscapedBreakpoints);
+#ifdef _WIN64
+    pointers->ContextRecord->Rip = reinterpret_cast<DWORD64>(gBreakpointAddress);
+#else
+    pointers->ContextRecord->Eip = reinterpret_cast<DWORD>(gBreakpointAddress);
+#endif
+    return EXCEPTION_CONTINUE_EXECUTION;
+}
+
+#ifdef _WIN64
+static bool InitializeTestInstruction()
+{
+    auto ntdll = GetModuleHandleW(L"ntdll.dll");
+    auto stub = reinterpret_cast<const uint8_t*>(GetProcAddress(ntdll, "NtDelayExecution"));
+    if(stub == nullptr)
+        return false;
+
+    DWORD syscallNumber = 0;
+    bool found = false;
+    for(size_t i = 0; i + 5 < 32 && !found; ++i)
+    {
+        if(stub[i] != 0xB8)
+            continue;
+        for(size_t j = i + 5; j + 1 < 32; ++j)
+        {
+            if(stub[j] == 0x0F && stub[j + 1] == 0x05)
+            {
+                memcpy(&syscallNumber, stub + i + 1, sizeof(syscallNumber));
+                found = true;
+                break;
+            }
+        }
+    }
+    if(!found)
+        return false;
+
+    // mov r10,rcx; mov eax,syscallNumber; syscall; jmp done; nop; nop; nop; ret
+    uint8_t code[] =
+    {
+        0x4C, 0x8B, 0xD1,
+        0xB8, 0, 0, 0, 0,
+        0x0F, 0x05,
+        0xEB, 0x03,
+        0x90, 0x90, 0x90,
+        0xC3
+    };
+    memcpy(code + 4, &syscallNumber, sizeof(syscallNumber));
+
+    auto memory = static_cast<uint8_t*>(VirtualAlloc(nullptr, sizeof(code), MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE));
+    if(memory == nullptr)
+        return false;
+    memcpy(memory, code, sizeof(code));
+    FlushInstructionCache(GetCurrentProcess(), memory, sizeof(code));
+
+    gTestInstruction = reinterpret_cast<TestInstruction>(memory);
+    gBreakpointAddress = memory + 8; // syscall
+    return true;
+}
+#else
+static bool InitializeTestInstruction()
+{
+    const uint8_t code[] = { 0x90, 0xC3 }; // nop; ret
+    auto memory = static_cast<uint8_t*>(VirtualAlloc(nullptr, sizeof(code), MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE));
+    if(memory == nullptr)
+        return false;
+    memcpy(memory, code, sizeof(code));
+    FlushInstructionCache(GetCurrentProcess(), memory, sizeof(code));
+
+    gTestInstruction = reinterpret_cast<TestInstruction>(memory);
+    gBreakpointAddress = memory;
+    return true;
+}
+#endif
+
+static DWORD WINAPI Worker(void*)
+{
+    InterlockedIncrement(&gReady);
+    while(InterlockedCompareExchange(&gGo, 0, 0) == 0)
+        YieldProcessor();
+
+#ifdef _WIN64
+    LARGE_INTEGER delay;
+    delay.QuadPart = -250LL * 10000LL;
+    gTestInstruction(FALSE, &delay);
+#else
+    gTestInstruction();
+#endif
+    return 0;
+}
+
+extern "C" __declspec(dllexport) __declspec(noinline) void Ready()
+{
+    InterlockedExchange(&gPhase, 1);
+}
+
+extern "C" __declspec(dllexport) __declspec(noinline) void Finish()
+{
+    InterlockedExchange(&gPhase, 2);
+}
+
+int main()
+{
+    if(!InitializeTestInstruction())
+        return 100;
+    Ready();
+    AddVectoredExceptionHandler(1, BreakpointHandler);
+
+    constexpr DWORD ThreadCount = 64;
+    HANDLE threads[ThreadCount] = {};
+    for(DWORD i = 0; i < ThreadCount; ++i)
+    {
+        threads[i] = CreateThread(nullptr, 0, Worker, nullptr, 0, nullptr);
+        if(threads[i] == nullptr)
+            return 101;
+    }
+
+    while(InterlockedCompareExchange(&gReady, 0, 0) != ThreadCount)
+        Sleep(0);
+    InterlockedExchange(&gGo, 1);
+
+    WaitForMultipleObjects(ThreadCount, threads, TRUE, INFINITE);
+    for(auto thread : threads)
+        CloseHandle(thread);
+
+    Finish();
+    return gEscapedBreakpoints == 0 ? 0 : 102;
+}

src/tests/swbp_stale/test.step.txt

@@ -0,0 +1,20 @@
+settingset Events, SystemBreakpoint, 1
+settingset Events, EntryBreakpoint, 0
+init tests/swbp_stale.exe
+bp swbp_stale:Ready
+run
+testassert cip == swbp_stale:Ready, "target initialized the dynamic test instruction"
+bpd swbp_stale:Ready
+bp [swbp_stale:gBreakpointAddress]
+bp swbp_stale:Finish
+run
+testassert cip == [swbp_stale:gBreakpointAddress], "dynamic short INT3 breakpoint was reached"
+bpd [swbp_stale:gBreakpointAddress]
+sti
+testassert cip != [swbp_stale:gBreakpointAddress], "the restored instruction completed its single-step"
+run
+testassert cip == swbp_stale:Finish, "all stale short INT3 events stayed inside the breakpoint engine after stepping"
+testassert dword:[swbp_stale:gEscapedBreakpoints] == 0, "no managed breakpoint exception reached the debuggee VEH"
+bpd swbp_stale:Finish
+run
+testfinalize

src/tests/swbp_stale/test.txt

@@ -0,0 +1,18 @@
+settingset Events, SystemBreakpoint, 1
+settingset Events, EntryBreakpoint, 0
+init tests/swbp_stale.exe
+bp swbp_stale:Ready
+run
+testassert cip == swbp_stale:Ready, "target initialized the dynamic test instruction"
+bpd swbp_stale:Ready
+bp [swbp_stale:gBreakpointAddress]
+bp swbp_stale:Finish
+run
+testassert cip == [swbp_stale:gBreakpointAddress], "dynamic short INT3 breakpoint was reached"
+bpd [swbp_stale:gBreakpointAddress]
+run
+testassert cip == swbp_stale:Finish, "all stale short INT3 events stayed inside the breakpoint engine"
+testassert dword:[swbp_stale:gEscapedBreakpoints] == 0, "no managed breakpoint exception reached the debuggee VEH"
+bpd swbp_stale:Finish
+run
+testfinalize

src/third_party/GleeBug

@@ -1 +1 @@
-Subproject commit 53f93ca7efe22fee7c3dd311518803414ea3cda5
+Subproject commit 6d37c95192004766c5ff947f495782ff8f318939

src/third_party/TitanEngine

@@ -1 +1 @@
-Subproject commit 810ab65f1092563f9eea6d81dc6f36578e0f41aa
+Subproject commit ccac889f27622255dfb199b2f534d69e2e1e9050