Repository navigation
feat: a disassembly can carry the caller's clock, bounded between lines - #200
Merged
Merged
Conversation
Closes #149. ## The decision the issue asked for first #149 is the sibling of #95: `disassemble(address, count)` reaches the engine with no deadline, and each line it renders resolves symbols, so a module with deferred symbols can send one line to a symbol server mid-loop. The issue asks the same two questions #95 did — whether a watchdog's `SetInterrupt` can reach a blocked typed call, or whether the honest answer is that the bound is unkeepable; and whether the budget covers the whole loop or each line. Whole-loop, and no watchdog — because this is `read_memory`'s shape, not `reload`'s. `disassemble` is already a *loop* of `IDebugControl::Disassemble` calls, one per line, in this crate's own code. So the bound is the loop, checked between lines, exactly as `read_memory_bounded`'s is checked between pages — and like that one it "needs no answer about `SetInterrupt` at all" (#168's words). `reload_symbols_bounded` armed a watchdog because a reload is a single call with no loop to bound and the watchdog was *measured* to reach it; a disassembly has a loop, so it does not. ## The bound this crate can keep `disassemble_bounded(address, count, budget_ms)` checks the deadline **before each line** and reads a host's `interrupt()` between lines too, so `budget_ms == 0` still buys a walk a host can stop. It returns `Disassembly { instructions, cut_short }` — the shape `MemoryRead` and `CommandRun` have, and for the same reason: a listing cut short by the clock is a prefix of a function still there, and reading its last instruction as the function's end invents a fall-through the code does not have. - **The residual is one line, and that is the whole of what #149 is about.** A single `Disassemble` blocked loading a symbol runs to its own completion — nothing lies between its start and its return — so a thousand-instruction function no longer spends a thousand symbol loads past the deadline, only the one it was inside when the clock ran out. Whether `SetInterrupt` reaches that one blocked call is left open exactly as #95/#168 left it for `ReadVirtual`: a live kernel behind a link slow enough for a call to sit in is the only thing that would settle it. - `disassemble` stays **unbounded** — the hot path for the listing tool, the parallel of `read_memory` staying unbounded beside `read_memory_bounded`. Both now drive one `disassemble_lines` loop, so they cannot drift in how they render a line or what they make of one that fails. ## Evidence `disassemble_lines` takes its line-renderer and halt as closures, the way `read_chunked` takes its reader, so every rule is asserted with no debuggee: the stop lands between two renderings, a walk that finished is never reported cut short, a host request is read between lines, the first rendering failing is the caller's error while a later one ends the walk with what it had, a budget already behind reads and renders nothing, and the non-advance guard still holds. `cargo check --all-targets` and `cargo fmt --all --check` clean on `x86_64-pc-windows-msvc` from a Mac; `cargo test --lib` green on the ARM64 bench (466 passed, 0 failed, 13 ignored) against this crate's 0.3.0 base. Two mutations verified by hand against that bench: moving the halt check after the render fails `..._out_of_time_before_the_first_line_reads_nothing` and `..._stops_between_lines_and_keeps_what_it_had` with their own diagnostics, and dropping the `cut_short` on the halt return fails the three tests that expect `Some` while leaving the two that expect `None`. ## Dependent change windbg-mcp's reachability walk (`function_listing` in `src/worker.rs`) calls `e.disassemble` in runs and polls only between its outer calls, which cannot bound a run blocked inside the engine; it carries this issue's note as a comment at the call site and in `docs/limitations.md`. It passes the walk's remaining budget to `disassemble_bounded` once this merges and the `rev` pin moves, the way the `uf` half already spends its budget — FOLLOWUPS items 13 and 56. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01H7Gnmok17PyEYbcc3aHUm5
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
Closes #149.
The decision the issue asked for first
#149 is the sibling of #95:
disassemble(address, count)reaches the engine with no deadline, and each line it renders resolves symbols, so a module with deferred symbols can send one line to a symbol server mid-loop. The issue asks the same two questions #95 did — whether a watchdog'sSetInterruptcan reach a blocked typed call, or whether the honest answer is that the bound is unkeepable; and whether the budget covers the whole loop or each line.Whole-loop, and no watchdog — because this is
read_memory's shape, notreload's.disassembleis already a loop ofIDebugControl::Disassemblecalls, one per line, in this crate's own code. So the bound is the loop, checked between lines, exactly asread_memory_bounded's is checked between pages — and like that one it "needs no answer aboutSetInterruptat all" (#168's words).reload_symbols_boundedarmed a watchdog because a reload is a single call with no loop to bound and the watchdog was measured to reach it; a disassembly has a loop, so it does not.The bound this crate can keep
disassemble_bounded(address, count, budget_ms)checks the deadline before each line and reads a host'sinterrupt()between lines too, sobudget_ms == 0still buys a walk a host can stop. It returnsDisassembly { instructions, cut_short }— the shapeMemoryReadandCommandRunhave, and for the same reason: a listing cut short by the clock is a prefix of a function still there, and reading its last instruction as the function's end invents a fall-through the code does not have.Disassembleblocked loading a symbol runs to its own completion — nothing lies between its start and its return — so a thousand-instruction function no longer spends a thousand symbol loads past the deadline, only the one it was inside when the clock ran out. WhetherSetInterruptreaches that one blocked call is left open exactly as read_memory has no watchdog, so a caller cannot promise when it will finish #95/feat: a read can be bounded, without pretending a watchdog reaches it #168 left it forReadVirtual: a live kernel behind a link slow enough for a call to sit in is the only thing that would settle it.disassemblestays unbounded — the hot path for the listing tool, the parallel ofread_memorystaying unbounded besideread_memory_bounded. Both now drive onedisassemble_linesloop, so they cannot drift in how they render a line or what they make of one that fails.Evidence
disassemble_linestakes its line-renderer and halt as closures, the wayread_chunkedtakes its reader, so every rule is asserted with no debuggee: the stop lands between two renderings, a walk that finished is never reported cut short, a host request is read between lines, the first rendering failing is the caller's error while a later one ends the walk with what it had, a budget already behind reads and renders nothing, and the non-advance guard still holds.cargo check --all-targetsandcargo fmt --all --checkclean onx86_64-pc-windows-msvcfrom a Mac;cargo test --libgreen on the ARM64 bench (466 passed, 0 failed, 13 ignored) against this crate's 0.3.0 base. Two mutations verified by hand against that bench: moving the halt check after the render fails..._out_of_time_before_the_first_line_reads_nothingand..._stops_between_lines_and_keeps_what_it_hadwith their own diagnostics, and dropping thecut_shorton the halt return fails the three tests that expectSomewhile leaving the two that expectNone.Dependent change
windbg-mcp's reachability walk (
function_listinginsrc/worker.rs) callse.disassemblein runs and polls only between its outer calls, which cannot bound a run blocked inside the engine; it carries this issue's note as a comment at the call site and indocs/limitations.md. It passes the walk's remaining budget todisassemble_boundedonce this merges and therevpin moves, the way theufhalf already spends its budget (glslang/windbg-mcp FOLLOWUPS items 13 and 56).🤖 Generated with Claude Code
https://claude.ai/code/session_01H7Gnmok17PyEYbcc3aHUm5