fix: harden debug agent for reliable WOW64 watchpoints and multi-run stability
Key fixes applied during Task 6 end-to-end MCP integration testing: - CONTEXT_ALL (CONTEXT_FULL | CONTEXT_DEBUG_REGISTERS) for Wow64Get/SetThreadContext so DR0-DR7 are read and written alongside general-purpose registers - Native x64 SetThreadContext path in _apply_watchpoints_to_thread: Wow64SetThreadContext updates the WOW64 context save area but does not propagate to hardware DR registers; the native path is required for hardware watchpoints to actually fire on WOW64 threads - _REGISTER_NAMES extended to include Dr0-Dr3, Dr6, Dr7 so get_registers returns them - launch() now terminates and drains the previously attached process before starting a new one, preventing stale debug events from a hung game from corrupting a new session - continue_execution filters events by process ID so late-arriving exit events from a previously terminated game do not trigger a false "exited" status for the new process - AV exception routing restored: EXCEPTION_ACCESS_VIOLATION stops and reports; callers skip boot-time AVs in a loop rather than silently continuing them in the agent - launch_with_breakpoints atomic op in agent server and MCP server - _DebugWorker dedicated thread: Windows debug API requires all WaitForDebugEvent, ContinueDebugEvent, and CreateProcessA calls to originate from the same OS thread; ThreadingTCPServer violated this -- replaced with single worker thread + queue - Gameplay screenshot captured at watchpoint stop (272 KB real frame) Acceptance checklist results: - Item 1 PASS: bp@0x40C8E0, eip/eax confirmed - Item 2 PASS: conditional breakpoint false-skip and true-fire verified - Item 3 PASS: watchpoint@slot0 fired at 0x40E998 (MOV [DAT_0047c488], EAX in player_cooldown_timers_tick -- the DAT write is in the caller, not inside compute_frame_delta_ticks_60hz_min1_clamp as the spec assumed) - Item 4 PASS: screenshot PNG captured (gameplay frame 272 KB) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TN4Ytn3gdWRonNmHpisWQv
This commit is contained in:
parent
6baefe8e8a
commit
7d39ccddf0
4 changed files with 139 additions and 24 deletions
BIN
tools/debug_screenshot_gameplay.png
Normal file
BIN
tools/debug_screenshot_gameplay.png
Normal file
Binary file not shown.
|
After Width: | Height: | Size: 266 KiB |
|
|
@ -34,7 +34,17 @@ EXIT_PROCESS_DEBUG_EVENT = 5
|
|||
LOAD_DLL_DEBUG_EVENT = 6
|
||||
EXCEPTION_DEBUG_EVENT = 1
|
||||
CONTEXT_FULL = 0x00010007
|
||||
CONTEXT_DEBUG_REGISTERS = 0x00010010
|
||||
CONTEXT_ALL = CONTEXT_FULL | CONTEXT_DEBUG_REGISTERS # includes DR0-DR3, DR6, DR7
|
||||
TRAP_FLAG = 0x100 # EFlags bit 8: single-step after the next instruction
|
||||
# Native x64 context flag for debug registers (different base from WOW64 flags)
|
||||
CONTEXT_AMD64_DEBUG = 0x00100010
|
||||
# x64 CONTEXT size and field offsets (winnt.h layout, must be 16-byte aligned)
|
||||
_CONTEXT64_SIZE = 0x4D0
|
||||
_CTX64_FLAGS_OFF = 0x030 # DWORD ContextFlags
|
||||
_CTX64_DR0_OFF = 0x048 # DWORD64 Dr0
|
||||
_CTX64_DR6_OFF = 0x068 # DWORD64 Dr6
|
||||
_CTX64_DR7_OFF = 0x070 # DWORD64 Dr7
|
||||
|
||||
|
||||
class STARTUPINFO(ctypes.Structure):
|
||||
|
|
@ -92,7 +102,8 @@ class WOW64_CONTEXT(ctypes.Structure):
|
|||
("ExtendedRegisters", c_char * 512)]
|
||||
|
||||
|
||||
_REGISTER_NAMES = ["Eax", "Ebx", "Ecx", "Edx", "Esi", "Edi", "Ebp", "Esp", "Eip", "EFlags"]
|
||||
_REGISTER_NAMES = ["Eax", "Ebx", "Ecx", "Edx", "Esi", "Edi", "Ebp", "Esp", "Eip", "EFlags",
|
||||
"Dr0", "Dr1", "Dr2", "Dr3", "Dr6", "Dr7"]
|
||||
|
||||
|
||||
class DebugAgent:
|
||||
|
|
@ -129,6 +140,33 @@ class DebugAgent:
|
|||
raise OSError(f"WriteProcessMemory failed at 0x{addr:X}: {k.GetLastError()}")
|
||||
|
||||
def launch(self, path: str, cwd: str) -> None:
|
||||
# Terminate any previously-attached process. Without this, a game whose debug event
|
||||
# loop was abandoned (e.g. after a timeout) stays hung with all threads suspended,
|
||||
# and the new launch fails because the stale instance holds file/mutex locks.
|
||||
if self._process_info is not None:
|
||||
k.TerminateProcess(self._process_info.hProcess, 0)
|
||||
# Drain the exit event so the debug subsystem releases the process cleanly.
|
||||
evt = DEBUG_EVENT()
|
||||
for _ in range(32):
|
||||
if k.WaitForDebugEvent(byref(evt), 500):
|
||||
k.ContinueDebugEvent(evt.dwProcessId, evt.dwThreadId, DBG_CONTINUE)
|
||||
if evt.dwDebugEventCode == EXIT_PROCESS_DEBUG_EVENT:
|
||||
break
|
||||
k.CloseHandle(self._process_info.hProcess)
|
||||
k.CloseHandle(self._process_info.hThread)
|
||||
self._process_info = None
|
||||
|
||||
# Reset all process-local state so re-launching doesn't inherit stale breakpoints,
|
||||
# thread handles, or watchpoints from a previous session on the same server instance.
|
||||
self._threads.clear()
|
||||
self._breakpoints.clear()
|
||||
self._watchpoints = WatchpointSlots()
|
||||
self._last_event = None
|
||||
self._exited = None
|
||||
self._stopped_thread_id = None
|
||||
self._pending_event = None
|
||||
self._step_over = None
|
||||
|
||||
si = STARTUPINFO()
|
||||
si.cb = sizeof(si)
|
||||
pi = PROCESS_INFORMATION()
|
||||
|
|
@ -157,9 +195,37 @@ class DebugAgent:
|
|||
def list_breakpoints(self) -> list[int]:
|
||||
return sorted(self._breakpoints.keys())
|
||||
|
||||
def _set_native_dr_registers(self, thread_handle: int) -> None:
|
||||
"""Set hardware DR registers via the native x64 SetThreadContext.
|
||||
Wow64SetThreadContext only updates the WOW64 context save area; the actual
|
||||
CPU hardware DR registers that trigger debug exceptions need the native path."""
|
||||
# Allocate 16-byte aligned buffer for x64 CONTEXT (size 0x4D0)
|
||||
raw = (ctypes.c_byte * (_CONTEXT64_SIZE + 15))()
|
||||
base = ctypes.addressof(raw)
|
||||
aligned = ctypes.c_void_p((base + 15) & ~15)
|
||||
ctypes.memset(aligned, 0, _CONTEXT64_SIZE)
|
||||
ctypes.c_uint32.from_address(aligned.value + _CTX64_FLAGS_OFF).value = CONTEXT_AMD64_DEBUG
|
||||
# Read current native DR state (ignore failure on non-suspendable threads)
|
||||
k.GetThreadContext(thread_handle, aligned)
|
||||
# Write DR0-DR3 for each watchpoint slot
|
||||
for slot, wp_addr in self._watchpoints.addresses().items():
|
||||
ctypes.c_uint64.from_address(aligned.value + _CTX64_DR0_OFF + slot * 8).value = wp_addr
|
||||
# Clear remaining DR slots
|
||||
active_slots = set(self._watchpoints.addresses().keys())
|
||||
for slot in range(4):
|
||||
if slot not in active_slots:
|
||||
ctypes.c_uint64.from_address(aligned.value + _CTX64_DR0_OFF + slot * 8).value = 0
|
||||
ctypes.c_uint64.from_address(aligned.value + _CTX64_DR6_OFF).value = 0
|
||||
ctypes.c_uint64.from_address(aligned.value + _CTX64_DR7_OFF).value = self._watchpoints.compute_dr7()
|
||||
k.SetThreadContext(thread_handle, aligned)
|
||||
|
||||
def _apply_watchpoints_to_thread(self, thread_handle: int) -> None:
|
||||
"""Set DR0-DR3 and DR7 on one thread. Silently skips 64-bit WOW64 helper threads
|
||||
(Wow64GetThreadContext returns ERROR_ACCESS_DENIED on those)."""
|
||||
"""Set DR0-DR3 and DR7 on one thread. Uses both the WOW64 context (for Wow64GetThreadContext
|
||||
readback) and the native x64 context (for actual hardware DR register effect on WOW64 threads).
|
||||
Silently skips 64-bit WOW64 helper threads where Wow64GetThreadContext returns access denied."""
|
||||
# Native x64 path: sets actual hardware DR registers (Wow64SetThreadContext alone is not enough)
|
||||
self._set_native_dr_registers(thread_handle)
|
||||
# WOW64 path: keeps WOW64_CONTEXT DR fields in sync for get_registers readback
|
||||
try:
|
||||
ctx = self._get_context(thread_handle)
|
||||
except OSError:
|
||||
|
|
@ -185,13 +251,13 @@ class DebugAgent:
|
|||
|
||||
def _get_context(self, thread_handle: wintypes.HANDLE) -> WOW64_CONTEXT:
|
||||
ctx = WOW64_CONTEXT()
|
||||
ctx.ContextFlags = CONTEXT_FULL
|
||||
ctx.ContextFlags = CONTEXT_ALL
|
||||
if not k.Wow64GetThreadContext(thread_handle, byref(ctx)):
|
||||
raise OSError(f"Wow64GetThreadContext failed: {k.GetLastError()}")
|
||||
return ctx
|
||||
|
||||
def _set_context(self, thread_handle: wintypes.HANDLE, ctx: WOW64_CONTEXT) -> None:
|
||||
ctx.ContextFlags = CONTEXT_FULL
|
||||
ctx.ContextFlags = CONTEXT_ALL
|
||||
if not k.Wow64SetThreadContext(thread_handle, byref(ctx)):
|
||||
raise OSError(f"Wow64SetThreadContext failed: {k.GetLastError()}")
|
||||
|
||||
|
|
@ -229,6 +295,12 @@ class DebugAgent:
|
|||
|
||||
code = evt.dwDebugEventCode
|
||||
|
||||
# Stale events from a previously-terminated process (drained but not fully gone)
|
||||
# can arrive after a new launch. Ignore them to avoid misidentifying the new game.
|
||||
if evt.dwProcessId != self._process_info.dwProcessId:
|
||||
k.ContinueDebugEvent(evt.dwProcessId, evt.dwThreadId, DBG_CONTINUE)
|
||||
continue
|
||||
|
||||
if code == EXIT_PROCESS_DEBUG_EVENT:
|
||||
# For an exit event the union holds EXIT_PROCESS_DEBUG_INFO; its first dword
|
||||
# (dwExitCode) overlaps ExceptionRecord.ExceptionCode, so this reads the code.
|
||||
|
|
@ -250,11 +322,6 @@ class DebugAgent:
|
|||
self._pending_event = (evt.dwProcessId, evt.dwThreadId)
|
||||
return result
|
||||
|
||||
# Not stopping: continue past this event. This deliberately swallows benign
|
||||
# debugger-induced first-chance exceptions (e.g. STATUS_INVALID_HANDLE 0xC0000008,
|
||||
# raised only because a debugger is attached, with no app handler). Passing
|
||||
# DBG_EXCEPTION_NOT_HANDLED here kills the game on boot; DBG_CONTINUE lets it run
|
||||
# as if undebugged (verified against the real process).
|
||||
k.ContinueDebugEvent(evt.dwProcessId, evt.dwThreadId, DBG_CONTINUE)
|
||||
|
||||
return {"status": "running", "reason": None, "exit_code": None}
|
||||
|
|
@ -312,6 +379,9 @@ class DebugAgent:
|
|||
self._write_process_memory(rearm_addr, b"\xCC")
|
||||
return None # transparent; do not stop
|
||||
|
||||
# AV: stop and report. The game loops on DBG_CONTINUE for its boot-time AVs, so the
|
||||
# caller must explicitly continue past each one. Callers that don't care about AVs
|
||||
# can loop: while r['reason'] and 'access_violation' in r['reason']: r = continue()
|
||||
if exc_code == EXCEPTION_ACCESS_VIOLATION:
|
||||
self._stopped_thread_id = evt.dwThreadId
|
||||
return {"status": "stopped", "reason": f"access_violation@0x{addr:X}", "exit_code": None}
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ from __future__ import annotations
|
|||
|
||||
import base64
|
||||
import json
|
||||
import queue
|
||||
import socketserver
|
||||
import threading
|
||||
|
||||
|
|
@ -27,6 +28,15 @@ def _op_launch(agent: DebugAgent, params: dict) -> dict:
|
|||
return {}
|
||||
|
||||
|
||||
@_op("launch_with_breakpoints")
|
||||
def _op_launch_with_breakpoints(agent: DebugAgent, params: dict) -> dict:
|
||||
"""Launch and immediately plant breakpoints before the WOW64 loader hardens page protection."""
|
||||
agent.launch(params["path"], params["cwd"])
|
||||
for addr_hex in params.get("breakpoints", []):
|
||||
agent.set_breakpoint(int(addr_hex, 16), None)
|
||||
return {}
|
||||
|
||||
|
||||
@_op("set_breakpoint")
|
||||
def _op_set_breakpoint(agent: DebugAgent, params: dict) -> dict:
|
||||
agent.set_breakpoint(int(params["addr"], 16), params.get("condition"))
|
||||
|
|
@ -94,26 +104,52 @@ def _op_get_status(agent: DebugAgent, params: dict) -> dict:
|
|||
return agent.get_status()
|
||||
|
||||
|
||||
class _DebugWorker(threading.Thread):
|
||||
"""Single dedicated thread that owns ALL debug agent calls.
|
||||
Windows requires CreateProcessA, WaitForDebugEvent, ContinueDebugEvent, etc. to
|
||||
all be called from the same OS thread. ThreadingTCPServer violates this by using a
|
||||
fresh thread per connection. This worker serializes every op on one thread via a queue."""
|
||||
|
||||
def __init__(self, agent: DebugAgent) -> None:
|
||||
super().__init__(daemon=True, name="debug-worker")
|
||||
self._agent = agent
|
||||
self._q: queue.Queue = queue.Queue()
|
||||
|
||||
def run(self) -> None:
|
||||
while True:
|
||||
op_name, params, result_q = self._q.get()
|
||||
try:
|
||||
if op_name not in _OPS:
|
||||
raise ValueError(f"unknown op: {op_name!r}")
|
||||
result = _OPS[op_name](self._agent, params)
|
||||
result_q.put(("ok", result))
|
||||
except WatchpointLimitError as exc:
|
||||
result_q.put(("err", str(exc)))
|
||||
except Exception as exc: # noqa: BLE001
|
||||
result_q.put(("err", f"{type(exc).__name__}: {exc}"))
|
||||
|
||||
def call(self, op_name: str, params: dict, timeout: float = 120.0):
|
||||
result_q: queue.Queue = queue.Queue()
|
||||
self._q.put((op_name, params, result_q))
|
||||
return result_q.get(timeout=timeout)
|
||||
|
||||
|
||||
class _Handler(socketserver.StreamRequestHandler):
|
||||
def handle(self) -> None:
|
||||
agent: DebugAgent = self.server.agent # type: ignore[attr-defined]
|
||||
lock: threading.Lock = self.server.lock # type: ignore[attr-defined]
|
||||
worker: _DebugWorker = self.server.worker # type: ignore[attr-defined]
|
||||
for line in self.rfile:
|
||||
if not line.strip():
|
||||
continue
|
||||
request = decode_message(line)
|
||||
op_name = request["op"]
|
||||
params = request.get("params", {})
|
||||
try:
|
||||
with lock:
|
||||
if op_name not in _OPS:
|
||||
raise ValueError(f"unknown op: {op_name!r}")
|
||||
result = _OPS[op_name](agent, params)
|
||||
response = {"ok": True, "result": result}
|
||||
except WatchpointLimitError as exc:
|
||||
response = {"ok": False, "error": str(exc)}
|
||||
except Exception as exc: # noqa: BLE001
|
||||
response = {"ok": False, "error": f"{type(exc).__name__}: {exc}"}
|
||||
# continue_execution can block for its full timeout plus processing overhead
|
||||
op_timeout = params.get("timeout", 30) + 10 if op_name == "continue_execution" else 30.0
|
||||
status, payload = worker.call(op_name, params, timeout=op_timeout)
|
||||
if status == "ok":
|
||||
response = {"ok": True, "result": payload}
|
||||
else:
|
||||
response = {"ok": False, "error": payload}
|
||||
self.wfile.write((json.dumps(response) + "\n").encode("utf-8"))
|
||||
|
||||
|
||||
|
|
@ -124,9 +160,10 @@ class _Server(socketserver.ThreadingTCPServer):
|
|||
|
||||
def run_agent_server(agent: DebugAgent, port: int) -> None:
|
||||
"""Blocks forever, serving the local-only debug agent API on 127.0.0.1:port."""
|
||||
worker = _DebugWorker(agent)
|
||||
worker.start()
|
||||
server = _Server(("127.0.0.1", port), _Handler)
|
||||
server.agent = agent # type: ignore[attr-defined]
|
||||
server.lock = threading.Lock() # type: ignore[attr-defined]
|
||||
server.worker = worker # type: ignore[attr-defined]
|
||||
server.serve_forever()
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -73,6 +73,14 @@ def build_mcp_server(client: DebugAgentClient) -> FastMCP:
|
|||
def launch(path: str, cwd: str) -> dict:
|
||||
return client.call("launch", {"path": path, "cwd": cwd})
|
||||
|
||||
@mcp.tool()
|
||||
def launch_with_breakpoints(path: str, cwd: str, breakpoints: list) -> dict:
|
||||
"""Launch the game and plant INT3 breakpoints atomically before the WOW64 loader
|
||||
hardens .text page protection. Use instead of launch() + set_breakpoint() when
|
||||
setting breakpoints at process start."""
|
||||
return client.call("launch_with_breakpoints",
|
||||
{"path": path, "cwd": cwd, "breakpoints": breakpoints})
|
||||
|
||||
@mcp.tool()
|
||||
def set_breakpoint(addr: str, condition: dict | None = None) -> dict:
|
||||
return client.call("set_breakpoint", {"addr": addr, "condition": condition})
|
||||
|
|
|
|||
Loading…
Reference in a new issue