[Draft] X2WinRpcAdapter: remote Windows debugging over FlatBuffers RPC - #1174
Draft
Weitao-Sun wants to merge 14 commits into
Draft
[Draft] X2WinRpcAdapter: remote Windows debugging over FlatBuffers RPC#1174Weitao-Sun wants to merge 14 commits into
Weitao-Sun wants to merge 14 commits into
Conversation
Introduces a new cross-platform X2WIN_RPC debug adapter that will talk to a Windows-side stub (x2winstub, WIN32-only, scaffolded but not yet implemented) over a custom TCP RPC protocol, to support debugging Windows targets from macOS/Linux without depending on lldb-server's immature Windows support or DbgEng's Windows-only client library. Lifecycle (Attach/Connect/Execute/Detach/Quit) and GetTargetArchitecture are implemented against the wire protocol; the remaining DebugAdapter methods are placeholder stubs to keep the class concrete while the protocol and stub are built out incrementally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Fixes the connect/reconnect crash risk (ConnectSocket now no-ops if already connected instead of reassigning a live thread), reads the stub address from adapter settings instead of a hardcoded value, and adds the attach.pid setting the built-in Attach-to-Process flow relies on internally to carry the selected pid. Also wires up the TargetStopped event end-to-end: Detach/Quit now post DetachedEventType/TargetExitedEventType so DebuggerController's connection-state tracking and WaitForAdapterStop() don't get stuck, and ReaderLoop() decodes the stub's stop-reason byte into a real DebugStopReason instead of dropping Event frames on the floor. Verified end-to-end against a throwaway Python stub: connect, list fake processes, attach, receive the stopped notification, detach, and attach again all work. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Replaces the hand-rolled frame format (manual FrameType/MethodId enums and byte-packing helpers) with a single protobuf Envelope message using a oneof to distinguish requests/responses/events, defined in protocol/x2win.proto (replacing the empty placeholder). This removes an entire class of manual encode/decode bugs and gives the not-yet-written Windows stub an unambiguous schema to implement against instead of reverse-engineering byte offsets. Protobuf is wired into core/CMakeLists.txt the same way LLDB already is: an externally-built dependency located via a PROTOBUF_PATH environment variable with a platform-appropriate default, not vendored or fetched by the build. build.md documents building it from source as a static lib (so debuggercore doesn't pick up a runtime dependency on a system-installed Protobuf); the CMAKE_CXX_STANDARD=20 flag in those instructions is required to avoid an Abseil ABI mismatch between its installed headers and compiled binaries. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Go() and both AddBreakpoint() overloads were still stub returns that never touched the wire; Resume and SetBreakpoint requests silently did nothing. ConnectToDebugServer() was unimplemented entirely. All three now round-trip through CallSync() the same way Attach()/Detach() already did. ReaderLoop() also stashes the reason/address from each TargetStoppedEvent into new atomic members so StopReason()/GetInstructionOffset() can report real values instead of hardcoded UnknownReason/0 -- needed for DebuggerController's stop-reason-driven resume logic to behave correctly. AddBreakpoint(ModuleNameAndOffset&) needed ResolveModuleAddress(), which was declared but never defined; added it following LldbAdapter's pattern. protocol/x2win.proto gains the corresponding ConnectServerRequest/Response, GoRequest/Response, SetBreakpointRequest/Response + BreakpointType, and an address field on TargetStoppedEvent plus STOP_REASON_INITIAL_BREAKPOINT. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Protocol: - Finish the switch from protobuf to flatbuffers (vendor/flatbuffers submodule, protocol/x2win.fbs replaces x2win.proto) and bring x2winstub's local mirror fully up to date (main.cpp, net/, debug/WindowsDebugEngine port, x2win_session). - Add StepIntoRequest/StepOverRequest/BreakIntoRequest/RemoveBreakpointRequest message pairs, and a size field on ModuleEntry. core/adapters/x2winrpcadapter.cpp: - Wire StepInto()/StepOver()/BreakInto()/RemoveBreakpoint() over the new RPCs, same CallSync pattern as Go(). - GetBreakpointList() now serves from a locally-maintained cache (kept in sync by AddBreakpoint()/RemoveBreakpoint()) instead of a live RPC, since the base class declares it const and CallSync() can't be called from a const method. - GetModuleList() extracts the module basename itself (recognizing both '/' and '\\') before storing it as short_name -- DebugModule::GetPathBaseName() only recognizes '\\' when compiled for Windows, which broke module-name matching (and therefore auto-rebase) since X2WinRpcAdapter is the first adapter where BN core can run on a different OS than the Windows debug target. - common.inputFile is now auto-populated from the BinaryView's file path (GenerateDefaultAdapterSettings, same convention as every other adapter), fixing the same rebase-matching path from the other side. - Go()/StepInto()/StepOver() now post ResumeEventType/StepIntoEventType/ StepOverEventType on success, which is what actually drives DebuggerState::IsRunning() -- previously always false for this adapter, which also meant CanResumeTarget() never blocked a second Go/Step while one was already in flight. core/debuggercontroller.cpp: - ApplyOwnStateForEvent: add StepOverEventType alongside Resume/StepIntoEventType so it also flips execution status to Running (additive only -- no existing adapter ever posts this event, so no behavior change for anyone else). x2winstub/CMakeLists.txt: - Add NOMINMAX/WIN32_LEAN_AND_MEAN so <Windows.h>'s max/min macros stop mangling flatbuffers' std::numeric_limits<T>::max() calls -- this was only surfacing on a genuinely clean build; incremental builds had been silently reusing stale .obj files for main.cpp/net/connection.cpp/x2win_session.cpp across several rounds of protocol changes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cAdapter Register read/write: - protocol/x2win.fbs gains ReadAllRegistersRequest/Response, ReadRegisterRequest/Response, WriteRegisterRequest/Response, and a RegisterEntry table (name/value/width/register_index). Values are uint64 -- X2Win only ever targets x86/x64 Windows. - X2WinRpcAdapter::ReadAllRegisters()/ReadRegister()/WriteRegister() were stub returns; now round-trip through CallSync() like the other RPCs. Breakpoint resync after (re)connect: - DebuggerBreakpoints::Apply() replays every known breakpoint from CreateDebugAdapter(), which runs before Attach()/ExecuteWithArgs()/ Connect() has actually opened the socket -- AddBreakpoint() used to just fail silently in that window, so breakpoints never made it to a freshly (re)connected stub. AddBreakpoint(ModuleNameAndOffset&) now stages into m_pendingBreakpoints when not yet connected (or when the module isn't resolvable yet), and the new ApplyBreakPoints() flushes it once connected and again on every TargetStoppedEvent -- same shape as LldbAdapter::ApplyBreakpoints()'s pending-breakpoint handling. - RemoveBreakpoint() now also checks m_pendingBreakpoints first, so removing a breakpoint that hadn't been flushed yet doesn't silently no-op and then reappear on the next flush. - TeardownConnection() now clears m_breakpoints -- entries from a dead connection aren't trustworthy after a reconnect (fresh stub session, or a resend from DebuggerBreakpoints::Apply() racing a stale cached entry into a duplicate/ghost breakpoint). GetProcessList() no longer self-connects: - It used to call ConnectFromSettings() itself, independent of the controller's Launch/Attach/Connect/ConnectToDebugServer lifecycle. In target mode this could open a connection to a stub that immediately pushes an unsolicited TargetStoppedEvent on accept, which could drive DetectLoadedModule()/autoRebase through a path that never ran CreateDebuggerBinaryView() -- crashing on a null memory accessor. Now it just checks m_connected, matching GdbAdapter (unimplemented) and LldbAdapter (only ever queries an already-live backend session). Launch/Restart: - launch.executablePath/workingDirectory/commandLineArguments were never registered as adapter settings, so DebuggerState::GetExecutablePath() always returned "" and any Launch (including Restart's Quit-then-Launch) sent an empty path to the stub. Settings added, deliberately without a local file-picker uiSelectionAction since the path is a remote Windows path, not a local one. - ExecuteWithArgs() now refuses immediately (before touching the network) when the last successful connection was via Connect() (the target-mode entry point, UI: "Connect to Remote Process") -- a target-mode stub only ever owns the one debuggee it was started with, same as plain gdbserver vs gdbserver --multi. Without this, Restart in target mode would Quit the debuggee (causing the stub to exit, per its reconnect-loop design) and then hang trying to reconnect to a stub that no longer exists. Also drops x2winstub/engine_port_task.md and read_memory_task.md, superseded by the x2winstub/instruction_note/ task-doc workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
SupportFeature() always returned false, so DebuggerController's StepOverAndWaitInternal() never used the already-wired native StepOver RPC and instead fell back to software step-over emulation. Report StepOver and Modules as supported since both are implemented over RPC; StepReturn, StepOverReverse, Threads, and TTD remain false since the stub doesn't support them yet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
protocol/x2win.fbs gains WriteMemoryRequest/WriteMemoryResponse, mirroring ReadMemoryRequest/Response's synchronous request/response shape (address + byte vector in, success bool out, no separate async event). X2WinRpcAdapter::WriteMemory() was a stub returning false; now round-trips through CallSync() like ReadMemory()/WriteRegister(). This is what backs DebuggerFileAccessor::Write() (core/debuggerfileaccessor.cpp), i.e. editing bytes in the hex view or bv.write() against the live process view during a debug session. Verified end-to-end against the stub (write + read-back). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…worker deadlock in X2WinRpcAdapter Thread management: - protocol/x2win.fbs gains GetThreadListRequest/Response (ThreadEntry: tid/rip/is_frozen), GetActiveThreadIdRequest/Response, SetActiveThreadIdRequest/Response, SuspendThreadRequest/Response, and ResumeThreadRequest/Response. - X2WinRpcAdapter::GetThreadList()/GetActiveThread()/GetActiveThreadId()/ SetActiveThread()/SetActiveThreadId()/SuspendThread()/ResumeThread() were stub returns; now round-trip through CallSync(). GetActiveThread() derives rip from GetInstructionOffset() (the last reported stop) rather than a separate RPC, since BN only ever stops the whole process, never a single thread. - SupportFeature() now reports DebugAdapterSupportThreads. Hardware breakpoints: - protocol/x2win.fbs gains SetHardwareBreakpointRequest/Response and RemoveHardwareBreakpointRequest/Response (address/type/size triple, not an allocated id -- mirrors a debug register slot's own identity rule). - The 4 AddHardwareBreakpoint()/RemoveHardwareBreakpoint() overloads (absolute address and ModuleNameAndOffset) always returned false; now wire through CallSync(), reusing core's PendingHardwareBreakpoint to stage before the adapter is connected -- DebuggerBreakpoints::Apply() calls these unconditionally from CreateDebugAdapter(), same pre-connect timing problem AddBreakpoint(ModuleNameAndOffset&) already had to solve. ApplyBreakPoints() now flushes both the software and hardware pending lists. Report process exit over the wire (fixes a worker-thread deadlock): - StopReason gains EXITED, and TargetStoppedEvent gains exit_code. Stub-side process exit was previously invisible to BN core entirely -- the stub detects it (WindowsDebugEngine posts an internal TargetExited event) but nothing on the wire ever reported it, so DebuggerController:: WaitForAdapterStop() (an untimed condition_variable::wait) would block forever after a Go() whose target ran to completion on its own, and the real Detach()/Quit() RPC -- queued behind that stuck worker op -- would never even reach the stub. Only the out-of-band RequestInterrupt() -> BreakInto() (fired once per Detach/Quit click, on its own thread) made it onto the wire, uselessly, since the process was already gone. - ReaderLoop() now branches on StopReason_EXITED: caches the exit code, sets m_lastStopReason to ProcessExited, and posts TargetExitedEventType instead of AdapterStoppedEventType (skipping the ApplyBreakPoints() resync -- nothing to resend to). ExitCode() now returns the cached value instead of a hardcoded 0. - BreakInto() skips the RPC round trip entirely when m_lastStopReason is already ProcessExited, instead of logging a "stub reported failure" that isn't telling us anything new (RequestInterrupt() calls it unconditionally before every Detach()/Quit(), regardless of whether the target is still running). Also strips a stray trailing "\n" from one LogWarn call (Log already appends its own newline). Corresponding stub-side changes (x2win_session.cpp HandleRequest cases for the new thread/hardware-breakpoint RPCs, and OnEngineEvent forwarding TargetExited) delivered separately via x2winstub/instruction_note/ task docs, per the BN-core/stub split -- see x2winrpcadapter-task-doc-workflow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Call stacks:
- protocol/x2win.fbs gains FrameEntry (index/pc/sp/fp/function_name/
function_start/module -- mirrors BN's DebugFrame) and
GetFramesOfThreadRequest/Response.
- X2WinRpcAdapter::GetFramesOfThread() fell back to DebugAdapter's default
(always {}), so the Stack Trace sidebar was always empty; now round-trips
through CallSync() like GetThreadList(). WindowsDebugEngine::
GetFramesOfThread() (StackWalk64-based, ported from WindowsNativeAdapter)
already did the actual unwinding, just wasn't wired through the proto
surface.
StepReturn:
- protocol/x2win.fbs gains StepReturnRequest/Response (no fields, mirrors
StepIntoRequest/StepOverRequest's shape).
- X2WinRpcAdapter::StepReturn() was unimplemented (same default-false
fallback), now wired the same way. WindowsDebugEngine::StepReturn()
already existed and uses the newly-wired GetFramesOfThread() internally
(direct C++ call, not a second RPC round trip) to find the caller's
return address and set a temporary breakpoint there.
- SupportFeature() now reports DebugAdapterSupportStepReturn.
Verified end-to-end against a multi-threaded test binary: call stacks
correctly unwind through user code -> CRT startup -> kernel32/ntdll thread
trampolines for every thread, and StepReturn correctly stops at the return
address in the caller rather than single-stepping.
Corresponding stub-side changes (x2win_session.cpp HandleRequest cases for
the two new RPCs) delivered separately via x2winstub/instruction_note/ task
docs, per the BN-core/stub split -- see x2winrpcadapter-task-doc-workflow.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GetStackPointer:
- X2WinRpcAdapter didn't override this, so it fell back to DebugAdapter's
default (always 0). No new RPC needed -- same trick as
GdbMiAdapter::GetStackPointer(): reuse the already-wired ReadRegister()
and read rsp/esp (X2Win only ever targets x86/x64 Windows, so no need for
GdbMiAdapter's fuller architecture-name switch).
GetMemoryMap:
- protocol/x2win.fbs gains MemoryRegionEntry (start/size/name/read/write/
execute/shared -- mirrors BN's DebugMemoryRegion) and
GetMemoryMapRequest/Response.
- X2WinRpcAdapter::GetMemoryMap() fell back to DebugAdapter's default
(always {}), so the Memory Map sidebar was always empty; now round-trips
through CallSync() like GetModuleList(). WindowsDebugEngine::
GetMemoryMap() (ported from WindowsNativeAdapter) already did the actual
region enumeration, just wasn't wired through the proto surface.
Verified end-to-end: SP now shows a real value in the register view instead
of 0, and the Memory Map sidebar populates with the target's regions.
Corresponding stub-side change (x2win_session.cpp's Body_GetMemoryMapRequest
case) delivered separately via x2winstub/instruction_note/ task docs, per
the BN-core/stub split -- see x2winrpcadapter-task-doc-workflow.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…in X2WinRpcAdapter - DisconnectDebugServer(): send QuitRequest and tear down the connection, mirroring the Server-mode counterpart to ConnectToDebugServer. - Detach()/Quit(): only fully TeardownConnection() for target-mode connections; for server-mode, reset session state instead so the underlying socket connection to the stub survives (it can still be reused for a subsequent Launch()/Attach()). - Factor the breakpoint/stop-state clearing out of TeardownConnection() into a shared ResetSessionState(), and extend it to also clear pending (hardware) breakpoints and last-stop/exit-code state.
This monorepo's x2winstub/ mirror had fallen behind the actual X2WinStub
repo checked out on the remote Windows box (10.42.4.10), which is where
it's actually built/run/debugged and carries its own git history. Pulled
the current tracked state of that repo (branch
wire-get-memory-map-and-fix-arg-parsing, 75fec60) into this mirror, i.e.
everything its own .gitignore doesn't exclude (build/, clangdLoc/,
.claude/, .vscode/, instruction_note/) and skipping testBinaries/ (also
untracked there) and vendor/flatbuffers (a real git submodule there,
building standalone; this mirror instead reuses this repo's own
vendor/flatbuffers via the nested add_subdirectory(x2winstub) path, so it
doesn't need its own copy -- see x2winstub/CMakeLists.txt's
`if(NOT TARGET x2win_fbs)` guard).
Covers remote's last several commits, wiring up over RPC: StepInto/
StepOver, BreakInto/RemoveBreakpoint, GetProcessList (+ restricting
Attach to Server mode), registers, WriteMemory, thread management,
hardware breakpoints/watchpoints, TargetExited forwarding,
GetFramesOfThread/GetMemoryMap/StepReturn, and a target-mode
reconnect/--ip/--port argument-parsing fix -- matching the BN-core side
already wired in this repo's own recent commits.
Also pulled over KNOWN_ISSUES.md (untracked on remote, not yet committed
there either) and debug/debug_loop.{cpp,h}.superseded, the pre-port
WinAPI debug loop kept there for reference (superseded by
windows_debug_engine.cpp).
Verified: debuggercore still builds clean locally (x2winstub itself is
Windows-only and can't be built on this machine).
Fix build.md: describe FlatBuffers, not stale Protobuf/Abseil wording. The wire protocol switched from Protobuf to FlatBuffers a while back (see protocol/x2win.fbs, vendor/flatbuffers), but this doc's build instructions never got updated to match -- it still described a two-submodule Protobuf+Abseil setup. Found while sweeping the repo for leftover protobuf references (everything else -- PROTOBUF_PATH, find_package(Protobuf), .proto/.pb.h/.pb.cc, vendor/protobuf submodule entries -- was already clean). Rename x2winstub/KNOWN_ISSUES.md to x2winstub/STATUS.md and expand it: - Add a top-level summary of what X2WinRpcAdapter/x2winstub currently supports and doesn't. - Note that build/run against the remote Windows dev box is confirmed, but passing this repo's Jenkins CI build is not yet confirmed. - Add known issue: X2WinRpcAdapter::Go() never posts a ResumeEventType, so the Binary Ninja UI doesn't reflect the target running until the next stop event arrives. - Drop the --ip/--port known issue (fixed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft PR — not ready to merge. Opening early for visibility while the
remaining known issues below get fixed and Jenkins CI status is confirmed.
What's here
X2WinRpcAdapter(BN-core) +x2winstub(remote Windows stub), talking overa FlatBuffers RPC protocol, for debugging Windows targets on a separate box.
Supported: Server-mode and Target-mode connect, launch/attach/detach/quit,
full execution control (go/step into/over/return/break-into), software +
hardware breakpoints, memory read/write + memory map, register read/write,
thread list/switch/suspend/resume, module list, call stack, target arch.
Not supported: reverse step-over, Time Travel Debugging (TTD).
Known issues (see STATUS.md for details)
shared breakpoint is hit by more than one thread at once.
reused across Attach/Launch cycles (not yet observed, but reachable).
X2WinRpcAdapter::Go()doesn't post aResumeEventType, so the BN UIdoesn't show the target as running until it next stops.
Build status
Builds/runs against the remote Windows dev box. Not yet confirmed to pass
this repo's Jenkins CI build.
🤖 Generated with Claude Code