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