From 1f67db352857a3ceb5a852fa60ec580902abc683 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 08:04:30 +0100 Subject: [PATCH 01/12] feat: disassembly carries its operands, and an unwind region is not a function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Instruction` gains `mnemonic`, `operands` and `flow`. The engine has no structured disassembly, so the third column was a string and every caller wanting an immediate or a branch target re-parsed it downstream — twice over, in windbg-mcp's case, once for the walk and once for the recipe. The parse belongs at the seam that produces the rendering. Two rules the shapes are chosen for. An operand this does not recognise is `Other` with its text, never forced into one of the other four. And every destination in `Flow` is an `Option`, because "the engine printed a resolvable address" and "this is indirect" are different facts; a caller reading `None` as no edge stays sound. Gated on `InstructionSet`. x86 and x64 are read; ARM64 reports its mnemonic and `Flow::Unknown`, not the `Fallthrough` most instructions happen to be — an unread `b.eq` called a fall-through hands a walk one edge of two. Measured rather than composed. `examples/typed_disassembly.rs` runs the reading over a whole real dispatch routine: 376 instructions of `mountmgr!MountMgrDeviceControl` on a 26100 image, zero unrecognised operands, zero unknown flows, and its eleven control-code compares recovered as values. It found both defects now pinned by tests — literals must be `u64`, since that routine renders `8000000000000000h` and `0FFFFFFFFFFFFFFFFh`, and registers must be matched before literals, since `ah`, `bh`, `ch` and `dh` are both. `function_extent` reads the `.pdata` entry for an address. It is deliberately named a region: MSVC splits a function across several, and this answers `0x14750..0x147a3` for that routine — 83 bytes, which `.fnent` confirms — while its compare chain lives past `0x147dd`. Bounding a walk with it recovered zero control codes where following the flow recovered twelve. The x64 entry is three `u32` RVAs rather than the 64-bit addresses the API's name suggests, and reading them as `u64` reports no entry for a function that plainly has one. `symbol_for` exposes the symbol lookup that was already there privately. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 28 ++ examples/typed_disassembly.rs | 200 +++++++++ src/dbgeng.rs | 805 +++++++++++++++++++++++++++++++++- 3 files changed, 1011 insertions(+), 22 deletions(-) create mode 100644 examples/typed_disassembly.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index a6f6b50..cf7eda5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,34 @@ All notable changes to this project are documented here. The format follows ### Added +- **Disassembly carries its operands as values.** `Instruction` gains `mnemonic`, `operands` and + `flow` beside the `text` it already had, so a caller asking what an instruction *compares + against* or *branches to* reads a field instead of re-parsing a rendering downstream. `Operand` + is `Register`, `Immediate`, `Memory`, `Target` or `Other`, and `Other` is the whole safety + story — an operand this does not recognise keeps its text rather than being forced into a shape. + `Flow` carries every destination as an `Option`, because a resolvable target and an indirect one + are different facts, and a caller treating `None` as "no edge" stays sound. + Reading is gated on `InstructionSet`: x86 and x64 are read, and anything else — ARM64 today — + reports its mnemonic, no operands and `Flow::Unknown`. That is a refusal rather than a guess, + and the flow is `Unknown` rather than the `Fallthrough` most instructions happen to be, because + an unread `b.eq` reported as falling through hands a walk one edge of two. + Measured against a whole real dispatch routine rather than composed lines + (`examples/typed_disassembly.rs`): 376 instructions of `mountmgr!MountMgrDeviceControl` on a + 26100 image read with **zero** unrecognised operands and zero unknown flows, and its eleven + control-code compares came back as values. Two defects that measurement found, both now pinned: + literals are `u64` rather than `i64`, since the routine renders `8000000000000000h` and + `0FFFFFFFFFFFFFFFFh`; and registers are matched before literals, since `ah`, `bh`, `ch` and `dh` + are both. +- `DebugEngine::function_extent` returns the unwind **region** containing an address, from the + image's `.pdata`, rebased. A region is **not** a function, and using it as one loses code: MSVC + splits a function across several entries, and this answers `0x14750..0x147a3` for + `mountmgr!MountMgrDeviceControl` — 83 bytes, which `.fnent` confirms — while that routine's + compare chain lives past `0x147dd`. Bounding a walk with it recovered zero control codes where + following the flow recovered twelve. The x64 entry is three `u32` RVAs and not the 64-bit + addresses the API's name suggests; reading them as `u64` reports "no entry" for a function that + plainly has one. +- `DebugEngine::symbol_for` is the public half of the existing symbol lookup: the `module!Symbol` + an address resolves to and how far past it, or `None` for a driver with no PDB. - **Exception events are readable as values.** `DebugEngine::last_event` returns a `DebugEvent` — kind, engine process and thread, and, when the event carried one, an `ExceptionRecord` with the code, flags, faulting address and parameters. That is `.exr -1` typed, and it is the user-mode diff --git a/examples/typed_disassembly.rs b/examples/typed_disassembly.rs new file mode 100644 index 0000000..2235bf2 --- /dev/null +++ b/examples/typed_disassembly.rs @@ -0,0 +1,200 @@ +//! Measurement for the typed-disassembly fields: does the operand reading survive what the engine +//! actually renders, rather than what a test composed? +//! +//! The unit tests in `dbgeng.rs` are written against renderings copied by hand. That is enough to +//! pin the *rules* and not enough to claim the parser works, because a hand-copied rendering is +//! chosen from the shapes its author already knew about. This runs the reader over a whole real +//! dispatch routine and reports what it could **not** read — the number that matters is the +//! `Other` count and the `Unknown` flow count, both of which should be zero on x64. +//! +//! It also answers the question the reading exists for: how many `cmp`/`sub` immediates inside an +//! IOCTL dispatch routine decode as plausible `CTL_CODE` values, which is the premise a static +//! IOCTL map rests on. +//! +//! ```text +//! cargo run --example typed_disassembly -- ! [image search path] +//! cargo run --example typed_disassembly -- C:\dumps\kernel.dmp mountmgr!MountMgrDeviceControl \ +//! SRV*C:\sym*https://msdl.microsoft.com/download/symbols +//! ``` +//! +//! A kernel minidump carries no driver code pages, so the image search path is not optional for a +//! driver that is not `nt`: without it every instruction reads `???` and the run reports exactly +//! that, which is itself the measurement of what a dump alone can answer. + +use dbgscope::dbgeng::{DebugEngine, Flow, Operand}; + +fn main() { + let mut args = std::env::args().skip(1); + let (Some(dump), Some(symbol)) = (args.next(), args.next()) else { + eprintln!("usage: typed_disassembly ! [image search path]"); + std::process::exit(2); + }; + let image_path = args.next(); + + let e = DebugEngine::new(); + e.open_dump(&dump).expect("opening the dump failed"); + // `OpenDumpFileWide` only names the file; the target is loaded by the first wait, and until + // it is there is no debuggee for a command to run against. + e.wait_for_event(30_000) + .expect("the dump did not load within thirty seconds"); + + if let Some(path) = &image_path { + // The engine takes an image search path the same way it takes a symbol one, and a symbol + // server serves the image binary as well as the PDB. + e.execute_command(&format!(".exepath+ {path}")) + .expect("setting the image search path failed"); + e.reload_symbols("/f").expect("reloading failed"); + } + + let set = e.instruction_set(); + println!( + "instruction set: {set:?} (operands read: {})", + set.operands_are_read() + ); + + let entry = e + .symbol_offset(&symbol) + .unwrap_or_else(|error| panic!("{symbol} did not resolve: {error}")); + println!("{symbol} at {entry:#x}"); + + match e.function_extent(entry) { + Ok(Some((begin, end))) => println!( + "unwind region: {begin:#x}..{end:#x} ({} bytes) — a region, not the function", + end - begin + ), + Ok(None) => println!("unwind region: none (leaf, or not code)"), + Err(error) => println!("unwind region: unavailable ({error})"), + } + + // Follow the flow rather than reading forward. A linear read runs into whatever follows the + // function and fills the candidate list with other routines' constants; the unwind region is + // no substitute, because MSVC splits one function across several of them. The module bounds + // the walk so a tail jump out of the driver does not take it with them. + let module = e + .module_at(entry) + .ok() + .flatten() + .expect("the entry is in no module"); + let (low, high) = (module.base, module.base + module.size as u64); + + let mut seen = std::collections::HashSet::new(); + let mut queue = vec![entry]; + let mut instructions = Vec::new(); + while let Some(at) = queue.pop() { + if at < low || at >= high || !seen.insert(at) || seen.len() > 20_000 { + continue; + } + // Two, so the second one's address is this one's fall-through — the engine's own + // arithmetic rather than a length guessed from the encoding. + let Ok(pair) = e.disassemble(at, 2) else { + continue; + }; + let fall_through = pair.get(1).map(|next| next.address); + let Some(instruction) = pair.into_iter().next() else { + continue; + }; + if let Some(target) = instruction.flow.target() { + // A call leaves this function; every other edge stays in it. + if !matches!(instruction.flow, Flow::Call(_)) { + queue.push(target); + } + } + if instruction.flow.falls_through() { + queue.extend(fall_through); + } + instructions.push(instruction); + } + instructions.sort_by_key(|instruction| instruction.address); + println!("walked {} instructions\n", instructions.len()); + + let (mut unreadable, mut other_operands, mut unknown_flow) = (0usize, 0usize, 0usize); + let mut candidates: Vec<(u64, u64)> = Vec::new(); + let mut calls: Vec<(u64, String)> = Vec::new(); + + for instruction in &instructions { + if instruction.text.starts_with('?') { + unreadable += 1; + continue; + } + if instruction.flow == Flow::Unknown { + unknown_flow += 1; + println!( + " UNKNOWN FLOW {:#x} {}", + instruction.address, instruction.text + ); + } + for operand in &instruction.operands { + if let Operand::Other(text) = operand { + other_operands += 1; + println!( + " UNREAD OPERAND {:#x} {} <- {text:?}", + instruction.address, instruction.text + ); + } + } + + // The premise: a compare against a control code, recovered as a value. + if matches!(instruction.mnemonic.as_str(), "cmp" | "sub" | "xor" | "add") + && let Some(Operand::Immediate(value)) = instruction.operands.get(1) + && plausible_ioctl(*value) + { + candidates.push((instruction.address, *value)); + } + if let Flow::Call(Some(target)) = instruction.flow { + let name = e + .symbol_for(target) + .map(|(name, _)| name) + .unwrap_or_else(|| format!("{target:#x}")); + calls.push((instruction.address, name)); + } + if let Some(Operand::Memory(memory)) = instruction.operands.first() + && let (Flow::Call(None), Some(symbol)) = (instruction.flow, &memory.symbol) + { + calls.push((instruction.address, format!("[{symbol}]"))); + } + } + + println!("\n--- what the reading could not read ---"); + println!("instructions the engine could not render: {unreadable}"); + println!("operands kept as Other: {other_operands}"); + println!("instructions with Unknown flow: {unknown_flow}"); + + println!( + "\n--- plausible CTL_CODE immediates ({}) ---", + candidates.len() + ); + for (address, value) in &candidates { + let value = *value; + println!( + " {address:#x} {value:#010x} device {:#06x} function {:#05x} method {} access {}", + (value >> 16) & 0xffff, + (value >> 2) & 0xfff, + value & 3, + (value >> 14) & 3 + ); + } + + println!("\n--- direct and thunked calls ({}) ---", calls.len()); + for (address, name) in &calls { + println!(" {address:#x} {name}"); + } +} + +/// The `CTL_CODE` shape: a device type that is not zero, and a value that is not something else +/// with the same bit width. +/// +/// The exclusions are measured, not defensive. An unbounded first run over `mountmgr` offered +/// `0x80000005` and `0xc0000023` as control codes; both are `NTSTATUS` — `STATUS_BUFFER_OVERFLOW` +/// and `STATUS_BUFFER_TOO_SMALL` — compared against a return value in a routine further down the +/// image, and `0xffffffff` came from a compare against -1. A control code's top bit is clear in +/// every code Windows defines, which is what separates the three. +fn plausible_ioctl(value: u64) -> bool { + if value == 0 || value > u32::MAX as u64 { + return false; + } + // `NTSTATUS` severity lives in the top two bits; a defined control code has none set. + if value & 0x8000_0000 != 0 { + return false; + } + (value >> 16) & 0xffff != 0 +} diff --git a/src/dbgeng.rs b/src/dbgeng.rs index d9eac77..89bf2cc 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -1776,6 +1776,149 @@ pub struct StackFrame { pub displacement: u64, } +/// The instruction set a rendering is in, which decides whether its operands can be read. +/// +/// The engine renders every architecture through the same three columns, and the columns are all +/// [`split_instruction`] needs — but *inside* the third one the syntaxes diverge completely +/// (`sub rsp,40h` against `stp fp,lr,[sp,#-0x10]!`). So operand reading is gated on the set, and +/// anything this does not implement is [`Self::Other`]: mnemonic kept, operands empty, +/// [`Flow::Unknown`]. That is a refusal rather than a guess, and it is why a caller can tell "no +/// operands were read" apart from "this instruction has none" by looking at the flow. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum InstructionSet { + /// x86, 32-bit. + X86, + /// x86-64. + Amd64, + /// Anything else — ARM64 today. The `IMAGE_FILE_MACHINE_*` value is kept so a caller can say + /// which one it declined to read. + Other(u32), +} + +impl InstructionSet { + /// The set a `GetActualProcessorType` value names. + pub fn from_processor_type(machine: u32) -> Self { + match machine { + 0x014c => Self::X86, + 0x8664 => Self::Amd64, + other => Self::Other(other), + } + } + + /// Whether operands and flow are read for this set. False means every instruction comes back + /// [`Flow::Unknown`] with no operands. + pub fn operands_are_read(self) -> bool { + matches!(self, Self::X86 | Self::Amd64) + } +} + +/// What one instruction does to control flow, as far as the **rendering** says. +/// +/// Every destination is an [`Option`] for one reason: the engine prints a resolvable target as a +/// parenthesised address and prints nothing resolvable for an indirect one, and those are +/// different facts. `Call(None)` is `call rax` or `call qword ptr [rax*8+…]`; `Jmp(None)` is a +/// jump table or a function pointer. A caller that treats `None` as "no edge" stays sound — it +/// never invents one — which is the property [`Self::target`] exists to keep obvious. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Flow { + /// Falls through to the next instruction — the common case. + Fallthrough, + /// A `call`: schedules the destination, then falls through. `None` when it is indirect. + Call(Option), + /// An unconditional `jmp`: control goes to the destination only, never to the next + /// instruction. `None` when it is indirect. + Jmp(Option), + /// A conditional branch: the destination **or** the next instruction. + Branch(Option), + /// `ret`/`iret`: flow stops. + Return, + /// A `noreturn` trap — `int 29h` (`__fastfail`), `int 3`, `ud2`, `hlt`. Flow stops, so a walk + /// must not fall through one. + Trap, + /// Not read: an instruction set whose operands this does not decode, or a rendering the + /// split did not recognise. Says nothing about what the instruction does. + Unknown, +} + +impl Flow { + /// The destination this instruction names, when it named one resolvably. + pub fn target(self) -> Option { + match self { + Self::Call(target) | Self::Jmp(target) | Self::Branch(target) => target, + _ => None, + } + } + + /// Whether control can continue at the next instruction. + /// + /// [`Flow::Unknown`] answers **true**, because a walk that stopped there would silently drop + /// the rest of a function on an architecture whose operands are not read. + pub fn falls_through(self) -> bool { + matches!( + self, + Self::Fallthrough | Self::Call(_) | Self::Branch(_) | Self::Unknown + ) + } +} + +/// A memory operand, as the engine renders it — `qword ptr [rdx+0B8h]`, `gs:[188h]`, +/// `[rax+rcx*8+20h]`, ``qword ptr [nt!_imp_ExAllocatePool2 (fffff803`…)]``. +/// +/// Every field is what was *printed*. Nothing here is computed against a register context, so +/// `[rdx+0B8h]` is the displacement `0xb8` off whatever `rdx` held, and this says only that. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct MemoryOperand { + /// The access width in bytes, from the `qword ptr` prefix; `None` when the engine printed + /// none, which it omits where the mnemonic already fixes the width. + pub size: Option, + /// A segment override — `gs` in `gs:[188h]`, which is how kernel code reaches the KPCR. + pub segment: Option, + /// The base register, when one was printed. + pub base: Option, + /// The index register of a `base+index*scale` form. + pub index: Option, + /// The index's scale; 1 when none was printed. + pub scale: u8, + /// The signed displacement, zero when none was printed. + pub displacement: i64, + /// A symbol printed inside the brackets — `nt!_imp_ExAllocatePool2`, which is what an import + /// thunk looks like on a driver whose symbols resolve. + pub symbol: Option, + /// An absolute address printed inside the brackets, whether bare or beside a symbol. + pub address: Option, +} + +/// One operand of an instruction. +/// +/// [`Self::Other`] is the whole safety story: an operand this does not recognise keeps its text +/// and is never forced into one of the shapes above. Order matters in the reading, and one +/// collision is worth knowing about — `ah`, `bh`, `ch` and `dh` are registers *and* well-formed +/// `h`-suffixed hexadecimal literals, so registers are matched first and `mov ah,5` does not +/// report a destination of `0xa`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Operand { + /// A register, by the engine's own name for it. + Register(String), + /// A literal, as the bit pattern the engine printed. `40h` is hexadecimal by its suffix; a + /// bare digit run is decimal, which agrees with hexadecimal below ten and is what the engine + /// prints for small counts. Unsigned because a real routine renders `8000000000000000h` and + /// `0FFFFFFFFFFFFFFFFh`, and a leading `-` is two's complement for the same reason. + Immediate(u64), + /// A memory reference. + Memory(MemoryOperand), + /// A branch or call destination as rendered: ``nt!KeBugCheckEx (fffff803`3e2547f0)``, a symbol + /// with no address, or a bare address. An address here has at least eight hexadecimal digits, + /// which is what keeps a short immediate from being read as one. + Target { + /// `module!Symbol` or `module!Symbol+0x1c`, when the engine resolved one. + symbol: Option, + /// The absolute destination, when the engine printed one. + address: Option, + }, + /// Anything else, verbatim. + Other(String), +} + /// One disassembled instruction, as [`DebugEngine::disassemble`] reports it. /// /// The engine has no structured disassembly — `IDebugControl::Disassemble` renders one line of @@ -1783,6 +1926,11 @@ pub struct StackFrame { /// boundaries, with the address taken from the walk rather than parsed back out of it. A line the /// split does not recognise keeps everything after the address in [`Self::text`] and leaves /// [`Self::bytes`] empty, rather than guessing. +/// +/// [`Self::mnemonic`], [`Self::operands`] and [`Self::flow`] read that third column, so a caller +/// asking what an instruction *compares against* or *branches to* reads a field rather than +/// re-parsing a rendering downstream. [`Self::text`] stays verbatim beside them: it is what a +/// listing prints, and the fields do not replace it. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Instruction { /// Where the instruction is. **Not** parsed from the rendered line: it is the offset this @@ -1793,8 +1941,18 @@ pub struct Instruction { pub bytes: String, /// The mnemonic and its operands — `mov qword ptr [rsp+8],rbx` — with the engine's column /// padding collapsed to single spaces, since the columns it was aligning are separate fields - /// here. Operand symbols are the engine's own (`call nt!KeBugCheckEx (fffff803`...)`). + /// here. Operand symbols are the engine's own (``call nt!KeBugCheckEx (fffff803`...)``). pub text: String, + /// The operative mnemonic — `mov`, `cmp`, `call`. A `lock`/`rep` prefix is **skipped** rather + /// than joined to it, so `lock inc dword ptr [rax]` reports `inc`; the prefix is still in + /// [`Self::text`]. Empty only when the rendering was. + pub mnemonic: String, + /// The operands, in the order printed. Empty for an instruction that takes none — and also + /// for an instruction set whose operands are not read, which [`Self::flow`] tells apart by + /// being [`Flow::Unknown`]. + pub operands: Vec, + /// What the instruction does to control flow. + pub flow: Flow, } /// The engine's current **scope**: which instruction, which frame, and the register context @@ -2288,29 +2446,340 @@ fn read_engine_string( /// on a rendering it does not otherwise trust. Anything the shape does not fit keeps its whole /// remainder as text, so an engine that renders differently loses a column rather than an /// instruction. -fn split_instruction(address: u64, line: &str) -> Instruction { +fn split_instruction(address: u64, line: &str, set: InstructionSet) -> Instruction { let mut columns = line.trim().splitn(3, char::is_whitespace); let (_address, bytes, rest) = (columns.next(), columns.next(), columns.next()); - match (bytes, rest) { - (Some(bytes), Some(rest)) => Instruction { - address, - bytes: bytes.to_string(), - text: collapse_spaces(rest), - }, + let (bytes, text) = match (bytes, rest) { + (Some(bytes), Some(rest)) => (bytes.to_string(), collapse_spaces(rest)), // One column past the address, or none: keep it whole rather than calling it an encoding. - (Some(only), None) => Instruction { - address, - bytes: String::new(), - text: collapse_spaces(only), - }, - _ => Instruction { - address, - bytes: String::new(), - text: collapse_spaces(line), - }, + (Some(only), None) => (String::new(), collapse_spaces(only)), + _ => (String::new(), collapse_spaces(line)), + }; + let (mnemonic, operands, flow) = read_operation(&text, set); + Instruction { + address, + bytes, + text, + mnemonic, + operands, + flow, + } +} + +/// Prefixes that are not the operation: `lock inc dword ptr [rax]` is an `inc`. +const MNEMONIC_PREFIXES: [&str; 7] = ["lock", "rep", "repe", "repz", "repne", "repnz", "bnd"]; + +/// The third column read as an operation — mnemonic, operands, control flow. +/// +/// An instruction set whose operands are not read still reports its mnemonic, because the first +/// token is the mnemonic in every syntax the engine renders. What it does not report is anything +/// that would be a guess: no operands, and [`Flow::Unknown`] rather than the `Fallthrough` that +/// most instructions happen to be — a walk told "falls through" about an unread `b.eq` would +/// silently take one edge of two. +fn read_operation(text: &str, set: InstructionSet) -> (String, Vec, Flow) { + let mut tokens = text.trim().splitn(2, char::is_whitespace); + let mut mnemonic = tokens.next().unwrap_or_default().to_string(); + let mut rest = tokens.next().unwrap_or_default().trim().to_string(); + // A prefix is not the operation. Take the next token as the mnemonic, if there is one. + while MNEMONIC_PREFIXES.contains(&mnemonic.as_str()) && !rest.is_empty() { + let mut next = rest.splitn(2, char::is_whitespace); + mnemonic = next.next().unwrap_or_default().to_string(); + rest = next.next().unwrap_or_default().trim().to_string(); + } + if !set.operands_are_read() || mnemonic.is_empty() { + return (mnemonic, Vec::new(), Flow::Unknown); + } + // `???` is the engine saying it could not read the bytes, not an instruction. + if mnemonic.starts_with('?') { + return (mnemonic, Vec::new(), Flow::Unknown); + } + let operands: Vec = split_operands(&rest) + .into_iter() + .map(|operand| read_operand(operand.trim())) + .collect(); + let flow = classify_flow(&mnemonic, &operands); + (mnemonic, operands, flow) +} + +/// Splits an operand list on the commas that separate operands — the ones outside brackets and +/// parentheses. `[rax+rcx*8]` has no top-level comma; a symbolised target's `(fffff803`…)` has no +/// comma at all, and both are stepped over the same way. +fn split_operands(rest: &str) -> Vec<&str> { + if rest.is_empty() { + return Vec::new(); + } + let (mut depth, mut start, mut out) = (0i32, 0usize, Vec::new()); + for (index, character) in rest.char_indices() { + match character { + '[' | '(' => depth += 1, + ']' | ')' => depth -= 1, + ',' if depth <= 0 => { + out.push(&rest[start..index]); + start = index + 1; + } + _ => {} + } + } + out.push(&rest[start..]); + out +} + +/// The `qword ptr` family, as a width in bytes. +fn operand_size(word: &str) -> Option { + Some(match word { + "byte" => 1, + "word" => 2, + "dword" => 4, + "fword" => 6, + "qword" => 8, + "tbyte" => 10, + "mmword" | "xmmword" => 16, + "ymmword" => 32, + "zmmword" => 64, + _ => return None, + }) +} + +/// One operand, read in an order that resolves the collisions rather than tripping over them. +fn read_operand(text: &str) -> Operand { + if text.is_empty() { + return Operand::Other(String::new()); + } + // Registers first: `ah`, `bh`, `ch` and `dh` are also well-formed `h`-suffixed literals. + if is_register(text) { + return Operand::Register(text.to_string()); + } + if text.contains('[') { + if let Some(memory) = read_memory_operand(text) { + return Operand::Memory(memory); + } + return Operand::Other(text.to_string()); + } + // A symbolised destination, or a bare address. `!` is what makes it a symbol; the address is + // the parenthesised one the engine prints beside it. + if text.contains('!') { + let (symbol, address) = split_symbol_and_address(text); + return Operand::Target { symbol, address }; + } + if let Some(address) = parse_engine_address(text) { + return Operand::Target { + symbol: None, + address: Some(address), + }; + } + if let Some(value) = parse_engine_number(text) { + return Operand::Immediate(value); + } + Operand::Other(text.to_string()) +} + +/// `nt!KeBugCheckEx (fffff803`3e2547f0)` as its two halves. Either may be missing. +fn split_symbol_and_address(text: &str) -> (Option, Option) { + let (name, address) = match text.split_once('(') { + Some((name, tail)) => (name.trim(), tail.trim_end_matches(')').trim().to_string()), + None => (text.trim(), String::new()), + }; + let symbol = (!name.is_empty()).then(|| name.to_string()); + (symbol, parse_engine_address(&address)) +} + +/// The inside of a memory operand, with whatever `qword ptr` and segment override preceded it. +fn read_memory_operand(text: &str) -> Option { + let mut memory = MemoryOperand { + scale: 1, + ..MemoryOperand::default() + }; + let open = text.find('[')?; + let close = text.rfind(']')?; + if close < open { + return None; + } + // Everything before the bracket: an optional `qword ptr`, an optional `gs:`. + for word in text[..open] + .split(|c: char| c.is_whitespace() || c == ':') + .filter(|w| !w.is_empty()) + { + if let Some(size) = operand_size(word) { + memory.size = Some(size); + } else if word != "ptr" { + memory.segment = Some(word.to_string()); + } + } + // A segment override written `gs:[188h]` leaves `gs` immediately before the bracket, which + // the same split already caught; nothing else is expected there. + let inside = &text[open + 1..close]; + if inside.contains('!') { + let (symbol, address) = split_symbol_and_address(inside); + memory.symbol = symbol; + memory.address = address; + return Some(memory); + } + let mut negative = false; + let mut term = String::new(); + let flush = |term: &mut String, negative: &mut bool, memory: &mut MemoryOperand| { + let text = term.trim().to_string(); + term.clear(); + let was_negative = std::mem::replace(negative, false); + if text.is_empty() { + return; + } + if let Some((index, scale)) = text.split_once('*') { + memory.index = Some(index.trim().to_string()); + memory.scale = scale.trim().parse().unwrap_or(1); + } else if is_register(&text) { + if memory.base.is_none() { + memory.base = Some(text); + } else { + memory.index = Some(text); + } + } else if let Some(address) = parse_engine_address(&text) { + memory.address = Some(address); + } else if let Some(value) = parse_engine_number(&text) { + let value = value as i64; + memory.displacement = if was_negative { -value } else { value }; + } + }; + for character in inside.chars() { + match character { + '+' => flush(&mut term, &mut negative, &mut memory), + '-' => { + flush(&mut term, &mut negative, &mut memory); + negative = true; + } + _ => term.push(character), + } + } + flush(&mut term, &mut negative, &mut memory); + Some(memory) +} + +/// An engine-rendered address: the `hi`lo` backtick form or a plain hexadecimal run, requiring +/// **eight** digits so a short immediate is never read as an address. +fn parse_engine_address(token: &str) -> Option { + let cleaned: String = token + .trim() + .trim_matches(|c| c == '(' || c == ')' || c == ',') + .chars() + .filter(|&c| c != '`') + .collect(); + if cleaned.len() < 8 || !cleaned.chars().all(|c| c.is_ascii_hexdigit()) { + return None; + } + u64::from_str_radix(&cleaned, 16).ok() +} + +/// A literal, as the **bit pattern** the engine printed. `40h` is hexadecimal by its suffix, +/// `0x40` by its prefix, and a bare digit run is decimal — which is what the engine prints for a +/// small count, and agrees with hexadecimal below ten either way. +/// +/// Unsigned on purpose, and measured rather than assumed: a real dispatch routine renders +/// `mov rax,8000000000000000h` and `mov qword ptr [rbp+0A8h],0FFFFFFFFFFFFFFFFh`, both of which +/// overflow a signed parse and came back as unread operands until this stopped being an `i64`. +/// A leading `-` is taken as two's complement for the same reason: what a caller compares against +/// a control code or a mask is the pattern, not an arithmetic sign. +fn parse_engine_number(token: &str) -> Option { + let token = token.trim(); + let (negative, token) = match token.strip_prefix('-') { + Some(rest) => (true, rest), + None => (false, token), + }; + let value = if let Some(hex) = token + .strip_prefix("0x") + .or_else(|| token.strip_prefix("0X")) + { + u64::from_str_radix(hex, 16).ok()? + } else if let Some(hex) = token.strip_suffix('h').or_else(|| token.strip_suffix('H')) { + u64::from_str_radix(hex, 16).ok()? + } else if token.chars().all(|c| c.is_ascii_digit()) && !token.is_empty() { + token.parse().ok()? + } else { + return None; + }; + Some(if negative { + value.wrapping_neg() + } else { + value + }) +} + +/// Whether a token is one of the engine's register names. +fn is_register(token: &str) -> bool { + const NAMED: [&str; 24] = [ + "rax", "rbx", "rcx", "rdx", "rsi", "rdi", "rbp", "rsp", "eax", "ebx", "ecx", "edx", "esi", + "edi", "ebp", "esp", "ax", "bx", "cx", "dx", "si", "di", "bp", "sp", + ]; + const BYTE: [&str; 12] = [ + "al", "bl", "cl", "dl", "ah", "bh", "ch", "dh", "sil", "dil", "bpl", "spl", + ]; + const SEGMENT: [&str; 6] = ["cs", "ds", "es", "fs", "gs", "ss"]; + const POINTER: [&str; 4] = ["rip", "eip", "ip", "eflags"]; + let token = token.trim(); + if NAMED.contains(&token) || BYTE.contains(&token) || SEGMENT.contains(&token) { + return true; + } + if POINTER.contains(&token) { + return true; + } + // `r8`..`r15` with their `d`/`w`/`b` widths, and the vector and control/debug files. + for (prefix, count) in [ + ("r", 16u32), + ("xmm", 32), + ("ymm", 32), + ("zmm", 32), + ("cr", 16), + ("dr", 16), + ] { + if let Some(tail) = token.strip_prefix(prefix) { + let digits = tail.trim_end_matches(['d', 'w', 'b']); + if prefix != "r" && digits.len() != tail.len() { + continue; + } + if let Ok(number) = digits.parse::() + && number < count + && (prefix != "r" || number >= 8) + { + return true; + } + } + } + false +} + +/// The control flow an x86 mnemonic implies, with its destination taken from the operand that +/// was already read rather than from the line. +fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { + let destination = || match operands.first() { + Some(Operand::Target { address, .. }) => *address, + // A near jump to an absolute the engine printed bare, without eight digits, is not one + // this reads: an immediate here is a relative displacement, not a destination. + _ => None, + }; + match mnemonic { + "call" | "callf" => Flow::Call(destination()), + "jmp" | "jmpf" => Flow::Jmp(destination()), + "ret" | "retf" | "retn" | "iret" | "iretd" | "iretq" | "sysret" | "sysexit" => Flow::Return, + "ud0" | "ud1" | "ud2" | "hlt" => Flow::Trap, + "int" | "int1" | "int3" | "into" => Flow::Trap, + _ if is_conditional_branch(mnemonic) => Flow::Branch(destination()), + _ => Flow::Fallthrough, } } +/// `jcc`, `loop` and `jcxz` — the conditional transfers, which take one edge or the other. +fn is_conditional_branch(mnemonic: &str) -> bool { + const CONDITIONS: [&str; 32] = [ + "o", "no", "b", "c", "nae", "ae", "nb", "nc", "e", "z", "ne", "nz", "be", "na", "a", "nbe", + "s", "ns", "p", "pe", "np", "po", "l", "nge", "ge", "nl", "le", "ng", "g", "nle", "cxz", + "ecxz", + ]; + if let Some(condition) = mnemonic.strip_prefix('j') + && (condition == "rcxz" || CONDITIONS.contains(&condition)) + { + return true; + } + matches!(mnemonic, "loop" | "loope" | "loopne" | "loopz" | "loopnz") +} + /// Runs of whitespace as one space. The engine pads its columns to align them in a listing, and /// the alignment means nothing once the columns are separate fields. fn collapse_spaces(text: &str) -> String { @@ -5408,6 +5877,9 @@ impl DebugEngine { /// annotation, which is a fact about the *current register context* rather than about the /// instruction and would make two identical calls differ. pub fn disassemble(&self, address: u64, count: usize) -> Result, DbgEngError> { + // Asked once for the whole walk: the target's architecture cannot change inside one, and + // asking per instruction would put a COM call between every pair of them. + let set = self.instruction_set(); let mut out = Vec::with_capacity(count.min(64)); let mut at = address; for _ in 0..count { @@ -5433,7 +5905,7 @@ impl DebugEngine { }); } }; - out.push(split_instruction(at, &line)); + out.push(split_instruction(at, &line, set)); // An engine that does not advance would spin here forever rendering one instruction. if next <= at { break; @@ -5443,6 +5915,93 @@ impl DebugEngine { Ok(out) } + /// The instruction set the target's disassembly is rendered in. + /// + /// Falls back to [`InstructionSet::Other`] when the engine will not say, which reads + /// operands out of nothing rather than reading them wrongly — a target that cannot name its + /// processor is not one to guess x86 for. + pub fn instruction_set(&self) -> InstructionSet { + match self.processor_type() { + Ok(machine) => InstructionSet::from_processor_type(machine), + Err(_) => InstructionSet::Other(0), + } + } + + /// Where the unwind **region** containing `address` begins and ends, from the target's own + /// unwind data rather than from a guess or a symbol's extent. + /// + /// `IDebugSymbols3::GetFunctionEntryByOffset` reads the `RUNTIME_FUNCTION` the image's + /// `.pdata` carries, so it is exact for the x64 code this crate walks, and the table is in a + /// read-only section — which is why it still answers on a dump whose data pages were never + /// captured. + /// + /// # A region is not a function, and using it as one loses code + /// + /// MSVC splits a function into **several** `RUNTIME_FUNCTION` entries whenever it separates + /// cold or shared blocks, and each entry bounds only its own region. Measured against + /// `mountmgr!MountMgrDeviceControl` on a 26100 image: this answers `0x14750..0x147a3`, an + /// 83-byte region of 23 instructions, and `.fnent` agrees exactly — while the routine's IOCTL + /// compare chain lives past `0x147dd`, in regions this entry says nothing about. Bounding a + /// walk with it recovered **zero** control codes where following the flow recovered twelve. + /// + /// So this is a sanity bound and a "which region is this" answer, **not** a function's extent. + /// A walk over a whole function follows [`Instruction::flow`] from its entry. + /// + /// `None` is a fact rather than a failure: a leaf function may have no entry, and neither has + /// an address that is not code. A caller that needs the distinction has [`Self::symbol_for`]. + /// + /// # The shape, which is not the one the name suggests + /// + /// On x64 the engine fills an `IMAGE_RUNTIME_FUNCTION_ENTRY` — **three `u32` RVAs** + /// (`BeginAddress`, `EndAddress`, `UnwindInfoAddress`), not the 64-bit addresses + /// `IMAGE_FUNCTION_ENTRY64` would carry. Reading them as `u64` packs two RVAs into one number + /// and yields an extent that fails its own sanity check, which is how this was first written + /// and how it reported "no entry" for a 2,108-byte dispatch routine that plainly had one. + /// They are also relative, so they are rebased here against the module that holds the address + /// — what `.fnent` prints, at the RVAs it prints them. + pub fn function_extent(&self, address: u64) -> Result, DbgEngError> { + let mut entry = [0u32; 3]; + let mut needed = 0u32; + if unsafe { + self.symbols.GetFunctionEntryByOffset( + address, + 0, + Some(entry.as_mut_ptr().cast()), + std::mem::size_of_val(&entry) as u32, + Some(&mut needed), + ) + } + .is_err() + { + // No entry for this address is the ordinary answer for a leaf or for data. + return Ok(None); + } + let (begin, end) = (entry[0] as u64, entry[1] as u64); + if begin == 0 || end <= begin { + return Ok(None); + } + let Some(module) = self.module_at(address)? else { + return Ok(None); + }; + let (begin, end) = (module.base + begin, module.base + end); + // The extent has to contain the address it was asked about; anything else means the entry + // read back does not describe this function and is not worth returning. + if !(begin..end).contains(&address) { + return Ok(None); + } + Ok(Some((begin, end))) + } + + /// The `module!Symbol` an address resolves to and how far past it the address is. + /// + /// The public half of [`Self::symbol_at`]: what names a call's destination once a walk has + /// one. Infallible for the same reason — an address that resolves to nothing is the normal + /// case for a driver with no PDB, and a caller reads `None` rather than an error. + pub fn symbol_for(&self, address: u64) -> Option<(String, u64)> { + let (name, displacement) = self.symbol_at(address); + name.map(|name| (name, displacement)) + } + /// The `module!Symbol` an address resolves to and how far past it the address is. /// /// Infallible: an address that resolves to nothing is the normal case for a module without @@ -7697,6 +8256,7 @@ mod tests { let x64 = split_instruction( 0xfffff803_89201234, "fffff803`89201234 48895c2408 mov qword ptr [rsp+8],rbx\n", + InstructionSet::Amd64, ); assert_eq!(x64.address, 0xfffff803_89201234); assert_eq!(x64.bytes, "48895c2408"); @@ -7705,6 +8265,7 @@ mod tests { let arm64 = split_instruction( 0xfffff803_89201234, "fffff803`89201234 a9bf7bfd stp fp,lr,[sp,#-0x10]!\n", + InstructionSet::Other(0xaa64), ); assert_eq!(arm64.bytes, "a9bf7bfd"); assert_eq!(arm64.text, "stp fp,lr,[sp,#-0x10]!"); @@ -7714,7 +8275,7 @@ mod tests { /// agreeing lines cannot tell the two sources apart. #[test] fn test_an_instructions_address_comes_from_the_walk_not_the_rendering() { - let one = split_instruction(0x1000, "deadbeef`deadbeef 90 nop"); + let one = split_instruction(0x1000, "deadbeef`deadbeef 90 nop", InstructionSet::Amd64); assert_eq!(one.address, 0x1000); assert_eq!(one.text, "nop"); } @@ -7723,13 +8284,213 @@ mod tests { /// the remainder is kept as text and nothing is presented as an encoding that is not one. #[test] fn test_an_unrecognised_line_keeps_its_text_rather_than_inventing_an_encoding() { - let two_columns = split_instruction(0x1000, "fffff803`89201234 ????"); + let two_columns = + split_instruction(0x1000, "fffff803`89201234 ????", InstructionSet::Amd64); assert!(two_columns.bytes.is_empty(), "{two_columns:?}"); assert_eq!(two_columns.text, "????"); - let one_column = split_instruction(0x1000, "???"); + let one_column = split_instruction(0x1000, "???", InstructionSet::Amd64); assert!(one_column.bytes.is_empty(), "{one_column:?}"); assert_eq!(one_column.text, "???"); + + // An unreadable rendering must not claim a flow either — `???` is the engine saying it + // could not read the bytes, and `Fallthrough` there would walk a walk into nothing. + assert_eq!(one_column.flow, Flow::Unknown, "{one_column:?}"); + assert_eq!(two_columns.flow, Flow::Unknown, "{two_columns:?}"); + } + + /// An instruction set whose operands are not read refuses rather than guesses. + /// + /// The mnemonic survives — the first token is the mnemonic in every syntax the engine renders + /// — and nothing else is claimed. The flow matters most: ARM64's `b.eq` is a conditional + /// branch, and reporting the `Fallthrough` that an unread instruction would otherwise default + /// to would hand a caller one edge of two and call the walk complete. + #[test] + fn test_an_unread_instruction_set_reports_no_operands_and_no_flow() { + let arm64 = split_instruction( + 0xfffff803_89201234, + "fffff803`89201234 54000060 b.eq nt!KiFoo+0x1c (fffff803`89201240)", + InstructionSet::Other(0xaa64), + ); + assert_eq!(arm64.mnemonic, "b.eq"); + assert!(arm64.operands.is_empty(), "{arm64:?}"); + assert_eq!(arm64.flow, Flow::Unknown); + assert!( + arm64.text.contains("nt!KiFoo"), + "the rendering is still the rendering: {arm64:?}" + ); + } + + /// The operands of the shapes this crate's callers actually walk. + /// + /// Every one of these is a rendering taken from a real x64 kernel target rather than + /// composed: the compare against an IOCTL code, the `IO_STACK_LOCATION` field load, the + /// symbolised call, the import thunk, the scaled index of a jump table, and the KPCR read + /// through a segment override. + #[test] + fn test_x64_operands_are_read_as_values() { + let one = |text: &str| { + split_instruction( + 0x1000, + &format!("00001000 90 {text}"), + InstructionSet::Amd64, + ) + }; + + // The compare an IOCTL map is recovered from. + let cmp = one("cmp r13d,6D0030h"); + assert_eq!(cmp.mnemonic, "cmp"); + assert_eq!( + cmp.operands, + vec![ + Operand::Register("r13d".into()), + Operand::Immediate(0x6d_0030) + ] + ); + assert_eq!(cmp.flow, Flow::Fallthrough); + + // `IRP_SP = [Irp+0xb8]`, the load every dispatch routine opens with. + let load = one("mov rax,qword ptr [rdx+0B8h]"); + let Some(Operand::Memory(memory)) = load.operands.get(1) else { + panic!("the memory operand was not read: {load:?}"); + }; + assert_eq!(memory.size, Some(8)); + assert_eq!(memory.base.as_deref(), Some("rdx")); + assert_eq!(memory.displacement, 0xb8); + assert_eq!(memory.index, None); + + // A symbolised direct call: the destination is the parenthesised address. + let call = one("call nt!KeBugCheckEx (fffff803`3e2547f0)"); + assert_eq!(call.flow, Flow::Call(Some(0xfffff803_3e2547f0))); + assert_eq!( + call.operands, + vec![Operand::Target { + symbol: Some("nt!KeBugCheckEx".into()), + address: Some(0xfffff803_3e2547f0), + }] + ); + + // An import thunk: indirect, so no destination — and the symbol names the slot. + let thunk = one("call qword ptr [mountmgr!_imp_ExAllocatePool2 (fffff803`3e25a018)]"); + assert_eq!(thunk.flow, Flow::Call(None)); + let Some(Operand::Memory(slot)) = thunk.operands.first() else { + panic!("the thunk's operand was not read as memory: {thunk:?}"); + }; + assert_eq!( + slot.symbol.as_deref(), + Some("mountmgr!_imp_ExAllocatePool2") + ); + assert_eq!(slot.address, Some(0xfffff803_3e25a018)); + + // A jump table: base, scaled index and the table's own address. + let table = one("jmp qword ptr [rax*8+fffff803`3e25a000]"); + assert_eq!(table.flow, Flow::Jmp(None)); + let Some(Operand::Memory(entry)) = table.operands.first() else { + panic!("the jump table's operand was not read as memory: {table:?}"); + }; + assert_eq!(entry.index.as_deref(), Some("rax")); + assert_eq!(entry.scale, 8); + assert_eq!(entry.address, Some(0xfffff803_3e25a000)); + + // A segment override, which is how kernel code reaches the KPCR. + let kpcr = one("mov rax,qword ptr gs:[188h]"); + let Some(Operand::Memory(pcr)) = kpcr.operands.get(1) else { + panic!("the segment-overridden operand was not read: {kpcr:?}"); + }; + assert_eq!(pcr.segment.as_deref(), Some("gs")); + assert_eq!(pcr.displacement, 0x188); + assert_eq!(pcr.base, None); + } + + /// The flow classification, including the three endings a walk must not fall through. + #[test] + fn test_flow_separates_the_edges_a_walk_may_take() { + let one = |text: &str| { + split_instruction( + 0x1000, + &format!("00001000 90 {text}"), + InstructionSet::Amd64, + ) + .flow + }; + + assert_eq!( + one("je mountmgr!MountMgrQueryPoints+0x1c (fffff803`3e2547f0)"), + Flow::Branch(Some(0xfffff803_3e2547f0)) + ); + assert_eq!(one("ret"), Flow::Return); + assert_eq!(one("int 29h"), Flow::Trap); + assert_eq!(one("ud2"), Flow::Trap); + assert_eq!(one("call rax"), Flow::Call(None)); + assert_eq!(one("jmp rax"), Flow::Jmp(None)); + assert_eq!(one("xor ebx,ebx"), Flow::Fallthrough); + + // A prefix is not the operation, and skipping it must not lose the operation's flow. + let locked = split_instruction( + 0x1000, + "00001000 f0ff05 lock inc dword ptr [rax]", + InstructionSet::Amd64, + ); + assert_eq!(locked.mnemonic, "inc"); + assert_eq!(locked.flow, Flow::Fallthrough); + + // Sound in the one direction that matters: an unresolved destination is `None`, never a + // borrowed address from somewhere else on the line. + assert_eq!(one("jmp qword ptr [rax*8+1234h]").target(), None); + } + + /// `ah`, `bh`, `ch` and `dh` are registers *and* well-formed `h`-suffixed hexadecimal, and + /// reading them as literals silently turns a destination register into a number. + /// + /// Pinned because the ordering that avoids it is invisible: registers are matched before + /// numbers, and swapping those two arms passes every other test in this file. + #[test] + fn test_a_byte_register_is_not_read_as_a_hexadecimal_literal() { + let one = split_instruction(0x1000, "00001000 b405 mov ah,5", InstructionSet::Amd64); + assert_eq!( + one.operands, + vec![Operand::Register("ah".into()), Operand::Immediate(5)], + "a register was read as a literal: {one:?}" + ); + + // And the literal case still reads: the same four letters with more digits in front. + let two = split_instruction(0x1000, "00001000 b40a mov al,0Ah", InstructionSet::Amd64); + assert_eq!( + two.operands, + vec![Operand::Register("al".into()), Operand::Immediate(0xa)] + ); + } + + /// A literal that does not fit a signed 64-bit integer is still a literal. + /// + /// Both renderings here are real, from a walk of `mountmgr!MountMgrDeviceControl` on a 26100 + /// image, and both came back as unread `Other` operands while this parsed into an `i64` — + /// three of the three unread operands in that whole routine were this one bug. A caller + /// matching an operand against a mask or a control code wants the bit pattern, so the value + /// is unsigned and a leading `-` is two's complement. + #[test] + fn test_a_literal_too_wide_for_a_signed_integer_is_still_read() { + let one = |text: &str| { + split_instruction( + 0x1000, + &format!("00001000 90 {text}"), + InstructionSet::Amd64, + ) + .operands + }; + + assert_eq!( + one("mov rax,8000000000000000h").get(1), + Some(&Operand::Immediate(0x8000_0000_0000_0000)) + ); + assert_eq!( + one("mov qword ptr [rbp+0A8h],0FFFFFFFFFFFFFFFFh").get(1), + Some(&Operand::Immediate(u64::MAX)) + ); + assert_eq!( + one("sub rsp,-8").get(1), + Some(&Operand::Immediate(8u64.wrapping_neg())) + ); } /// glslang/dbgscope#82: a borrowed engine's lifecycle used to die with the wrapper. From 6e01a4716afe3f8f7eefdb92a70b660236603a53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 10:37:53 +0100 Subject: [PATCH 02/12] fix: three review findings, and a probe for the two that needed measuring All three findings on #145 were correct. Two of them turned on facts the API's documentation does not state, so `examples/function_entry_probe.rs` measures them rather than arguing: which failure means "no entry", and what the record looks like off x64. `function_extent` answers a three-state `FunctionExtent` instead of an `Option`, because the two non-answers were different facts collapsed into one. `NoEntry` is now reported for the single measured failure that means it -- `E_NOINTERFACE`, which a real dbgeng 10.x gives for address zero, for a module's header page, and for every x86 address, 32-bit Windows having no unwind table -- and every other failure is returned as an error rather than read as a leaf. `Unsupported` is every instruction set but x64, and refusing rather than decoding is the choice here. ARM64's record is two words whose second is packed unwind data or an `.xdata` RVA: measured on an ARM64 kernel dump, `nt!KeBugCheckEx` fills `needed = 8` with `[0x0025df60, 0x0005f218]`. Read as an end address that is a bogus region, and for any function whose `BeginAddress` is below the `.xdata` RVA it is a bogus region that contains the address asked about, so it passes every sanity check in the function. A wrong region that looks right is worse than no region. Decoding ARM64's packed unwind length is the other option the finding offered and buys nothing yet: the operand reading already refuses that architecture, so nothing downstream could use the bound. The engine's own `needed` is now checked against the x64 shape as well, so the layout is a check rather than an assumption. A software interrupt is classified by its vector rather than its mnemonic. `int 2eh` is the 32-bit system-call path and returns; classifying every `int` as a trap made `falls_through()` false and discarded every instruction after a syscall. `int 29h` and `int 3` still stop a walk -- the second by vector as well as by the `int3` spelling, since that is how the engine renders `0xcc`. Mutation-verified: restoring the blanket `int` trap fails the new vector test and nothing else. The two engine-dependent fixes are verified by the probe against a real ARM64 dump and a real x64 one. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 13 +++ examples/function_entry_probe.rs | 66 +++++++++++++++ examples/typed_disassembly.rs | 9 +- src/dbgeng.rs | 136 +++++++++++++++++++++++++++---- 4 files changed, 204 insertions(+), 20 deletions(-) create mode 100644 examples/function_entry_probe.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index cf7eda5..f267387 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,9 @@ All notable changes to this project are documented here. The format follows literals are `u64` rather than `i64`, since the routine renders `8000000000000000h` and `0FFFFFFFFFFFFFFFFh`; and registers are matched before literals, since `ah`, `bh`, `ch` and `dh` are both. + A software interrupt is classified by its **vector**, not by its mnemonic: `int 29h` + (`__fastfail`) and `int 3` stop a walk, and every other one falls through — notably `int 2eh`, + the 32-bit system-call path, where stopping would discard every instruction after a syscall. - `DebugEngine::function_extent` returns the unwind **region** containing an address, from the image's `.pdata`, rebased. A region is **not** a function, and using it as one loses code: MSVC splits a function across several entries, and this answers `0x14750..0x147a3` for @@ -34,6 +37,16 @@ All notable changes to this project are documented here. The format follows following the flow recovered twelve. The x64 entry is three `u32` RVAs and not the 64-bit addresses the API's name suggests; reading them as `u64` reports "no entry" for a function that plainly has one. + It answers a three-state `FunctionExtent` rather than an `Option`, because the two non-answers + are different facts. `NoEntry` is reported for the one measured failure that means it — + `E_NOINTERFACE`, which a real dbgeng 10.x gives for address zero, for a header page, and for + every x86 address, 32-bit Windows having no unwind table — and any other failure is returned as + an error rather than read as a leaf. `Unsupported` covers every instruction set but x64: + ARM64's record is two words whose second is packed unwind data, measured as `needed = 8` with + `[0x0025df60, 0x0005f218]` for `nt!KeBugCheckEx` on an ARM64 kernel dump, and read as an end + address that is a bogus region which — for any function below the `.xdata` RVA — contains the + address asked about and passes every sanity check. The engine's own `needed` is checked against + the x64 shape rather than assumed. - `DebugEngine::symbol_for` is the public half of the existing symbol lookup: the `module!Symbol` an address resolves to and how far past it, or `None` for a driver with no PDB. - **Exception events are readable as values.** `DebugEngine::last_event` returns a `DebugEvent` — diff --git a/examples/function_entry_probe.rs b/examples/function_entry_probe.rs new file mode 100644 index 0000000..db27003 --- /dev/null +++ b/examples/function_entry_probe.rs @@ -0,0 +1,66 @@ +//! Measurement for `DebugEngine::function_extent`: what the engine fills in, and what it says +//! when there is nothing to fill. +//! +//! Two questions the API's documentation does not answer, and both decide code: +//! +//! - **Which failure means "no entry"?** `GetFunctionEntryByOffset` returns `Result<()>`, and a +//! leaf function, a data address and a broken engine are all `Err`. Mapping every one of them to +//! `None` makes a failed query indistinguishable from a function that has no unwind record. +//! - **What is the entry's layout off x64?** The x64 record is three `u32` RVAs. ARM64's is two +//! words whose second is packed unwind data or an `.xdata` RVA — so reading it as an end address +//! is a bogus extent rather than an error, which is the worst shape a wrong answer can take. +//! +//! ```text +//! cargo run --example function_entry_probe -- ... [--exepath ] +//! ``` + +use dbgscope::dbgeng::{DebugEngine, FunctionExtent}; + +fn main() { + let mut args = std::env::args().skip(1); + let Some(dump) = args.next() else { + eprintln!("usage: function_entry_probe ... [--exepath ]"); + std::process::exit(2); + }; + let mut wanted = Vec::new(); + let mut image_path = None; + while let Some(arg) = args.next() { + if arg == "--exepath" { + image_path = args.next(); + } else { + wanted.push(arg); + } + } + + let e = DebugEngine::new(); + e.open_dump(&dump).expect("opening the dump failed"); + e.wait_for_event(30_000).expect("the dump did not load"); + if let Some(path) = &image_path { + e.execute_command(&format!(".exepath+ {path}")) + .expect("setting the image search path failed"); + e.reload_symbols("/f").expect("reloading failed"); + } + + println!("instruction set: {:?}\n", e.instruction_set()); + for name in &wanted { + let address = match name.strip_prefix("0x") { + Some(hex) => u64::from_str_radix(hex, 16).expect("a hexadecimal address"), + None => match e.symbol_offset(name) { + Ok(address) => address, + Err(error) => { + println!("{name}: did not resolve ({error})\n"); + continue; + } + }, + }; + println!("{name} = {address:#x}"); + match e.function_extent(address) { + Ok(FunctionExtent::Region { begin, end }) => { + println!(" region {begin:#x}..{end:#x} ({} bytes)\n", end - begin) + } + Ok(FunctionExtent::NoEntry) => println!(" no entry\n"), + Ok(FunctionExtent::Unsupported(set)) => println!(" not decoded for {set:?}\n"), + Err(error) => println!(" error: {error}\n"), + } + } +} diff --git a/examples/typed_disassembly.rs b/examples/typed_disassembly.rs index 2235bf2..17789f2 100644 --- a/examples/typed_disassembly.rs +++ b/examples/typed_disassembly.rs @@ -21,7 +21,7 @@ //! driver that is not `nt`: without it every instruction reads `???` and the run reports exactly //! that, which is itself the measurement of what a dump alone can answer. -use dbgscope::dbgeng::{DebugEngine, Flow, Operand}; +use dbgscope::dbgeng::{DebugEngine, Flow, FunctionExtent, Operand}; fn main() { let mut args = std::env::args().skip(1); @@ -58,11 +58,14 @@ fn main() { println!("{symbol} at {entry:#x}"); match e.function_extent(entry) { - Ok(Some((begin, end))) => println!( + Ok(FunctionExtent::Region { begin, end }) => println!( "unwind region: {begin:#x}..{end:#x} ({} bytes) — a region, not the function", end - begin ), - Ok(None) => println!("unwind region: none (leaf, or not code)"), + Ok(FunctionExtent::NoEntry) => println!("unwind region: no entry (leaf, or not code)"), + Ok(FunctionExtent::Unsupported(set)) => { + println!("unwind region: not decoded for {set:?}") + } Err(error) => println!("unwind region: unavailable ({error})"), } diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 89bf2cc..6c56501 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -1861,6 +1861,37 @@ impl Flow { } } +/// What a target's unwind data says about the region holding an address, as +/// [`DebugEngine::function_extent`] reports it. +/// +/// Three outcomes rather than an [`Option`], because the two that are not a region are different +/// facts with different remedies: one says this image has no entry covering that address, the +/// other says this build does not decode that target's entry layout at all. Collapsing them lets a +/// caller read "not decoded" as "a leaf function", which is the reading that turns an +/// architecture this cannot answer for into an architecture it answers wrongly for. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FunctionExtent { + /// The region containing the address, rebased into the target's address space. + Region { begin: u64, end: u64 }, + /// The image has no unwind entry covering the address — a leaf function, or an address that + /// is not code. Every x86 address answers this, 32-bit Windows having no unwind table. + NoEntry, + /// This instruction set's entry layout is not decoded here, so nothing is claimed about the + /// address at all. + Unsupported(InstructionSet), +} + +impl FunctionExtent { + /// The region, where there is one. `None` covers both of the other outcomes, so use it only + /// where "no bound available" is the whole question. + pub fn region(self) -> Option<(u64, u64)> { + match self { + Self::Region { begin, end } => Some((begin, end)), + _ => None, + } + } +} + /// A memory operand, as the engine renders it — `qword ptr [rdx+0B8h]`, `gs:[188h]`, /// `[rax+rcx*8+20h]`, ``qword ptr [nt!_imp_ExAllocatePool2 (fffff803`…)]``. /// @@ -2759,7 +2790,16 @@ fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { "jmp" | "jmpf" => Flow::Jmp(destination()), "ret" | "retf" | "retn" | "iret" | "iretd" | "iretq" | "sysret" | "sysexit" => Flow::Return, "ud0" | "ud1" | "ud2" | "hlt" => Flow::Trap, - "int" | "int1" | "int3" | "into" => Flow::Trap, + "int1" | "int3" => Flow::Trap, + // A software interrupt is classified by its **vector**, not by its mnemonic. Most of them + // return, and the one that matters is `int 2eh` — the 32-bit system-call path — where + // stopping the walk discards every instruction after a syscall. Two do not return: + // `int 29h` is `__fastfail`, and `int 3` is the breakpoint a compiler emits as padding + // behind unreachable code. `into` traps only on overflow, so it returns. + "int" => match operands.first() { + Some(Operand::Immediate(0x29 | 0x3)) => Flow::Trap, + _ => Flow::Fallthrough, + }, _ if is_conditional_branch(mnemonic) => Flow::Branch(destination()), _ => Flow::Fallthrough, } @@ -5947,8 +5987,22 @@ impl DebugEngine { /// So this is a sanity bound and a "which region is this" answer, **not** a function's extent. /// A walk over a whole function follows [`Instruction::flow`] from its entry. /// - /// `None` is a fact rather than a failure: a leaf function may have no entry, and neither has - /// an address that is not code. A caller that needs the distinction has [`Self::symbol_for`]. + /// # Three outcomes, because collapsing them hides two different wrong answers + /// + /// [`FunctionExtent::NoEntry`] is a fact rather than a failure — a leaf function has no entry, + /// and neither has an address that is not code. It is reported for the **one** measured + /// failure that means it, `E_NOINTERFACE` (`0x80004002`), which is what a real dbgeng 10.x + /// answers for address zero, for a module's header page and for every x86 address (32-bit + /// Windows has no unwind table at all — measured against `cppthrow-fastfail-x86.dmp`). Any + /// other failure is returned as one, so a broken engine is not read as a leaf. + /// + /// [`FunctionExtent::Unsupported`] is every instruction set but x64, and it is not caution. + /// ARM64's record is **two** words whose second is packed unwind data or an `.xdata` RVA, not + /// an end address: measured on an ARM64 kernel dump, `nt!KeBugCheckEx` fills `needed = 8` with + /// `[0x0025df60, 0x0005f218]`. Read as an end that is a bogus extent, and for any function + /// whose `BeginAddress` is below the `.xdata` RVA it is a bogus extent that **contains the + /// address asked about** and so passes every sanity check below. A wrong region that looks + /// right is worse than no region. /// /// # The shape, which is not the one the name suggests /// @@ -5958,11 +6012,17 @@ impl DebugEngine { /// and yields an extent that fails its own sanity check, which is how this was first written /// and how it reported "no entry" for a 2,108-byte dispatch routine that plainly had one. /// They are also relative, so they are rebased here against the module that holds the address - /// — what `.fnent` prints, at the RVAs it prints them. - pub fn function_extent(&self, address: u64) -> Result, DbgEngError> { + /// — what `.fnent` prints, at the RVAs it prints them. The engine's own `needed` is checked + /// against that shape rather than assumed, since it is the field that says ARM64's is + /// different. + pub fn function_extent(&self, address: u64) -> Result { + let set = self.instruction_set(); + if set != InstructionSet::Amd64 { + return Ok(FunctionExtent::Unsupported(set)); + } let mut entry = [0u32; 3]; let mut needed = 0u32; - if unsafe { + if let Err(source) = unsafe { self.symbols.GetFunctionEntryByOffset( address, 0, @@ -5970,26 +6030,35 @@ impl DebugEngine { std::mem::size_of_val(&entry) as u32, Some(&mut needed), ) + } { + // The one failure that means "this address has no unwind entry". + if source.code() == E_NOINTERFACE { + return Ok(FunctionExtent::NoEntry); + } + return Err(DbgEngError::Context { + operation: format!("reading the function entry for {address:#x}"), + source, + }); } - .is_err() - { - // No entry for this address is the ordinary answer for a leaf or for data. - return Ok(None); + // The engine says how big the record it filled is. An x64 `RUNTIME_FUNCTION` is twelve + // bytes; anything else is a layout this does not decode, whatever the processor said. + if needed as usize != std::mem::size_of_val(&entry) { + return Ok(FunctionExtent::Unsupported(set)); } let (begin, end) = (entry[0] as u64, entry[1] as u64); if begin == 0 || end <= begin { - return Ok(None); + return Ok(FunctionExtent::NoEntry); } let Some(module) = self.module_at(address)? else { - return Ok(None); + return Ok(FunctionExtent::NoEntry); }; let (begin, end) = (module.base + begin, module.base + end); - // The extent has to contain the address it was asked about; anything else means the entry - // read back does not describe this function and is not worth returning. + // The region has to contain the address it was asked about; anything else means the entry + // read back does not describe this code and is not worth returning. if !(begin..end).contains(&address) { - return Ok(None); + return Ok(FunctionExtent::NoEntry); } - Ok(Some((begin, end))) + Ok(FunctionExtent::Region { begin, end }) } /// The `module!Symbol` an address resolves to and how far past it the address is. @@ -8419,7 +8488,6 @@ mod tests { Flow::Branch(Some(0xfffff803_3e2547f0)) ); assert_eq!(one("ret"), Flow::Return); - assert_eq!(one("int 29h"), Flow::Trap); assert_eq!(one("ud2"), Flow::Trap); assert_eq!(one("call rax"), Flow::Call(None)); assert_eq!(one("jmp rax"), Flow::Jmp(None)); @@ -8439,6 +8507,40 @@ mod tests { assert_eq!(one("jmp qword ptr [rax*8+1234h]").target(), None); } + /// A software interrupt is classified by its vector, because most of them return. + /// + /// `int 2eh` is the 32-bit system-call path: a walk that stops there discards every + /// instruction after a syscall, which on that architecture is most of a function. The two that + /// do not return are `int 29h` (`__fastfail`) and `int 3` — the breakpoint a compiler emits as + /// padding behind unreachable code, and the form the engine renders `0xcc` as, so it has to be + /// caught by vector rather than by the `int3` spelling alone. + #[test] + fn test_a_software_interrupt_is_classified_by_its_vector() { + let one = |text: &str| { + split_instruction( + 0x1000, + &format!("00001000 90 {text}"), + InstructionSet::Amd64, + ) + .flow + }; + + assert_eq!(one("int 29h"), Flow::Trap, "__fastfail does not return"); + assert_eq!(one("int 3"), Flow::Trap, "a breakpoint stops the walk"); + assert_eq!(one("int3"), Flow::Trap); + assert_eq!( + one("int 2Eh"), + Flow::Fallthrough, + "the 32-bit system call returns, and the walk must go on past it" + ); + assert_eq!(one("int 2Dh"), Flow::Fallthrough); + // `into` traps only on overflow, so it returns. + assert_eq!(one("into"), Flow::Fallthrough); + + assert!(one("int 2Eh").falls_through()); + assert!(!one("int 29h").falls_through()); + } + /// `ah`, `bh`, `ch` and `dh` are registers *and* well-formed `h`-suffixed hexadecimal, and /// reading them as literals silently turns a destination register into a number. /// From 7fda435b46b8a41cd2b7f15049e4088019190e60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 11:07:58 +0100 Subject: [PATCH 03/12] fix: discriminate a rendering by the processor that rendered it Both findings on the second round were correct. `instruction_set` read `GetActualProcessorType`, which is the machine, while `Disassemble` renders with the *effective* one. They diverge wherever one machine runs another's code -- a WOW64 process, x64 emulated on ARM64, or any target after `.effmach` -- and discriminating a rendering by the machine underneath it reads x86 output with x64 rules and picks the wrong unwind record shape in `function_extent`. No fixture here has that divergence, so it was forced rather than left argued: `.effmach x86` on an x64 kernel dump moves the effective type to `0x14c` with the physical still `0x8664`, and the reading now follows it -- `nt!KeBugCheckEx` answers "not decoded for X86" instead of decoding an x64 entry against x86 output. `effective_processor_type` is a new method rather than a change to `processor_type`, because the pool and heap walkers read that one for pointer width, and a pointer's width is a fact about the machine rather than about a rendering. `mmword` was folded in with `xmmword` at sixteen bytes. An MMX operand is eight; the fold doubled the width reported for every MMX access, with nothing about the rendering to show for it. Every `ptr` width now has a test asserting the width its name means, including the two non-powers of two. Mutation-verified: restoring `mmword` to sixteen fails the new width test and nothing else. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 12 +++++ examples/function_entry_probe.rs | 25 ++++++++--- src/dbgeng.rs | 75 +++++++++++++++++++++++++++++--- 3 files changed, 101 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f267387..09c6ef8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,18 @@ All notable changes to this project are documented here. The format follows A software interrupt is classified by its **vector**, not by its mnemonic: `int 29h` (`__fastfail`) and `int 3` stop a walk, and every other one falls through — notably `int 2eh`, the 32-bit system-call path, where stopping would discard every instruction after a syscall. + An operand's width is the one its name means, and `mmword` is the trap: it is an MMX operand at + eight bytes, beside the `xmmword` that is sixteen, so folding the two doubles the width reported + for every MMX access while the rendering looks perfectly ordinary. +- `DebugEngine::effective_processor_type` reports the processor the engine is **rendering** in, as + against the physical one `processor_type` already answered. The two diverge wherever one machine + runs another's code — a WOW64 process, x64 emulated on ARM64, any target after `.effmach` — and + it is the effective one that discriminates a *rendering*, so `instruction_set` reads it. Measured + by forcing the divergence: `.effmach x86` on an x64 kernel dump moves the effective type to + `0x14c` while the physical stays `0x8664`, and the reading follows it rather than decoding an x64 + unwind record against x86 output. Anything reading the target's **structures** still wants the + physical type, a pointer's width being a fact about the machine rather than about a rendering, + so the pool and heap walkers are unchanged. - `DebugEngine::function_extent` returns the unwind **region** containing an address, from the image's `.pdata`, rebased. A region is **not** a function, and using it as one loses code: MSVC splits a function across several entries, and this answers `0x14750..0x147a3` for diff --git a/examples/function_entry_probe.rs b/examples/function_entry_probe.rs index db27003..a0ce52c 100644 --- a/examples/function_entry_probe.rs +++ b/examples/function_entry_probe.rs @@ -24,11 +24,14 @@ fn main() { }; let mut wanted = Vec::new(); let mut image_path = None; + let mut effmach = None; while let Some(arg) = args.next() { - if arg == "--exepath" { - image_path = args.next(); - } else { - wanted.push(arg); + match arg.as_str() { + "--exepath" => image_path = args.next(), + // `.effmach` is the one way to make the physical and effective processor types + // disagree on a fixture that is not a WOW64 or emulated target. + "--effmach" => effmach = args.next(), + _ => wanted.push(arg), } } @@ -41,7 +44,19 @@ fn main() { e.reload_symbols("/f").expect("reloading failed"); } - println!("instruction set: {:?}\n", e.instruction_set()); + if let Some(machine) = &effmach { + e.execute_command(&format!(".effmach {machine}")) + .expect("setting the effective machine failed"); + } + + // Physical against effective: `Disassemble` renders with the second, so the second is what + // discriminates the reading. They diverge wherever one machine runs another's code. + println!( + "processor: physical {:?} effective {:?} -> {:?}\n", + e.processor_type().map(|m| format!("{m:#x}")), + e.effective_processor_type().map(|m| format!("{m:#x}")), + e.instruction_set() + ); for name in &wanted { let address = match name.strip_prefix("0x") { Some(hex) => u64::from_str_radix(hex, 16).expect("a hexadecimal address"), diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 6c56501..1bec343 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2556,15 +2556,20 @@ fn split_operands(rest: &str) -> Vec<&str> { } /// The `qword ptr` family, as a width in bytes. +/// +/// `mmword` is an **MMX** operand and is eight bytes, not sixteen: it is the `xmmword` beside it +/// that is 128 bits, and folding the two together doubles the width reported for every MMX access. +/// `fword` is the 48-bit far pointer and `tbyte` the 80-bit float, neither of which is a power of +/// two. fn operand_size(word: &str) -> Option { Some(match word { "byte" => 1, "word" => 2, "dword" => 4, "fword" => 6, - "qword" => 8, + "qword" | "mmword" => 8, "tbyte" => 10, - "mmword" | "xmmword" => 16, + "xmmword" => 16, "ymmword" => 32, "zmmword" => 64, _ => return None, @@ -3668,6 +3673,21 @@ impl DebugEngine { }) } + /// The processor type the engine is currently **rendering** in, which is not always the one + /// the machine has. + /// + /// `.effmach` sets it, and it diverges from [`Self::processor_type`] wherever one machine runs + /// another's code: a WOW64 process on x64, x64 emulated on ARM64. Anything reading the + /// engine's *output* — a disassembly listing, a register file — is discriminated by this one, + /// while anything reading the target's *structures* wants the physical type, because a + /// pointer's width is a fact about the machine rather than about a rendering. + pub fn effective_processor_type(&self) -> Result { + unsafe { self.control.GetEffectiveProcessorType() }.map_err(|source| DbgEngError::Context { + operation: "querying effective processor type".into(), + source, + }) + } + pub fn is_kernel_target(&self) -> Result { let mut class = 0; let mut qualifier = 0; @@ -5957,11 +5977,18 @@ impl DebugEngine { /// The instruction set the target's disassembly is rendered in. /// - /// Falls back to [`InstructionSet::Other`] when the engine will not say, which reads - /// operands out of nothing rather than reading them wrongly — a target that cannot name its - /// processor is not one to guess x86 for. + /// The **effective** processor type and not the physical one, because that is what + /// `Disassemble` renders with: the two diverge wherever one machine runs another's code — a + /// WOW64 process on x64, x64 emulated on ARM64, or any target after `.effmach` — and + /// discriminating a rendering by the machine underneath it reads x86 output with x64 rules, + /// or refuses operands the engine did render. It also picks the unwind-entry layout in + /// [`Self::function_extent`], where the same divergence chooses the wrong record shape. + /// + /// Falls back to [`InstructionSet::Other`] when the engine will not say, which reads operands + /// out of nothing rather than reading them wrongly — a target that cannot name its processor + /// is not one to guess x86 for. pub fn instruction_set(&self) -> InstructionSet { - match self.processor_type() { + match self.effective_processor_type() { Ok(machine) => InstructionSet::from_processor_type(machine), Err(_) => InstructionSet::Other(0), } @@ -8507,6 +8534,42 @@ mod tests { assert_eq!(one("jmp qword ptr [rax*8+1234h]").target(), None); } + /// The `ptr` widths, each asserted against the width its name means rather than against the + /// table that produced it. + /// + /// `mmword` is the one worth a test of its own: it sits beside `xmmword` in every listing and + /// is **half** its width, so folding the two together doubles the reported width of every MMX + /// access and nothing about the rendering looks wrong. The two non-powers of two are here for + /// the same reason, being the ones a reader is most likely to round. + #[test] + fn test_an_operand_width_is_the_one_its_name_means() { + let width = |text: &str| { + let one = split_instruction( + 0x1000, + &format!("00001000 90 mov {text} [rax],rbx"), + InstructionSet::Amd64, + ); + match one.operands.first() { + Some(Operand::Memory(memory)) => memory.size, + other => panic!("{text} was not read as a memory operand: {other:?}"), + } + }; + + assert_eq!(width("byte ptr"), Some(1)); + assert_eq!(width("word ptr"), Some(2)); + assert_eq!(width("dword ptr"), Some(4)); + assert_eq!(width("fword ptr"), Some(6), "a far pointer is 48 bits"); + assert_eq!(width("qword ptr"), Some(8)); + assert_eq!(width("mmword ptr"), Some(8), "an MMX operand is 64 bits"); + assert_eq!(width("tbyte ptr"), Some(10), "an 80-bit float"); + assert_eq!(width("xmmword ptr"), Some(16)); + assert_eq!(width("ymmword ptr"), Some(32)); + assert_eq!(width("zmmword ptr"), Some(64)); + + // No `ptr` prefix at all is no width, rather than a guessed one. + assert_eq!(width(""), None); + } + /// A software interrupt is classified by its vector, because most of them return. /// /// `int 2eh` is the 32-bit system-call path: a walk that stops there discards every From de4a0db96df358e2bb6dce91ae3dcfeab6ff8a74 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 11:20:34 +0100 Subject: [PATCH 04/12] fix: a decorated symbol keeps its comma, xbegin keeps its second edge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings on the third round, all correct. A control transfer's operand text is no longer split on commas. It takes exactly one operand -- a far jump renders its segment with a colon -- and a demangled C++ name carries commas of its own, so `call module!std::map::insert (…)` was severed at the template argument, leaving a first fragment with no parenthesised address. That reports `Call(None)`, which a reachability walk reads as an indirect call and drops: a direct edge lost with nothing to show for it. Tracking angle-bracket depth is the other way to fix that and is not taken. `operator<<` and `operator<` leave it unbalanced, and an unbalanced opener swallows every later operand into one -- which on a `cmp` would take the immediate with it and lose exactly the control codes this reading exists to recover. Not splitting where there is nothing to split needs no such judgement. The cost is that a comma inside a symbol still severs it on a non-transfer instruction, where no edge is at stake. `xbegin` is a conditional branch. It falls through into the transaction and takes its operand on an abort or a failure to start, and the abort path is usually where the lock-based fallback lives, so classifying it as a plain fall-through misses a whole implementation of the routine. `function_extent` asks the processor directly instead of through `instruction_set`, which folds a failed query into `Other(0)`. That fold is right where a rendering still exists to hand back with its operands unread; here there is no partial answer, so a failed query is an error and `Unsupported` keeps meaning "a real architecture this does not decode". Both sides now say why they differ. Two of these landed on `classify_flow`, which is a hand-maintained table, so its doc now states what a missing entry costs rather than implying the table is complete: the default arm reads an unlisted transfer as a fall-through, which loses an edge and never invents one, so a gap degrades along the "reachable is sound, not-reachable is best-effort" boundary a walk over this already documents. Decoding ARM64's packed unwind length is now tracked as #146 and cited from the code. Mutation-verified one at a time: always splitting fails the decorated-symbol test and nothing else, and removing the `xbegin` arm fails its test and nothing else. The real-target measurement is unchanged at 376 instructions, zero unread operands. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 8 ++++ src/dbgeng.rs | 123 +++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 124 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09c6ef8..da97363 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,14 @@ All notable changes to this project are documented here. The format follows An operand's width is the one its name means, and `mmword` is the trap: it is an MMX operand at eight bytes, beside the `xmmword` that is sixteen, so folding the two doubles the width reported for every MMX access while the rendering looks perfectly ordinary. + A control transfer's operand text is **not** split on commas, because it takes exactly one + operand and a demangled C++ name carries commas of its own: split, + `call module!std::map::insert (…)` loses its parenthesised address and reports + `Call(None)`, which a walk reads as an indirect call and drops. Angle-bracket depth is + deliberately not tracked instead — `operator<<` and `operator<` leave it unbalanced, and an + unbalanced opener swallows a following operand, which on a `cmp` would take the control code + with it. `xbegin` is a conditional branch: it falls through into the transaction and takes its + operand on an abort, where the lock-based fallback usually lives. - `DebugEngine::effective_processor_type` reports the processor the engine is **rendering** in, as against the physical one `processor_type` already answered. The two diverge wherever one machine runs another's code — a WOW64 process, x64 emulated on ARM64, any target after `.effmach` — and diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 1bec343..7163a20 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2524,14 +2524,39 @@ fn read_operation(text: &str, set: InstructionSet) -> (String, Vec, Flo if mnemonic.starts_with('?') { return (mnemonic, Vec::new(), Flow::Unknown); } - let operands: Vec = split_operands(&rest) - .into_iter() - .map(|operand| read_operand(operand.trim())) - .collect(); + // A control transfer takes exactly one operand, so its text is not split at all. That is what + // keeps a comma *inside* a decorated symbol from severing a destination: + // `call module!std::map::insert (00007ff6`12345678)` has a top-level comma, and split + // on it the first fragment carries no parenthesised address — so the direct edge disappears + // and `classify_flow` reports `Call(None)`, which a reachability walk reads as indirect. + // + // Angle-bracket depth is deliberately **not** tracked instead. `operator<<` and `operator<` + // leave it unbalanced, and an unbalanced opener swallows every later operand into one — which + // on a `cmp` would take the immediate with it, losing exactly the control codes this reading + // exists to recover. Not splitting where there is nothing to split needs no such judgement. + let operands: Vec = if rest.is_empty() { + Vec::new() + } else if takes_one_destination(&mnemonic) { + vec![read_operand(rest.trim())] + } else { + split_operands(&rest) + .into_iter() + .map(|operand| read_operand(operand.trim())) + .collect() + }; let flow = classify_flow(&mnemonic, &operands); (mnemonic, operands, flow) } +/// Whether this mnemonic's whole operand text is one destination. +/// +/// Every x86 control transfer takes a single operand — a far jump renders its segment with a +/// colon rather than a comma — so there is never a comma here that separates two operands. +fn takes_one_destination(mnemonic: &str) -> bool { + matches!(mnemonic, "call" | "callf" | "jmp" | "jmpf" | "xbegin") + || is_conditional_branch(mnemonic) +} + /// Splits an operand list on the commas that separate operands — the ones outside brackets and /// parentheses. `[rax+rcx*8]` has no top-level comma; a symbolised target's `(fffff803`…)` has no /// comma at all, and both are stepped over the same way. @@ -2783,6 +2808,17 @@ fn is_register(token: &str) -> bool { /// The control flow an x86 mnemonic implies, with its destination taken from the operand that /// was already read rather than from the line. +/// +/// # What a missing entry costs, which is why the table need not be complete +/// +/// This is a hand-maintained list, so an exotic transfer will be missing from it, and the default +/// arm calls anything unlisted [`Flow::Fallthrough`]. That error has a **direction**: a transfer +/// read as a fall-through costs a walk one edge, so a reachable block can be missed, while nothing +/// unlisted ever invents an edge that does not exist. Which is exactly the boundary a reachability +/// walk over this already documents — "reachable" is sound and "not reachable" is best-effort +/// within bounds — so a gap here degrades along the axis callers are already told about rather +/// than opening a new one. Add to the table when a real target turns up an instruction it misses; +/// do not treat its incompleteness as a soundness hole. fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { let destination = || match operands.first() { Some(Operand::Target { address, .. }) => *address, @@ -2805,6 +2841,10 @@ fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { Some(Operand::Immediate(0x29 | 0x3)) => Flow::Trap, _ => Flow::Fallthrough, }, + // TSX. `xbegin` takes the transaction on its fall-through and its own operand on an abort + // or a failure to start, and the abort path is commonly where the lock-based fallback + // lives — so a walk that only falls through misses that whole implementation. + "xbegin" => Flow::Branch(destination()), _ if is_conditional_branch(mnemonic) => Flow::Branch(destination()), _ => Flow::Fallthrough, } @@ -5986,7 +6026,11 @@ impl DebugEngine { /// /// Falls back to [`InstructionSet::Other`] when the engine will not say, which reads operands /// out of nothing rather than reading them wrongly — a target that cannot name its processor - /// is not one to guess x86 for. + /// is not one to guess x86 for. That fold is deliberate **here** and not everywhere: a + /// disassembly whose processor query failed still has a rendering to hand back, with its + /// operands unread and its flow [`Flow::Unknown`], so there is a partial answer to give. + /// [`Self::function_extent`] has none, so it asks the processor directly and returns a failed + /// query as an error rather than as an unsupported architecture. pub fn instruction_set(&self) -> InstructionSet { match self.effective_processor_type() { Ok(machine) => InstructionSet::from_processor_type(machine), @@ -6029,7 +6073,9 @@ impl DebugEngine { /// `[0x0025df60, 0x0005f218]`. Read as an end that is a bogus extent, and for any function /// whose `BeginAddress` is below the `.xdata` RVA it is a bogus extent that **contains the /// address asked about** and so passes every sanity check below. A wrong region that looks - /// right is worse than no region. + /// right is worse than no region. Decoding that record's packed function length is + /// [#146](https://github.com/glslang/dbgscope/issues/146); it waits on something consuming a + /// bound on ARM64, since operand reading refuses that set as well. /// /// # The shape, which is not the one the name suggests /// @@ -6043,7 +6089,11 @@ impl DebugEngine { /// against that shape rather than assumed, since it is the field that says ARM64's is /// different. pub fn function_extent(&self, address: u64) -> Result { - let set = self.instruction_set(); + // Asked directly rather than through `instruction_set`, which folds a failed query into + // `Other(0)`. That fold is right where a rendering still exists to hand back with its + // operands unread; here there is no partial answer, so a processor query that fails is an + // error and `Unsupported` is left meaning "a real architecture this does not decode". + let set = InstructionSet::from_processor_type(self.effective_processor_type()?); if set != InstructionSet::Amd64 { return Ok(FunctionExtent::Unsupported(set)); } @@ -8534,6 +8584,65 @@ mod tests { assert_eq!(one("jmp qword ptr [rax*8+1234h]").target(), None); } + /// A comma inside a decorated symbol must not sever the destination it belongs to. + /// + /// A demangled C++ name carries commas between template arguments, and they sit outside the + /// brackets and parentheses the operand split steps over. Split there, the first fragment has + /// no parenthesised address, so the call reports `Call(None)` and a reachability walk reads a + /// direct edge as an indirect one and drops it. A control transfer takes one operand, so its + /// text is not split at all. + #[test] + fn test_a_comma_inside_a_decorated_symbol_does_not_sever_the_destination() { + let one = |text: &str| { + split_instruction( + 0x1000, + &format!("00001000 90 {text}"), + InstructionSet::Amd64, + ) + }; + + let call = one("call module!std::map::insert (00007ff6`12345678)"); + assert_eq!( + call.flow, + Flow::Call(Some(0x00007ff6_12345678)), + "the direct edge was lost: {call:?}" + ); + assert_eq!( + call.operands, + vec![Operand::Target { + symbol: Some("module!std::map::insert".into()), + address: Some(0x00007ff6_12345678), + }] + ); + + let branch = one("je module!foo (00007ff6`1234abcd)"); + assert_eq!(branch.flow, Flow::Branch(Some(0x00007ff6_1234abcd))); + + // And an ordinary two-operand instruction is still split, so the immediate a control-code + // compare carries is still its own operand. + let cmp = one("cmp r13d,6D0030h"); + assert_eq!(cmp.operands.len(), 2, "{cmp:?}"); + assert_eq!(cmp.operands[1], Operand::Immediate(0x6d_0030)); + } + + /// `xbegin` starts a transaction and takes its operand on an abort, so it has both edges. + /// + /// The abort path is commonly where the lock-based fallback lives, so a walk that only falls + /// through misses a whole implementation of the routine rather than a branch of it. + #[test] + fn test_xbegin_is_a_conditional_branch() { + let one = split_instruction( + 0x1000, + "00001000 c7f8 xbegin module!Lock+0x40 (00007ff6`12345678)", + InstructionSet::Amd64, + ); + assert_eq!(one.flow, Flow::Branch(Some(0x00007ff6_12345678))); + assert!( + one.flow.falls_through(), + "a transaction that starts goes on" + ); + } + /// The `ptr` widths, each asserted against the width its name means rather than against the /// table that produced it. /// From 5810f9f5730999541831c6ba20fea2ce91533595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 11:35:47 +0100 Subject: [PATCH 05/12] fix: a symbol's own parentheses, and what an unlisted mnemonic really costs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round four. Three taken, one declined. A destination is split from the **last** parenthesis, and only when what follows it parses as an address. `module!Functor::operator()` is a demangled call operator, and splitting at the first parenthesis took the name apart at `operator`, left `) (…` to parse as an address, and reported `Call(None)` -- the same direct edge the comma case lost, reached through a different character. Requiring the tail to parse is what keeps a symbol whose last parenthesis is its own intact. `function_extent`'s doc promised `NoEntry` for x86 while the gate has always answered `Unsupported`. Both readings are defensible and only one can be true, so the doc moves rather than the code: 32-bit Windows has no unwind table and every x86 address measured here answers `E_NOINTERFACE`, but that is a fact about a platform, and returning `NoEntry` would assert it about an address this never asked about. x86's record is also `IMAGE_FUNCTION_ENTRY`, whose fields are addresses rather than the RVAs rebased here, and nothing has measured one. One rule everywhere: decode the layout that has been measured, refuse the rest by name. The probe no longer follows a tail `jmp` out of the routine it was asked about. A neighbour is inside the module, so module bounds alone let the walk wander into that function and its own tail calls, and every number the probe prints would then describe more than the routine. Edges are kept inside the entry's symbol, with the module bound as the fallback where there is no symbol, and the run says which. Measured on `mountmgr!MountMgrDeviceControl`: zero cross-function jumps declined, 376 instructions and eleven control-code compares unchanged -- so the figures quoted for this branch were already describing the right routine. **Declined: `xabort` as non-fallthrough.** The finding's premise is that execution never continues at the next instruction, and outside a transaction that is not so -- the SDM specifies `IF RTM_ACTIVE = 0 THEN treat as NOP`, so it falls straight through. Inside one it resumes at the outer `xbegin`'s fallback, and no static reading tells the two apart. Classifying it as a transfer would drop every instruction after it wherever RTM is inactive, which on current parts is nearly everywhere. Listed explicitly as a fall-through so the decision is visible rather than left to the default arm. That finding did show up a sentence of mine that was wrong. `classify_flow`'s doc claimed an unlisted mnemonic "never invents an edge", which holds for a missing *conditional* transfer and not for an unconditional one, whose fall-through does not exist. The doc now separates the two and states that the table is complete for the second kind. Mutation-verified: splitting from the first parenthesis fails the new parentheses test and nothing else. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 10 ++- examples/typed_disassembly.rs | 38 +++++++++-- src/dbgeng.rs | 121 +++++++++++++++++++++++++++++----- 3 files changed, 144 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index da97363..67e6df7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,8 +38,14 @@ All notable changes to this project are documented here. The format follows `Call(None)`, which a walk reads as an indirect call and drops. Angle-bracket depth is deliberately not tracked instead — `operator<<` and `operator<` leave it unbalanced, and an unbalanced opener swallows a following operand, which on a `cmp` would take the control code - with it. `xbegin` is a conditional branch: it falls through into the transaction and takes its - operand on an abort, where the lock-based fallback usually lives. + with it. A symbol's own **parentheses** cost the same edge the same way, so a destination is + split from the *last* parenthesis and only when what follows it parses as an address: + `call module!Functor::operator() (…)` keeps both halves, and a name whose last parenthesis is + its own stays whole. `xbegin` is a conditional branch: it falls through into the transaction and + takes its operand on an abort, where the lock-based fallback usually lives. `xabort` is listed + as a fall-through on purpose — inside a transaction it resumes at the fallback, but the SDM + makes it a NOP when `RTM_ACTIVE = 0`, and no static reading can tell which, so classifying it as + a transfer would drop everything after it wherever RTM is inactive. - `DebugEngine::effective_processor_type` reports the processor the engine is **rendering** in, as against the physical one `processor_type` already answered. The two diverge wherever one machine runs another's code — a WOW64 process, x64 emulated on ARM64, any target after `.effmach` — and diff --git a/examples/typed_disassembly.rs b/examples/typed_disassembly.rs index 17789f2..5a3c14d 100644 --- a/examples/typed_disassembly.rs +++ b/examples/typed_disassembly.rs @@ -71,18 +71,33 @@ fn main() { // Follow the flow rather than reading forward. A linear read runs into whatever follows the // function and fills the candidate list with other routines' constants; the unwind region is - // no substitute, because MSVC splits one function across several of them. The module bounds - // the walk so a tail jump out of the driver does not take it with them. + // no substitute, because MSVC splits one function across several of them. + // + // A module bound alone is not enough either. A tail `jmp` to a neighbour is *inside* the + // module, so the walk would follow it into that function and its own tail calls, and every + // number below would describe more than the routine that was asked about. So a non-call edge + // is taken only while it stays inside the entry's own symbol. Where the entry has no symbol — + // a stripped driver — there is no ownership to test and the module bound is all there is, + // which the run says out loud rather than reporting a narrower walk as the same thing. let module = e .module_at(entry) .ok() .flatten() .expect("the entry is in no module"); let (low, high) = (module.base, module.base + module.size as u64); + let owner = e.symbol_for(entry).map(|(name, _)| name); + println!( + "ownership: {}", + match &owner { + Some(name) => format!("edges kept inside {name}"), + None => "no symbol for the entry — module bounds only".to_string(), + } + ); let mut seen = std::collections::HashSet::new(); let mut queue = vec![entry]; let mut instructions = Vec::new(); + let mut left_the_function = Vec::new(); while let Some(at) = queue.pop() { if at < low || at >= high || !seen.insert(at) || seen.len() > 20_000 { continue; @@ -96,10 +111,17 @@ fn main() { let Some(instruction) = pair.into_iter().next() else { continue; }; - if let Some(target) = instruction.flow.target() { - // A call leaves this function; every other edge stays in it. - if !matches!(instruction.flow, Flow::Call(_)) { + if let Some(target) = instruction.flow.target() + && !matches!(instruction.flow, Flow::Call(_)) + { + let stays = match &owner { + Some(owner) => e.symbol_for(target).is_some_and(|(name, _)| &name == owner), + None => true, + }; + if stays { queue.push(target); + } else { + left_the_function.push((instruction.address, target)); } } if instruction.flow.falls_through() { @@ -108,7 +130,11 @@ fn main() { instructions.push(instruction); } instructions.sort_by_key(|instruction| instruction.address); - println!("walked {} instructions\n", instructions.len()); + println!("walked {} instructions", instructions.len()); + println!( + "cross-function jumps declined: {}\n", + left_the_function.len() + ); let (mut unreadable, mut other_operands, mut unknown_flow) = (0usize, 0usize, 0usize); let mut candidates: Vec<(u64, u64)> = Vec::new(); diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 7163a20..5a1fbb3 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2634,14 +2634,25 @@ fn read_operand(text: &str) -> Operand { Operand::Other(text.to_string()) } -/// `nt!KeBugCheckEx (fffff803`3e2547f0)` as its two halves. Either may be missing. +/// ``nt!KeBugCheckEx (fffff803`3e2547f0)`` as its two halves. Either may be missing. +/// +/// Split from the **last** parenthesis, and only when what follows it is an address. A symbol can +/// contain parentheses of its own — `module!Functor::operator()` is the demangled name of a call +/// operator — and splitting at the first one takes the name apart at `operator`, leaves `) (…` to +/// parse as an address, and hands back a truncated symbol with no destination. The instruction is +/// then `Call(None)`, which a reachability walk reads as indirect and drops: a direct edge lost to +/// a name. Requiring the tail to *parse* is what makes the rule safe for a symbol whose last +/// parenthesis is its own, since then there is no address there and the whole text is the name. fn split_symbol_and_address(text: &str) -> (Option, Option) { - let (name, address) = match text.split_once('(') { - Some((name, tail)) => (name.trim(), tail.trim_end_matches(')').trim().to_string()), - None => (text.trim(), String::new()), - }; - let symbol = (!name.is_empty()).then(|| name.to_string()); - (symbol, parse_engine_address(&address)) + let text = text.trim(); + if let Some(open) = text.rfind('(') { + let inside = text[open + 1..].trim().trim_end_matches(')'); + if let Some(address) = parse_engine_address(inside) { + let name = text[..open].trim(); + return ((!name.is_empty()).then(|| name.to_string()), Some(address)); + } + } + ((!text.is_empty()).then(|| text.to_string()), None) } /// The inside of a memory operand, with whatever `qword ptr` and segment override preceded it. @@ -2812,13 +2823,19 @@ fn is_register(token: &str) -> bool { /// # What a missing entry costs, which is why the table need not be complete /// /// This is a hand-maintained list, so an exotic transfer will be missing from it, and the default -/// arm calls anything unlisted [`Flow::Fallthrough`]. That error has a **direction**: a transfer -/// read as a fall-through costs a walk one edge, so a reachable block can be missed, while nothing -/// unlisted ever invents an edge that does not exist. Which is exactly the boundary a reachability -/// walk over this already documents — "reachable" is sound and "not reachable" is best-effort -/// within bounds — so a gap here degrades along the axis callers are already told about rather -/// than opening a new one. Add to the table when a real target turns up an instruction it misses; -/// do not treat its incompleteness as a soundness hole. +/// arm calls anything unlisted [`Flow::Fallthrough`]. What that costs is **not** one thing, and +/// saying so was wrong the first time this paragraph was written: +/// +/// - A missing **conditional** transfer keeps its fall-through and loses its taken edge, so a +/// reachable block can be missed. That is the direction a reachability walk already documents — +/// "reachable" sound, "not reachable" best-effort within bounds. +/// - A missing **unconditional** one is worse, because the fall-through it is given does not +/// exist: instructions after it are reported reachable when nothing reaches them. That invents +/// an edge, which is the direction the walk does *not* have slack in. +/// +/// So the table is complete for the second kind — every unconditional transfer x86 has is listed — +/// and best-effort for the first. Add to it when a real target turns up something it misses, and +/// weigh a new entry by which of those two it would be. fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { let destination = || match operands.first() { Some(Operand::Target { address, .. }) => *address, @@ -2845,6 +2862,14 @@ fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { // or a failure to start, and the abort path is commonly where the lock-based fallback // lives — so a walk that only falls through misses that whole implementation. "xbegin" => Flow::Branch(destination()), + // And `xabort` is a fall-through **on purpose**, though it reads like a transfer. Inside a + // transaction it resumes at the outer `xbegin`'s fallback and the next instruction is not + // reached — but the SDM makes it a NOP when `RTM_ACTIVE = 0`, so outside one it falls + // straight through, and no static reading can tell which it is. Classifying it as a + // transfer would drop every instruction after it on any target where RTM is inactive, + // which today is most of them. Listed rather than left to the default so the decision is + // visible. + "xabort" => Flow::Fallthrough, _ if is_conditional_branch(mnemonic) => Flow::Branch(destination()), _ => Flow::Fallthrough, } @@ -6063,9 +6088,18 @@ impl DebugEngine { /// [`FunctionExtent::NoEntry`] is a fact rather than a failure — a leaf function has no entry, /// and neither has an address that is not code. It is reported for the **one** measured /// failure that means it, `E_NOINTERFACE` (`0x80004002`), which is what a real dbgeng 10.x - /// answers for address zero, for a module's header page and for every x86 address (32-bit - /// Windows has no unwind table at all — measured against `cppthrow-fastfail-x86.dmp`). Any - /// other failure is returned as one, so a broken engine is not read as a leaf. + /// answers for address zero and for a module's header page. Any other failure is returned as + /// one, so a broken engine is not read as a leaf. + /// + /// **x86 answers [`FunctionExtent::Unsupported`], not `NoEntry`**, and an earlier draft of + /// this paragraph promised the opposite — which the gate below has never done. Both readings + /// are defensible and only one can be true, so: 32-bit Windows has no unwind table, and every + /// x86 address measured here answers `E_NOINTERFACE` (`cppthrow-fastfail-x86.dmp`), so + /// implementing x86 would buy nothing. But that is a fact about a *platform*, and returning + /// `NoEntry` would assert it about an *address* this never asked about. x86's record is also a + /// different structure — `IMAGE_FUNCTION_ENTRY`, whose fields are addresses rather than the + /// RVAs rebased below — and nothing here has measured one. So the rule is the same rule + /// everywhere: decode the one layout that has been measured, and refuse the rest by name. /// /// [`FunctionExtent::Unsupported`] is every instruction set but x64, and it is not caution. /// ARM64's record is **two** words whose second is packed unwind data or an `.xdata` RVA, not @@ -8625,6 +8659,59 @@ mod tests { assert_eq!(cmp.operands[1], Operand::Immediate(0x6d_0030)); } + /// A symbol that contains parentheses keeps its destination. + /// + /// `module!Functor::operator()` is the demangled name of a call operator, and splitting at the + /// *first* parenthesis takes it apart at `operator`, leaves `) (…` to parse as an address, and + /// reports `Call(None)` — a direct edge lost to a name, exactly as the comma case lost one. + /// The split is from the last parenthesis and only when what follows parses as an address, + /// which is also what keeps a symbol whose *last* parenthesis is its own intact. + #[test] + fn test_a_symbol_containing_parentheses_keeps_its_destination() { + let one = |text: &str| { + split_instruction( + 0x1000, + &format!("00001000 90 {text}"), + InstructionSet::Amd64, + ) + }; + + let call = one("call module!Functor::operator() (00007ff6`12345678)"); + assert_eq!( + call.flow, + Flow::Call(Some(0x00007ff6_12345678)), + "the direct edge was lost: {call:?}" + ); + assert_eq!( + call.operands, + vec![Operand::Target { + symbol: Some("module!Functor::operator()".into()), + address: Some(0x00007ff6_12345678), + }] + ); + + // No address at all: the whole text is the name, parentheses and all. + let bare = one("call module!Functor::operator()"); + assert_eq!( + bare.operands, + vec![Operand::Target { + symbol: Some("module!Functor::operator()".into()), + address: None, + }] + ); + assert_eq!(bare.flow, Flow::Call(None)); + + // And the ordinary shape still splits where it always did. + let plain = one("call nt!KeBugCheckEx (fffff803`3e2547f0)"); + assert_eq!( + plain.operands, + vec![Operand::Target { + symbol: Some("nt!KeBugCheckEx".into()), + address: Some(0xfffff803_3e2547f0), + }] + ); + } + /// `xbegin` starts a transaction and takes its operand on an abort, so it has both edges. /// /// The abort path is commonly where the lock-based fallback lives, so a walk that only falls From 66eb5edbbf1ab00f15d57a2f3b8c6cad786c73c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 12:02:03 +0100 Subject: [PATCH 06/12] refactor: decode the encoding instead of parsing the rendering Five review rounds said this. Three of them found the same defect through a different character -- a comma inside `std::map`, a parenthesis inside `operator()`, a bracket inside `operator[]` -- each one severing a direct call edge that a walk then dropped as indirect, each fix locally correct, the next character already waiting. Four rounds added an entry to the mnemonic table: `int` by vector, `xbegin`, `xabort`, `hlt`. Neither list was ever going to end, because both were consequences of one choice: recovering structure from another program's rendering. So the rendering stops being the source. `mnemonic`, `operands` and `flow` now come from decoding `bytes` -- the engine's own read of the instruction, already in hand, so no extra round trip -- with `iced-x86`, default features off, `decoder` and `instr_info` only. No formatter: the engine's rendering is what this crate promises and it stays verbatim in `text`. What that deletes, rather than fixes. Symbols leave the picture: a destination is an address and naming it is `symbol_for`'s job, so no symbol's spelling can take an operand apart again -- `Operand::Target` and `MemoryOperand` lose their symbol halves. The mnemonic table is gone; flow control comes from the decoder and is complete by construction, leaving two decisions that are about semantics rather than spelling (a software interrupt's vector, and `xabort`), both keyed on `Code` and both carrying the reasoning that was established under review. Roughly 300 lines of parser and every special case in it go with them. `decode_range` is the other half: one memory read for a whole span instead of one engine call per instruction, which is what a bounded traversal over a hundred functions needs. Those instructions carry no `text`, nothing having rendered them, and the doc says to ask `disassemble` for the few a caller displays rather than filling the field from a second formatter -- two renderings of one instruction in one type is worse than none. `Flow::Unreadable` splits off `Unknown`, which round five caught. An instruction set this does not decode still has an instruction there and falls through; a `???` rendering has none, and a walk that fell through one would step through bytes, one address at a time, to its own cap. Checked before the instruction set, because unreadable bytes are unreadable on every architecture. `hlt` falls through, also from round five, and now for free: the decoder calls it `Next`, which is right -- a halted processor resumes when an interrupt wakes it, and grouping it with the undefined-instruction traps truncated every idle loop at the halt. Behaviour-preserving where it counts, measured rather than asserted. On `mountmgr!MountMgrDeviceControl`: 376 instructions, zero unreadable operands, zero unknown flows, the same eleven control-code compares at the same addresses, the same forty calls with the same import names -- identical before and after. The example now also compares the two decode paths over one region: 23 range-decoded, 23 compared, 0 disagreements. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 66 +- Cargo.lock | 16 + Cargo.toml | 10 + examples/typed_disassembly.rs | 51 +- src/dbgeng.rs | 1133 +++++++++++++-------------------- 5 files changed, 564 insertions(+), 712 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67e6df7..e0142d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,44 +8,38 @@ All notable changes to this project are documented here. The format follows ### Added -- **Disassembly carries its operands as values.** `Instruction` gains `mnemonic`, `operands` and - `flow` beside the `text` it already had, so a caller asking what an instruction *compares - against* or *branches to* reads a field instead of re-parsing a rendering downstream. `Operand` - is `Register`, `Immediate`, `Memory`, `Target` or `Other`, and `Other` is the whole safety - story — an operand this does not recognise keeps its text rather than being forced into a shape. - `Flow` carries every destination as an `Option`, because a resolvable target and an indirect one - are different facts, and a caller treating `None` as "no edge" stays sound. - Reading is gated on `InstructionSet`: x86 and x64 are read, and anything else — ARM64 today — - reports its mnemonic, no operands and `Flow::Unknown`. That is a refusal rather than a guess, - and the flow is `Unknown` rather than the `Fallthrough` most instructions happen to be, because - an unread `b.eq` reported as falling through hands a walk one edge of two. +- **Disassembly carries its operands as values, decoded from the encoding.** `Instruction` gains + `mnemonic`, `operands` and `flow` beside the `text` it already had, so a caller asking what an + instruction *compares against* or *branches to* reads a field instead of re-parsing a rendering + downstream. They come from decoding `bytes` — the engine's own read of the instruction, so no + extra round trip — with `iced-x86`, and the rendering stays verbatim in `text` because it is what + a listing prints. + **Decoding, rather than reading the rendering, is the whole design and was arrived at the hard + way.** The first implementation parsed the third column, and a symbol's own punctuation kept + taking operands apart: a comma inside `std::map`, a parenthesis inside `operator()`, a + bracket inside `operator[]` — three review rounds, three characters, each severing a direct call + edge that a walk then dropped as indirect. The mnemonic table had the same shape, growing an + entry a round for `int` by vector, `xbegin`, `xabort` and `hlt`, because a hand-written list of + what transfers control is never finished. An encoding has neither ambiguity, and a decoder's + flow control is complete by construction. Symbols leave the picture entirely: a destination is an + address, and naming it is `symbol_for`'s job. + `Operand` is `Register`, `Immediate`, `Memory`, `Target` or `Other`. `Flow` carries every + destination as an `Option`, because a direct transfer encodes a displacement and an indirect one + encodes a register, and a caller treating `None` as "no edge" stays sound. `Unknown` and + `Unreadable` are separate: an instruction set this does not decode still has an instruction + there, so it falls through, while a `???` rendering has none — a walk that fell through one would + step through *bytes*, one address at a time, to its own cap. + Reading is gated on `InstructionSet`: x86 and x64 are decoded, and anything else — ARM64 today — + reports its mnemonic, no operands and `Flow::Unknown`. Measured against a whole real dispatch routine rather than composed lines (`examples/typed_disassembly.rs`): 376 instructions of `mountmgr!MountMgrDeviceControl` on a - 26100 image read with **zero** unrecognised operands and zero unknown flows, and its eleven - control-code compares came back as values. Two defects that measurement found, both now pinned: - literals are `u64` rather than `i64`, since the routine renders `8000000000000000h` and - `0FFFFFFFFFFFFFFFFh`; and registers are matched before literals, since `ah`, `bh`, `ch` and `dh` - are both. - A software interrupt is classified by its **vector**, not by its mnemonic: `int 29h` - (`__fastfail`) and `int 3` stop a walk, and every other one falls through — notably `int 2eh`, - the 32-bit system-call path, where stopping would discard every instruction after a syscall. - An operand's width is the one its name means, and `mmword` is the trap: it is an MMX operand at - eight bytes, beside the `xmmword` that is sixteen, so folding the two doubles the width reported - for every MMX access while the rendering looks perfectly ordinary. - A control transfer's operand text is **not** split on commas, because it takes exactly one - operand and a demangled C++ name carries commas of its own: split, - `call module!std::map::insert (…)` loses its parenthesised address and reports - `Call(None)`, which a walk reads as an indirect call and drops. Angle-bracket depth is - deliberately not tracked instead — `operator<<` and `operator<` leave it unbalanced, and an - unbalanced opener swallows a following operand, which on a `cmp` would take the control code - with it. A symbol's own **parentheses** cost the same edge the same way, so a destination is - split from the *last* parenthesis and only when what follows it parses as an address: - `call module!Functor::operator() (…)` keeps both halves, and a name whose last parenthesis is - its own stays whole. `xbegin` is a conditional branch: it falls through into the transaction and - takes its operand on an abort, where the lock-based fallback usually lives. `xabort` is listed - as a fall-through on purpose — inside a transaction it resumes at the fallback, but the SDM - makes it a NOP when `RTM_ACTIVE = 0`, and no static reading can tell which, so classifying it as - a transfer would drop everything after it wherever RTM is inactive. + 26100 image, **zero** unrecognised operands, zero unknown flows, and its eleven control-code + compares recovered as values — identical before and after the decoder replaced the parser. +- `DebugEngine::decode_range` decodes a span from **one** memory read instead of one engine call + per instruction, which is what a bounded traversal over a hundred functions needs. Its + instructions carry no `text`, nothing having rendered them; a caller needing a rendering for the + few it displays asks `disassemble` for those. The two paths are compared against each other in + the example over a real function's first region: 23 instructions, 23 compared, 0 disagreements. - `DebugEngine::effective_processor_type` reports the processor the engine is **rendering** in, as against the physical one `processor_type` already answered. The two diverge wherever one machine runs another's code — a WOW64 process, x64 emulated on ARM64, any target after `.effmach` — and diff --git a/Cargo.lock b/Cargo.lock index 6d0abe0..7d6908a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7,6 +7,7 @@ name = "dbgscope" version = "0.1.0" dependencies = [ "hex", + "iced-x86", "thiserror", "windows", "windows-core", @@ -18,6 +19,21 @@ version = "0.4.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f24254aa9a54b5c858eaee2f5bccdb46aaf0e486a595ed5fd8f86ba55232a70" +[[package]] +name = "iced-x86" +version = "1.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c447cff8c7f384a7d4f741cfcff32f75f3ad02b406432e8d6c878d56b1edf6b" +dependencies = [ + "lazy_static", +] + +[[package]] +name = "lazy_static" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbd2bcb4c963f2ddae06a2efc7e9f3591312473c50c6685e1f298068316e66fe" + [[package]] name = "proc-macro2" version = "1.0.107" diff --git a/Cargo.toml b/Cargo.toml index 08954c6..ca758bc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,6 +29,16 @@ exclude = [ crate-type = ["rlib", "cdylib"] [dependencies] +# The x86/x64 instruction decoder behind `Instruction`'s typed fields. The engine renders +# disassembly as text and offers no structured form, and recovering operands from that rendering +# means deciding what a character means from the fact that it is present -- which does not survive +# real symbols: `std::map` puts a comma inside one, `operator()` a parenthesis, +# `operator[]` a bracket, and each of those severed a direct call edge in turn. Decoding the +# encoding the engine already hands back has none of those ambiguities and needs no extra round +# trip. Default features off: `decoder` and `instr_info` are the whole of what is used -- no +# formatter, because the engine's own rendering is what this crate promises, and no encoder. It +# brings one transitive dependency, `lazy_static`, for the tables `instr_info` reads. +iced-x86 = { version = "1.21", default-features = false, features = ["std", "decoder", "instr_info"] } hex = "0.4.3" thiserror = "2.0.18" windows-core = "0.62.2" diff --git a/examples/typed_disassembly.rs b/examples/typed_disassembly.rs index 5a3c14d..04d91ca 100644 --- a/examples/typed_disassembly.rs +++ b/examples/typed_disassembly.rs @@ -176,10 +176,57 @@ fn main() { .unwrap_or_else(|| format!("{target:#x}")); calls.push((instruction.address, name)); } + // An import thunk: an indirect call through a slot whose address the encoding pins. The + // operand carries no symbol — nothing here parses one — so the slot is named by asking. if let Some(Operand::Memory(memory)) = instruction.operands.first() - && let (Flow::Call(None), Some(symbol)) = (instruction.flow, &memory.symbol) + && let (Flow::Call(None), Some(slot)) = (instruction.flow, memory.address) { - calls.push((instruction.address, format!("[{symbol}]"))); + let name = e + .symbol_for(slot) + .map(|(name, _)| name) + .unwrap_or_else(|| format!("{slot:#x}")); + calls.push((instruction.address, format!("[{name}]"))); + } + } + + // The two decode paths, over the same bytes, compared. `disassemble` asks the engine to render + // each instruction and walks by the end it reports; `decode_range` reads the span once and + // decodes it locally. They should agree instruction for instruction, and a disagreement is + // worth more than either count on its own — it is the only signal that the local decoder and + // the engine's own read of the same bytes have diverged. + if let Ok(FunctionExtent::Region { begin, end }) = e.function_extent(entry) { + match e.decode_range(begin, (end - begin) as usize) { + Ok(ranged) => { + let walked: std::collections::HashMap = + instructions + .iter() + .map(|instruction| (instruction.address, instruction)) + .collect(); + let mut compared = 0usize; + let mut disagreed = 0usize; + for one in &ranged { + let Some(other) = walked.get(&one.address) else { + continue; + }; + compared += 1; + if one.bytes != other.bytes + || one.mnemonic != other.mnemonic + || one.flow != other.flow + { + disagreed += 1; + println!( + " DISAGREE {:#x} ranged {} {:?} walked {} {:?}", + one.address, one.mnemonic, one.flow, other.mnemonic, other.flow + ); + } + } + println!( + "\n--- the two decode paths over the first region ---\nrange-decoded {}, \ + compared {compared}, disagreed {disagreed}", + ranged.len() + ); + } + Err(error) => println!("\nrange decode unavailable: {error}"), } } diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 5a1fbb3..260d40e 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -135,6 +135,11 @@ pub enum DbgEngError { #[error("requested debugger buffer is too large: {0} bytes")] BufferTooLarge(usize), + /// A decode was asked for on an instruction set this build does not decode. Its own error + /// rather than a COM one, because no call failed: the question cannot be answered here. + #[error("this build does not decode instructions for machine {machine:#x}")] + UndecodedInstructionSet { machine: u32 }, + #[error("debugger text contains an interior NUL")] InvalidOutput, @@ -1812,13 +1817,14 @@ impl InstructionSet { } } -/// What one instruction does to control flow, as far as the **rendering** says. +/// What one instruction does to control flow, as its **encoding** says. /// -/// Every destination is an [`Option`] for one reason: the engine prints a resolvable target as a -/// parenthesised address and prints nothing resolvable for an indirect one, and those are -/// different facts. `Call(None)` is `call rax` or `call qword ptr [rax*8+…]`; `Jmp(None)` is a -/// jump table or a function pointer. A caller that treats `None` as "no edge" stays sound — it -/// never invents one — which is the property [`Self::target`] exists to keep obvious. +/// Every destination is an [`Option`] for one reason: a direct transfer encodes a displacement, +/// which adds up to an address, while an indirect one encodes a register or a memory reference +/// whose value is not in the instruction at all. Those are different facts. `Call(None)` is +/// `call rax` or `call qword ptr [rax*8+…]`; `Jmp(None)` is a jump table or a function pointer. A +/// caller that treats `None` as "no edge" stays sound — it never invents one — which is the +/// property [`Self::target`] exists to keep obvious. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Flow { /// Falls through to the next instruction — the common case. @@ -1835,9 +1841,12 @@ pub enum Flow { /// A `noreturn` trap — `int 29h` (`__fastfail`), `int 3`, `ud2`, `hlt`. Flow stops, so a walk /// must not fall through one. Trap, - /// Not read: an instruction set whose operands this does not decode, or a rendering the - /// split did not recognise. Says nothing about what the instruction does. + /// Not read: a real instruction, in an instruction set whose operands this does not decode. + /// Says nothing about what it does, but there *is* something there. Unknown, + /// There is no instruction here. The engine rendered `???`, which is what it prints when the + /// bytes could not be read at all — an unmapped page, or a dump that never captured the code. + Unreadable, } impl Flow { @@ -1851,8 +1860,13 @@ impl Flow { /// Whether control can continue at the next instruction. /// - /// [`Flow::Unknown`] answers **true**, because a walk that stopped there would silently drop - /// the rest of a function on an architecture whose operands are not read. + /// [`Flow::Unknown`] answers **true** and [`Flow::Unreadable`] answers **false**, and that is + /// the whole reason they are two variants. An unread instruction set still has an instruction + /// there, and nearly every instruction falls through, so continuing is the best-effort a walk + /// wants — stopping would drop the rest of a function on ARM64. An unreadable rendering has no + /// instruction at all: continuing walks *bytes*, one at a time, through whatever follows an + /// unmapped page, and on a dump with no code pages that runs to a walk's cap inventing every + /// address on the way. pub fn falls_through(self) -> bool { matches!( self, @@ -1892,11 +1906,13 @@ impl FunctionExtent { } } -/// A memory operand, as the engine renders it — `qword ptr [rdx+0B8h]`, `gs:[188h]`, -/// `[rax+rcx*8+20h]`, ``qword ptr [nt!_imp_ExAllocatePool2 (fffff803`…)]``. +/// A memory operand, as its encoding gives it — the shapes an engine would render +/// `qword ptr [rdx+0B8h]`, `gs:[188h]`, `[rax+rcx*8+20h]`, `qword ptr [rip+0x9018]`. /// -/// Every field is what was *printed*. Nothing here is computed against a register context, so -/// `[rdx+0B8h]` is the displacement `0xb8` off whatever `rdx` held, and this says only that. +/// Nothing here is computed against a register context, so `[rdx+0B8h]` is the displacement `0xb8` +/// off whatever `rdx` held, and this says only that. [`Self::address`] is the exception and is why +/// it is an [`Option`]: an absolute or RIP-relative reference needs no register, so its target is +/// known from the instruction alone. #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct MemoryOperand { /// The access width in bytes, from the `qword ptr` prefix; `None` when the engine printed @@ -1912,41 +1928,37 @@ pub struct MemoryOperand { pub scale: u8, /// The signed displacement, zero when none was printed. pub displacement: i64, - /// A symbol printed inside the brackets — `nt!_imp_ExAllocatePool2`, which is what an import - /// thunk looks like on a driver whose symbols resolve. - pub symbol: Option, - /// An absolute address printed inside the brackets, whether bare or beside a symbol. + /// The absolute address this operand refers to, when nothing at run time contributes to it — + /// an absolute displacement, or a RIP-relative reference, whose target is computed against the + /// instruction's own end. + /// + /// This is what names an import thunk. `call qword ptr [driver+0x9018]` is a RIP-relative + /// load, so the slot's address is here, and what lives at that slot is a question for the + /// image's import table or for [`DebugEngine::symbol_for`] — not for the operand's spelling. pub address: Option, } -/// One operand of an instruction. +/// One operand of an instruction, as its **encoding** says rather than as the engine printed it. /// -/// [`Self::Other`] is the whole safety story: an operand this does not recognise keeps its text -/// and is never forced into one of the shapes above. Order matters in the reading, and one -/// collision is worth knowing about — `ah`, `bh`, `ch` and `dh` are registers *and* well-formed -/// `h`-suffixed hexadecimal literals, so registers are matched first and `mov ah,5` does not -/// report a destination of `0xa`. +/// There is no symbol anywhere in here, and that is the point. A destination is an address; +/// naming it is [`DebugEngine::symbol_for`]'s job. While operands were read out of the rendering, +/// a symbol's own punctuation kept taking them apart — a comma inside `std::map`, a +/// parenthesis inside `operator()`, a bracket inside `operator[]` — and each of those severed a +/// direct call edge that a reachability walk then dropped as indirect. #[derive(Debug, Clone, PartialEq, Eq)] pub enum Operand { - /// A register, by the engine's own name for it. + /// A register, lowercased — the name the engine also prints. Register(String), - /// A literal, as the bit pattern the engine printed. `40h` is hexadecimal by its suffix; a - /// bare digit run is decimal, which agrees with hexadecimal below ten and is what the engine - /// prints for small counts. Unsigned because a real routine renders `8000000000000000h` and - /// `0FFFFFFFFFFFFFFFFh`, and a leading `-` is two's complement for the same reason. + /// A literal, as the bit pattern encoded. Unsigned, because what a caller matches against a + /// control code or a mask is the pattern rather than an arithmetic sign. Immediate(u64), /// A memory reference. Memory(MemoryOperand), - /// A branch or call destination as rendered: ``nt!KeBugCheckEx (fffff803`3e2547f0)``, a symbol - /// with no address, or a bare address. An address here has at least eight hexadecimal digits, - /// which is what keeps a short immediate from being read as one. - Target { - /// `module!Symbol` or `module!Symbol+0x1c`, when the engine resolved one. - symbol: Option, - /// The absolute destination, when the engine printed one. - address: Option, - }, - /// Anything else, verbatim. + /// The absolute destination of a direct branch or call, computed from the relative + /// displacement the encoding carries. + Target(u64), + /// An operand kind this does not shape — a far branch, or a string operation's implicit + /// operand — named rather than forced into one of the others. Other(String), } @@ -1958,10 +1970,12 @@ pub enum Operand { /// split does not recognise keeps everything after the address in [`Self::text`] and leaves /// [`Self::bytes`] empty, rather than guessing. /// -/// [`Self::mnemonic`], [`Self::operands`] and [`Self::flow`] read that third column, so a caller -/// asking what an instruction *compares against* or *branches to* reads a field rather than -/// re-parsing a rendering downstream. [`Self::text`] stays verbatim beside them: it is what a -/// listing prints, and the fields do not replace it. +/// [`Self::mnemonic`], [`Self::operands`] and [`Self::flow`] come from decoding [`Self::bytes`] — +/// the **encoding**, not the rendering — so a caller asking what an instruction *compares +/// against* or *branches to* reads a field rather than re-parsing a rendering downstream, and no +/// symbol's spelling can take an operand apart. [`Self::text`] stays verbatim beside them: it is +/// what a listing prints, and the fields do not replace it. An instruction from +/// [`DebugEngine::decode_range`] has fields and no text, nothing having rendered it. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Instruction { /// Where the instruction is. **Not** parsed from the rendered line: it is the offset this @@ -2486,7 +2500,21 @@ fn split_instruction(address: u64, line: &str, set: InstructionSet) -> Instructi (Some(only), None) => (String::new(), collapse_spaces(only)), _ => (String::new(), collapse_spaces(line)), }; - let (mnemonic, operands, flow) = read_operation(&text, set); + // The encoding is the engine's own read of this instruction, so decoding it costs no round + // trip. A column that is not hexadecimal is not one — a `???` line has none — and decodes to + // nothing. + let raw = hex::decode(&bytes).unwrap_or_default(); + let (mut mnemonic, operands, flow) = decode_operation(&raw, address, set); + if mnemonic.is_empty() { + // Nothing decoded: an instruction set this does not decode, or bytes that are not an + // instruction. The rendering's first token is the mnemonic in every syntax the engine + // prints, and it is the one thing still worth reporting. + mnemonic = text + .split_whitespace() + .next() + .unwrap_or_default() + .to_string(); + } Instruction { address, bytes, @@ -2497,397 +2525,172 @@ fn split_instruction(address: u64, line: &str, set: InstructionSet) -> Instructi } } -/// Prefixes that are not the operation: `lock inc dword ptr [rax]` is an `inc`. -const MNEMONIC_PREFIXES: [&str; 7] = ["lock", "rep", "repe", "repz", "repne", "repnz", "bnd"]; - -/// The third column read as an operation — mnemonic, operands, control flow. +/// One instruction's **encoding**, decoded. /// -/// An instruction set whose operands are not read still reports its mnemonic, because the first -/// token is the mnemonic in every syntax the engine renders. What it does not report is anything -/// that would be a guess: no operands, and [`Flow::Unknown`] rather than the `Fallthrough` that -/// most instructions happen to be — a walk told "falls through" about an unread `b.eq` would -/// silently take one edge of two. -fn read_operation(text: &str, set: InstructionSet) -> (String, Vec, Flow) { - let mut tokens = text.trim().splitn(2, char::is_whitespace); - let mut mnemonic = tokens.next().unwrap_or_default().to_string(); - let mut rest = tokens.next().unwrap_or_default().trim().to_string(); - // A prefix is not the operation. Take the next token as the mnemonic, if there is one. - while MNEMONIC_PREFIXES.contains(&mnemonic.as_str()) && !rest.is_empty() { - let mut next = rest.splitn(2, char::is_whitespace); - mnemonic = next.next().unwrap_or_default().to_string(); - rest = next.next().unwrap_or_default().trim().to_string(); - } - if !set.operands_are_read() || mnemonic.is_empty() { - return (mnemonic, Vec::new(), Flow::Unknown); - } - // `???` is the engine saying it could not read the bytes, not an instruction. - if mnemonic.starts_with('?') { - return (mnemonic, Vec::new(), Flow::Unknown); - } - // A control transfer takes exactly one operand, so its text is not split at all. That is what - // keeps a comma *inside* a decorated symbol from severing a destination: - // `call module!std::map::insert (00007ff6`12345678)` has a top-level comma, and split - // on it the first fragment carries no parenthesised address — so the direct edge disappears - // and `classify_flow` reports `Call(None)`, which a reachability walk reads as indirect. - // - // Angle-bracket depth is deliberately **not** tracked instead. `operator<<` and `operator<` - // leave it unbalanced, and an unbalanced opener swallows every later operand into one — which - // on a `cmp` would take the immediate with it, losing exactly the control codes this reading - // exists to recover. Not splitting where there is nothing to split needs no such judgement. - let operands: Vec = if rest.is_empty() { - Vec::new() - } else if takes_one_destination(&mnemonic) { - vec![read_operand(rest.trim())] - } else { - split_operands(&rest) - .into_iter() - .map(|operand| read_operand(operand.trim())) - .collect() - }; - let flow = classify_flow(&mnemonic, &operands); - (mnemonic, operands, flow) -} - -/// Whether this mnemonic's whole operand text is one destination. +/// # Why this is not read out of the rendering /// -/// Every x86 control transfer takes a single operand — a far jump renders its segment with a -/// colon rather than a comma — so there is never a comma here that separates two operands. -fn takes_one_destination(mnemonic: &str) -> bool { - matches!(mnemonic, "call" | "callf" | "jmp" | "jmpf" | "xbegin") - || is_conditional_branch(mnemonic) -} - -/// Splits an operand list on the commas that separate operands — the ones outside brackets and -/// parentheses. `[rax+rcx*8]` has no top-level comma; a symbolised target's `(fffff803`…)` has no -/// comma at all, and both are stepped over the same way. -fn split_operands(rest: &str) -> Vec<&str> { - if rest.is_empty() { - return Vec::new(); - } - let (mut depth, mut start, mut out) = (0i32, 0usize, Vec::new()); - for (index, character) in rest.char_indices() { - match character { - '[' | '(' => depth += 1, - ']' | ')' => depth -= 1, - ',' if depth <= 0 => { - out.push(&rest[start..index]); - start = index + 1; - } - _ => {} - } - } - out.push(&rest[start..]); - out -} - -/// The `qword ptr` family, as a width in bytes. +/// The engine's third column is a rendering, and recovering structure from it means deciding what +/// a character means from the fact that it is present. That was tried, at length: a comma is an +/// operand separator until a demangled `std::map` puts one inside a symbol; a parenthesis +/// introduces the resolved address until `operator()` puts one inside a symbol; a bracket opens a +/// memory expression until `operator[]` puts one inside a symbol. Three review rounds found the +/// same defect through three characters, each fix locally correct and the next one already +/// waiting. The same shape appeared in the mnemonic table — `int` by vector, then `xbegin`, then +/// `xabort`, then `hlt` — because a hand-maintained list of what transfers control is never +/// finished. /// -/// `mmword` is an **MMX** operand and is eight bytes, not sixteen: it is the `xmmword` beside it -/// that is 128 bits, and folding the two together doubles the width reported for every MMX access. -/// `fword` is the 48-bit far pointer and `tbyte` the 80-bit float, neither of which is a power of -/// two. -fn operand_size(word: &str) -> Option { - Some(match word { - "byte" => 1, - "word" => 2, - "dword" => 4, - "fword" => 6, - "qword" | "mmword" => 8, - "tbyte" => 10, - "xmmword" => 16, - "ymmword" => 32, - "zmmword" => 64, - _ => return None, - }) -} - -/// One operand, read in an order that resolves the collisions rather than tripping over them. -fn read_operand(text: &str) -> Operand { - if text.is_empty() { - return Operand::Other(String::new()); - } - // Registers first: `ah`, `bh`, `ch` and `dh` are also well-formed `h`-suffixed literals. - if is_register(text) { - return Operand::Register(text.to_string()); - } - if text.contains('[') { - if let Some(memory) = read_memory_operand(text) { - return Operand::Memory(memory); - } - return Operand::Other(text.to_string()); - } - // A symbolised destination, or a bare address. `!` is what makes it a symbol; the address is - // the parenthesised one the engine prints beside it. - if text.contains('!') { - let (symbol, address) = split_symbol_and_address(text); - return Operand::Target { symbol, address }; - } - if let Some(address) = parse_engine_address(text) { - return Operand::Target { - symbol: None, - address: Some(address), - }; - } - if let Some(value) = parse_engine_number(text) { - return Operand::Immediate(value); - } - Operand::Other(text.to_string()) -} - -/// ``nt!KeBugCheckEx (fffff803`3e2547f0)`` as its two halves. Either may be missing. +/// The encoding has none of those ambiguities. `bytes` is the engine's own read of the +/// instruction, so decoding it needs no extra round trip, and a decoder answers what the operands +/// *are* rather than how they were printed. Symbols leave the picture entirely: a destination is +/// an address, and naming it is [`DebugEngine::symbol_for`]'s job, so no symbol's spelling can +/// take an operand apart again. /// -/// Split from the **last** parenthesis, and only when what follows it is an address. A symbol can -/// contain parentheses of its own — `module!Functor::operator()` is the demangled name of a call -/// operator — and splitting at the first one takes the name apart at `operator`, leaves `) (…` to -/// parse as an address, and hands back a truncated symbol with no destination. The instruction is -/// then `Call(None)`, which a reachability walk reads as indirect and drops: a direct edge lost to -/// a name. Requiring the tail to *parse* is what makes the rule safe for a symbol whose last -/// parenthesis is its own, since then there is no address there and the whole text is the name. -fn split_symbol_and_address(text: &str) -> (Option, Option) { - let text = text.trim(); - if let Some(open) = text.rfind('(') { - let inside = text[open + 1..].trim().trim_end_matches(')'); - if let Some(address) = parse_engine_address(inside) { - let name = text[..open].trim(); - return ((!name.is_empty()).then(|| name.to_string()), Some(address)); - } - } - ((!text.is_empty()).then(|| text.to_string()), None) -} - -/// The inside of a memory operand, with whatever `qword ptr` and segment override preceded it. -fn read_memory_operand(text: &str) -> Option { - let mut memory = MemoryOperand { - scale: 1, - ..MemoryOperand::default() +/// The rendering stays in [`Instruction::text`] verbatim beside the fields, because it is what a +/// listing prints and this crate has always promised `u`'s own output. In principle the two could +/// disagree on an encoding one decoder knows and the other does not; they are the same bytes, and +/// the fields say which reading they came from. +fn decode_operation( + bytes: &[u8], + address: u64, + set: InstructionSet, +) -> (String, Vec, Flow) { + // Asked before the instruction set: bytes that are not there are not there on any + // architecture, and answering `Unknown` for them on ARM64 would let a walk step through the + // same unreadable page the x64 path is kept out of. + if bytes.is_empty() { + return (String::new(), Vec::new(), Flow::Unreadable); + } + let bitness = match set { + InstructionSet::X86 => 32, + InstructionSet::Amd64 => 64, + // Not decoded: ARM64 today. The mnemonic is still the rendering's first token, which every + // syntax puts first, and the caller reads `Flow::Unknown` as "nothing was claimed". + InstructionSet::Other(_) => return (String::new(), Vec::new(), Flow::Unknown), }; - let open = text.find('[')?; - let close = text.rfind(']')?; - if close < open { - return None; - } - // Everything before the bracket: an optional `qword ptr`, an optional `gs:`. - for word in text[..open] - .split(|c: char| c.is_whitespace() || c == ':') - .filter(|w| !w.is_empty()) - { - if let Some(size) = operand_size(word) { - memory.size = Some(size); - } else if word != "ptr" { - memory.segment = Some(word.to_string()); - } - } - // A segment override written `gs:[188h]` leaves `gs` immediately before the bracket, which - // the same split already caught; nothing else is expected there. - let inside = &text[open + 1..close]; - if inside.contains('!') { - let (symbol, address) = split_symbol_and_address(inside); - memory.symbol = symbol; - memory.address = address; - return Some(memory); - } - let mut negative = false; - let mut term = String::new(); - let flush = |term: &mut String, negative: &mut bool, memory: &mut MemoryOperand| { - let text = term.trim().to_string(); - term.clear(); - let was_negative = std::mem::replace(negative, false); - if text.is_empty() { - return; - } - if let Some((index, scale)) = text.split_once('*') { - memory.index = Some(index.trim().to_string()); - memory.scale = scale.trim().parse().unwrap_or(1); - } else if is_register(&text) { - if memory.base.is_none() { - memory.base = Some(text); - } else { - memory.index = Some(text); - } - } else if let Some(address) = parse_engine_address(&text) { - memory.address = Some(address); - } else if let Some(value) = parse_engine_number(&text) { - let value = value as i64; - memory.displacement = if was_negative { -value } else { value }; - } - }; - for character in inside.chars() { - match character { - '+' => flush(&mut term, &mut negative, &mut memory), - '-' => { - flush(&mut term, &mut negative, &mut memory); - negative = true; - } - _ => term.push(character), - } + let mut decoder = + iced_x86::Decoder::with_ip(bitness, bytes, address, iced_x86::DecoderOptions::NONE); + let decoded = decoder.decode(); + if decoded.is_invalid() { + return (String::new(), Vec::new(), Flow::Unreadable); } - flush(&mut term, &mut negative, &mut memory); - Some(memory) -} -/// An engine-rendered address: the `hi`lo` backtick form or a plain hexadecimal run, requiring -/// **eight** digits so a short immediate is never read as an address. -fn parse_engine_address(token: &str) -> Option { - let cleaned: String = token - .trim() - .trim_matches(|c| c == '(' || c == ')' || c == ',') - .chars() - .filter(|&c| c != '`') + let mnemonic = format!("{:?}", decoded.mnemonic()).to_lowercase(); + let operands = (0..decoded.op_count()) + .map(|index| read_decoded_operand(&decoded, index)) .collect(); - if cleaned.len() < 8 || !cleaned.chars().all(|c| c.is_ascii_hexdigit()) { - return None; - } - u64::from_str_radix(&cleaned, 16).ok() -} - -/// A literal, as the **bit pattern** the engine printed. `40h` is hexadecimal by its suffix, -/// `0x40` by its prefix, and a bare digit run is decimal — which is what the engine prints for a -/// small count, and agrees with hexadecimal below ten either way. -/// -/// Unsigned on purpose, and measured rather than assumed: a real dispatch routine renders -/// `mov rax,8000000000000000h` and `mov qword ptr [rbp+0A8h],0FFFFFFFFFFFFFFFFh`, both of which -/// overflow a signed parse and came back as unread operands until this stopped being an `i64`. -/// A leading `-` is taken as two's complement for the same reason: what a caller compares against -/// a control code or a mask is the pattern, not an arithmetic sign. -fn parse_engine_number(token: &str) -> Option { - let token = token.trim(); - let (negative, token) = match token.strip_prefix('-') { - Some(rest) => (true, rest), - None => (false, token), - }; - let value = if let Some(hex) = token - .strip_prefix("0x") - .or_else(|| token.strip_prefix("0X")) - { - u64::from_str_radix(hex, 16).ok()? - } else if let Some(hex) = token.strip_suffix('h').or_else(|| token.strip_suffix('H')) { - u64::from_str_radix(hex, 16).ok()? - } else if token.chars().all(|c| c.is_ascii_digit()) && !token.is_empty() { - token.parse().ok()? + (mnemonic, operands, decoded_flow(&decoded)) +} + +/// One decoded operand. +fn read_decoded_operand(decoded: &iced_x86::Instruction, index: u32) -> Operand { + use iced_x86::OpKind; + match decoded.op_kind(index) { + OpKind::Register => Operand::Register(register_name(decoded.op_register(index))), + OpKind::NearBranch16 | OpKind::NearBranch32 | OpKind::NearBranch64 => { + Operand::Target(decoded.near_branch_target()) + } + OpKind::Immediate8 + | OpKind::Immediate8_2nd + | OpKind::Immediate16 + | OpKind::Immediate32 + | OpKind::Immediate64 + | OpKind::Immediate8to16 + | OpKind::Immediate8to32 + | OpKind::Immediate8to64 + | OpKind::Immediate32to64 => Operand::Immediate(decoded.immediate(index)), + OpKind::Memory => Operand::Memory(read_decoded_memory(decoded)), + // Far branches, and the implicit string-operation operands. Kept rather than shaped. + other => Operand::Other(format!("{other:?}").to_lowercase()), + } +} + +/// The memory operand of a decoded instruction. +fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { + let size = decoded.memory_size().size(); + let base = decoded.memory_base(); + let index = decoded.memory_index(); + // Statically known only when nothing at run time contributes: an absolute displacement, or a + // RIP-relative reference, whose target the decoder has already computed against the + // instruction's own end. + let address = if base == iced_x86::Register::RIP || base == iced_x86::Register::EIP { + Some(decoded.ip_rel_memory_address()) + } else if base == iced_x86::Register::None && index == iced_x86::Register::None { + Some(decoded.memory_displacement64()) } else { - return None; + None }; - Some(if negative { - value.wrapping_neg() - } else { - value - }) + MemoryOperand { + size: (size != 0).then_some(size as u32), + // The *prefix*, not the segment the encoding implies: `[rsp+8]` is `ss` by rule and prints + // no override, and reporting one would say the instruction carried something it did not. + segment: (decoded.segment_prefix() != iced_x86::Register::None) + .then(|| register_name(decoded.segment_prefix())), + base: (base != iced_x86::Register::None).then(|| register_name(base)), + index: (index != iced_x86::Register::None).then(|| register_name(index)), + scale: decoded.memory_index_scale() as u8, + displacement: decoded.memory_displacement64() as i64, + address, + } } -/// Whether a token is one of the engine's register names. -fn is_register(token: &str) -> bool { - const NAMED: [&str; 24] = [ - "rax", "rbx", "rcx", "rdx", "rsi", "rdi", "rbp", "rsp", "eax", "ebx", "ecx", "edx", "esi", - "edi", "ebp", "esp", "ax", "bx", "cx", "dx", "si", "di", "bp", "sp", - ]; - const BYTE: [&str; 12] = [ - "al", "bl", "cl", "dl", "ah", "bh", "ch", "dh", "sil", "dil", "bpl", "spl", - ]; - const SEGMENT: [&str; 6] = ["cs", "ds", "es", "fs", "gs", "ss"]; - const POINTER: [&str; 4] = ["rip", "eip", "ip", "eflags"]; - let token = token.trim(); - if NAMED.contains(&token) || BYTE.contains(&token) || SEGMENT.contains(&token) { - return true; - } - if POINTER.contains(&token) { - return true; - } - // `r8`..`r15` with their `d`/`w`/`b` widths, and the vector and control/debug files. - for (prefix, count) in [ - ("r", 16u32), - ("xmm", 32), - ("ymm", 32), - ("zmm", 32), - ("cr", 16), - ("dr", 16), - ] { - if let Some(tail) = token.strip_prefix(prefix) { - let digits = tail.trim_end_matches(['d', 'w', 'b']); - if prefix != "r" && digits.len() != tail.len() { - continue; - } - if let Ok(number) = digits.parse::() - && number < count - && (prefix != "r" || number >= 8) - { - return true; - } - } - } - false +/// A register by the lowercase name the engine also prints. +fn register_name(register: iced_x86::Register) -> String { + format!("{register:?}").to_lowercase() } -/// The control flow an x86 mnemonic implies, with its destination taken from the operand that -/// was already read rather than from the line. -/// -/// # What a missing entry costs, which is why the table need not be complete -/// -/// This is a hand-maintained list, so an exotic transfer will be missing from it, and the default -/// arm calls anything unlisted [`Flow::Fallthrough`]. What that costs is **not** one thing, and -/// saying so was wrong the first time this paragraph was written: -/// -/// - A missing **conditional** transfer keeps its fall-through and loses its taken edge, so a -/// reachable block can be missed. That is the direction a reachability walk already documents — -/// "reachable" sound, "not reachable" best-effort within bounds. -/// - A missing **unconditional** one is worse, because the fall-through it is given does not -/// exist: instructions after it are reported reachable when nothing reaches them. That invents -/// an edge, which is the direction the walk does *not* have slack in. +/// What a decoded instruction does to control flow. /// -/// So the table is complete for the second kind — every unconditional transfer x86 has is listed — -/// and best-effort for the first. Add to it when a real target turns up something it misses, and -/// weigh a new entry by which of those two it would be. -fn classify_flow(mnemonic: &str, operands: &[Operand]) -> Flow { - let destination = || match operands.first() { - Some(Operand::Target { address, .. }) => *address, - // A near jump to an absolute the engine printed bare, without eight digits, is not one - // this reads: an immediate here is a relative displacement, not a destination. - _ => None, - }; - match mnemonic { - "call" | "callf" => Flow::Call(destination()), - "jmp" | "jmpf" => Flow::Jmp(destination()), - "ret" | "retf" | "retn" | "iret" | "iretd" | "iretq" | "sysret" | "sysexit" => Flow::Return, - "ud0" | "ud1" | "ud2" | "hlt" => Flow::Trap, - "int1" | "int3" => Flow::Trap, - // A software interrupt is classified by its **vector**, not by its mnemonic. Most of them - // return, and the one that matters is `int 2eh` — the 32-bit system-call path — where - // stopping the walk discards every instruction after a syscall. Two do not return: - // `int 29h` is `__fastfail`, and `int 3` is the breakpoint a compiler emits as padding - // behind unreachable code. `into` traps only on overflow, so it returns. - "int" => match operands.first() { - Some(Operand::Immediate(0x29 | 0x3)) => Flow::Trap, +/// Nearly all of this is the decoder's own `FlowControl`, which is complete by construction — the +/// property a hand-written mnemonic table could not have. Two families still need a decision, and +/// both are decisions about *semantics* rather than about spelling, so they are keyed on the +/// instruction's `Code` and not on a string. +fn decoded_flow(decoded: &iced_x86::Instruction) -> Flow { + use iced_x86::{Code, FlowControl}; + match decoded.flow_control() { + FlowControl::Next => Flow::Fallthrough, + FlowControl::UnconditionalBranch => Flow::Jmp(near_target(decoded)), + FlowControl::IndirectBranch => Flow::Jmp(None), + FlowControl::ConditionalBranch => Flow::Branch(near_target(decoded)), + FlowControl::Return => Flow::Return, + FlowControl::Call => Flow::Call(near_target(decoded)), + FlowControl::IndirectCall => Flow::Call(None), + // Always raises: `ud0`/`ud1`/`ud2`. `hlt` is deliberately **not** here — the decoder calls + // it `Next`, and rightly: a halted processor resumes at the next instruction when an + // interrupt wakes it, so treating it as an ending truncates every idle loop. + FlowControl::Exception => Flow::Trap, + // A software interrupt is classified by its **vector**. Most return, and `int 2eh` — the + // 32-bit system-call path — is the one that matters: stopping there discards every + // instruction after a syscall. Two do not return: `int 29h` is `__fastfail`, and `int 3` + // is the breakpoint a compiler emits as padding behind unreachable code. + FlowControl::Interrupt => match decoded.code() { + Code::Int3 => Flow::Trap, + Code::Int_imm8 if matches!(decoded.immediate8(), 0x29 | 0x03) => Flow::Trap, + _ => Flow::Fallthrough, + }, + // TSX. `xbegin` starts a transaction on its fall-through and takes its operand on an abort + // or a failure to start, and that abort path is commonly where the lock-based fallback + // lives. `xabort` is a fall-through on purpose: inside a transaction it resumes at the + // outer `xbegin`'s fallback, but the SDM makes it a NOP when `RTM_ACTIVE = 0`, and no + // static reading tells the two apart — so classifying it as a transfer would drop every + // instruction after it wherever RTM is inactive, which today is nearly everywhere. + FlowControl::XbeginXabortXend => match decoded.code() { + Code::Xbegin_rel16 | Code::Xbegin_rel32 => Flow::Branch(near_target(decoded)), _ => Flow::Fallthrough, }, - // TSX. `xbegin` takes the transaction on its fall-through and its own operand on an abort - // or a failure to start, and the abort path is commonly where the lock-based fallback - // lives — so a walk that only falls through misses that whole implementation. - "xbegin" => Flow::Branch(destination()), - // And `xabort` is a fall-through **on purpose**, though it reads like a transfer. Inside a - // transaction it resumes at the outer `xbegin`'s fallback and the next instruction is not - // reached — but the SDM makes it a NOP when `RTM_ACTIVE = 0`, so outside one it falls - // straight through, and no static reading can tell which it is. Classifying it as a - // transfer would drop every instruction after it on any target where RTM is inactive, - // which today is most of them. Listed rather than left to the default so the decision is - // visible. - "xabort" => Flow::Fallthrough, - _ if is_conditional_branch(mnemonic) => Flow::Branch(destination()), - _ => Flow::Fallthrough, } } -/// `jcc`, `loop` and `jcxz` — the conditional transfers, which take one edge or the other. -fn is_conditional_branch(mnemonic: &str) -> bool { - const CONDITIONS: [&str; 32] = [ - "o", "no", "b", "c", "nae", "ae", "nb", "nc", "e", "z", "ne", "nz", "be", "na", "a", "nbe", - "s", "ns", "p", "pe", "np", "po", "l", "nge", "ge", "nl", "le", "ng", "g", "nle", "cxz", - "ecxz", - ]; - if let Some(condition) = mnemonic.strip_prefix('j') - && (condition == "rcxz" || CONDITIONS.contains(&condition)) - { - return true; - } - matches!(mnemonic, "loop" | "loope" | "loopne" | "loopz" | "loopnz") +/// The destination of a direct branch or call, when the instruction carries one. +fn near_target(decoded: &iced_x86::Instruction) -> Option { + use iced_x86::OpKind; + (0..decoded.op_count()) + .any(|index| { + matches!( + decoded.op_kind(index), + OpKind::NearBranch16 | OpKind::NearBranch32 | OpKind::NearBranch64 + ) + }) + .then(|| decoded.near_branch_target()) } /// Runs of whitespace as one space. The engine pads its columns to align them in a listing, and @@ -6040,6 +5843,73 @@ impl DebugEngine { Ok(out) } + /// Decodes a **range** of code from one memory read, instead of one engine call per + /// instruction. + /// + /// [`Self::disassemble`] asks the engine to render each instruction and walks forward by the + /// end it reports, which is right for a listing and wrong for an analysis: a bounded walk over + /// a whole routine makes one COM call per instruction, and a traversal over a hundred + /// functions makes tens of thousands. This reads the span once and decodes it locally. + /// + /// **The instructions carry no [`Instruction::text`].** Nothing rendered them, and this crate + /// has always promised that field is the engine's own output — filling it from a second + /// formatter would put two renderings of one instruction into one type, which is worse than an + /// empty string. A caller that needs a rendering for the few instructions it displays asks + /// [`Self::disassemble`] for those. + /// + /// Decoding resynchronises the way a linear read must: a span that begins mid-instruction, or + /// runs into data, decodes as whatever those bytes are. Bound it with a function's own extent + /// where there is one, and read [`Flow`] rather than trusting a listing's shape. + pub fn decode_range( + &self, + address: u64, + length: usize, + ) -> Result, DbgEngError> { + let set = InstructionSet::from_processor_type(self.effective_processor_type()?); + let bitness = match set { + InstructionSet::X86 => 32, + InstructionSet::Amd64 => 64, + InstructionSet::Other(machine) => { + return Err(DbgEngError::UndecodedInstructionSet { machine }); + } + }; + let bytes = self.read_memory(address, length)?; + let mut decoder = + iced_x86::Decoder::with_ip(bitness, &bytes, address, iced_x86::DecoderOptions::NONE); + let mut out = Vec::new(); + while decoder.can_decode() { + let at = decoder.ip(); + let decoded = decoder.decode(); + let offset = (at - address) as usize; + let encoding = bytes + .get(offset..offset + decoded.len()) + .unwrap_or_default(); + out.push(Instruction { + address: at, + bytes: hex::encode(encoding), + text: String::new(), + mnemonic: if decoded.is_invalid() { + String::new() + } else { + format!("{:?}", decoded.mnemonic()).to_lowercase() + }, + operands: if decoded.is_invalid() { + Vec::new() + } else { + (0..decoded.op_count()) + .map(|index| read_decoded_operand(&decoded, index)) + .collect() + }, + flow: if decoded.is_invalid() { + Flow::Unreadable + } else { + decoded_flow(&decoded) + }, + }); + } + Ok(out) + } + /// The instruction set the target's disassembly is rendered in. /// /// The **effective** processor type and not the physical one, because that is what @@ -8458,6 +8328,7 @@ mod tests { let one = split_instruction(0x1000, "deadbeef`deadbeef 90 nop", InstructionSet::Amd64); assert_eq!(one.address, 0x1000); assert_eq!(one.text, "nop"); + assert_eq!(one.mnemonic, "nop"); } /// An engine that renders a shape this does not know loses a column, never an instruction: @@ -8472,19 +8343,40 @@ mod tests { let one_column = split_instruction(0x1000, "???", InstructionSet::Amd64); assert!(one_column.bytes.is_empty(), "{one_column:?}"); assert_eq!(one_column.text, "???"); + } - // An unreadable rendering must not claim a flow either — `???` is the engine saying it - // could not read the bytes, and `Fallthrough` there would walk a walk into nothing. - assert_eq!(one_column.flow, Flow::Unknown, "{one_column:?}"); - assert_eq!(two_columns.flow, Flow::Unknown, "{two_columns:?}"); + /// `???` is not an instruction, and a walk must not step through it. + /// + /// The engine prints it where the bytes could not be read — an unmapped page, or a dump with + /// no code pages. There is nothing to decode, so there is no fall-through either: a walk that + /// took one would step through *bytes*, one address at a time, inventing every one of them + /// until it hit its own cap. That is a different fact from an instruction set whose operands + /// are not decoded, where there is a real instruction and continuing is the right best-effort, + /// and the two are separate variants for exactly that reason. + #[test] + fn test_an_unreadable_rendering_is_not_walked_through() { + for set in [ + InstructionSet::Amd64, + InstructionSet::X86, + InstructionSet::Other(0xaa64), + ] { + let nothing = split_instruction(0x1000, "fffff803`89201234 ????", set); + assert_eq!(nothing.flow, Flow::Unreadable, "{set:?}: {nothing:?}"); + assert!( + !nothing.flow.falls_through(), + "{set:?}: a walk would step through unreadable bytes" + ); + assert_eq!(nothing.mnemonic, "????"); + } } - /// An instruction set whose operands are not read refuses rather than guesses. + /// An instruction set whose encoding this does not decode refuses rather than guesses. /// /// The mnemonic survives — the first token is the mnemonic in every syntax the engine renders /// — and nothing else is claimed. The flow matters most: ARM64's `b.eq` is a conditional - /// branch, and reporting the `Fallthrough` that an unread instruction would otherwise default - /// to would hand a caller one edge of two and call the walk complete. + /// branch, and reporting the `Fallthrough` an unread instruction would otherwise default to + /// would hand a caller one edge of two and call the walk complete. Unlike an unreadable + /// rendering, this one still falls through, because there **is** an instruction there. #[test] fn test_an_unread_instruction_set_reports_no_operands_and_no_flow() { let arm64 = split_instruction( @@ -8495,30 +8387,47 @@ mod tests { assert_eq!(arm64.mnemonic, "b.eq"); assert!(arm64.operands.is_empty(), "{arm64:?}"); assert_eq!(arm64.flow, Flow::Unknown); + assert!(arm64.flow.falls_through()); assert!( arm64.text.contains("nt!KiFoo"), "the rendering is still the rendering: {arm64:?}" ); } - /// The operands of the shapes this crate's callers actually walk. + /// A rendering plays no part in what the operands are. /// - /// Every one of these is a rendering taken from a real x64 kernel target rather than - /// composed: the compare against an IOCTL code, the `IO_STACK_LOCATION` field load, the - /// symbolised call, the import thunk, the scaled index of a jump table, and the KPCR read - /// through a segment override. + /// This is the property the decoder bought, and the one worth a test of its own: every + /// operand comes from the encoding, so a symbol's own punctuation cannot take one apart. + /// While operands were read out of the third column, `std::map` severed a call at its + /// comma, `operator()` at its parenthesis and `operator[]` at its bracket — three rounds of + /// review, three characters, one defect. Here the rendering is **deliberately a lie** and the + /// fields are still right. + #[test] + fn test_the_operands_come_from_the_encoding_and_not_from_the_rendering() { + let lied_to = split_instruction( + 0x1000, + // `e8 0b 00 00 00` is `call +0xb`, which from 0x1000 lands at 0x1010. The text says + // something else entirely, and carries every character that used to break the reading. + "00001000 e80b000000 call module!std::map::operator[]() (deadbeef`deadbeef)", + InstructionSet::Amd64, + ); + assert_eq!(lied_to.flow, Flow::Call(Some(0x1010)), "{lied_to:?}"); + assert_eq!(lied_to.operands, vec![Operand::Target(0x1010)]); + assert_eq!(lied_to.mnemonic, "call"); + } + + /// The operands of the shapes this crate's callers actually walk, decoded from real + /// encodings — the compare against an IOCTL code, the `IO_STACK_LOCATION` field load, the + /// direct call, the import thunk, the scaled index of a jump table, and the KPCR read through + /// a segment override. #[test] fn test_x64_operands_are_read_as_values() { - let one = |text: &str| { - split_instruction( - 0x1000, - &format!("00001000 90 {text}"), - InstructionSet::Amd64, - ) + let one = |encoding: &str, at: u64| { + split_instruction(at, &format!("00001000 {encoding} x"), InstructionSet::Amd64) }; - // The compare an IOCTL map is recovered from. - let cmp = one("cmp r13d,6D0030h"); + // `41 81 fd 30 00 6d 00` — cmp r13d,6D0030h. The compare an IOCTL map is recovered from. + let cmp = one("4181fd30006d00", 0x1000); assert_eq!(cmp.mnemonic, "cmp"); assert_eq!( cmp.operands, @@ -8529,8 +8438,9 @@ mod tests { ); assert_eq!(cmp.flow, Flow::Fallthrough); - // `IRP_SP = [Irp+0xb8]`, the load every dispatch routine opens with. - let load = one("mov rax,qword ptr [rdx+0B8h]"); + // `48 8b 82 b8 00 00 00` — mov rax,qword ptr [rdx+0B8h]. `IRP_SP = [Irp+0xb8]`, the load + // every dispatch routine opens with. Taken verbatim from a walk of mountmgr. + let load = one("488b82b8000000", 0x1000); let Some(Operand::Memory(memory)) = load.operands.get(1) else { panic!("the memory operand was not read: {load:?}"); }; @@ -8538,320 +8448,195 @@ mod tests { assert_eq!(memory.base.as_deref(), Some("rdx")); assert_eq!(memory.displacement, 0xb8); assert_eq!(memory.index, None); - - // A symbolised direct call: the destination is the parenthesised address. - let call = one("call nt!KeBugCheckEx (fffff803`3e2547f0)"); - assert_eq!(call.flow, Flow::Call(Some(0xfffff803_3e2547f0))); assert_eq!( - call.operands, - vec![Operand::Target { - symbol: Some("nt!KeBugCheckEx".into()), - address: Some(0xfffff803_3e2547f0), - }] + memory.address, None, + "a based reference has no static address" ); - // An import thunk: indirect, so no destination — and the symbol names the slot. - let thunk = one("call qword ptr [mountmgr!_imp_ExAllocatePool2 (fffff803`3e25a018)]"); + // `e8 fb 0f 00 00` — call +0xffb, which from 0x1000 lands at 0x2000. + let call = one("e8fb0f0000", 0x1000); + assert_eq!(call.flow, Flow::Call(Some(0x2000))); + assert_eq!(call.operands, vec![Operand::Target(0x2000)]); + + // `ff 15 fa 0f 00 00` — call qword ptr [rip+0xffa], an import thunk. Indirect, so no + // destination; the slot's address is what names it, computed against the instruction's end. + let thunk = one("ff15fa0f0000", 0x1000); assert_eq!(thunk.flow, Flow::Call(None)); let Some(Operand::Memory(slot)) = thunk.operands.first() else { panic!("the thunk's operand was not read as memory: {thunk:?}"); }; - assert_eq!( - slot.symbol.as_deref(), - Some("mountmgr!_imp_ExAllocatePool2") - ); - assert_eq!(slot.address, Some(0xfffff803_3e25a018)); + assert_eq!(slot.base.as_deref(), Some("rip")); + assert_eq!(slot.address, Some(0x2000), "{slot:?}"); - // A jump table: base, scaled index and the table's own address. - let table = one("jmp qword ptr [rax*8+fffff803`3e25a000]"); + // `ff 24 c5 00 20 00 00` — jmp qword ptr [rax*8+0x2000], a jump table. + let table = one("ff24c500200000", 0x1000); assert_eq!(table.flow, Flow::Jmp(None)); let Some(Operand::Memory(entry)) = table.operands.first() else { panic!("the jump table's operand was not read as memory: {table:?}"); }; assert_eq!(entry.index.as_deref(), Some("rax")); assert_eq!(entry.scale, 8); - assert_eq!(entry.address, Some(0xfffff803_3e25a000)); + assert_eq!(entry.displacement, 0x2000); - // A segment override, which is how kernel code reaches the KPCR. - let kpcr = one("mov rax,qword ptr gs:[188h]"); + // `65 48 8b 04 25 88 01 00 00` — mov rax,qword ptr gs:[188h]. How kernel code reaches the + // KPCR, and the one place a segment override shows up. + let kpcr = one("65488b042588010000", 0x1000); let Some(Operand::Memory(pcr)) = kpcr.operands.get(1) else { panic!("the segment-overridden operand was not read: {kpcr:?}"); }; assert_eq!(pcr.segment.as_deref(), Some("gs")); assert_eq!(pcr.displacement, 0x188); assert_eq!(pcr.base, None); - } - - /// The flow classification, including the three endings a walk must not fall through. - #[test] - fn test_flow_separates_the_edges_a_walk_may_take() { - let one = |text: &str| { - split_instruction( - 0x1000, - &format!("00001000 90 {text}"), - InstructionSet::Amd64, - ) - .flow - }; + // `b4 05` — mov ah,5. A byte register and a small literal, which the text reading had to + // order carefully because `ah` is also well-formed `h`-suffixed hexadecimal. The encoding + // has no such collision. + let byte_register = one("b405", 0x1000); assert_eq!( - one("je mountmgr!MountMgrQueryPoints+0x1c (fffff803`3e2547f0)"), - Flow::Branch(Some(0xfffff803_3e2547f0)) + byte_register.operands, + vec![Operand::Register("ah".into()), Operand::Immediate(5)] ); - assert_eq!(one("ret"), Flow::Return); - assert_eq!(one("ud2"), Flow::Trap); - assert_eq!(one("call rax"), Flow::Call(None)); - assert_eq!(one("jmp rax"), Flow::Jmp(None)); - assert_eq!(one("xor ebx,ebx"), Flow::Fallthrough); - // A prefix is not the operation, and skipping it must not lose the operation's flow. - let locked = split_instruction( - 0x1000, - "00001000 f0ff05 lock inc dword ptr [rax]", - InstructionSet::Amd64, - ); - assert_eq!(locked.mnemonic, "inc"); - assert_eq!(locked.flow, Flow::Fallthrough); - - // Sound in the one direction that matters: an unresolved destination is `None`, never a - // borrowed address from somewhere else on the line. - assert_eq!(one("jmp qword ptr [rax*8+1234h]").target(), None); - } - - /// A comma inside a decorated symbol must not sever the destination it belongs to. - /// - /// A demangled C++ name carries commas between template arguments, and they sit outside the - /// brackets and parentheses the operand split steps over. Split there, the first fragment has - /// no parenthesised address, so the call reports `Call(None)` and a reachability walk reads a - /// direct edge as an indirect one and drops it. A control transfer takes one operand, so its - /// text is not split at all. - #[test] - fn test_a_comma_inside_a_decorated_symbol_does_not_sever_the_destination() { - let one = |text: &str| { - split_instruction( - 0x1000, - &format!("00001000 90 {text}"), - InstructionSet::Amd64, - ) - }; - - let call = one("call module!std::map::insert (00007ff6`12345678)"); - assert_eq!( - call.flow, - Flow::Call(Some(0x00007ff6_12345678)), - "the direct edge was lost: {call:?}" - ); + // `48 b8 00 00 00 00 00 00 00 80` — mov rax,8000000000000000h. A literal that does not fit + // a signed 64-bit integer is still a literal; a real routine renders this one. + let wide = one("48b80000000000000080", 0x1000); assert_eq!( - call.operands, - vec![Operand::Target { - symbol: Some("module!std::map::insert".into()), - address: Some(0x00007ff6_12345678), - }] + wide.operands.get(1), + Some(&Operand::Immediate(0x8000_0000_0000_0000)) ); - - let branch = one("je module!foo (00007ff6`1234abcd)"); - assert_eq!(branch.flow, Flow::Branch(Some(0x00007ff6_1234abcd))); - - // And an ordinary two-operand instruction is still split, so the immediate a control-code - // compare carries is still its own operand. - let cmp = one("cmp r13d,6D0030h"); - assert_eq!(cmp.operands.len(), 2, "{cmp:?}"); - assert_eq!(cmp.operands[1], Operand::Immediate(0x6d_0030)); } - /// A symbol that contains parentheses keeps its destination. - /// - /// `module!Functor::operator()` is the demangled name of a call operator, and splitting at the - /// *first* parenthesis takes it apart at `operator`, leaves `) (…` to parse as an address, and - /// reports `Call(None)` — a direct edge lost to a name, exactly as the comma case lost one. - /// The split is from the last parenthesis and only when what follows parses as an address, - /// which is also what keeps a symbol whose *last* parenthesis is its own intact. + /// The flow classification, including the endings a walk must not fall through and the ones + /// that only look like endings. #[test] - fn test_a_symbol_containing_parentheses_keeps_its_destination() { - let one = |text: &str| { + fn test_flow_separates_the_edges_a_walk_may_take() { + let one = |encoding: &str| { split_instruction( 0x1000, - &format!("00001000 90 {text}"), + &format!("00001000 {encoding} x"), InstructionSet::Amd64, ) + .flow }; - let call = one("call module!Functor::operator() (00007ff6`12345678)"); - assert_eq!( - call.flow, - Flow::Call(Some(0x00007ff6_12345678)), - "the direct edge was lost: {call:?}" - ); - assert_eq!( - call.operands, - vec![Operand::Target { - symbol: Some("module!Functor::operator()".into()), - address: Some(0x00007ff6_12345678), - }] - ); - - // No address at all: the whole text is the name, parentheses and all. - let bare = one("call module!Functor::operator()"); - assert_eq!( - bare.operands, - vec![Operand::Target { - symbol: Some("module!Functor::operator()".into()), - address: None, - }] - ); - assert_eq!(bare.flow, Flow::Call(None)); + assert_eq!(one("c3"), Flow::Return, "ret"); + assert_eq!(one("0f0b"), Flow::Trap, "ud2"); + assert_eq!(one("ffd0"), Flow::Call(None), "call rax"); + assert_eq!(one("ffe0"), Flow::Jmp(None), "jmp rax"); + assert_eq!(one("31db"), Flow::Fallthrough, "xor ebx,ebx"); + // `74 0e` — je +0xe, which from 0x1000 lands at 0x1010. + assert_eq!(one("740e"), Flow::Branch(Some(0x1010)), "je"); + // `eb 0e` — jmp +0xe. + assert_eq!(one("eb0e"), Flow::Jmp(Some(0x1010)), "jmp short"); - // And the ordinary shape still splits where it always did. - let plain = one("call nt!KeBugCheckEx (fffff803`3e2547f0)"); - assert_eq!( - plain.operands, - vec![Operand::Target { - symbol: Some("nt!KeBugCheckEx".into()), - address: Some(0xfffff803_3e2547f0), - }] - ); + // A prefix is not the operation, and skipping it must not lose the operation's flow. + let locked = split_instruction(0x1000, "00001000 f0ff00 x", InstructionSet::Amd64); + assert_eq!(locked.mnemonic, "inc", "lock inc dword ptr [rax]"); + assert_eq!(locked.flow, Flow::Fallthrough); } - /// `xbegin` starts a transaction and takes its operand on an abort, so it has both edges. + /// `hlt` is not an ending, though it is grouped with one in every mnemonic list. /// - /// The abort path is commonly where the lock-based fallback lives, so a walk that only falls - /// through misses a whole implementation of the routine rather than a branch of it. + /// A halted processor resumes at the next instruction when an interrupt or NMI wakes it, and + /// kernel idle loops are built on exactly that. Classifying it with the undefined-instruction + /// traps truncates every one of them at the halt. #[test] - fn test_xbegin_is_a_conditional_branch() { - let one = split_instruction( - 0x1000, - "00001000 c7f8 xbegin module!Lock+0x40 (00007ff6`12345678)", - InstructionSet::Amd64, - ); - assert_eq!(one.flow, Flow::Branch(Some(0x00007ff6_12345678))); + fn test_hlt_keeps_its_wake_up_edge() { + let halt = split_instruction(0x1000, "00001000 f4 hlt", InstructionSet::Amd64); + assert_eq!(halt.mnemonic, "hlt"); + assert_eq!(halt.flow, Flow::Fallthrough); assert!( - one.flow.falls_through(), - "a transaction that starts goes on" + halt.flow.falls_through(), + "an idle loop continues past its halt" ); } - /// The `ptr` widths, each asserted against the width its name means rather than against the - /// table that produced it. - /// - /// `mmword` is the one worth a test of its own: it sits beside `xmmword` in every listing and - /// is **half** its width, so folding the two together doubles the reported width of every MMX - /// access and nothing about the rendering looks wrong. The two non-powers of two are here for - /// the same reason, being the ones a reader is most likely to round. - #[test] - fn test_an_operand_width_is_the_one_its_name_means() { - let width = |text: &str| { - let one = split_instruction( - 0x1000, - &format!("00001000 90 mov {text} [rax],rbx"), - InstructionSet::Amd64, - ); - match one.operands.first() { - Some(Operand::Memory(memory)) => memory.size, - other => panic!("{text} was not read as a memory operand: {other:?}"), - } - }; - - assert_eq!(width("byte ptr"), Some(1)); - assert_eq!(width("word ptr"), Some(2)); - assert_eq!(width("dword ptr"), Some(4)); - assert_eq!(width("fword ptr"), Some(6), "a far pointer is 48 bits"); - assert_eq!(width("qword ptr"), Some(8)); - assert_eq!(width("mmword ptr"), Some(8), "an MMX operand is 64 bits"); - assert_eq!(width("tbyte ptr"), Some(10), "an 80-bit float"); - assert_eq!(width("xmmword ptr"), Some(16)); - assert_eq!(width("ymmword ptr"), Some(32)); - assert_eq!(width("zmmword ptr"), Some(64)); - - // No `ptr` prefix at all is no width, rather than a guessed one. - assert_eq!(width(""), None); - } - /// A software interrupt is classified by its vector, because most of them return. /// /// `int 2eh` is the 32-bit system-call path: a walk that stops there discards every /// instruction after a syscall, which on that architecture is most of a function. The two that /// do not return are `int 29h` (`__fastfail`) and `int 3` — the breakpoint a compiler emits as - /// padding behind unreachable code, and the form the engine renders `0xcc` as, so it has to be - /// caught by vector rather than by the `int3` spelling alone. + /// padding behind unreachable code, which encodes as the one-byte `0xcc` as well as the + /// two-byte `cd 03`, so both have to be caught. #[test] fn test_a_software_interrupt_is_classified_by_its_vector() { - let one = |text: &str| { + let one = |encoding: &str| { split_instruction( 0x1000, - &format!("00001000 90 {text}"), + &format!("00001000 {encoding} x"), InstructionSet::Amd64, ) .flow }; - assert_eq!(one("int 29h"), Flow::Trap, "__fastfail does not return"); - assert_eq!(one("int 3"), Flow::Trap, "a breakpoint stops the walk"); - assert_eq!(one("int3"), Flow::Trap); + assert_eq!(one("cd29"), Flow::Trap, "int 29h — __fastfail"); + assert_eq!(one("cc"), Flow::Trap, "int 3 — the one-byte breakpoint"); + assert_eq!(one("cd03"), Flow::Trap, "int 3 — the two-byte form"); assert_eq!( - one("int 2Eh"), + one("cd2e"), Flow::Fallthrough, "the 32-bit system call returns, and the walk must go on past it" ); - assert_eq!(one("int 2Dh"), Flow::Fallthrough); - // `into` traps only on overflow, so it returns. - assert_eq!(one("into"), Flow::Fallthrough); + assert_eq!(one("cd2d"), Flow::Fallthrough); - assert!(one("int 2Eh").falls_through()); - assert!(!one("int 29h").falls_through()); + assert!(one("cd2e").falls_through()); + assert!(!one("cd29").falls_through()); } - /// `ah`, `bh`, `ch` and `dh` are registers *and* well-formed `h`-suffixed hexadecimal, and - /// reading them as literals silently turns a destination register into a number. + /// `xbegin` starts a transaction and takes its operand on an abort, so it has both edges. /// - /// Pinned because the ordering that avoids it is invisible: registers are matched before - /// numbers, and swapping those two arms passes every other test in this file. + /// The abort path is commonly where the lock-based fallback lives, so a walk that only falls + /// through misses a whole implementation of the routine rather than a branch of it. `xabort` + /// is the opposite call and is deliberate: the SDM makes it a NOP outside a transaction, so + /// treating it as a transfer would drop everything after it wherever RTM is inactive. #[test] - fn test_a_byte_register_is_not_read_as_a_hexadecimal_literal() { - let one = split_instruction(0x1000, "00001000 b405 mov ah,5", InstructionSet::Amd64); - assert_eq!( - one.operands, - vec![Operand::Register("ah".into()), Operand::Immediate(5)], - "a register was read as a literal: {one:?}" + fn test_the_transactional_instructions_keep_the_edges_they_have() { + // `c7 f8 0a 00 00 00` — xbegin +0xa, which from 0x1000 lands at 0x1010. + let begin = split_instruction(0x1000, "00001000 c7f80a000000 x", InstructionSet::Amd64); + assert_eq!(begin.mnemonic, "xbegin"); + assert_eq!(begin.flow, Flow::Branch(Some(0x1010))); + assert!( + begin.flow.falls_through(), + "a transaction that starts goes on" ); - // And the literal case still reads: the same four letters with more digits in front. - let two = split_instruction(0x1000, "00001000 b40a mov al,0Ah", InstructionSet::Amd64); - assert_eq!( - two.operands, - vec![Operand::Register("al".into()), Operand::Immediate(0xa)] - ); + // `c6 f8 00` — xabort 0. + let abort = split_instruction(0x1000, "00001000 c6f800 x", InstructionSet::Amd64); + assert_eq!(abort.mnemonic, "xabort"); + assert_eq!(abort.flow, Flow::Fallthrough); } - /// A literal that does not fit a signed 64-bit integer is still a literal. - /// - /// Both renderings here are real, from a walk of `mountmgr!MountMgrDeviceControl` on a 26100 - /// image, and both came back as unread `Other` operands while this parsed into an `i64` — - /// three of the three unread operands in that whole routine were this one bug. A caller - /// matching an operand against a mask or a control code wants the bit pattern, so the value - /// is unsigned and a leading `-` is two's complement. + /// An operand's width is the encoding's, and the two vector families are the ones worth + /// pinning: an MMX operand is 64 bits and the `xmm` beside it is 128, so a reading that folds + /// them doubles every MMX access while nothing about it looks wrong. #[test] - fn test_a_literal_too_wide_for_a_signed_integer_is_still_read() { - let one = |text: &str| { - split_instruction( + fn test_an_operand_width_comes_from_the_encoding() { + let width = |encoding: &str, index: u32| { + let one = split_instruction( 0x1000, - &format!("00001000 90 {text}"), + &format!("00001000 {encoding} x"), InstructionSet::Amd64, - ) - .operands + ); + match one.operands.get(index as usize) { + Some(Operand::Memory(memory)) => memory.size, + other => panic!("{encoding} operand {index} was not memory: {other:?} in {one:?}"), + } }; - assert_eq!( - one("mov rax,8000000000000000h").get(1), - Some(&Operand::Immediate(0x8000_0000_0000_0000)) - ); - assert_eq!( - one("mov qword ptr [rbp+0A8h],0FFFFFFFFFFFFFFFFh").get(1), - Some(&Operand::Immediate(u64::MAX)) - ); - assert_eq!( - one("sub rsp,-8").get(1), - Some(&Operand::Immediate(8u64.wrapping_neg())) - ); + // `88 18` — mov byte ptr [rax],bl. + assert_eq!(width("8818", 0), Some(1)); + // `66 89 18` — mov word ptr [rax],bx. + assert_eq!(width("668918", 0), Some(2)); + // `89 18` — mov dword ptr [rax],ebx. + assert_eq!(width("8918", 0), Some(4)); + // `48 89 18` — mov qword ptr [rax],rbx. + assert_eq!(width("488918", 0), Some(8)); + // `0f 6f 00` — movq mm0,mmword ptr [rax]. Sixty-four bits. + assert_eq!(width("0f6f00", 1), Some(8), "an MMX operand is 64 bits"); + // `66 0f 6f 00` — movdqa xmm0,xmmword ptr [rax]. A hundred and twenty-eight. + assert_eq!(width("660f6f00", 1), Some(16)); + // `c5 fd 6f 00` — vmovdqa ymm0,ymmword ptr [rax]. + assert_eq!(width("c5fd6f00", 1), Some(32)); } /// glslang/dbgscope#82: a borrowed engine's lifecycle used to die with the wrapper. From 86649e23a090cccc1f07f858b24fd602549ffc6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 12:13:31 +0100 Subject: [PATCH 07/12] fix: claim a static address only where the instruction determines one Round six, all three taken, and none of them about symbol punctuation or the mnemonic table -- those stopped existing with the decoder. These are edges in the new code. Bytes that were read and did not decode are `Unknown`, not `Unreadable`. The line between the two is whether there are bytes at all: a `???` rendering has none, and a walk must stop rather than step through them one address at a time; an encoding this decoder does not know -- an extension newer than the pinned version, or a span entered mid-instruction -- has an instruction there, and stopping would discard the rest of a routine over a version skew. The same correction applies in `decode_range`, where a linear read runs into data as a matter of course. A segment override rules out a static address. `gs:[188h]` has no base and no index, so it was reported as address `0x188`; it is the KPCR, its linear address is the segment base plus that displacement, and the base is a runtime fact. A consumer taking the old value would read or symbolise a low address that means nothing. A RIP-relative operand keeps the displacement it encodes. The decoder normalises that displacement to its target, so taking it verbatim reported `[rip+0xffa]` at `0x1000` as `0x2000` -- a second copy of `address`, where the addressing expression should be. Derived back against the instruction's own end, which is what the displacement is relative to. Mutation-verified one at a time: each of the three, reverted, fails exactly one test and no other. Re-measured on `mountmgr!MountMgrDeviceControl` unchanged at 376 instructions, eleven control-code compares, and 23 of 23 instructions agreeing between the two decode paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 14 ++++-- src/dbgeng.rs | 120 ++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 122 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0142d2..e040efb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,9 +26,17 @@ All notable changes to this project are documented here. The format follows `Operand` is `Register`, `Immediate`, `Memory`, `Target` or `Other`. `Flow` carries every destination as an `Option`, because a direct transfer encodes a displacement and an indirect one encodes a register, and a caller treating `None` as "no edge" stays sound. `Unknown` and - `Unreadable` are separate: an instruction set this does not decode still has an instruction - there, so it falls through, while a `???` rendering has none — a walk that fell through one would - step through *bytes*, one address at a time, to its own cap. + `Unreadable` are separate, and the line between them is whether there are bytes: a `???` + rendering has none and stops a walk, while an instruction set this does not decode — or an + encoding newer than the pinned decoder — has an instruction there and falls through. A walk that + fell through the first would step through *bytes*, one address at a time, to its own cap; one + that stopped at the second would discard the rest of a routine over a version skew. + A memory operand claims a static `address` only where the instruction alone determines one. A + segment override does not: `gs:[188h]` is the KPCR, its linear address is the segment base plus + the displacement, and that base is a runtime fact. A RIP-relative operand keeps the displacement + it **encodes** rather than the decoder's normalised target, which would otherwise report + `[rip+0xffa]` at `0x1000` as a displacement of `0x2000` — a second copy of `address` where the + addressing expression should be. Reading is gated on `InstructionSet`: x86 and x64 are decoded, and anything else — ARM64 today — reports its mnemonic, no operands and `Flow::Unknown`. Measured against a whole real dispatch routine rather than composed lines diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 260d40e..11aa0ae 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2571,7 +2571,12 @@ fn decode_operation( iced_x86::Decoder::with_ip(bitness, bytes, address, iced_x86::DecoderOptions::NONE); let decoded = decoder.decode(); if decoded.is_invalid() { - return (String::new(), Vec::new(), Flow::Unreadable); + // Bytes that were read and did not decode. That is **not** [`Flow::Unreadable`], which + // means there is no instruction here: the engine rendered one, so there is, and this + // decoder simply does not know it — an extension newer than the version pinned, or a + // span entered mid-instruction. `Unknown` says exactly that, and falls through, where + // `Unreadable` would stop a walk and discard the rest of a routine over a version skew. + return (String::new(), Vec::new(), Flow::Unknown); } let mnemonic = format!("{:?}", decoded.mnemonic()).to_lowercase(); @@ -2609,26 +2614,40 @@ fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { let size = decoded.memory_size().size(); let base = decoded.memory_base(); let index = decoded.memory_index(); - // Statically known only when nothing at run time contributes: an absolute displacement, or a - // RIP-relative reference, whose target the decoder has already computed against the - // instruction's own end. - let address = if base == iced_x86::Register::RIP || base == iced_x86::Register::EIP { + let rip_relative = base == iced_x86::Register::RIP || base == iced_x86::Register::EIP; + // A segment override means the linear address is the segment's base plus this, and that base + // is a runtime fact — `gs:[188h]` is the KPCR and emphatically not address `0x188`. So an + // override rules out a static address entirely, rather than being ignored because both the + // base and index registers happen to be absent. + let overridden = decoded.segment_prefix() != iced_x86::Register::None; + let address = if overridden { + None + } else if rip_relative { Some(decoded.ip_rel_memory_address()) } else if base == iced_x86::Register::None && index == iced_x86::Register::None { Some(decoded.memory_displacement64()) } else { None }; + // For a RIP-relative operand the decoder's displacement is the **normalised target**, not the + // signed number the instruction encodes, so taking it verbatim would report `[rip+0xffa]` at + // `0x1000` as a displacement of `0x2000` — a duplicate of `address` where the addressing + // expression should be. Derived back against the instruction's own end, which is what the + // displacement is relative to. + let displacement = if rip_relative { + (decoded.ip_rel_memory_address() as i64).wrapping_sub(decoded.next_ip() as i64) + } else { + decoded.memory_displacement64() as i64 + }; MemoryOperand { size: (size != 0).then_some(size as u32), // The *prefix*, not the segment the encoding implies: `[rsp+8]` is `ss` by rule and prints // no override, and reporting one would say the instruction carried something it did not. - segment: (decoded.segment_prefix() != iced_x86::Register::None) - .then(|| register_name(decoded.segment_prefix())), + segment: overridden.then(|| register_name(decoded.segment_prefix())), base: (base != iced_x86::Register::None).then(|| register_name(base)), index: (index != iced_x86::Register::None).then(|| register_name(index)), scale: decoded.memory_index_scale() as u8, - displacement: decoded.memory_displacement64() as i64, + displacement, address, } } @@ -5900,8 +5919,11 @@ impl DebugEngine { .map(|index| read_decoded_operand(&decoded, index)) .collect() }, + // Bytes that were read and did not decode: `Unknown`, not `Unreadable`. A linear + // read runs into data and into the middle of instructions as a matter of course, + // and neither is "there is nothing here". flow: if decoded.is_invalid() { - Flow::Unreadable + Flow::Unknown } else { decoded_flow(&decoded) }, @@ -8535,6 +8557,86 @@ mod tests { assert_eq!(locked.flow, Flow::Fallthrough); } + /// Bytes that were read and did not decode are `Unknown`, never `Unreadable`. + /// + /// The two are separate facts and only one of them stops a walk. `Unreadable` means there is + /// no instruction here; if the engine rendered one and this decoder does not know it — an + /// extension newer than the pinned version, or a span entered mid-instruction — there *is* an + /// instruction and the walk should carry on. Getting that backwards discards the rest of a + /// routine over a version skew. + /// + /// `06` is the case with no version skew needed: `push es`, a perfectly good 32-bit + /// instruction and not an encoding at all in 64-bit mode. + #[test] + fn test_bytes_that_do_not_decode_are_unknown_rather_than_unreadable() { + let in_64 = split_instruction(0x1000, "00001000 06 x", InstructionSet::Amd64); + assert_eq!(in_64.flow, Flow::Unknown, "{in_64:?}"); + assert!( + in_64.flow.falls_through(), + "a walk must not stop at an instruction this decoder does not know" + ); + + // And the same byte in the mode where it is an instruction. + let in_32 = split_instruction(0x1000, "00001000 06 x", InstructionSet::X86); + assert_eq!(in_32.mnemonic, "push"); + assert_eq!(in_32.flow, Flow::Fallthrough); + } + + /// A segment override rules out a static address, and a RIP-relative operand keeps the + /// displacement it encodes rather than a copy of its target. + /// + /// `gs:[188h]` is the KPCR and emphatically not address `0x188`: the linear address is the + /// segment base plus that, and the base is a runtime fact. Reporting one would invite a + /// consumer to read or symbolise a low address that means nothing. + /// + /// The RIP case is the mirror image. The decoder normalises a RIP-relative displacement to its + /// target, so taking it verbatim reports `[rip+0xffa]` at `0x1000` as `0x2000` — a duplicate + /// of `address`, where the addressing expression should be. + #[test] + fn test_a_static_address_is_only_claimed_where_one_exists() { + let one = |encoding: &str| { + let decoded = split_instruction( + 0x1000, + &format!("00001000 {encoding} x"), + InstructionSet::Amd64, + ); + decoded + .operands + .iter() + .find_map(|operand| match operand { + Operand::Memory(memory) => Some(memory.clone()), + _ => None, + }) + .unwrap_or_else(|| panic!("no memory operand in {decoded:?}")) + }; + + // `65 48 8b 04 25 88 01 00 00` — mov rax,qword ptr gs:[188h]. + let kpcr = one("65488b042588010000"); + assert_eq!(kpcr.segment.as_deref(), Some("gs")); + assert_eq!(kpcr.displacement, 0x188); + assert_eq!( + kpcr.address, None, + "a segment override has no address the instruction alone knows: {kpcr:?}" + ); + + // `ff 15 fa 0f 00 00` — call qword ptr [rip+0xffa], six bytes, so the next IP is 0x1006 + // and the target is 0x2000. + let thunk = one("ff15fa0f0000"); + assert_eq!(thunk.base.as_deref(), Some("rip")); + assert_eq!(thunk.address, Some(0x2000), "{thunk:?}"); + assert_eq!( + thunk.displacement, 0xffa, + "the encoded displacement, not a second copy of the target: {thunk:?}" + ); + + // An absolute reference with no override still has one. + // `48 8b 04 25 00 20 00 00` — mov rax,qword ptr [2000h]. + let absolute = one("488b042500200000"); + assert_eq!(absolute.address, Some(0x2000)); + assert_eq!(absolute.displacement, 0x2000); + assert_eq!(absolute.segment, None); + } + /// `hlt` is not an ending, though it is grouped with one in every mnemonic list. /// /// A halted processor resumes at the next instruction when an interrupt or NMI wakes it, and From dfc3e64822db6579a658c6eb36dbdb25bc5c8c11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 12:24:05 +0100 Subject: [PATCH 08/12] fix: a negative displacement is negative at any address width Round seven, both taken. The decoder keeps a narrow effective address in its own width, so `[ebp-8]` comes back as `0xfffffff8` and the widening cast reported the commonest local-variable reference there is as 4,294,967,288 -- against a field whose whole promise is a *signed* displacement. Measured before fixing: the test went in asserting -8 and printed 4294967288. Sign-extended from the width the address registers are, that being what the addressing form is computed in, and an absolute reference with no register left unsigned, because a 32-bit `[0xfffff000]` is a high address and not a negative offset. The 64-bit form was already right, and is now pinned beside the 32-bit one so the two cannot drift. And the `FunctionExtent::NoEntry` variant still promised that every x86 address answers it, which the gate has never done and which the *method* doc was corrected about last round. Fixing one site and leaving the other is how a contract ends up saying two things; both now say x86 answers `Unsupported`, and why. Mutation-verified: dropping the sign extension fails the new displacement test and nothing else. Re-measured on `mountmgr!MountMgrDeviceControl` unchanged -- 376 instructions, eleven control-code compares, 23 of 23 agreeing across the two decode paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 6 ++++- src/dbgeng.rs | 65 ++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e040efb..e42f779 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,11 @@ All notable changes to this project are documented here. The format follows the displacement, and that base is a runtime fact. A RIP-relative operand keeps the displacement it **encodes** rather than the decoder's normalised target, which would otherwise report `[rip+0xffa]` at `0x1000` as a displacement of `0x2000` — a second copy of `address` where the - addressing expression should be. + addressing expression should be. A displacement is signed at the width its *address registers* + are: the decoder keeps a 32-bit effective address in 32 bits, so `[ebp-8]` arrives as + `0xfffffff8` and a straight widening cast reported the commonest local-variable reference there + is as 4,294,967,288. An absolute reference with no register stays unsigned, a 32-bit + `[0xfffff000]` being a high address rather than a negative offset. Reading is gated on `InstructionSet`: x86 and x64 are decoded, and anything else — ARM64 today — reports its mnemonic, no operands and `Flow::Unknown`. Measured against a whole real dispatch routine rather than composed lines diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 11aa0ae..ae3bfbc 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -1888,10 +1888,13 @@ pub enum FunctionExtent { /// The region containing the address, rebased into the target's address space. Region { begin: u64, end: u64 }, /// The image has no unwind entry covering the address — a leaf function, or an address that - /// is not code. Every x86 address answers this, 32-bit Windows having no unwind table. + /// is not code. Reported only for x64, that being the one entry layout this decodes. NoEntry, /// This instruction set's entry layout is not decoded here, so nothing is claimed about the - /// address at all. + /// address at all. **x86 answers this**, not [`Self::NoEntry`]: 32-bit Windows has no unwind + /// table, so implementing it would buy nothing, but that is a fact about a platform and + /// `NoEntry` would assert it about an address that was never queried. + /// [`DebugEngine::function_extent`] has the reasoning. Unsupported(InstructionSet), } @@ -2634,10 +2637,22 @@ fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { // `0x1000` as a displacement of `0x2000` — a duplicate of `address` where the addressing // expression should be. Derived back against the instruction's own end, which is what the // displacement is relative to. + // And a narrow effective address keeps its own width in the decoder, so `[ebp-8]` comes back + // as `0xfffffff8` and a straight widening cast reports it as 4,294,967,288. The width to + // sign-extend from is the *address registers'*, that being what the addressing form is + // computed in; an absolute reference with no register is left unsigned, because a 32-bit + // `[0xfffff000]` is a high address rather than a negative offset. + let address_bits = if base != iced_x86::Register::None { + base.size() * 8 + } else if index != iced_x86::Register::None { + index.size() * 8 + } else { + 64 + }; let displacement = if rip_relative { (decoded.ip_rel_memory_address() as i64).wrapping_sub(decoded.next_ip() as i64) } else { - decoded.memory_displacement64() as i64 + sign_extend(decoded.memory_displacement64(), address_bits) }; MemoryOperand { size: (size != 0).then_some(size as u32), @@ -2652,6 +2667,15 @@ fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { } } +/// A value carried in `bits` of a wider word, read as the signed number it is. +fn sign_extend(value: u64, bits: usize) -> i64 { + if bits == 0 || bits >= 64 { + return value as i64; + } + let shift = 64 - bits; + ((value << shift) as i64) >> shift +} + /// A register by the lowercase name the engine also prints. fn register_name(register: iced_x86::Register) -> String { format!("{register:?}").to_lowercase() @@ -8637,6 +8661,41 @@ mod tests { assert_eq!(absolute.segment, None); } + /// A negative displacement is negative, whatever the address width. + /// + /// The decoder keeps a 32-bit effective address in its 32-bit representation, so `[ebp-8]` + /// comes back as `0xfffffff8` and a straight widening cast reports it as four billion. The + /// field promises a *signed* displacement, and `[ebp-8]` is the commonest local-variable + /// reference there is. + #[test] + fn test_a_negative_displacement_survives_a_narrow_address_width() { + let memory = |encoding: &str, set: InstructionSet| { + let decoded = split_instruction(0x1000, &format!("00001000 {encoding} x"), set); + decoded + .operands + .iter() + .find_map(|operand| match operand { + Operand::Memory(memory) => Some(memory.clone()), + _ => None, + }) + .unwrap_or_else(|| panic!("no memory operand in {decoded:?}")) + }; + + // `8b 45 f8` — mov eax,dword ptr [ebp-8], in 32-bit code. + let local32 = memory("8b45f8", InstructionSet::X86); + assert_eq!(local32.base.as_deref(), Some("ebp")); + assert_eq!(local32.displacement, -8, "{local32:?}"); + + // `48 8b 45 f8` — mov rax,qword ptr [rbp-8], the 64-bit form, which was already right. + let local64 = memory("488b45f8", InstructionSet::Amd64); + assert_eq!(local64.base.as_deref(), Some("rbp")); + assert_eq!(local64.displacement, -8, "{local64:?}"); + + // And a positive one is unchanged in both. + assert_eq!(memory("8b4508", InstructionSet::X86).displacement, 8); + assert_eq!(memory("488b4508", InstructionSet::Amd64).displacement, 8); + } + /// `hlt` is not an ending, though it is grouped with one in every mnemonic list. /// /// A halted processor resumes at the next instruction when an interrupt or NMI wakes it, and From b1fae017db412b254dbc215386c5cb502f6fa483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 12:37:26 +0100 Subject: [PATCH 09/12] fix: a 32-bit branch target lands in its instruction's address space Round eight, one finding, taken. Measured before fixing: at `ffffffff`80001000` with a 32-bit effective machine, `call +0xffb` reported its destination as `0x80002000` rather than `ffffffff`80002000`. Decoding 32-bit code computes a 32-bit target, so an instruction whose own address carries a high half named a destination in a different address space from itself -- which a module-bounds walk rejects as out of range, and which anything reading or symbolising follows to the wrong place. `.effmach x86` over a 64-bit kernel target is a reachable way to be there, and this branch already knows that mode exists. The high half is inherited from the instruction rather than sign-extended. Both would fix the reported case; inheriting takes the address form from the caller's own value instead of assuming which convention the engine uses, and this code has no business deciding that on the engine's behalf. 64-bit decoding is deliberately untouched, and that is not just "it needs no help": a `rel32` reaches two gigabytes either way, so it can legitimately cross a 4 GB boundary, and masking there would drag a correct target back by four gigabytes. Both directions are asserted, and each of the two mutations -- dropping the canonicalization, and applying it to 64-bit as well -- fails the test. The one case this does not get right is the mirror image, a 32-bit branch wrapping across its own sign boundary, which keeps the high half it started in. Said in the doc rather than left to be discovered. Re-measured on `mountmgr!MountMgrDeviceControl` unchanged: 376 instructions, eleven control-code compares, 23 of 23 agreeing across the two decode paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 8 ++++++ src/dbgeng.rs | 76 +++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 82 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e42f779..9191803 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,14 @@ All notable changes to this project are documented here. The format follows `0xfffffff8` and a straight widening cast reported the commonest local-variable reference there is as 4,294,967,288. An absolute reference with no register stays unsigned, a 32-bit `[0xfffff000]` being a high address rather than a negative offset. + A near branch's destination lands in the address space its instruction came from. Decoding 32-bit + code computes a 32-bit target, so an instruction whose own address carries a high half — a narrow + effective machine over a wide address, which `.effmach x86` produces — would otherwise name a + destination in a different address space from itself, which a module-bounds check rejects and a + reader follows to the wrong place. The high half is inherited from the instruction rather than + sign-extended, taking the address form from the caller's own value instead of assuming the + engine's. 64-bit decoding is left alone deliberately: a `rel32` reaches ±2 GB and so may cross a + 4 GB boundary, where inheriting would drag a correct target back four gigabytes. Reading is gated on `InstructionSet`: x86 and x64 are decoded, and anything else — ARM64 today — reports its mnemonic, no operands and `Flow::Unknown`. Measured against a whole real dispatch routine rather than composed lines diff --git a/src/dbgeng.rs b/src/dbgeng.rs index ae3bfbc..b2c2de4 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2595,7 +2595,7 @@ fn read_decoded_operand(decoded: &iced_x86::Instruction, index: u32) -> Operand match decoded.op_kind(index) { OpKind::Register => Operand::Register(register_name(decoded.op_register(index))), OpKind::NearBranch16 | OpKind::NearBranch32 | OpKind::NearBranch64 => { - Operand::Target(decoded.near_branch_target()) + Operand::Target(canonical_target(decoded)) } OpKind::Immediate8 | OpKind::Immediate8_2nd @@ -2733,7 +2733,29 @@ fn near_target(decoded: &iced_x86::Instruction) -> Option { OpKind::NearBranch16 | OpKind::NearBranch32 | OpKind::NearBranch64 ) }) - .then(|| decoded.near_branch_target()) + .then(|| canonical_target(decoded)) +} + +/// A near branch's destination, in the address space its instruction came from. +/// +/// Decoding 16- or 32-bit code computes a target of that width, so an instruction whose own +/// address carries a high half — a narrow effective machine over a wide address, which is exactly +/// what `.effmach x86` produces, and any target whose addresses arrive sign-extended — would get a +/// destination in a different address space from the instruction naming it. The high half is +/// inherited from the instruction rather than sign-extended, because that takes the address form +/// from the caller's own value instead of assuming which one the engine uses. +/// +/// **64-bit decoding is left alone**, and not merely because it needs no help: a `rel32` branch +/// reaches ±2 GB, so it can legitimately cross a 4 GB boundary, and inheriting a high half there +/// would drag a correct target backwards by four gigabytes. The one case this does not get right +/// is the mirror of that — a 32-bit branch wrapping across its own sign boundary keeps the high +/// half it started in. +fn canonical_target(decoded: &iced_x86::Instruction) -> u64 { + let target = decoded.near_branch_target(); + if decoded.code_size() == iced_x86::CodeSize::Code64 { + return target; + } + (decoded.ip() & 0xffff_ffff_0000_0000) | (target & 0xffff_ffff) } /// Runs of whitespace as one space. The engine pads its columns to align them in a listing, and @@ -8696,6 +8718,56 @@ mod tests { assert_eq!(memory("488b4508", InstructionSet::Amd64).displacement, 8); } + /// A 32-bit branch target lands in the address space its instruction was given. + /// + /// Decoding 32-bit code computes a 32-bit target, so an instruction whose own address carries + /// high bits — a 32-bit effective machine over a 64-bit address, which is what `.effmach x86` + /// produces, and any target whose addresses the engine hands out sign-extended — gets a + /// destination in a different address space from the instruction that names it. A module-bounds + /// check then rejects the edge, and anything reading or symbolising it reads the wrong place. + #[test] + fn test_a_32_bit_target_stays_in_its_instructions_address_space() { + // `e8 fb 0f 00 00` — call +0xffb. Five bytes, so from `…1000` the next IP is `…1005` and + // the destination is `…2000`, whatever the high half is. + let high = split_instruction( + 0xffffffff_80001000, + "ffffffff`80001000 e8fb0f0000 x", + InstructionSet::X86, + ); + assert_eq!( + high.flow, + Flow::Call(Some(0xffffffff_80002000)), + "the high half of the address was dropped: {high:?}" + ); + assert_eq!(high.operands, vec![Operand::Target(0xffffffff_80002000)]); + + // And an ordinary low 32-bit address is untouched. + let low = split_instruction(0x00401000, "00401000 e8fb0f0000 x", InstructionSet::X86); + assert_eq!(low.flow, Flow::Call(Some(0x00402000)), "{low:?}"); + + // 64-bit decoding was never affected: its own arithmetic is already 64-bit. + let wide = split_instruction( + 0xfffff805_5ec04750, + "fffff805`5ec04750 e8fb0f0000 x", + InstructionSet::Amd64, + ); + assert_eq!(wide.flow, Flow::Call(Some(0xfffff805_5ec05750)), "{wide:?}"); + + // And 64-bit must **not** inherit a high half, because a `rel32` reaches ±2 GB and so can + // legitimately cross a 4 GB boundary. From `…f805`fffffff0` the destination is in + // `…f806`, and masking it back into `…f805` would move it four gigabytes. + let crossing = split_instruction( + 0xfffff805_fffffff0, + "fffff805`fffffff0 e8fb0f0000 x", + InstructionSet::Amd64, + ); + assert_eq!( + crossing.flow, + Flow::Call(Some(0xfffff806_00000ff0)), + "a 64-bit branch across a 4 GB boundary was dragged back: {crossing:?}" + ); + } + /// `hlt` is not an ending, though it is grouped with one in every mnemonic list. /// /// A halted processor resumes at the next instruction when an interrupt or NMI wakes it, and From c2e35e2710c40658670d3062dfe93bbc2bbb2912 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 12:48:31 +0100 Subject: [PATCH 10/12] fix: canonicalise absolute addresses, and take a displacement's width from the field Round nine, both taken, and both are the previous two rounds' fixes not carried to their siblings. An absolute memory address was left in the decoder's 32-bit space while a branch target was canonicalised into the instruction's. `[0x80002000]` under an instruction at `ffffffff`80001000` is an address in a different space from the code reading it, and absolute globals and import slots are ordinary in x86 kernel code, so this is the field a consumer follows. Same rule, same helper, same reason 64-bit is exempt. And a displacement's signed width now comes from the displacement field rather than from the address registers. The register width was the first answer and is wrong for a VSIB gather, whose index is an `xmm`/`ymm`/`zmm`: `index.size() * 8` is 128 or more, `sign_extend` becomes a no-op, and a negative displacement comes back as four billion -- exactly the defect the sign extension was added to fix, surviving in the one addressing form whose index is not a general-purpose register. The encoded field is the right width by construction, a `disp8` of `0xf8` being `-8` whatever computes the address, and it sidesteps the address-size override too. Mutation-verified one at a time: each reverted fix fails exactly one test. The VSIB case is a real encoding rather than a shape argued about -- `c4 e2 79 92 0c 95 f8 ff ff ff`, and the test asserts the index really is a vector register before asserting the displacement, so it cannot pass by decoding something else. Re-measured on `mountmgr!MountMgrDeviceControl` unchanged: 376 instructions, eleven control-code compares, 23 of 23 agreeing across the two decode paths, and its import thunks still naming themselves. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 9 +++++- src/dbgeng.rs | 90 +++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9191803..44f214d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,7 +48,14 @@ All notable changes to this project are documented here. The format follows reader follows to the wrong place. The high half is inherited from the instruction rather than sign-extended, taking the address form from the caller's own value instead of assuming the engine's. 64-bit decoding is left alone deliberately: a `rel32` reaches ±2 GB and so may cross a - 4 GB boundary, where inheriting would drag a correct target back four gigabytes. + 4 GB boundary, where inheriting would drag a correct target back four gigabytes. An **absolute** + memory address is canonicalised the same way and for the same reason, absolute globals and + import slots being ordinary in x86 kernel code. + A displacement's signed width comes from the **displacement field itself**, not from the address + registers. Those were the first answer and are wrong for a VSIB gather, whose index is an + `xmm`/`ymm`/`zmm`: a width taken from the index is 128 bits or more, the sign extension becomes + a no-op, and a negative displacement comes back as four billion. The encoded field is the right + width by construction, and it sidesteps the address-size override as well. Reading is gated on `InstructionSet`: x86 and x64 are decoded, and anything else — ARM64 today — reports its mnemonic, no operands and `Flow::Unknown`. Measured against a whole real dispatch routine rather than composed lines diff --git a/src/dbgeng.rs b/src/dbgeng.rs index b2c2de4..69bff1b 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2623,12 +2623,16 @@ fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { // override rules out a static address entirely, rather than being ignored because both the // base and index registers happen to be absent. let overridden = decoded.segment_prefix() != iced_x86::Register::None; + // Canonicalised for the same reason a branch target is: 32-bit decoding computes a 32-bit + // value, and an absolute `[0x80002000]` under an instruction living at `ffffffff`80001000` is + // an address in a different space from the instruction that reads it. Absolute globals and + // import slots are ordinary in x86 kernel code, so this is the field a consumer would follow. let address = if overridden { None } else if rip_relative { - Some(decoded.ip_rel_memory_address()) + Some(canonical_address(decoded, decoded.ip_rel_memory_address())) } else if base == iced_x86::Register::None && index == iced_x86::Register::None { - Some(decoded.memory_displacement64()) + Some(canonical_address(decoded, decoded.memory_displacement64())) } else { None }; @@ -2638,21 +2642,26 @@ fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { // expression should be. Derived back against the instruction's own end, which is what the // displacement is relative to. // And a narrow effective address keeps its own width in the decoder, so `[ebp-8]` comes back - // as `0xfffffff8` and a straight widening cast reports it as 4,294,967,288. The width to - // sign-extend from is the *address registers'*, that being what the addressing form is - // computed in; an absolute reference with no register is left unsigned, because a 32-bit - // `[0xfffff000]` is a high address rather than a negative offset. - let address_bits = if base != iced_x86::Register::None { - base.size() * 8 - } else if index != iced_x86::Register::None { - index.size() * 8 - } else { - 64 - }; + // as `0xfffffff8` and a straight widening cast reports it as 4,294,967,288. + // + // The width to sign-extend from is the **displacement field's own**, not the address + // registers'. Those were the first answer and are wrong for a VSIB gather, whose index is an + // `xmm`/`ymm`/`zmm`: `index.size() * 8` is then 128 or more, the extension becomes a no-op, + // and a negative displacement comes back positive. The encoded field is the right width by + // construction — a `disp8` of `0xf8` is `-8` whatever computes the address — and it sidesteps + // the address-size override too. + // + // An absolute reference with no register stays unsigned, because a 32-bit `[0xfffff000]` is a + // high address rather than a negative offset. let displacement = if rip_relative { (decoded.ip_rel_memory_address() as i64).wrapping_sub(decoded.next_ip() as i64) + } else if base == iced_x86::Register::None && index == iced_x86::Register::None { + decoded.memory_displacement64() as i64 } else { - sign_extend(decoded.memory_displacement64(), address_bits) + sign_extend( + decoded.memory_displacement64(), + decoded.memory_displ_size() as usize * 8, + ) }; MemoryOperand { size: (size != 0).then_some(size as u32), @@ -2751,11 +2760,17 @@ fn near_target(decoded: &iced_x86::Instruction) -> Option { /// is the mirror of that — a 32-bit branch wrapping across its own sign boundary keeps the high /// half it started in. fn canonical_target(decoded: &iced_x86::Instruction) -> u64 { - let target = decoded.near_branch_target(); + canonical_address(decoded, decoded.near_branch_target()) +} + +/// A decoded address, put back into the address space its instruction came from. +/// +/// See [`canonical_target`] for why, and for why 64-bit is left alone. +fn canonical_address(decoded: &iced_x86::Instruction, value: u64) -> u64 { if decoded.code_size() == iced_x86::CodeSize::Code64 { - return target; + return value; } - (decoded.ip() & 0xffff_ffff_0000_0000) | (target & 0xffff_ffff) + (decoded.ip() & 0xffff_ffff_0000_0000) | (value & 0xffff_ffff) } /// Runs of whitespace as one space. The engine pads its columns to align them in a listing, and @@ -8681,6 +8696,24 @@ mod tests { assert_eq!(absolute.address, Some(0x2000)); assert_eq!(absolute.displacement, 0x2000); assert_eq!(absolute.segment, None); + + // And an absolute address is canonicalised into its instruction's space exactly as a + // branch target is. Absolute globals and import slots are ordinary in x86 kernel code, + // so this is the field a consumer follows. + // `a1 00 20 00 80` — mov eax,dword ptr [80002000h], in 32-bit code at a high address. + let high = split_instruction( + 0xffffffff_80001000, + "ffffffff`80001000 a100200080 x", + InstructionSet::X86, + ); + let Some(Operand::Memory(global)) = high.operands.get(1) else { + panic!("no absolute operand in {high:?}"); + }; + assert_eq!( + global.address, + Some(0xffffffff_80002000), + "the high half was dropped: {global:?}" + ); } /// A negative displacement is negative, whatever the address width. @@ -8716,6 +8749,29 @@ mod tests { // And a positive one is unchanged in both. assert_eq!(memory("8b4508", InstructionSet::X86).displacement, 8); assert_eq!(memory("488b4508", InstructionSet::Amd64).displacement, 8); + + // A scaled index does not change the reading either. + // `8b 44 88 f8` — mov eax,dword ptr [eax+ecx*4-8]. + let scaled = memory("8b4488f8", InstructionSet::X86); + assert_eq!(scaled.index.as_deref(), Some("ecx")); + assert_eq!(scaled.scale, 4); + assert_eq!(scaled.displacement, -8, "{scaled:?}"); + + // The width comes from the **displacement field**, not from the address registers, and a + // VSIB gather is where that matters: its index is a vector register, so a width taken + // from the index would be 128 bits or more, the sign extension would be a no-op, and a + // negative displacement would come back as four billion. + // `c4 e2 79 92 0c 95 f8 ff ff ff` — vgatherdps xmm1,dword ptr [xmm2*4-8],xmm0. + let gather = memory("c4e279920c95f8ffffff", InstructionSet::X86); + assert!( + gather + .index + .as_deref() + .is_some_and(|r| r.starts_with("xmm")), + "not the VSIB form this is about: {gather:?}" + ); + assert_eq!(gather.base, None); + assert_eq!(gather.displacement, -8, "{gather:?}"); } /// A 32-bit branch target lands in the address space its instruction was given. From f2a2feb3d2341504dcdfa3e8c77b1fc9267d7615 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 13:03:33 +0100 Subject: [PATCH 11/12] fix: a displacement's width is the address width, and neither shortcut to it Round ten, both taken, and both measured before fixing: the EVEX case printed -128 for a real +128, and the address-size-overridden relative operand printed 4294971386 for an encoded 0xffa -- four gigabytes out, exactly as claimed. The width to sign-extend a displacement from is the **effective address width**. Two simpler readings of that have now each been tried and each is wrong in one addressing form: * the index register's width breaks a VSIB gather, whose index is a vector register, so the extension is a no-op and a negative displacement comes back as four billion; * the encoded field's width breaks EVEX, whose `disp8` is compressed -- the decoder returns it already multiplied by the tuple scale while the field is still one byte, so extending the expanded value from eight bits turns a real `+128` into `-128`. So: the address registers give the width where there are any, an address-size override being precisely what makes them narrow; and where there is no general-purpose register -- a pure VSIB form -- the encoding makes the displacement field the address width, and compression cannot arise there because a compressed `disp8` needs a base. All three readings are pinned by mutation, including the two wrong ones, so the next round finds a failing test rather than a plausible-looking alternative. And an `EIP`-relative operand computes its displacement at 32 bits. An address-size override on a 64-bit instruction leaves the decoder wrapping the target to 32 while the instruction's own next address stays 64, and subtracting across that puts the answer 2^32 out. Re-measured on `mountmgr!MountMgrDeviceControl` unchanged: 376 instructions, eleven control-code compares, 23 of 23 agreeing across the two decode paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 17 +++++++--- src/dbgeng.rs | 89 ++++++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 90 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 44f214d..68120bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,11 +51,18 @@ All notable changes to this project are documented here. The format follows 4 GB boundary, where inheriting would drag a correct target back four gigabytes. An **absolute** memory address is canonicalised the same way and for the same reason, absolute globals and import slots being ordinary in x86 kernel code. - A displacement's signed width comes from the **displacement field itself**, not from the address - registers. Those were the first answer and are wrong for a VSIB gather, whose index is an - `xmm`/`ymm`/`zmm`: a width taken from the index is 128 bits or more, the sign extension becomes - a no-op, and a negative displacement comes back as four billion. The encoded field is the right - width by construction, and it sidesteps the address-size override as well. + A displacement's signed width is the **effective address width**, and neither of the two simpler + readings of that survives: the *index register's* width breaks a VSIB gather, whose index is an + `xmm`/`ymm`/`zmm`, so the extension becomes a no-op and a negative displacement comes back as + four billion; the *encoded field's* width breaks EVEX, whose `disp8` is compressed, so the + decoder returns it already scaled by the tuple while the field is still one byte and extending + from eight bits turns a real `+128` into `-128`. The address registers give the width where + there are any, an address-size override being exactly what makes them narrow, and the encoded + field gives it for a pure VSIB form, where compression cannot arise because it needs a base. All + three readings are pinned, so the two wrong ones fail a test rather than being re-proposed. + An `EIP`-relative operand — a 64-bit instruction under an address-size override — has its + displacement computed at 32 bits, the decoder wrapping the target to that width while the + instruction's own next address stays 64, which otherwise puts the delta four gigabytes out. Reading is gated on `InstructionSet`: x86 and x64 are decoded, and anything else — ARM64 today — reports its mnemonic, no operands and `Flow::Unknown`. Measured against a whole real dispatch routine rather than composed lines diff --git a/src/dbgeng.rs b/src/dbgeng.rs index 69bff1b..ab04d90 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2644,24 +2644,49 @@ fn read_decoded_memory(decoded: &iced_x86::Instruction) -> MemoryOperand { // And a narrow effective address keeps its own width in the decoder, so `[ebp-8]` comes back // as `0xfffffff8` and a straight widening cast reports it as 4,294,967,288. // - // The width to sign-extend from is the **displacement field's own**, not the address - // registers'. Those were the first answer and are wrong for a VSIB gather, whose index is an - // `xmm`/`ymm`/`zmm`: `index.size() * 8` is then 128 or more, the extension becomes a no-op, - // and a negative displacement comes back positive. The encoded field is the right width by - // construction — a `disp8` of `0xf8` is `-8` whatever computes the address — and it sidesteps - // the address-size override too. + // # The width to sign-extend from, which is neither of the two obvious answers + // + // It is the **effective address width**, and both simpler readings of that are wrong in one + // addressing form each: + // + // * The *index register's* width breaks a VSIB gather, whose index is an `xmm`/`ymm`/`zmm`: + // 128 bits or more makes the extension a no-op and a negative displacement comes back as + // four billion. + // * The *encoded field's* width breaks EVEX, whose `disp8` is **compressed** — the decoder + // returns it already multiplied by the tuple's scale while the field is still one byte, so + // extending the expanded value from eight bits turns a real `+128` into `-128`. + // + // The address registers give the width where there are any, an address-size override being + // exactly what makes them narrow. Where there is no general-purpose register — a pure VSIB + // form — the encoding makes the displacement field the address width, and EVEX compression + // cannot arise there, since a compressed `disp8` needs a base. // // An absolute reference with no register stays unsigned, because a 32-bit `[0xfffff000]` is a // high address rather than a negative offset. + let address_bits = if base != iced_x86::Register::None { + base.size() * 8 + } else if index != iced_x86::Register::None && index.size() <= 8 { + index.size() * 8 + } else { + decoded.memory_displ_size() as usize * 8 + }; let displacement = if rip_relative { - (decoded.ip_rel_memory_address() as i64).wrapping_sub(decoded.next_ip() as i64) + // An address-size override leaves this `EIP`-relative, and then the decoder's target is + // wrapped to 32 bits while the instruction's own next address is still 64 — so the + // subtraction has to happen at the narrower width or it reports the delta four gigabytes + // out. + let delta = decoded + .ip_rel_memory_address() + .wrapping_sub(decoded.next_ip()); + if base == iced_x86::Register::EIP { + sign_extend(delta & 0xffff_ffff, 32) + } else { + delta as i64 + } } else if base == iced_x86::Register::None && index == iced_x86::Register::None { decoded.memory_displacement64() as i64 } else { - sign_extend( - decoded.memory_displacement64(), - decoded.memory_displ_size() as usize * 8, - ) + sign_extend(decoded.memory_displacement64(), address_bits) }; MemoryOperand { size: (size != 0).then_some(size as u32), @@ -8772,6 +8797,48 @@ mod tests { ); assert_eq!(gather.base, None); assert_eq!(gather.displacement, -8, "{gather:?}"); + + // And the mirror of that: an EVEX operand's `disp8` is **compressed**, so the decoder + // hands back the displacement already multiplied by the tuple's scale while the encoded + // field is still one byte. Sign-extending the expanded value from eight bits turns a + // legitimate `+128` into `-128`, which is why the width cannot come from the field either. + // `62 f1 7c 48 28 41 02` — vmovaps zmm0,zmmword ptr [rcx+80h]: an encoded `disp8` of 2, + // scaled by the 64-byte tuple. + let evex = memory("62f17c48284102", InstructionSet::Amd64); + assert_eq!(evex.base.as_deref(), Some("rcx"), "{evex:?}"); + assert_eq!(evex.displacement, 128, "{evex:?}"); + } + + /// An address-size override makes a 64-bit instruction's relative operand `EIP`-relative, and + /// the two halves of the subtraction stop being the same width. + /// + /// The decoder wraps the target to 32 bits while the instruction's own next address stays 64, + /// so subtracting them directly reports an enormous negative displacement rather than the + /// small number the instruction encodes. + #[test] + fn test_an_eip_relative_displacement_is_computed_at_its_own_width() { + let operand = |at: u64, encoding: &str| { + let decoded = + split_instruction(at, &format!("00001000 {encoding} x"), InstructionSet::Amd64); + decoded + .operands + .iter() + .find_map(|operand| match operand { + Operand::Memory(memory) => Some(memory.clone()), + _ => None, + }) + .unwrap_or_else(|| panic!("no memory operand in {decoded:?}")) + }; + + // `67 48 8b 05 fa 0f 00 00` — mov rax,qword ptr [eip+0xffa], eight bytes. + let overridden = operand(0xffffffff_80001000, "67488b05fa0f0000"); + assert_eq!(overridden.base.as_deref(), Some("eip"), "{overridden:?}"); + assert_eq!(overridden.displacement, 0xffa, "{overridden:?}"); + + // The ordinary 64-bit form is unchanged: `48 8b 05 fa 0f 00 00`, seven bytes. + let plain = operand(0xfffff805_5ec04750, "488b05fa0f0000"); + assert_eq!(plain.base.as_deref(), Some("rip")); + assert_eq!(plain.displacement, 0xffa, "{plain:?}"); } /// A 32-bit branch target lands in the address space its instruction was given. From 0ec420cea8a880be2fa45c5d0e6c38f1b0dd027d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Carvalho?= Date: Thu, 10 Sep 2026 13:15:15 +0100 Subject: [PATCH 12/12] test: pin that a 32-bit address is not sign-extended, and say why Round eleven, one finding, **declined** -- and measured rather than argued, because it asks to reverse round eight's choice and the whole thing turns on one fact nobody had checked: does the engine hand out high x86 addresses in sign-extended form? It does not. Measured on this build against a 32-bit user target (`cppthrow-fastfail-x86.dmp`): `? 80002000` evaluates to `80002000`, and `.formats` prints `Hex: 80002000` -- eight digits, unextended. Sign-extending would therefore invent `ffffffff80002000` for an address the engine itself calls `80002000`, and on a 32-bit kernel, where most code lives above bit 31, it would do that for nearly every address. Inheriting the instruction's own half reproduces the measurement exactly, a genuine 32-bit target's instructions having a zero high half. The finding is right that the tests covered only same-half cases, so the straddling one it names is now asserted: a low instruction reaching a high absolute keeps the low half. That is the residual cost of this choice and it is the narrower of the two exposures -- the sign-extending rule would be wrong for every high address rather than for the ones that straddle. Mutation-verified in the direction that matters here: swapping the inherit for a sign-extension fails the new test. So the alternative is no longer a plausible-looking suggestion, it is a red build with the measurement in the test's name. What would reopen it is a measurement from a 32-bit **kernel** target, which no fixture in this repo has. Said in the doc rather than left implicit. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Ayv1beKpAVkmDJDLfYqoyf --- CHANGELOG.md | 6 ++++++ src/dbgeng.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68120bb..6e0cc49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -51,6 +51,12 @@ All notable changes to this project are documented here. The format follows 4 GB boundary, where inheriting would drag a correct target back four gigabytes. An **absolute** memory address is canonicalised the same way and for the same reason, absolute globals and import slots being ordinary in x86 kernel code. + Inherited rather than sign-extended, and that is measured rather than preferred: against a + 32-bit target, `? 80002000` evaluates to `80002000` and `.formats` prints `Hex: 80002000` — + eight digits, unextended — so sign-extending would invent `ffffffff80002000` for an address the + engine calls `80002000`, and on a 32-bit kernel would do so for nearly every address. The cost + is one straddling case, a low instruction reaching a high absolute, which keeps the low half and + is pinned by a test naming the measurement. A displacement's signed width is the **effective address width**, and neither of the two simpler readings of that survives: the *index register's* width breaks a VSIB gather, whose index is an `xmm`/`ymm`/`zmm`, so the extension becomes a no-op and a negative displacement comes back as diff --git a/src/dbgeng.rs b/src/dbgeng.rs index ab04d90..93d019b 100644 --- a/src/dbgeng.rs +++ b/src/dbgeng.rs @@ -2781,9 +2781,24 @@ fn near_target(decoded: &iced_x86::Instruction) -> Option { /// /// **64-bit decoding is left alone**, and not merely because it needs no help: a `rel32` branch /// reaches ±2 GB, so it can legitimately cross a 4 GB boundary, and inheriting a high half there -/// would drag a correct target backwards by four gigabytes. The one case this does not get right -/// is the mirror of that — a 32-bit branch wrapping across its own sign boundary keeps the high -/// half it started in. +/// would drag a correct target backwards by four gigabytes. +/// +/// # Inherited rather than sign-extended, and that is measured +/// +/// The alternative is to sign-extend a 32-bit value, on the theory that the engine hands out high +/// x86 addresses in sign-extended form; it has been proposed twice. **It does not.** Measured on +/// this build against a 32-bit user target (`cppthrow-fastfail-x86.dmp`), `? 80002000` evaluates +/// to `80002000` and `.formats` prints `Hex: 80002000` — eight digits, unextended. Sign-extending +/// would therefore invent `ffffffff80002000` for an address the engine calls `80002000`, and on a +/// 32-bit kernel, where most code lives above bit 31, it would do so for nearly every address. +/// Inheriting the instruction's own half reproduces exactly what was measured, because a genuine +/// 32-bit target's instructions have a zero high half. +/// +/// What that leaves un-handled is one straddling case: a 32-bit instruction on one side of bit 31 +/// reaching a target on the other keeps the half it started in. It is pinned by test rather than +/// left to be rediscovered, and it is the *narrower* of the two exposures — the sign-extending +/// rule would be wrong for every high address rather than for the ones that straddle. Reopen this +/// only with a measurement from a 32-bit **kernel** target, which no fixture here has. fn canonical_target(decoded: &iced_x86::Instruction) -> u64 { canonical_address(decoded, decoded.near_branch_target()) } @@ -8891,6 +8906,39 @@ mod tests { ); } + /// A 32-bit address keeps the half its instruction is in, and is **not** sign-extended. + /// + /// Sign-extending has been proposed twice, on the theory that the engine hands out high x86 + /// addresses in sign-extended form. Measured on this build against a 32-bit user target + /// (`cppthrow-fastfail-x86.dmp`): `? 80002000` evaluates to `80002000` and `.formats` prints + /// `Hex: 80002000` — eight digits, unextended. So sign-extending would invent + /// `ffffffff80002000` for an address the engine calls `80002000`, and on a 32-bit kernel, + /// where most code is above bit 31, it would do so for nearly every address. + /// + /// The cost of inheriting instead is one straddling case, asserted here so the choice is + /// pinned rather than merely written down: a low instruction reaching a high absolute keeps + /// the low half. That is the narrower exposure of the two, and the doc on `canonical_target` + /// says what would reopen it. + #[test] + fn test_a_32_bit_address_is_not_sign_extended() { + // `a1 00 20 00 80` — mov eax,dword ptr [80002000h], from a *low* instruction address. + let straddling = + split_instruction(0x00401000, "00401000 a100200080 x", InstructionSet::X86); + let Some(Operand::Memory(global)) = straddling.operands.get(1) else { + panic!("no absolute operand in {straddling:?}"); + }; + assert_eq!( + global.address, + Some(0x0000_0000_8000_2000), + "a 32-bit address was sign-extended into a space the engine does not use: {global:?}" + ); + + // And a genuine 32-bit target's addresses are all in the low half anyway, which is why + // inheriting reproduces the measurement exactly. + let ordinary = split_instruction(0x00401000, "00401000 e8fb0f0000 x", InstructionSet::X86); + assert_eq!(ordinary.flow, Flow::Call(Some(0x00402000))); + } + /// `hlt` is not an ending, though it is grouped with one in every mnemonic list. /// /// A halted processor resumes at the next instruction when an interrupt or NMI wakes it, and