From 43ea63ece40c98f400908a00f7504a7d80cdb932 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 24 Sep 2026 16:35:08 +0100 Subject: [PATCH] fix(heap): reserved address space is not a hole in the walk's coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A user-mode heap walk answered `coverage: Partial` on every healthy live process. What held it there was not damage: it was the tails of subsegments and page ranges, reserved and never committed, filed as `Unreadable` because *would not read* was the only thing the walk could observe. A signal that fires on everything says nothing, which cost the one signal the walker has for "we could not see something". Three states hide behind one failed read — reserved, committed and paged out, committed and present — and the allocator's own records answer only the first split, from three structures that move between builds (`CommittedPageCount`, a VS `CommitBitmap`, an LFH commit state at `CommitStateOffset`). The memory manager answers all of it in one call and answers about the target rather than about what one allocator believes, so `DebugEngine::virtual_region` wraps `IDebugDataSpaces2::QueryVirtual` and `PoolState::Uncommitted` joins `Unreadable`, with `PoolState::is_coverage_gap` the single definition of which gap costs a walk its `complete`. Only a positive `MEM_RESERVE`/`MEM_FREE` excuses a gap. A failed query, a run that cannot advance, a state this crate does not name and a source that cannot be asked are each `None`, and `None` keeps the conservative reading — the excuse is never granted by an absence of evidence. Kernel sessions are not asked at all, so the pool walk is unchanged there, which for paged pool is the truth. A free chunk whose middle the allocator decommitted is the same question reached another way: it runs past the committed extent it starts in, and `walk_vs` emitted no span for it and cleared `complete` with no diagnostic. A span is geometry and state, both known there, so where the tail holds nothing the chunk is now reported; where it is memory the process has it is still refused, and now the walk names the chunk it dropped. The query's output buffer must be 16-byte aligned, which `MEMORY_BASIC_INFORMATION64` is not: the engine's live path copies its answer out with three `movaps` stores and faults inside dbgeng otherwise, while the dump path copies field by field and never complains. Testing it on a dump proves nothing about it. Measured on a live 26200 process (`sihost`, four Segment Heaps, 20,426 chunks, 2026-09-24): all 33 gaps `MEM_RESERVE`, an allocated chunk and a free one both `MEM_COMMIT`, and an answer that had been `Partial` is `Complete`. Checked from the other end on a thin dump of the same process: an address whose page the dump does not carry still answers `Committed`, the read still fails, and the walk still counts it. windbg-mcp FOLLOWUPS item 98. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MUhLUt9rB6zd42Y25btB3h --- CHANGELOG.md | 49 ++++ Cargo.toml | 13 +- README.md | 1 + docs/unknown-not-absent.md | 62 ++++- examples/heap_coverage.rs | 159 +++++++++++ src/dbgeng.rs | 124 +++++++++ src/heap.rs | 37 ++- src/pool.rs | 44 ++++ src/pool/index.rs | 27 +- src/pool/render.rs | 7 +- src/pool/snapshot.rs | 523 +++++++++++++++++++++++++++++++++++-- 11 files changed, 1015 insertions(+), 31 deletions(-) create mode 100644 examples/heap_coverage.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 070584c..a58c242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,57 @@ All notable changes to this project are documented here. The format follows ## [Unreleased] +### Added + +- **`DebugEngine::virtual_region`** — `IDebugDataSpaces2::QueryVirtual` as a typed answer + (`VirtualRegion`, `VirtualState`), which is what the memory manager says about a run of pages + rather than what the debugger can read there. `VirtualState::Unknown(u32)` keeps a state this + crate does not name instead of folding it into reserved or committed, because either guess is + what would make the primitive lie. User-mode only; a kernel session has no answer to give and + says so as an error. + + **Its output buffer must be 16-byte aligned**, which `MEMORY_BASIC_INFORMATION64` is not: the + engine's live-target path copies the 48-byte answer out with three `movaps` stores and takes + an access violation *inside dbgeng* otherwise. The dump path copies field by field, so the + same call against a full dump answered 22 queries from an 8-aligned buffer without complaint — + testing this on a dump proves nothing about it. Measured on 26200, 2026-09-24 + (`dbgeng!Ordinal367+0x14f96`, `movaps xmmword ptr [rbx],xmm0`). + +- **`examples/heap_coverage.rs`** — what, if anything, holds a user heap walk short of + `Complete`: every gap it filed, put back to the memory manager, with an allocated chunk and a + free one as controls. An address as a second argument answers that one question, which is how + the dump direction is checked. + ### Changed +- **A heap walk no longer calls reserved address space a hole in its own coverage.** A user-mode + walk came back `coverage: Partial` on every healthy live process, because the tails of + subsegments and page ranges — reserved and never committed — read the same way a paged-out + page does, and *would not read* was the only thing the walk could observe. `PoolState` gains + `Uncommitted` (and `HeapState` with it), and `PoolState::is_coverage_gap` is now the single + definition of which gap costs a walk its `complete`. + + What separates them is `virtual_region`, not the allocator's records: `CommittedPageCount`, + `CommitBitmap` and the LFH commit state are three structures that move between builds and say + what one allocator believes, while the memory manager answers about the target in one call. + Only a positive `MEM_RESERVE`/`MEM_FREE` excuses a gap — a failed query, a run that cannot + advance, an unnamed state and a source that cannot be asked are each conservative, so the + excuse is never granted by an absence of evidence. The kernel pool walk is not asked at all + and is unchanged. + + A free chunk whose middle the allocator decommitted is the same question reached another way: + it runs past the committed extent it starts in, and `walk_vs` emitted no span for it and + cleared `complete` **without a diagnostic**. A span is geometry and state, both of which are + known there, so where the tail holds nothing the chunk is now reported; where it is memory the + process has, it is still refused, and the walk now names the chunk it dropped. + + Measured on a live 26200 process (`sihost`, four Segment Heaps, 20,426 chunks, 2026-09-24): + every one of its 33 gaps `MEM_RESERVE`, both controls `MEM_COMMIT`, and an answer that had + been `Partial` is `Complete`. Checked from the other end on a thin dump of the same process: + an address whose page the dump does not carry still answers `Committed`, the read still fails, + and the walk still counts it. `HeapWalkReport` gains `uncommitted_gaps` so that what a + `Complete` answer forgave is still on the report. windbg-mcp `FOLLOWUPS.md` item 98. + - **The kernel pool walker takes ARM64 targets.** `pool::query` accepted `IMAGE_FILE_MACHINE_AMD64` and nothing else, so every `pool_*` query against an ARM64 kernel came back `pool walking supports x64 targets only (machine 0xaa64)`. It now admits ARM64 diff --git a/Cargo.toml b/Cargo.toml index 23c52f3..826e8df 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -63,6 +63,10 @@ features = [ "Win32_Foundation", "Win32_System_Diagnostics_Debug", "Win32_System_Diagnostics_Debug_Extensions", + # `MEMORY_BASIC_INFORMATION64` and the `MEM_*` state constants, which is what + # `IDebugDataSpaces2::QueryVirtual` answers in — the memory manager's own record of which + # pages are committed, as `DebugEngine::virtual_region` returns it. + "Win32_System_Memory", "Win32_System_SystemInformation", # `RtlUpcaseUnicodeChar`, which is how `object::same_object_name` folds a name: it is the # object manager's own fold rather than a reproduction of it. Under `Wdk` rather than `Win32` @@ -81,11 +85,16 @@ disarm64 = "0.2" # Only `examples/user_heap_smoke.rs` allocates on the host heap it then walks. Cargo unifies # this with the normal dependency when building examples and tests, and a downstream consumer -# of the library still resolves the four features above and no more. +# of the library still resolves the features above and no more. +# +# `Win32_System_Memory` is deliberately **not** repeated here even though the example calls +# `HeapCreate`/`HeapAlloc` from it: the library dependency now carries it, and listing it twice +# would leave a reader unable to tell which of the two is load-bearing — so deleting it from the +# library list above would still build, and the deletion would only be found by a downstream +# consumer. [dev-dependencies.windows] version = "0.62.2" features = [ - "Win32_System_Memory", "Win32_System_Threading", # `GetConsoleProcessList` and `EnumWindows`/`IsWindowVisible`, which is how the launch tests # check that a debuggee gets a console of its own and that the console has no window diff --git a/README.md b/README.md index cfce132..346ba3a 100644 --- a/README.md +++ b/README.md @@ -305,6 +305,7 @@ Per-session paged heaps are outside the initial pool-map scope, and the command | `examples/split_open.rs` | Re-validates the two-step openers, including a guard dropped before the engine is pumped. | | `examples/typed_context.rs` | Typed reads next to the debugger's own text for the same state. | | `examples/user_heap_smoke.rs` | Launches a child, allocates across size regimes, walks its Segment Heap and checks it against the child's own `HeapWalk`. | +| `examples/heap_coverage.rs` | What, if anything, holds a user heap walk short of `Complete` — every gap it filed, against the memory manager's own record of what is behind it. | | `examples/register_description.rs` | The full register description, not one flag of it. | ## Building diff --git a/docs/unknown-not-absent.md b/docs/unknown-not-absent.md index 25fd693..3d91b3c 100644 --- a/docs/unknown-not-absent.md +++ b/docs/unknown-not-absent.md @@ -65,10 +65,62 @@ Two further properties of it are load-bearing: `WalkCoverage`. No caller has to know that "incomplete" has more than one cause, and none can invent the distinction differently. -**It is not implied by the diagnostics.** A walk can end incomplete having said nothing at all — -`walk_vs` clears completeness when a readable region stops mid-chunk, without a message. A -caller that wants to reject partial results consults `coverage` and never the message list. -The inverse also holds: a walk with thousands of diagnostics can be `Complete`. +**It is not implied by the diagnostics.** A walk can end incomplete having said nothing at all, +and a walk with thousands of diagnostics can be `Complete`. A caller that wants to reject +partial results consults `coverage` and never the message list. (`walk_vs` clearing +completeness at a chunk running past a committed extent used to be the standing example of the +silent case; it now names the chunk it dropped — see below.) + +### A page that will not read is three things, and only one of them is a gap + +A user-mode heap walk was `Partial` on every healthy live process. That is the same failure as +reporting a partial reading as a total one, reached from the other side: a signal that fires on +everything says nothing. What held it there was not damage. It was the tails of subsegments and +page ranges — address space the allocator reserved and never committed — filed as `Unreadable`, +because *would not read* was the only thing the walk could observe. + +Three states hide behind that one observation, and the failed read does not separate them: + +| What it is | What it means for coverage | +|---|---| +| Reserved, or decommitted — no pages behind it | Nothing could have been there, so nothing was missed. | +| Committed and paged out, or absent from a dump | The target has this memory and the walk did not see it. A real gap. | +| Committed and present | Not a gap at all — it read. | + +The allocator's own records can answer the first distinction: a page range descriptor's +`CommittedPageCount`, a VS subsegment's `CommitBitmap`, an LFH subsegment's commit state at +`CommitStateOffset`. That is three structures, each of which moves between builds, to learn +what one allocator believes. The **memory manager** answers all of it in one call, about the +target rather than about the allocator: `DebugEngine::virtual_region`, which is +`IDebugDataSpaces2::QueryVirtual` returning `MEM_RESERVE`, `MEM_COMMIT` or `MEM_FREE`. + +So `PoolState::Uncommitted` joins `Unreadable`, and `PoolState::is_coverage_gap` is the single +definition of which one costs a walk its `complete`. Three properties keep it honest: + +- **Only a positive answer excuses a gap.** A query that fails, a run that cannot advance, a + state this crate does not name, and a source that cannot be asked at all are each `None`, and + `None` keeps the conservative reading. The excuse is never granted by an absence of evidence. +- **It is asked of the memory manager, never inferred from where the span lies.** Position is a + good guess and it is a guess: on a target trimming paged pool the pages that will not read + are committed and written, and a walk calling them empty would claim coverage it never had. +- **A kernel session is not asked at all.** `QueryVirtual` is a user-mode question, so the + kernel pool walk keeps counting every unreadable page against its coverage — which, for paged + pool, is the truth. + +The same question answers a second one. A free chunk whose middle the allocator decommitted +runs past the committed extent it starts in, and `walk_vs` emitted no span for it. A span is +geometry and state, both of which are known there — the header was read, the size came out of +it and passed the subsegment bound, the state comes from the free tree — and the only thing +missing is the chunk's *contents*, which no span carries. So where the tail holds nothing the +chunk is reported; where it is memory the process has, it is not, and now the walk says so. + +Measured on a live 26200 process (`sihost`, four Segment Heaps, 19,448 chunks, 2026-09-24): all +48 gaps were `MEM_RESERVE`, both controls — an allocated chunk and a free one — were +`MEM_COMMIT`, five chunks with decommitted middles were the last thing holding the walk short, +and the answer that had been `Partial` was `Complete`. The dump direction was checked from the +other end, on a thin dump of the same process: an address whose page the dump does not carry +still answers `Committed`, the read of it still fails, and the walk still counts it. A dump +that lacks committed pages is exactly the case this must not sweep up. ### Running out of time is not an error @@ -415,6 +467,8 @@ Once you adopt it, it stops being a pool-walker concern. | `Module::name` empty | For an unloaded module there is no name to qualify symbols by. Empty is the fact, not a truncation. | | `PoolSpan::requested_size: Option` | Set only where allocator metadata validates it. Kernel pool and LFH/VS spans leave it `None` rather than guessing from capacity. | | `PoolState::Unreadable` | A distinct state from `Allocated` and the two free states — the walker reached the chunk and could not read it. | +| `PoolState::Uncommitted` | Distinct from `Unreadable`, and the one gap that does *not* cost the walk its `complete`: the memory manager says there are no pages behind it, so nothing was missed. Never inferred — only a positive `MEM_RESERVE`/`MEM_FREE` puts a span here. | +| `VirtualState::Unknown(u32)` | A `State` the engine returned that this crate does not name, kept rather than folded into reserved or committed — guessing either way is the one thing that would make the primitive lie. | | `query::chunk_at` → `Ok(None)` | "Not covered by the snapshot at all", which is a different answer from "it is a free hole" — that comes back as a chunk whose `PoolState` is not `Allocated`. | | `find_tag` indexes allocated chunks only | A freed chunk's tag is not reliably preserved by the allocator, so "freed chunks with this tag" would be inventing information. | | `PoolKind`'s eight variants | Not collapsed to paged/nonpaged, because crossing one of those boundaries creates false holes. | diff --git a/examples/heap_coverage.rs b/examples/heap_coverage.rs new file mode 100644 index 0000000..d832e0a --- /dev/null +++ b/examples/heap_coverage.rs @@ -0,0 +1,159 @@ +//! Opt-in live probe: what, if anything, holds a user heap walk short of `Complete`. +//! +//! ```text +//! cargo run --example heap_coverage -- 6176 # a live process, by pid +//! cargo run --example heap_coverage -- C:\dumps\sihost.dmp # or a user-mode dump +//! cargo run --example heap_coverage -- C:\dumps\sihost.dmp 0x1ec81102040 +//! ``` +//! +//! Walks every Segment Heap in the target, then asks the **memory manager** about every gap the +//! walk filed — which is the question `PoolState::Uncommitted` turns on, and the only way to +//! tell a reserved subsegment tail from a page the process has and the debugger could not read. +//! With an address as a second argument it answers that one question and nothing else, which is +//! how the dump direction is checked: a page a thin dump does not carry still answers +//! `Committed`, and reading it still fails. +//! +//! Nothing here is asserted, because nothing here is a property of this crate — it is a reading +//! of whatever target it is pointed at. Set `_NT_SYMBOL_PATH`, or the public symbol server is +//! used. + +use std::collections::BTreeMap; +use std::time::Duration; + +use dbgscope::dbgeng::{DebugEngine, VirtualState}; +use dbgscope::heap::{self, HeapState, HeapWalk}; + +/// The walk's budget. Generous: this is a probe, and a run that expires reports the budget +/// rather than the target. +const BUDGET: Duration = Duration::from_secs(120); + +fn open(target: &str) -> Result> { + let engine = DebugEngine::new(); + engine.set_symbol_path(&std::env::var("_NT_SYMBOL_PATH").unwrap_or_else(|_| { + "srv*C:\\ProgramData\\dbg\\sym*https://msdl.microsoft.com/download/symbols".into() + }))?; + match target.parse::() { + // Already includes the break-in wait. + Ok(pid) => engine.attach_process(pid)?, + Err(_) => { + engine.open_dump(target)?; + engine.wait_for_event(60_000)?; + } + } + // The heap walker needs `ntdll`'s private types, and a deferred module has none. + engine.execute_command(".reload /f ntdll.dll")?; + Ok(engine) +} + +fn describe(state: VirtualState) -> String { + match state { + VirtualState::Committed => "committed".to_string(), + VirtualState::Reserved => "reserved".to_string(), + VirtualState::Free => "free".to_string(), + VirtualState::Unknown(state) => format!("unknown state {state:#x}"), + } +} + +fn coverage(target: &str) -> Result<(), Box> { + let engine = open(target)?; + let answer = heap::allocations(&engine, HeapWalk::refreshed().within(BUDGET))?; + println!( + "coverage {:?}: {} chunks, {} unreadable gaps, {} uncommitted gaps", + answer.walk.coverage, + answer.found.len(), + answer.walk.unreadable_gaps, + answer.walk.uncommitted_gaps + ); + println!( + " refused {} headers, {:#x} unplaced bytes, {:?}", + answer.walk.refused_headers, answer.walk.unplaced_bytes, answer.walk.stalls + ); + + // Every gap, against the memory manager — one row per answer, sized. A walk that reports + // `Complete` should have nothing but reserved runs here; anything committed is memory the + // target has and the walk did not see, and is what the coverage figure is about. + let mut tally: BTreeMap = BTreeMap::new(); + let mut note = |label: String, bytes: u64| { + let row = tally.entry(label).or_insert((0, 0)); + row.0 += 1; + row.1 += bytes; + }; + for gap in answer.found.iter().filter(|gap| !gap.state.is_chunk()) { + let mut cursor = gap.header_address; + let end = gap.end(); + while cursor < end { + match engine.virtual_region(cursor) { + Ok(region) if region.contains(cursor) => { + let stop = region.end().unwrap_or(end).min(end).max(cursor + 1); + note( + format!("{:?} / {}", gap.state, describe(region.state)), + stop - cursor, + ); + cursor = stop; + } + Ok(region) => { + note( + format!( + "{:?} / answered about {:#x}+{:#x}", + gap.state, region.base, region.size + ), + end - cursor, + ); + break; + } + Err(why) => { + note(format!("{:?} / no answer: {why}", gap.state), end - cursor); + break; + } + } + } + } + for (label, (runs, bytes)) in &tally { + println!(" {label}: {runs} runs, {bytes:#x} bytes"); + } + + // Controls. A chunk the walk *did* read has to come back committed; if it does not, the + // rows above are measuring the query rather than the target. + for state in [HeapState::Allocated, HeapState::ReusableFree] { + if let Some(chunk) = answer.found.iter().find(|chunk| chunk.state == state) { + println!( + " control {state:?} {:#x}: {}", + chunk.user_address, + engine.virtual_region(chunk.user_address).map_or_else( + |why| format!("no answer: {why}"), + |region| describe(region.state) + ) + ); + } + } + + let diagnostics = heap::diagnostics(&engine, HeapWalk::cached())?; + for shape in &diagnostics.found.categories { + println!(" {} x {}", shape.total, shape.shape); + } + engine.end_session()?; + Ok(()) +} + +fn one_address(target: &str, address: u64) -> Result<(), Box> { + let engine = open(target)?; + println!("{address:#x}: {:?}", engine.virtual_region(address)); + println!( + " read: {:?}", + engine.read_memory(address, 8).map(|bytes| bytes.len()) + ); + engine.end_session()?; + Ok(()) +} + +fn main() -> Result<(), Box> { + let arguments: Vec = std::env::args().skip(1).collect(); + match arguments.as_slice() { + [target] => coverage(target), + [target, address] => one_address( + target, + u64::from_str_radix(address.trim_start_matches("0x"), 16)?, + ), + _ => Err("usage: heap_coverage [address]".into()), + } +} diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 3554455..f10a79f 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -38,6 +38,9 @@ use windows::Win32::System::Diagnostics::Debug::Extensions::{ IDebugSystemObjects, }; use windows::Win32::System::Diagnostics::Debug::{EXCEPTION_RECORD64, IMAGEHLP_MODULEW64}; +use windows::Win32::System::Memory::{ + MEM_COMMIT, MEM_FREE, MEM_RESERVE, MEMORY_BASIC_INFORMATION64, +}; /// Callback type for breakpoint events that receives the breakpoint, context, and flags pub type BreakpointCallback = @@ -1478,6 +1481,96 @@ pub struct MemoryRead { pub cut_short: Option, } +/// `QueryVirtual`'s output buffer, **16-byte aligned**, which the type it holds is not. +/// +/// `MEMORY_BASIC_INFORMATION64` is 48 bytes of 8-byte fields, so Rust aligns it to 8 — and the +/// engine's *live-target* path copies the answer out with three `movaps` stores, which fault on +/// anything less than 16. Measured on 26200 (`dbgeng!Ordinal367+0x14f96`, +/// `movaps xmmword ptr [rbx],xmm0`, 2026-09-24): an 8-aligned buffer took an access violation +/// **inside dbgeng** on the first call against a live process, with nothing in the Rust frames +/// to suggest the caller was at fault. +/// +/// The crash is path-dependent, which is what makes it worth a type rather than a comment: the +/// dump path copies the same structure field by field, so the identical call against a full +/// dump answered 22 queries with no alignment at all. Testing this on a dump proves nothing +/// about it. +#[repr(C, align(16))] +#[derive(Default)] +struct AlignedMemoryInfo(MEMORY_BASIC_INFORMATION64); + +/// One run of pages as the memory manager describes it; see [`DebugEngine::virtual_region`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct VirtualRegion { + /// The first address of the run, which is at or below the address asked about. + pub base: u64, + /// Its length in bytes. Zero would mean the engine described nothing, and callers that act + /// on a region have to check for it rather than assume a query that succeeded covers the + /// address it was given. + pub size: u64, + pub state: VirtualState, + /// `PAGE_*` protection, verbatim. Meaningless on a [`VirtualState::Free`] run, and for + /// [`VirtualState::Reserved`] it is `AllocationProtect` rather than a page protection. + pub protect: u32, +} + +impl VirtualRegion { + /// Whether this run covers `address` — what a caller needs before reading anything off it. + pub fn contains(&self, address: u64) -> bool { + address >= self.base && address - self.base < self.size + } + + /// The last address in the run, or `None` for an empty one. + pub fn end(&self) -> Option { + (self.size != 0).then(|| self.base.saturating_add(self.size)) + } +} + +/// Whether a run of pages holds anything. +/// +/// The distinction [`DebugEngine::virtual_region`] exists for: address space that was never +/// committed can hold nothing, so a walk that cannot read it has missed nothing, while committed +/// memory that will not read is a real hole in what the walk saw. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum VirtualState { + /// `MEM_FREE`: not reserved at all. + Free, + /// `MEM_RESERVE`: address space claimed, no pages behind it. + Reserved, + /// `MEM_COMMIT`: pages exist. They may still be unreadable — paged out, or absent from a + /// dump — which is exactly the case a walk must keep counting against its coverage. + Committed, + /// A `State` this build of the engine returned and this crate does not name. Kept rather + /// than folded into one of the three above, because guessing which way it folds is the one + /// thing that would make this primitive lie: read as `Reserved` it would excuse a gap that + /// is real, and read as `Committed` it would refuse to excuse one that is not. + Unknown(u32), +} + +impl VirtualState { + fn from_mem_state(state: u32) -> Self { + // `VIRTUAL_ALLOCATION_TYPE` is a flag word, and the three states are disjoint bits in + // it, so this matches on the bit rather than on equality: `MEM_COMMIT` arrives beside + // other bits on some queries and an `==` would send it to `Unknown`. + match state { + _ if state & MEM_COMMIT.0 != 0 => Self::Committed, + _ if state & MEM_RESERVE.0 != 0 => Self::Reserved, + _ if state & MEM_FREE.0 != 0 => Self::Free, + other => Self::Unknown(other), + } + } + + /// Whether pages are behind this address space. `false` for both `MEM_RESERVE` and + /// `MEM_FREE`; `None` when the engine named a state this crate does not know, where the + /// caller must fall back rather than choose. + pub fn committed(self) -> Option { + match self { + Self::Committed => Some(true), + Self::Reserved | Self::Free => Some(false), + Self::Unknown(_) => None, + } + } +} + /// How much of a bounded read to ask the engine for per `ReadVirtual`. /// /// **This sets the residual overshoot, and that is the whole guarantee.** A bounded read is not @@ -4675,6 +4768,37 @@ impl DebugEngine { Ok((valid_base, valid_size as usize)) } + /// What the **memory manager** says about the run of pages `address` lies in, rather than + /// what the debugger can read there. + /// + /// The two are different questions and [`Self::valid_virtual_region`] only answers the + /// second. A page that will not read is reserved, or decommitted, or committed and paged + /// out, and a walk that cannot tell those apart has to treat all three as a hole in its own + /// coverage. This tells them apart: `MEM_RESERVE` is address space nothing was ever put in, + /// so nothing was missed there, while `MEM_COMMIT` that will not read is memory the target + /// does have and the debugger could not see. + /// + /// **User-mode targets only.** `QueryVirtual` is `NtQueryVirtualMemory` as the engine reaches + /// it, so a kernel session has no answer to give and this is an error there rather than a + /// state — callers must keep whatever conservative reading they had. On a crash dump it is + /// the `MemoryInfoListStream`, which a full dump carries and a minidump need not, so the + /// same error stands in for "this dump does not say". + pub fn virtual_region(&self, address: u64) -> Result { + let mut info = AlignedMemoryInfo::default(); + unsafe { self.dataspaces.QueryVirtual(address, &mut info.0) }.map_err(|source| { + DbgEngError::Context { + operation: format!("querying the memory manager about {address:#x}"), + source, + } + })?; + Ok(VirtualRegion { + base: info.0.BaseAddress, + size: info.0.RegionSize, + state: VirtualState::from_mem_state(info.0.State.0), + protect: info.0.Protect.0, + }) + } + pub fn interrupted(&self) -> Result { // The generated windows wrapper calls HRESULT::ok(), which deliberately // maps both S_OK and S_FALSE to Ok(()). GetInterrupt uses that distinction: diff --git a/src/heap.rs b/src/heap.rs index f5d6fce..f38d22e 100644 --- a/src/heap.rs +++ b/src/heap.rs @@ -112,7 +112,24 @@ pub enum HeapState { Allocated, ReusableFree, CachedFree, + /// Memory the walk could not read that the process does have; see + /// [`PoolState::Unreadable`]. Unreadable, + /// Address space in a heap region with no pages behind it; see + /// [`PoolState::Uncommitted`]. Not a gap in the walk's coverage — a reserved subsegment + /// tail is the allocator working, not something the walk missed. + Uncommitted, +} + +impl HeapState { + /// Whether this is a chunk the allocator laid out rather than a gap; see + /// [`PoolState::is_chunk`]. + pub fn is_chunk(self) -> bool { + match self { + Self::Allocated | Self::ReusableFree | Self::CachedFree => true, + Self::Unreadable | Self::Uncommitted => false, + } + } } impl From for HeapState { @@ -122,6 +139,7 @@ impl From for HeapState { PoolState::ReusableFree => Self::ReusableFree, PoolState::CachedFree => Self::CachedFree, PoolState::Unreadable => Self::Unreadable, + PoolState::Uncommitted => Self::Uncommitted, } } } @@ -164,7 +182,18 @@ pub struct HeapWalkReport { pub total_chunks: usize, pub allocated_chunks: usize, pub diagnostic_count: usize, + /// Spans the walk could not read and the process **does** have; each one clears + /// [`WalkCoverage::complete`]. pub unreadable_gaps: usize, + /// Spans with no pages behind them — reserved subsegment tails and the like, which the + /// memory manager confirmed rather than the walk assumed. + /// + /// Reported beside `unreadable_gaps` rather than folded into it, and rather than left out: + /// these are the spans the walk stopped counting against its coverage, so a reader who + /// wants to know what a `Complete` answer forgave has the figure. Always zero where + /// nothing could be asked — a kernel walk, or a dump that records no memory information — + /// because there the walk keeps its old conservative reading. + pub uncommitted_gaps: usize, pub refused_headers: u64, /// Committed VS bytes the walk declined to decode because it could not place a chunk /// boundary in them; see [`crate::pool::PoolSnapshot::unplaced_bytes`]. @@ -954,6 +983,10 @@ fn from_pool_snapshot( .iter() .filter(|allocation| allocation.state == HeapState::Unreadable) .count(), + uncommitted_gaps: allocations + .iter() + .filter(|allocation| allocation.state == HeapState::Uncommitted) + .count(), refused_headers: snapshot.refused_chunks, unplaced_bytes: snapshot.unplaced_bytes, stalls: snapshot.stalls, @@ -1244,7 +1277,9 @@ fn neighbourhood_at(allocations: &[HeapAllocation], address: u64) -> Option bool { + match self { + Self::Allocated | Self::ReusableFree | Self::CachedFree => true, + Self::Unreadable | Self::Uncommitted => false, + } + } + + /// Whether a span in this state counts *against* the walk's coverage — the one place that + /// decides what `complete` means. + pub fn is_coverage_gap(self) -> bool { + match self { + Self::Unreadable => true, + Self::Allocated | Self::ReusableFree | Self::CachedFree | Self::Uncommitted => false, + } + } } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] diff --git a/src/pool/index.rs b/src/pool/index.rs index e1372cd..ca1f465 100644 --- a/src/pool/index.rs +++ b/src/pool/index.rs @@ -108,8 +108,13 @@ impl PoolIndex { && left.heap == right.heap && left.backend == right.backend && left.subsegment == right.subsegment - && left.state != PoolState::Unreadable - && right.state != PoolState::Unreadable + // Both have to be *chunks*, not merely readable ones: a gap has no header and no + // successor, so letting one be a neighbour puts an allocation next to memory + // nothing was decoded out of. Asked through `is_chunk` rather than named here, so + // that a second kind of gap cannot arrive and pass this test by default — which is + // exactly what `PoolState::Uncommitted` would have done. + && left.state.is_chunk() + && right.state.is_chunk() } pub(crate) fn predecessor(&self, index: usize) -> Option { @@ -314,6 +319,24 @@ mod tests { assert_eq!(contextual.context_for_tag(tag), vec![0, 1, 2, 3, 4]); assert_eq!(contextual.successor(2), None); + // The other kind of gap, and the same answer. A span with nothing behind it has no + // header and no successor either, so it cannot be an allocation's neighbour — and + // arriving after this test was written is exactly why the check asks `is_chunk` + // rather than naming `Unreadable`. + let mut uncommitted = span(0x1060, 0, PoolState::Uncommitted, 1); + uncommitted.size_class = 0x20; + let across_a_gap = PoolIndex::build(PoolSnapshot { + spans: vec![ + span(0x1040, 0, PoolState::ReusableFree, 1), + uncommitted, + span(0x1080, other, PoolState::Allocated, 1), + ], + complete: true, + ..PoolSnapshot::default() + }); + assert_eq!(across_a_gap.successor(0), None); + assert_eq!(across_a_gap.predecessor(2), None); + // Distinct targets, not just distinct generations: the cache keys on the whole // SessionKey so a snapshot cannot outlive the target it described. let session = |value: u64| super::super::layout::SessionKey { diff --git a/src/pool/render.rs b/src/pool/render.rs index ead8580..9a24f68 100644 --- a/src/pool/render.rs +++ b/src/pool/render.rs @@ -74,6 +74,10 @@ fn glyph(span: &PoolSpan, selected: bool) -> (char, &'static str) { PoolState::ReusableFree => ('.', "0xf4d03f"), PoolState::CachedFree => ('c', "0xe67e22"), PoolState::Unreadable => ('?', "0x7f8c8d"), + // Distinct from `?` on purpose: the two look the same to a reader of the map and + // mean opposite things. `?` is a hole in what the walk saw; `-` is address space + // with nothing behind it, which the map should show as the empty span it is. + PoolState::Uncommitted => ('-', "0x566573"), } } } @@ -259,7 +263,7 @@ pub(crate) fn render_pool_map( } push_chunked( &mut chunks, - "Legend: S selected tag, A unrelated allocation, . reusable hole, c cached/delay-free, ? unreadable\n", + "Legend: S selected tag, A unrelated allocation, . reusable hole, c cached/delay-free, ? unreadable, - uncommitted\n", options.dml, ); if let Some(tag) = options.tag { @@ -365,6 +369,7 @@ pub(crate) fn render_advice(index: &PoolIndex, tag: u32, dml: bool) -> String { PoolState::ReusableFree => "reusable", PoolState::CachedFree => "cached/delay-free", PoolState::Unreadable => "unreadable", + PoolState::Uncommitted => "uncommitted", PoolState::Allocated => "allocated", }, hole.address, diff --git a/src/pool/snapshot.rs b/src/pool/snapshot.rs index fcdfcee..06150ed 100644 --- a/src/pool/snapshot.rs +++ b/src/pool/snapshot.rs @@ -135,6 +135,17 @@ pub(crate) struct PoolRegion { pub big_pool: Arc>, } +/// One run of address space the target's memory manager describes the same way; see +/// [`PoolMemory::committed_run`]. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) struct CommitRun { + /// One past the last byte of the run, and always above the address it was asked about — + /// so a caller stepping a span with it cannot fail to advance. + pub end: u64, + /// Whether pages exist behind it. + pub committed: bool, +} + pub(crate) trait PoolMemory { fn read_exact(&self, address: u64, size: usize) -> Result, SnapshotError>; fn valid_region(&self, address: u64, size: usize) -> Result<(u64, usize), SnapshotError>; @@ -147,6 +158,20 @@ pub(crate) trait PoolMemory { fn out_of_budget(&self) -> bool { false } + + /// What the target's **memory manager** says about the run of pages `address` lies in — + /// a different question from whether the debugger can read it, and the only one that + /// separates *nothing was ever here* from *we could not see what is here*. + /// + /// `None` means this source cannot say, and the caller then keeps the conservative + /// reading: what did not read stays a hole in the walk's coverage. **That default is + /// deliberate.** A source that does not override this behaves exactly as it did, so no + /// fixture starts excusing gaps by accident — the ones that exercise + /// [`PoolState::Uncommitted`] have to say so, which is what makes their assertions mean + /// anything. + fn committed_run(&self, _address: u64) -> Option { + None + } } /// A memory source with a deadline attached. @@ -192,6 +217,10 @@ impl PoolMemory for Budgeted<'_, M> { .is_some_and(|deadline| Instant::now() >= deadline) || self.inner.out_of_budget() } + + fn committed_run(&self, address: u64) -> Option { + self.inner.committed_run(address) + } } impl PoolMemory for crate::dbgeng::DebugEngine { @@ -220,6 +249,32 @@ impl PoolMemory for crate::dbgeng::DebugEngine { } }) } + + /// **Not asked of a kernel session at all.** `QueryVirtual` is the engine reaching + /// `NtQueryVirtualMemory` in the debuggee, which a kernel target has no equivalent of, so + /// every call there fails — and a kernel walk files thousands of unreadable spans, which + /// would be thousands of failed calls to learn the same thing each time. The kernel walk + /// therefore keeps counting every unreadable page against its coverage, which on a target + /// trimming paged pool is the truth: those pages are committed and the walk really did + /// miss what was in them (`windbg-mcp` FOLLOWUPS item 100). + /// + /// Every other answer that is not plainly *committed* or *not committed* — a query that + /// fails, a zero-length run, a state this crate does not name — is `None` rather than a + /// guess, because the guess is what would make this lie: read as uncommitted it excuses a + /// gap that is real. + fn committed_run(&self, address: u64) -> Option { + if crate::dbgeng::DebugEngine::is_kernel_target(self).ok()? { + return None; + } + let region = self.virtual_region(address).ok()?; + if !region.contains(address) { + return None; + } + Some(CommitRun { + end: region.end()?, + committed: region.state.committed()?, + }) + } } /// The one place a walk asks whether it is still allowed to run. @@ -2301,14 +2356,21 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { } }; let valid_base = reported_base.max(cursor).min(requested_end); - if valid_base > cursor { + // Reported only when some of that space is memory the target **has** — which, + // before `committed_run` could be asked, was the only reading available. On a + // healthy live user-mode heap the whole of it is reserved subsegment tails, and + // saying "unreadable space extends" about address space holding nothing is a + // diagnostic that both describes nothing wrong and is wrong about what it + // describes. Emitting it after the spans rather than before them is what lets it + // be conditional at all: only filing them establishes which kind this is. + if valid_base > cursor && self.unreadable(region, cursor, valid_base - cursor, snapshot) + { snapshot.diagnostics.push(format!( "region {:#x}+{:#x} is only committed through {cursor:#x}; unreadable space extends {:#x} bytes", region.address, region.size, valid_base - cursor )); - self.unreadable(region, cursor, valid_base - cursor, snapshot); } let valid_end = reported_base .saturating_add(reported_size as u64) @@ -2547,24 +2609,89 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { } } + /// Files address space the walk did not decode, split by what the memory manager says is + /// behind it. + /// + /// Every caller reaches this having failed to read something, which is where the walk used + /// to stop reasoning: it recorded one [`PoolState::Unreadable`] span and cleared + /// `complete`. That is right for a page the target has and wrong for address space that + /// holds nothing, and the two are not distinguishable from the failure — a reserved tail + /// and a trimmed page fail a read identically. [`PoolMemory::committed_run`] is what + /// separates them, and it is asked **per run** rather than per page, so one reserved tail + /// of any size costs one query. + /// + /// Split rather than classified whole: a span can cross the boundary — the tail of a + /// committed run and the reserved space after it arrive here as one failure — and calling + /// the whole of it by either name misreports the other half. Splitting also keeps the byte + /// totals addable, which is the only way the figure downstream means anything. + /// + /// Answers **whether any of it counted against coverage**, so a caller whose diagnostic is + /// about missed memory can stay quiet when nothing was missed. fn unreadable( &self, region: &PoolRegion, address: u64, size: u64, snapshot: &mut PoolSnapshot, - ) { - if size != 0 { + ) -> bool { + let Some(end) = (size != 0).then(|| address.saturating_add(size)) else { + return false; + }; + let mut cursor = address; + let mut missed = false; + while cursor < end { + // `> cursor` is not a formality: a run that does not advance would spin here, and + // the engine answering about a *preceding* run is exactly the shape `walk_region` + // already has to defend against. An answer that cannot move the cursor is no + // answer, so the rest of the span is filed the conservative way and the loop ends. + let Some(run) = self + .memory + .committed_run(cursor) + .filter(|run| run.end > cursor) + else { + return self.gap( + region, + cursor, + end - cursor, + PoolState::Unreadable, + snapshot, + ) || missed; + }; + let stop = run.end.min(end); + let state = if run.committed { + PoolState::Unreadable + } else { + PoolState::Uncommitted + }; + missed |= self.gap(region, cursor, stop - cursor, state, snapshot); + cursor = stop; + } + missed + } + + /// Records one gap span and, when it is a gap in *coverage*, clears `complete`; answers + /// whether it was one. + /// + /// The clearing is here rather than at the call sites so the two can never disagree: + /// `complete` means "no span says something may have been missed", and + /// [`PoolState::is_coverage_gap`] is the one definition of which states say that. + fn gap( + &self, + region: &PoolRegion, + address: u64, + size: u64, + state: PoolState, + snapshot: &mut PoolSnapshot, + ) -> bool { + if size == 0 { + return false; + } + let missed = state.is_coverage_gap(); + if missed { snapshot.complete = false; - snapshot.record_span(self.base_span( - region, - address, - address, - size, - 0, - PoolState::Unreadable, - )); } + snapshot.record_span(self.base_span(region, address, address, size, 0, state)); + missed } /// Decodes a Driver Verifier special-pool region, one allocation per page. @@ -2863,16 +2990,38 @@ impl<'a, M: PoolMemory> SnapshotWalker<'a, M> { snapshot.complete = false; } let chunk_size = chunk.size; - if offset.saturating_add(chunk_size) > bytes.len() { - // The chunk reaches past the committed extent, which after the bound check in - // `decode_vs_chunk` can only mean a hole ahead of it inside the subsegment — - // so this is the ordinary free chunk with a decommitted interior, not a walk - // running out of bytes. It carries the chain over the hole, which is the whole - // reason the expectation is returned rather than recomputed per extent. - // - // Still incomplete, and for the reason `complete` exists: no span is emitted for - // this chunk, so the snapshot omits it however well the walk understands it. + // The chunk reaches past the committed extent, which after the bound check in + // `decode_vs_chunk` can only mean a hole ahead of it inside the subsegment — so + // this is the ordinary free chunk with a decommitted interior, not a walk running + // out of bytes. It carries the chain over the hole, which is the whole reason the + // expectation is returned rather than recomputed per extent. + // + // **What the walk loses by it depends on what is in the hole**, and the same + // question the gap spans ask answers this one. A chunk's span is geometry and + // state, not contents: its header was read, its size came out of that header and + // passed the bound check, and its state comes from the free tree — so where the + // tail holds nothing, nothing about the chunk is unknown and the span is emitted + // below like any other. Where the tail is memory the process has, the walk really + // did fail to see part of this chunk, and then no span is emitted and `complete` + // clears. + // + // Measured on `sihost` (26200, 2026-09-24): five free chunks of 0x1660 to 0xfe70 + // bytes, each with a decommitted middle, were the *only* thing left holding that + // walk at `Partial` once the gap spans were classified — and this site cleared + // `complete` without a diagnostic, so nothing in the answer said why. + if offset.saturating_add(chunk_size) > bytes.len() + && !self + .memory + .committed_run(base.saturating_add(bytes.len() as u64)) + .is_some_and(|run| !run.committed) + { resume = header_address.saturating_add(chunk_size as u64); + snapshot.diagnostics.push(format!( + "VS chunk at {header_address:#x} is {chunk_size:#x} bytes and runs \ + {:#x} past the committed extent at {:#x}; no span is emitted for it", + offset.saturating_add(chunk_size) - bytes.len(), + base.saturating_add(bytes.len() as u64) + )); snapshot.complete = false; break; } @@ -4213,7 +4362,16 @@ mod tests { /// something and failing to size it. The two look alike at the call site and mean /// opposite things about what lies behind them. blind: HashSet, + /// What the *memory manager* says, which is a separate axis from every field above: + /// those decide what reads, this decides whether there was anything there to read. + /// + /// `None` is a source that cannot answer — the debugger against a kernel target, or a + /// dump recording no memory information — and is the default, so every test written + /// before this existed keeps its old meaning. `Some` names the pages with no pages + /// behind them. + uncommitted: Option>, queries: Cell, + commit_queries: Cell, } impl HoleyMemory { @@ -4224,10 +4382,18 @@ mod tests { holes: HashSet::new(), stalls: HashSet::new(), blind: HashSet::new(), + uncommitted: None, queries: Cell::new(0), + commit_queries: Cell::new(0), } } + /// Give this source a memory manager, telling it which pages hold nothing. + fn with_memory_manager(mut self, uncommitted: &[u64]) -> Self { + self.uncommitted = Some(uncommitted.iter().copied().collect()); + self + } + fn end(&self) -> u64 { self.base + self.bytes.len() as u64 } @@ -4281,6 +4447,33 @@ mod tests { fn interrupted(&self) -> Result { Ok(false) } + + /// Answers in **runs**, as the memory manager does: like pages are merged, so a walk + /// stepping a gap of any length costs one query per run and not one per page. The + /// counter is what lets a test say so. + fn committed_run(&self, address: u64) -> Option { + let uncommitted = self.uncommitted.as_ref()?; + self.commit_queries.set(self.commit_queries.get() + 1); + let mut page = address & !(PAGE_SIZE - 1); + let committed = !uncommitted.contains(&page); + loop { + page += PAGE_SIZE; + if page >= self.end() { + // Past the fixture the whole rest of the address space is one run of the + // same kind, which keeps a run always able to advance the caller. + return Some(CommitRun { + end: u64::MAX, + committed, + }); + } + if !uncommitted.contains(&page) != committed { + return Some(CommitRun { + end: page, + committed, + }); + } + } + } } /// Three special-pool pages, so each committed page the walk reaches shows up as exactly @@ -4388,6 +4581,221 @@ mod tests { ); } + /// The gap spans of a walk, as `(state, address, size)`, in address order. + fn gaps(snapshot: &PoolSnapshot) -> Vec<(PoolState, u64, u64)> { + let mut gaps: Vec<_> = snapshot + .spans + .iter() + .filter(|span| !span.state.is_chunk()) + .map(|span| (span.state, span.header_address, span.size)) + .collect(); + gaps.sort_by_key(|&(_, address, _)| address); + gaps + } + + /// `windbg-mcp` FOLLOWUPS item 98. The same page that does not read is a hole in the walk's + /// coverage or is nothing at all, and **only the memory manager can say which** — so this + /// runs one fixture past three memory managers and changes nothing else. + /// + /// The three answers are the whole rule. A source that says the page holds nothing files + /// [`PoolState::Uncommitted`] and leaves `complete` alone; one that says the page is + /// committed files [`PoolState::Unreadable`] and clears it; and one that cannot say keeps + /// the second reading, because *unknown* has to fall on the conservative side or the + /// excuse is being granted by the absence of evidence. + /// + /// Measured live before it was written: on `sihost` (26200, 4 Segment Heaps, 19,459 + /// chunks, 2026-09-24) every one of the 45 unreadable spans holding the walk at `Partial` + /// was `MEM_RESERVE`, and both controls — an allocated chunk and a free one — were + /// `MEM_COMMIT`. + #[test] + fn test_only_the_memory_manager_decides_whether_a_gap_costs_coverage() { + let gap = SPECIAL_PAGE + PAGE_SIZE; + let walk = |memory: HoleyMemory| { + let mut memory = memory; + memory.holes.insert(gap); + let snapshot = walk_holey(&memory, &special_region(SPECIAL_PAGE, 4)); + // The pages on either side are walked whatever the answer is: this changes how a + // gap is *described*, never how much is read. + assert_eq!( + snapshot + .spans + .iter() + .filter(|span| span.state == PoolState::Allocated) + .count(), + 3, + "the gap is one page of four" + ); + (snapshot, memory.commit_queries.get()) + }; + + let (nothing_there, queries) = + walk(HoleyMemory::new(SPECIAL_PAGE, special_pages(4)).with_memory_manager(&[gap])); + assert_eq!( + gaps(¬hing_there), + [(PoolState::Uncommitted, gap, PAGE_SIZE)], + "a page with nothing behind it is not a hole in what the walk saw" + ); + assert!( + nothing_there.complete, + "and so the walk covered everything it set out to" + ); + assert_eq!( + queries, 1, + "asked per run, not per page — a reserved tail of any length costs one query" + ); + + let (really_missed, _) = + walk(HoleyMemory::new(SPECIAL_PAGE, special_pages(4)).with_memory_manager(&[])); + assert_eq!( + gaps(&really_missed), + [(PoolState::Unreadable, gap, PAGE_SIZE)], + "a committed page that will not read is memory the target has and we did not see" + ); + assert!(!really_missed.complete); + + let (cannot_say, _) = walk(HoleyMemory::new(SPECIAL_PAGE, special_pages(4))); + assert_eq!( + gaps(&cannot_say), + gaps(&really_missed), + "a source with no answer keeps the conservative reading, not the convenient one" + ); + assert!(!cannot_say.complete); + } + + /// The diagnostic that goes with the gap, which is about *missed memory* and so has to be + /// as conditional as the coverage is. + /// + /// It fired on every reserved tail before this, which on a live user-mode heap is all of + /// them — a line per gap describing nothing wrong, and wrong about what it describes + /// ("unreadable space" for address space that holds nothing). The same failure mode as + /// glslang/dbgscope#94, where 3,285 such lines drowned out the diagnostics that meant + /// something. + #[test] + fn test_a_gap_with_nothing_behind_it_is_not_worth_a_diagnostic() { + let gap = SPECIAL_PAGE + PAGE_SIZE; + let complains = |memory: HoleyMemory| { + let mut memory = memory; + memory.holes.insert(gap); + let snapshot = walk_holey(&memory, &special_region(SPECIAL_PAGE, 4)); + snapshot + .diagnostics + .examples() + .iter() + .any(|message| message.contains("only committed through")) + }; + + assert!( + !complains( + HoleyMemory::new(SPECIAL_PAGE, special_pages(4)).with_memory_manager(&[gap]) + ), + "nothing was missed, so there is nothing to report" + ); + assert!( + complains(HoleyMemory::new(SPECIAL_PAGE, special_pages(4)).with_memory_manager(&[])), + "a committed page that would not read is still worth saying" + ); + assert!( + complains(HoleyMemory::new(SPECIAL_PAGE, special_pages(4))), + "and so is one nothing could be asked about" + ); + } + + /// A gap that crosses the boundary is **split**, because calling the whole of it by either + /// name misreports the other half — and the half that is really missed still costs the + /// walk its `complete`. + /// + /// This is the shape a live target actually produces: a subsegment's committed pages end + /// part-way through the range that holds it, so the tail of a committed run and the + /// reserved space after it reach the walk as one failed read. + #[test] + fn test_a_gap_straddling_the_commit_boundary_is_split_at_it() { + let mut memory = HoleyMemory::new(SPECIAL_PAGE, special_pages(4)) + // Only the second of the two pages that will not read holds nothing. + .with_memory_manager(&[SPECIAL_PAGE + 2 * PAGE_SIZE]); + memory.holes.insert(SPECIAL_PAGE + PAGE_SIZE); + memory.holes.insert(SPECIAL_PAGE + 2 * PAGE_SIZE); + let snapshot = walk_holey(&memory, &special_region(SPECIAL_PAGE, 4)); + + assert_eq!( + gaps(&snapshot), + [ + (PoolState::Unreadable, SPECIAL_PAGE + PAGE_SIZE, PAGE_SIZE), + ( + PoolState::Uncommitted, + SPECIAL_PAGE + 2 * PAGE_SIZE, + PAGE_SIZE + ), + ], + "two spans, each the size of what it describes" + ); + assert!( + !snapshot.complete, + "half of it was memory the target has, and that half is still missing" + ); + } + + /// A memory manager whose answer cannot advance the walk, which is the one way this loop + /// could hang rather than misreport. + /// + /// Not hypothetical: `walk_region` already defends against the engine naming a region + /// *behind* the cursor, and the same answer reaches here. The rest of the span is filed + /// the conservative way and the loop ends. + /// + /// **Deleting the guard hangs this test rather than failing it** — verified 2026-09-24, + /// which is the point: without it a walk that met such an answer would never return, and + /// a wedged engine process is the one failure this module cannot report its way out of. + #[test] + fn test_a_commit_run_that_cannot_advance_is_no_answer_at_all() { + struct Stuck(HoleyMemory); + impl PoolMemory for Stuck { + fn read_exact(&self, address: u64, size: usize) -> Result, SnapshotError> { + self.0.read_exact(address, size) + } + fn valid_region( + &self, + address: u64, + size: usize, + ) -> Result<(u64, usize), SnapshotError> { + self.0.valid_region(address, size) + } + fn interrupted(&self) -> Result { + Ok(false) + } + fn committed_run(&self, address: u64) -> Option { + // Names the address it was given, so a caller that trusted it would ask the + // same question forever. + Some(CommitRun { + end: address, + committed: false, + }) + } + } + + let mut inner = HoleyMemory::new(SPECIAL_PAGE, special_pages(4)); + inner.holes.insert(SPECIAL_PAGE + PAGE_SIZE); + let memory = Stuck(inner); + let layout = vs_layout(VsFixture::Inline); + let walker = SnapshotWalker { + memory: &memory, + layout: &layout, + traversal_limit: 1000, + }; + let mut snapshot = PoolSnapshot { + complete: true, + ..PoolSnapshot::default() + }; + walker + .walk_region(&special_region(SPECIAL_PAGE, 4), &mut snapshot) + .unwrap(); + + assert_eq!( + gaps(&snapshot), + [(PoolState::Unreadable, SPECIAL_PAGE + PAGE_SIZE, PAGE_SIZE)], + "an answer that cannot advance is discarded, not acted on" + ); + assert!(!snapshot.complete); + } + /// A VS subsegment with a hole in it, walked in two committed extents. /// /// `bytes` tiles the whole region with chunks the way the allocator does; `holes` names the @@ -4396,10 +4804,20 @@ mod tests { /// ranges anywhere inside the subsegment and tracks them in `_HEAP_VS_SUBSEGMENT.CommitBitmap` /// — and it is the shape the old walk had no way to survive. fn walk_vs_with_holes(chunks: &[(usize, usize)], holes: &[u64]) -> PoolSnapshot { + walk_vs_holes(chunks, holes, false) + } + + /// `walk_vs_with_holes`, plus a memory manager that confirms the holes hold nothing — + /// which is what a live target's does, and what the walk needs before it can treat a + /// chunk running into one as fully understood. + fn walk_vs_holes(chunks: &[(usize, usize)], holes: &[u64], asked: bool) -> PoolSnapshot { let bytes = vs_extent(chunks); let region = vs_region(bytes.len()); let mut memory = HoleyMemory::new(VS_BASE, bytes); memory.holes.extend(holes.iter().copied()); + if asked { + memory = memory.with_memory_manager(holes); + } walk_holey(&memory, ®ion) } @@ -4513,6 +4931,69 @@ mod tests { ); } + /// The chunk in the test above **is** billed as coverage the walk gave up, and that is the + /// other half of `windbg-mcp` FOLLOWUPS item 98: a free chunk whose middle the allocator + /// decommitted runs past the committed extent, so no span was emitted for it and + /// `complete` cleared — silently, with no diagnostic saying which chunk or why. + /// + /// A span is geometry and state, and both are known here: the header was read, the size + /// came out of it and passed the subsegment bound, and the state comes from the free tree. + /// The only thing the walk could not see is the chunk's *contents*, which no span carries + /// — so where the tail holds nothing the chunk is emitted like any other. + /// + /// The control is the same fixture with no memory manager, and it must still refuse: a + /// tail the walk cannot ask about may be memory the target has, and a chunk that large + /// with an unread middle is a real hole in what was seen. + /// + /// On `sihost` (26200, 2026-09-24) this was the last thing between a healthy live walk and + /// `Complete` — five chunks, 0x1660 to 0xfe70 bytes, after all 48 gap spans had been + /// classified. + #[test] + fn test_a_chunk_running_into_nothing_is_still_a_chunk() { + let chunks = [(0x1000usize, 0usize), (0x3000, 0x1000)]; + let holes = [VS_BASE + 0x2000]; + + let asked = walk_vs_holes(&chunks, &holes, true); + assert_eq!( + asked + .spans + .iter() + .filter(|span| span.state.is_chunk()) + .map(|span| (span.header_address, span.size)) + .collect::>(), + [(VS_BASE + 0x10, 0xfe0), (VS_BASE + 0x1010, 0x2fe0)], + "the chunk that spans the hole is understood, so it is reported" + ); + assert!( + asked.complete, + "and nothing about it is unknown, so the walk covered everything: {:?}", + asked.diagnostics.examples() + ); + + let unasked = walk_vs_holes(&chunks, &holes, false); + assert_eq!( + unasked + .spans + .iter() + .filter(|span| span.state.is_chunk()) + .map(|span| span.header_address) + .collect::>(), + [VS_BASE + 0x10], + "with no answer about the tail the chunk is not invented" + ); + assert!(!unasked.complete); + assert!( + unasked + .diagnostics + .examples() + .iter() + .any(|message| message.contains("runs") + && message.contains("past the committed extent")), + "and the walk says which chunk it dropped: {:?}", + unasked.diagnostics.examples() + ); + } + /// glslang/dbgscope#104, settled on live 26100: every stall the walk met answered `0x0+0x0`, /// and the eight verbatim samples were eight *consecutive* pages — one dead region stepping /// itself into the give-up bound, paying eight KD round trips to be told eight times what the