From c110fb245b4856362c2c685ddc8c60e4f8551d18 Mon Sep 17 00:00:00 2001 From: alex Date: Tue, 8 Sep 2026 01:28:35 -0400 Subject: [PATCH] harness: compile-time Coverage on every descriptor, guard regions, replace-mode records; audit of 23 undeclared side effects --- docs/harness-audit.md | 307 +++++++++++++++++++++++++++++ docs/shim-trace.md | 27 ++- src/shim/hooks/colony_turn.cpp | 19 ++ src/shim/hooks/colony_turn.h | 31 +++ src/shim/hooks/compute_budget.cpp | 19 ++ src/shim/hooks/compute_budget.h | 23 +++ src/shim/hooks/dictionaries.h | 45 +++++ src/shim/hooks/fleet_movement.cpp | 18 ++ src/shim/hooks/fleet_movement.h | 59 ++++++ src/shim/hooks/global_consts.h | 19 ++ src/shim/hooks/research.cpp | 72 +++++++ src/shim/hooks/research.h | 32 +++ src/shim/hooks/tech_effects.cpp | 21 ++ src/shim/hooks/tech_effects.h | 29 +++ src/shim/main.cpp | 5 + src/shim/trace/emitter.cpp | 69 ++++++- src/shim/trace/emitter.h | 62 +++++- src/shim/trace/hook.h | 127 +++++++++++- src/shim/trace/selftest.cpp | 45 +++++ src/shim/trace/selftest.h | 42 ++++ src/shim/trace/tracer.cpp | 63 +++++- src/shim/trace/tracer.h | 42 +++- tests/shim_trace/CMakeLists.txt | 6 +- tests/shim_trace/oracle_emit.py | 22 ++- tests/shim_trace/test_coverage.cpp | 170 ++++++++++++++++ tests/shim_trace/test_emitter.cpp | 70 ++++++- tests/shim_trace/test_hook.cpp | 23 ++- 27 files changed, 1434 insertions(+), 33 deletions(-) create mode 100644 docs/harness-audit.md create mode 100644 tests/shim_trace/test_coverage.cpp diff --git a/docs/harness-audit.md b/docs/harness-audit.md new file mode 100644 index 0000000..e4cd9a0 --- /dev/null +++ b/docs/harness-audit.md @@ -0,0 +1,307 @@ +# Harness audit: undeclared side effects + +**Status: framework change landed, host tests green (31/31). Nothing has been run on the VM. +Five hooks changed their region set and need their golden traces recaptured (see §5).** + +B3 shipped a result that read as evidence and was not: 13 of 15 compares clean, zero +divergences on the hook that matters — while replace mode wrote an autosave that differed from +the oracle by one `EVENT_RESEARCH_OVERBUDGET` entry. The event is appended to the owner's event +list. The event list was never a declared region. The diff could not see it, so it said +nothing, and "said nothing" was read as "found nothing". + +That is not a B3 bug. It is a property of the harness: **a declared region is a check, and +everything else is invisible**. Every hook written so far had it. This document is the audit, +the mechanism that makes the blind spot visible, and what still cannot be checked. + +--- + +## 1. The audit table + +Ranked by how likely each undeclared effect is to hide a real divergence: how directly it +feeds simulation state, whether any layer of the harness could currently notice it, and how +often it fires. + +| # | hook | undeclared side effect of the original | why it can hide a divergence | now | +|---|---|---|---|---| +| 1 | `TechTree::ProcessResearch` (B3) | posts **`EVENT_RESEARCH_OVERBUDGET`** on the owner's `EventStorage` (`ServerPlayer+0x29c`), bumping `EvNxID` and the turn's list | **the known defect.** Same branch that sets the modelled `node.flag = 2`; serialized into the save, so it changes the oracle | **declared** as region `events`; `ours` still does not post it, so it now diverges loudly instead of passing | +| 2 | `ServerPlayer::OnTechResearched` (B2) | posts **`EVENT_RESEARCH_COMPLETE` / `_UNDERBUDGET` / `_TEMPERANCE`** on the same `EventStorage` | identical shape to #1, and *worse*: B2 has no replace-mode oracle behind it — docs/B2.md gotcha 4 says a changed save hash on a completion turn is expected, so nothing would have flagged it | guard region `player` reports it | +| 3 | `ServerSystem::ProcessTurn` (B4) | the addiction sweep constructs **MoraleEvents** and appends them to the system's capped history | same shape again. Worse still: `sim::ProcessColonyTurn` *computes* the morale events, and `DescribeMoraleEvents` exists — but the hook never calls it, so they are dropped on the floor: not compared, not even logged | guard region `system` reports it; the dead describer is called out below | +| 4 | `StrategyServer::ProcessFleetMovement` (B4) | `OnFleetArrived` posts **`EVENT_FLEET_ARRIVED`**; the pass also writes `FPdpos` into every fleet and clears flags `0x2` and `0x100` on every fleet | fourth instance of the same class. No replace mode for this hook either, so neither layer could see it | coverage notes only (a guard would have to span every fleet; see §6) | +| 5 | `StrategyServer::ProcessFleetMovement` (B4) | **`ours` re-reads the LIVE fleet list after the original has run** | breaks the compare invariant that ours never touches game memory, and makes the verdict partly self-fulfilling: our gate-traffic total is computed from the original's own post-move state | coverage note, risk `high` | +| 6 | `ServerPlayer::ComputeBudget` (B1) | `ServerSystem::ComputeOutput` **repairs damaged ships in orbit**; replace mode runs the original a *second* time on a scratch Budget to harvest six unmodelled slots | a real double-effect per turn in replace mode, on objects no region reaches. B1's oracle held, so it is either benign or was not exercised — unknown, not cleared | coverage note, risk `high`; unverifiable from inside this hook | +| 7 | `ServerPlayer::OnTechResearched` (B2) | writes **other objects**: every owned system's AI flag, the arcology civilian-cap re-evaluation, the addiction cure, the plague clear across systems **and ships** | writes through pointers the region model cannot follow; all of it is simulation state | coverage note; out of reach of any guard on the player | +| 8 | `TechTree::ProcessResearch` (B3) | `SetResearched`: turn/order stamps, the child-unlock cascade, the recursive research of zero-cost children, and the owner's tech-effect callback (which consumes one RNG word) | the extra draw desynchronises the generator, which *is* a declared region — so it surfaces, but as an unexplained RNG divergence rather than as its cause | coverage note + guard `tree_header` | +| 9 | `TechTree::ProcessResearch` (B3) | bumps the tree's completion-order counter at **`TechTree+0x20`** | the per-node `order` word is compared but the counter it is read from was not a region — a compare could match every node and still be running on a different counter | guard region `tree_header` | +| 10 | `StrategyServer::MoveFleet` (B4) | departure cancels every still-acting ship and calls `FleetDeparts` (system ownership bits); arrival dispatches `SEFleetArrived` and one of three arrival handlers | the declared boundary, but "expected to differ" was never *measured*: an arriving call was simply clean | guard region `fleet` (the fleet's own words only) | +| 11 | `ServerPlayer::ComputeBudget` (B1) | the **over-budget int at Budget+0x64** | the original writes it. B1 captured it only as an *argument* (`tail_before`), and args are never compared — so it was neither checked nor watched | guard region `budget_object` | +| 12 | `WeaponDictionary::Init` / `SectionDictionary::SectionDictionary` (M2) | `LoadWeapon` / `LoadSection` register every definition's name with the **string table** and resolve `requires` against the live **TechTree** | `ours` delegates to those same functions *after* the original has run, so a compare run performs every registration a second time. This is the leading hypothesis for the section hook's compare-mode crash | coverage notes, risk `high`; compare mode for the section hook remains unsafe | +| 13 | `ServerPlayer::OnTechResearched` (B2) | the pending plague-cure roll draws **exactly one RNG word**, unconditionally | it is the extra draw B3 measured. `ours` clears the guarded words and reports rather than rolling — correct, but it means the generator is not verified across this hook | coverage note | +| 14 | `ServerSystem::ProcessTurn` (B4) | every callee: plague, imperial/civilian growth, the resource debit, in-orbit refuel, slaves, rebellion, the build queue (creates ships, bumps per-player `ShipRecords`, raises `SEBuildCompleted`) | the declared input boundary. Large, and previously invisible: a clean compare on ten words implied nothing about the other 700 bytes of the system | guard region `system` measures how much moves | +| 15 | `ServerPlayer::ComputeBudget` (B1) | six slots (1, 2, 3, 4, 7, 11) are copied out of the original's own output back into the same slots | they **match by construction** and prove nothing, but they were counted in the headline "4437 compares, 0 divergences" | coverage note, risk `high` | +| 16 | `Mars::GlobalConsts::LoadFile` (M1) | **erases each consumed key from the caller's `std::map`** (a red-black tree unlink + free) | the erase is what implements first-occurrence-wins. The *effect* is modelled in `game::config::apply`; the container is not, and the map is a `LoadAll` temporary that no region can reach | coverage note | +| 17 | `ServerPlayer::OnTechResearched` (B2) | allocates or frees the node-bore block at `ServerPlayer+0x308` | `ours` has no allocator the game's runtime could free, so replace mode calls the game's own updater — meaning replace mode never exercises our node-bore selection at all | coverage note; region `node_bore` exists only when the block already does | +| 18 | `StrategyServer::MoveFleet` (B4) | a node-line waypoint's step comes from the stutter profile; a missed probabilistic jump scatters the fleet in a random direction (second draw) | `NodeLineStep` / `BuildStutterSegments` are written and unit-tested but **not wired in**: the hook steps every waypoint type as `speed × dt`, so a node-line leg is knowingly mis-stepped | coverage note (a declared gap, but it was only in prose) | +| 19 | `ServerSystem::ProcessTurn` (B4) | `ApplyInfraBonus` / `ApplyPopBonus` read the owner's home-system id; the build queue writes the owning `ServerPlayer` | writes through a pointer to another object | coverage note | +| 20 | `StrategyServer::ProcessFleetMovement` (B4) | the original accumulates by `player->index` but writes back by the player's *position* in the server vector, into a fixed 32-int array with **no bounds check** | a latent bug in the original that ours reproduces only while `index == position`; the reference save never separates them | coverage note | +| 21 | `StrategyServer::ProcessFleetMovement` (B4) | `PassSchedule()` is never called by the hook and `FleetSummary::targetFleetId` / `relation` are never filled | the header claims ours predicts the call order "so a trace can be checked against it". It does not. `DescribeGateTraffic` and `DescribeFleetStep` are dead too | coverage note | +| 22 | M1 / M2 / B3 | **log output** (three `GlobalConsts` messages, missing-manifest lines, the research completion line) | not simulation state; recorded for completeness | coverage notes, risk `low` | +| 23 | M1 / M2 | heap traffic: M1's long-string leak per compare call; M2's 123 `WeaponDef` (0x278 B) and 885 `SectionDef` (0x3d8 B) allocations, both leaked per compare run | start-up only, but they are real allocator effects a save-hash oracle cannot see | coverage notes, risk `low` | + +Things I could **not** settle offline, and did not guess: + +- Whether the double `ComputeOutput` in B1 replace mode (#6) actually double-repairs ships. + It needs a hook on `ComputeOutputFromRates`, or a VM run with a damaged ship in orbit. +- The exact stride of `EventStorage::TurnEvents`, so the `events` region reports the outer + vector's **byte** span (`turns_bytes`) rather than an element count. `EvNxID` is the field + that actually carries the signal. +- Whether the section dictionary's `LoadSection` appends to the dictionary's own vector + (docs/M2.md raises it as a hypothesis). Still a hypothesis; recorded as `risk: high`. +- `ServerPlayer` / `ServerSystem` / `StarFleet` sizes come from the recovered object table in + `sots-re/findings/`, not from the instruction stream. The guard spans use them, and each + guard is behind a `readable()` probe so an over-long span degrades to "no guard" rather than + to a fault. + +--- + +## 2. The mechanism + +Four changes, chosen so the failure mode is *noisy* rather than *silent*, and so keeping them +correct does not depend on anyone remembering to. + +### 2.1 `Coverage` — a compile-time-required admission (the honesty layer) + +Every descriptor must define + +```cpp +static void coverage(Coverage& c); +``` + +and either list what it does not model or explicitly claim it models everything: + +```cpp +c.unmodelled("posts EVENT_RESEARCH_OVERBUDGET on the owner's EventStorage", + Risk::High, "the message text is composed from the tech name", + "region:events"); +c.complete("Fill writes buf[0..n) and nothing else"); // the other option +``` + +`Hook` carries a `static_assert` on it, so **a hook without a coverage statement does not +compile**. The notes go into `meta.hooks..coverage`, and `tracecmp.py` prints them under +every report — including a clean one. A passing run now always ends with a paragraph saying +what it did not check. + +Three states: `complete`, `partial` (the honest normal case — all nine game hooks), and +`unstated`. `unstated` is unreachable in new code and is what an old log reads as; the tracer +logs it to `shim.log` at start-up and `tracecmp.py` warns per hook. + +*Why this and not just documentation:* the boundary prose already existed — B2's header lists +its four unmodelled effects almost exactly. It was in a comment, so it never reached the +report a human actually reads after a run. + +### 2.2 Guard regions — the part that finds what nobody wrote down (the detection layer) + +`Region` gains a `kind`: + +- **`Result`** — the classic region: snapshotted, copied to scratch, handed to `ours`, diffed. + This is the verdict. +- **`Guard`** — a coarse span (usually the whole object). Snapshotted before and after the + **original only**. Never copied to scratch, never given to `ours`, never diffed. After the + call, every byte run inside it that moved and that **no Result region covers** is emitted as + an *undeclared write*. + +That masking step is what makes it usable: a guard over the whole `ServerPlayer` during B2 +reports the event storage and the plague words while staying quiet about the thirteen field +groups the descriptor already checks. A guard is a few hundred bytes of memcpy — cheap enough +to put over every object a hook can reach. + +*Why not a checksum:* a hash tells you something moved. A masked byte-run diff tells you +*where*, which is the difference between "B2 diverges somewhere" and "B2 wrote +`player+0x2b0`, which is `EvNxID`". + +*Why guards do not fail the run by default:* on a hook that admits its boundary, guard hits +are the expected consequence of that boundary, and treating them as failures would train +people to ignore them. They are always reported. They fail the run in exactly one case — the +one that matters: + +> **a hook whose `coverage.state` is `complete` while a guard caught an undeclared write has +> been proved wrong about its own model.** `tracecmp.py` counts that as a divergence (exit 1). + +So a hook can only produce a clean, quiet pass by claiming completeness — and if that claim is +false, the guard turns it into a failure. `--strict-coverage` widens failure to any undeclared +write or any unstated hook, for a gate that wants zero tolerance. + +Scratch is built from Result regions only, so a guard never occupies a scratch slot and +existing descriptors' recorded indices keep working — provided guards are pushed **last**. +`regions_well_ordered()` checks that on every call and turns a violation into an `err`, so the +footgun cannot fire quietly either. + +### 2.3 `replace` mode emits records + +`replace` used to log nothing, which is precisely why B3's divergence could only be found by +hashing a 609 KB autosave. It now writes one record per call — the same regions, measured +around `ours` — plus its own guard findings. Consequences: + +- a replace run is inspectable and `--replay`-able instead of pass/fail on a save hash; +- guards in replace mode report what **ours** writes outside the declared regions; +- comparing the two, `tracecmp.py` names the asymmetry directly: + `ONLY the original writes these: player+0x2b0:4`. That line *is* the B3 bug, printed. + +### 2.4 `tracecmp.py` coverage reporting + +Every report now ends with a `coverage` section: per hook, the verdict, the regions actually +compared (derived from the records, not from a claim), the guards, the undeclared writes they +caught with byte spans, and the descriptor's unmodelled list. `--json-out` carries the same +under `hooks..coverage` plus top-level `coverage_unstated` / `coverage_contradicted` and +`totals.{guarded_calls, undeclared_calls, undeclared_writes, coverage_unstated, +coverage_contradicted}`. Exit codes are unchanged (0 / 1 / 2). + +### What I rejected + +- **A checksum-only guard.** Detects, but does not localise; a moved hash on a 1 KB object is + barely more actionable than no signal at all. +- **A replace-mode assertion against a compare-mode snapshot.** Attractive, but the two runs + are different game sessions with different call ids and different RNG positions; matching + them up is the save-file oracle's job, and it already exists. Emitting replace records plus + the guard-span asymmetry gets most of the value with none of the plumbing. +- **Declaring the event list as a `Result` region on B2 and B4 as well.** It would turn every + completion and every colony turn into a divergence for a reason already known, drowning real + signal. B3 gets a Result region because closing that gap *is* B3's remaining work; the + others get guards, which report without polluting the verdict. + +--- + +## 3. What changed + +**Framework** (`src/shim/trace/`) + +- `emitter.h/.cpp`: `Risk`, `CoverageNote`, `Coverage`, `HookMeta`, `UndeclaredWrite`; record + fields `has_coverage` / `guards` / `undeclared` / `undeclared_total`; emission of + `meta.hooks..coverage` and the record's `coverage` block. +- `tracer.h/.cpp`: `Region::Kind`; `undeclared_writes()` (masked byte-run diff); + `regions_well_ordered()`; `register_hook(name, policy, coverage)`; `unstated_hooks()`. +- `hook.h`: the `static_assert`; Result/Guard split in `run<>`; `run_replace()`; the + region-order check. +- `selftest.{h,cpp}`: `Blob` + `FillCounted` + `FillGuardHook` / `FillGuardLyingHook` — the B3 + shape in miniature (a Result region `ours` reproduces exactly, next to a counter only the + original writes). +- `main.cpp`: logs `COVERAGE:` for any hook registered without a statement. + +**Descriptors** — `coverage()` on all nine, plus: + +| hook | region change | +|---|---| +| `TechTree::ProcessResearch` | **+ Result `events`** (owner+0x29c, 0x1c, `{turns_bytes, next_id, vec_*}`); **+ Guard `player`** (owner, 0x3e0); **+ Guard `tree_header`** (tree, 0x24) | +| `ServerPlayer::OnTechResearched` | **+ Guard `player`** (self, 0x3e0) | +| `ServerPlayer::ComputeBudget` | **+ Guard `budget_object`** (budget, 0x68) | +| `ServerSystem::ProcessTurn` | **+ Guard `system`** (self, 0x2d8) | +| `StrategyServer::MoveFleet` | **+ Guard `fleet`** (fleet, 0x120) | +| the other four | coverage notes only; regions unchanged | + +**Harness** (`sots-re/verify/harness/compare/`) + +- `tracecmp.py`: `coverage` in `RECORD_KEYS`; `validate_coverage()` / + `validate_meta_coverage()`; per-hook coverage accounting; the `coverage` report section + including the compare-vs-replace asymmetry line; `--strict-coverage`; contradicted claims + count as divergences. +- `mkfixture.py`: `coverage` in `KEY_ORDER`; `coverage()` / `guard_block()` helpers; default + coverage in `meta()`; three new fixture logs (`coverage_guarded`, `coverage_lying`, + `coverage_unstated`). +- `test_tracecmp.py`: a `CoverageTest` class (8 cases) — meta coverage reaching the report, + guard findings reported without failing an honest hook, `--strict-coverage`, a false + completeness claim exiting 1, unstated logs being called out, block validation, an invalid + block failing the record, and the compare-vs-replace asymmetry line. +- `TRACE_FORMAT.md`: `coverage` in the record table and the key order, coverage in the `meta` + example, new section 8. + +**Tests**: `tests/shim_trace/test_coverage.cpp` (new ctest `shim_trace_coverage`) instantiates +every descriptor's `Hook<>` on the host — which is what fires the `static_assert`, since the +descriptors otherwise only reach a compiler on the MinGW cross build — and checks that every +hook states its coverage, that no note is empty, and that the guard machinery reports the +undeclared write, stays quiet when there is none, and counts honestly when it truncates. +`test_emitter.cpp` and `test_hook.cpp` updated for the new meta/record shape and for replace +mode now emitting. + +`ctest`: **31/31 green** (was 30/30; +`shim_trace_coverage`). +`test_tracecmp.py`: **38 tests, 2 failures** — both pre-existing and unrelated +(`OracleBridgeTest::test_per_kind_output` and `::test_cli_and_replay_round_trip`, in +`oracle_parsers.py`'s bare-item key and duplicate-id error count; failing identically before +this change). + +--- + +## 4. What a report looks like now + +``` +- calls: 12 compared: 12 diverged: 0 invalid records: 0 warnings: 0 +- coverage: 12 guarded call(s), 4 undeclared write(s) in 4 call(s); 0 hook(s) unstated, 0 contradicted + +### coverage + +| hook | verdict | compared regions | guards | undeclared writes | unmodelled | +|---|---|---|---|---|---| +| Game::TechTree::ProcessResearch | partial | events, node[0], … +214 | player, tree_header | 4 in 4 call(s) | 5 | + +#### Game::TechTree::ProcessResearch — not checked by this run +- (high) posts EVENT_RESEARCH_OVERBUDGET on the owner's EventStorage … [region:events] +- guard hits in compare mode: player+0x2b0:4 +- ONLY the original writes these: player+0x2b0:4 +``` + +The last three lines are the B3 defect, stated by the harness, on a run that reports zero +divergences elsewhere. + +--- + +## 5. Golden traces that need recapturing + +A region-set change invalidates a golden trace (docs/M2.md made this mistake once already). +These five need a fresh capture on the VM before their logs mean anything: + +| hook | golden | why | +|---|---|---| +| `Game::TechTree::ProcessResearch` | `b3-trace-golden.jsonl` | new `events` side entry + two guards; **expect new divergences** on over-budget and completion calls — they are the defect becoming visible, not a regression | +| `Game::ServerPlayer::ComputeBudget` | `b1-trace-golden.jsonl` | new `coverage` block per record (side entries unchanged, so the *diff* verdict should be identical: 0 divergences) | +| `Game::ServerPlayer::OnTechResearched` | none yet (B2 never captured) | capture with the guard in place from the start | +| `Game::ServerSystem::ProcessTurn` | none yet (B4 never captured) | ditto | +| `Game::StrategyServer::MoveFleet` | none yet (B4 never captured) | ditto | + +`m1-trace-golden.jsonl` and `m2-trace-golden.jsonl` keep their side entries; only their `meta` +line grows a `coverage` object. They do not need re-capturing to stay valid, but any strict +byte-comparison against them will differ on line 1. + +**First VM run to do:** B3 in compare mode on the reference save, `--json-out` to +`verify/results/compare/`. Two things to read: whether `side.events.after.v.next_id` diverges +on exactly the over-budget call (it should — that is the mechanism working), and what +`player` / `tree_header` report as undeclared writes on a *completion* call, which is the +cheapest available map of what `SetResearched` actually touches. + +--- + +## 6. What still cannot be checked + +Honest limits of the mechanism, so nobody reads a clean coverage section as more than it is. + +- **Writes to other objects reached through pointers.** A guard covers one span. B2's writes + to every owned system and every ship, B4's writes to other fleets and to the owning player, + and `ComputeBudget`'s ship repair are all outside every guard. Covering them means declaring + a guard per reachable object — possible for a bounded set (the owner, the tree), not for + "every ship in the empire". These stay coverage notes. +- **Heap allocation and container growth as such.** A guard sees a `std::vector`'s three words + move; it does not see the elements, and it cannot see a `std::map` node unlinked from a tree + that is not in the span. M1's map erase and M2's definition allocations are structurally out + of reach of a before/after byte diff. +- **Log output, file I/O, and anything outside the process's data.** Not modelled and not + modellable this way. +- **The message text of an event.** `events` proves an event was or was not posted, and which + counter moved. It cannot prove the text, which the game composes from the tech name. Closing + B3 properly still means calling the game's own event API from `ours`, the way M2 delegates + to `LoadWeapon`. +- **A guard cannot distinguish "the original wrote X" from "a callee of the original wrote + X".** It reports the byte, not the writer. That is usually enough to start, and never enough + to finish. +- **Anything on a call the hook never sees.** An empty trace still proves nothing. The + coverage section reports `not watched` for a hook with no guarded calls, which at least + makes the absence explicit. +- **B2 and B4 remain entirely unverified against the game.** Every claim in their rows above + is read from the disassembly and the RE notes, not measured. The guards are how they will be + measured. diff --git a/docs/shim-trace.md b/docs/shim-trace.md index c1d1f0b..1f84fcf 100644 --- a/docs/shim-trace.md +++ b/docs/shim-trace.md @@ -24,13 +24,18 @@ Host tests: `tests/shim_trace/` (ctest `shim_trace_*`). They build the same sour | `off` | original | nothing | original's result | | `trace` | original | `args`, `ret`, `side` (before/after of every declared region) | original's result | | `compare` | original, then **ours on a copy of the pre-call state** | trace fields + `ours{ret,side}`, `diverged`, `diff` | original's result | -| `replace` | ours | nothing | ours' result | +| `replace` | ours | `args`, `ret`, `side` (before/after around **ours**), `coverage` | ours' result | `compare` never lets ours touch game memory: the declared regions are snapshotted *before* the original runs, those snapshots are copied into scratch buffers, the arguments are rebound onto the scratch buffers (`rebind`), and ours runs there. The diff compares the original's `after` with the scratch buffers' contents and both return values. +`replace` used to log nothing at all, which is why B3's replace-mode divergence could only be +found by hashing the autosave. It now writes one record per call — the same regions, measured +around `ours` instead of around the original — so a replace run is readable, `--replay`-able, +and directly comparable with a compare log's guard findings (docs/harness-audit.md). + A throw from ours, or from any describe/regions/rebind callback, is caught inside the hook and becomes `err` on the record (which the harness counts as a divergence). The original is always invoked in `off`/`trace`/`compare`, whatever happened during capture. Nothing propagates across @@ -107,6 +112,16 @@ struct CfgVarRegisterKeyHook { static bool ours(CfgTable* t, const char* key, int value); // the reimplementation static HookPolicy policy() { return HookPolicy{}; } // ftol / ftol_kind / ptr_exact / unordered + + // REQUIRED (compile error if missing): everything the original writes that no Result + // region above covers. See docs/harness-audit.md. + static void coverage(Coverage& c) { + c.unmodelled("erases the consumed key from the caller's map", Risk::Medium, + "a red-black tree cannot be snapshotted before the call", + "guard:cfg_table"); + // ...or, when the regions really are everything: + // c.complete("RegisterKey writes only the table slot declared above"); + } }; ``` @@ -128,6 +143,16 @@ Rules of the road: (ignored by default policy) unless you declare the pointed-to memory as another region. - **Region sizes must be known at call time.** A region whose length is only known after the call cannot be snapshotted "before"; declare an upper bound or split the hook. +- **A clean compare bounds only what you declared.** Everything else the original writes is + invisible to the diff. Two things exist so that is never silent: `coverage()`, which the + compiler makes you fill in, and **guard regions** — `Region::kind = Region::Kind::Guard`, a + coarse span (usually the whole object) that is watched around the original, never handed to + `ours` and never diffed. Any byte in it that moved and that no Result region covers is + reported as an undeclared write. Guards are cheap; declare one over every object the hook + can reach. +- **Push guards LAST.** Scratch holds Result regions only, so a descriptor that remembers a + region index as `out.size()` at push time must not interleave guards. `Hook<>` checks the + order on every call and turns a violation into an `err` rather than a shifted mapping. - **Prefer `struct` describers** over raw `bytes` for anything with fields: the harness then points at `side.cfg_table.after.v.count` instead of a byte offset. - **Floats**: `tv::f32` stores at float32 width and prints `%.9g`; the diff rounds both sides diff --git a/src/shim/hooks/colony_turn.cpp b/src/shim/hooks/colony_turn.cpp index cb2a405..44999d9 100644 --- a/src/shim/hooks/colony_turn.cpp +++ b/src/shim/hooks/colony_turn.cpp @@ -29,6 +29,9 @@ constexpr std::size_t kRngSize = A::RNG_size; // 0x9cc constexpr int kMtWords = mars::rng::MT19937::N; constexpr std::size_t kPopGroupStride = 0x18; // {?, int type @+4, int species @+8, int64 @+0x10} constexpr std::size_t kMaxPopGroups = 4096; +// Whole-object guard span. ServerSystem is 0x2d8 bytes (findings/control-flow/turn-spine.md +// object table); the generated header's highest declared offset is +0x2cc. +constexpr std::size_t kSystemGuardSize = 0x2d8; using MaxPopFn = int(SHIM_THISCALL*)(void* sys, void* player, int flag); using IsStableFn = bool(SHIM_THISCALL*)(void* sys); @@ -365,6 +368,22 @@ void ServerSystemProcessTurnHook::regions(std::vector& out, void* in.size = sizeof(ColonySnapshot); in.describe = &DescribeColonySnapshot; out.push_back(in); + + // ---- guard: pushed LAST so rebind()'s positional scratch mapping is unaffected ---------- + // + // The whole ServerSystem. Ten of its words are Result regions and are masked out; every + // other byte the dispatcher's callees write -- population, morale (including the morale + // events the addiction sweep raises), resources, plague, slaves, rebellion state -- comes + // back as an undeclared write. That is the declared input boundary made *visible*: it stays + // uncompared, but a clean run can no longer imply it was checked. + if (readable(self, kSystemGuardSize)) { + trace::Region g; + g.name = "system"; + g.ptr = self; + g.size = kSystemGuardSize; + g.kind = trace::Region::Kind::Guard; + out.push_back(g); + } } ServerSystemProcessTurnHook::Args ServerSystemProcessTurnHook::rebind(trace::Scratch& s, diff --git a/src/shim/hooks/colony_turn.h b/src/shim/hooks/colony_turn.h index 1acc24f..d29ad33 100644 --- a/src/shim/hooks/colony_turn.h +++ b/src/shim/hooks/colony_turn.h @@ -38,6 +38,37 @@ struct ServerSystemProcessTurnHook { static Args rebind(trace::Scratch& s, void* self); static void ours(void* self); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("the addiction sweep raises MoraleEvents, which are constructed and appended " + "to the system's capped morale history", + trace::Risk::High, + "the same class of write as B3's defect. sim::ProcessColonyTurn does compute " + "the morale events (ColonyTurnResult), but the hook never emits them: " + "DescribeMoraleEvents is dead code, so they are neither compared nor logged", + "guard:system"); + c.unmodelled("every callee: the plague pass, imperial and civilian growth, the resource " + "debit, in-orbit refuel, slaves, rebellion and the build queue", + trace::Risk::High, + "declared input boundary -- ProcessTurn is a dispatcher and only the words " + "it writes itself are modelled. The callees raise EVENT_SLAVES_DEAD, " + "EVENT_SYSTEM_REBELLION_CONTINUES, the plague events and SEBuildCompleted, " + "create ships and bump per-player ShipRecords counters", + "guard:system covers the system object only, not the other objects"); + c.unmodelled("ApplyInfraBonus / ApplyPopBonus read the owner's home-system id, and the " + "build queue writes the owning ServerPlayer", + trace::Risk::Medium, + "writes through a pointer to another object; no region reaches the player"); + c.unmodelled("ProcessRebellion is the pass's only RNG consumer and its draw count is " + "data-dependent", + trace::Risk::Low, + "the generator IS a declared region, so a moved post-state is visible and " + "names the system whose rebellion fired -- it is reported, not modelled"); + c.unmodelled("replace mode is refused for this hook", + trace::Risk::Medium, + "our side models the dispatcher's own writes and none of the callees, so a " + "replace run would silently skip a colony's whole turn. There is therefore " + "no oracle layer behind the compare for this hook"); + } }; // Process facts the hook needs (exe base for the RVAs, a line logger). Call once before diff --git a/src/shim/hooks/compute_budget.cpp b/src/shim/hooks/compute_budget.cpp index d83b373..1882158 100644 --- a/src/shim/hooks/compute_budget.cpp +++ b/src/shim/hooks/compute_budget.cpp @@ -25,6 +25,10 @@ void logf(const char* fmt, ...) { g_log_line(line); } +// The whole Budget object: 22 ints, the research-allocation vector (3 words at +0x58) and the +// over-budget int at +0x64. docs/B1.md, "The out parameter is not an int[25]". +constexpr std::size_t kBudgetObjectSize = kAllocVectorOffset + kAllocVectorSize + 4; + // ---- raw reads out of the game's objects (offsets come from the generated header) ---------- template @@ -179,6 +183,21 @@ void ComputeBudgetHook::regions(std::vector& out, void* self, std in.size = sizeof(BudgetSnapshot); in.describe = &DescribeSnapshot; out.push_back(in); + + // ---- guard: pushed LAST so the scratch indices rebind() uses are unaffected ------------- + // + // The whole Budget object, 0x68 bytes: the 22 ints, the research-allocation vector and the + // over-budget int at +0x64. The first two are Result regions and are masked out, so what + // this reports is the over-budget word -- which the original writes and which B1 only ever + // captured as an *argument* (`tail_before`), i.e. never compared and never even watched. + if (budget) { + trace::Region g; + g.name = "budget_object"; + g.ptr = budget; + g.size = kBudgetObjectSize; + g.kind = trace::Region::Kind::Guard; + out.push_back(g); + } } ComputeBudgetHook::Args ComputeBudgetHook::rebind(trace::Scratch& s, void* self, std::int32_t*, diff --git a/src/shim/hooks/compute_budget.h b/src/shim/hooks/compute_budget.h index 70c1338..b56b588 100644 --- a/src/shim/hooks/compute_budget.h +++ b/src/shim/hooks/compute_budget.h @@ -39,6 +39,29 @@ struct ComputeBudgetHook { static Args rebind(trace::Scratch& s, void* self, std::int32_t* budget, bool projected); static void ours(void* self, std::int32_t* budget, bool projected); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("slots 1, 2, 3, 4, 7 and 11 are produced by callees this milestone does not " + "model (per-system output, trade, ship-carried population, a second manager, " + "the build-queue spend)", + trace::Risk::High, + "they are copied out of the original's own output and back into the same " + "slots, so they match BY CONSTRUCTION and prove nothing", + "declared input boundary; see budget_inputs.h"); + c.unmodelled("ServerSystem::ComputeOutput repairs damaged ships in orbit", + trace::Risk::High, + "replace mode runs the original a second time on a scratch Budget to harvest " + "the six unmodelled slots, so that repair happens TWICE per turn in replace " + "mode and nothing in the trace would show it", + "guard:budget_object does not reach the ships; unverified"); + c.unmodelled("the difficulty-mods row from StrategyServer::GetDifficultyMods", + trace::Risk::Medium, + "not reachable from a ServerPlayer, so the two relevant entries are fitted " + "constants measured from the B1 trace rather than snapshotted inputs"); + c.unmodelled("the research-allocation vector's heap block", + trace::Risk::Low, + "only the element count is compared; the three words are heap pointers the " + "default policy ignores"); + } }; // Process facts the hook needs (a line logger for shim.log). Call once before installing. diff --git a/src/shim/hooks/dictionaries.h b/src/shim/hooks/dictionaries.h index 9badd80..1b9980c 100644 --- a/src/shim/hooks/dictionaries.h +++ b/src/shim/hooks/dictionaries.h @@ -42,6 +42,30 @@ struct WeaponDictionaryInitHook { static Args rebind(trace::Scratch& s, void* self); static void ours(void* self); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("LoadWeapon -> WeaponDef::ParseScript registers each weapon's name with the " + "string table and resolves `requires` against the live TechTree", + trace::Risk::High, + "per-file parsing is M3 scope; ours delegates to the game's own LoadWeapon, " + "so a compare run performs the registration a SECOND time and neither the " + "string table nor the tech tree is a declared region", + "suspected cause of the sibling section hook's compare crash (docs/M2.md)"); + c.unmodelled("allocates 123 WeaponDef objects (0x278 bytes each) on the game heap", + trace::Risk::Low, + "the definitions do not exist when the hook is entered, so they cannot be a " + "before-snapshot; the dictionary region compares them by id/name/path"); + c.unmodelled("the word at dictionary+0x14", + trace::Risk::Low, + "not modelled; emitted as an opaque pointer, which the default policy " + "ignores -- a change is visible in a trace but never a divergence", + "guard:dict"); + c.unmodelled("writes lines to the game log for a missing manifest", + trace::Risk::Low, "log text is not simulation state"); + c.unmodelled("std::sort tie order for equal weapon names", + trace::Risk::Low, + "msvc_sort.h replays MSVC 2010's introsort, but the shipped data has no tied " + "names, so the tie rule is unexercised rather than verified"); + } }; struct SectionDictionaryCtorHook { @@ -56,6 +80,27 @@ struct SectionDictionaryCtorHook { static Args rebind(trace::Scratch& s, void* self, void* tree); static void* ours(void* self, void* tree); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("LoadSection registers each section with the string table and the live " + "TechTree, and may append to the dictionary's own vector", + trace::Risk::High, + "M3 scope; ours delegates to the game's LoadSection after the original has " + "already built all 885 definitions, so the second pass registers duplicates " + "-- the leading hypothesis for this hook's compare-mode crash", + "see docs/M2.md; compare mode for this hook is not safe to run"); + c.unmodelled("post-load validation pass over every definition's @-token against the " + "string table", + trace::Risk::Medium, "runs after the loop and touches no declared region"); + c.unmodelled("allocates 885 SectionDef objects (0x3d8 bytes each) on the game heap", + trace::Risk::Low, + "they do not exist at hook entry; compared by index/species/id/token"); + c.unmodelled("the word at dictionary+0x14", + trace::Risk::Low, "not modelled; emitted as an ignored pointer", "guard:dict"); + c.unmodelled("the before-snapshot of the object is uninitialised heap", + trace::Risk::Medium, + "the hook is on the constructor, so `before` is meaningless and only " + "`after` carries information"); + } }; // Process facts the hooks need (exe base for the RVAs, the CRT allocator for the section diff --git a/src/shim/hooks/fleet_movement.cpp b/src/shim/hooks/fleet_movement.cpp index 1bce7bc..b6c3c2c 100644 --- a/src/shim/hooks/fleet_movement.cpp +++ b/src/shim/hooks/fleet_movement.cpp @@ -28,6 +28,9 @@ namespace A = sots::addr; constexpr std::size_t kRngSize = A::RNG_size; constexpr int kMtWords = mars::rng::MT19937::N; constexpr std::size_t kMaxPlayers = 64; +// Whole-object guard span. StarFleet is 0x120 bytes (findings/control-flow/turn-spine.md object +// table); the generated header's highest declared offset is +0x10c (Flags). +constexpr std::size_t kFleetGuardSize = 0x120; using ResolveWaypointFn = void*(SHIM_THISCALL*)(void* fleet); using RelationFn = int(SHIM_THISCALL*)(void* player, void* other); @@ -369,6 +372,21 @@ void StrategyServerMoveFleetHook::regions(std::vector& out, void* r.describe = &describe_rng; out.push_back(r); } + + // ---- guard: pushed LAST; rebind() maps scratch by position, and guards take no slot ----- + // + // The whole StarFleet. `pos` and `prev_pos` are masked out, so what this reports is every + // other word the step touches: the flags the departure hook clears, the waypoint vector the + // arrival handlers pop, FPdpos, the gate-traffic counter. All of it is the declared input + // boundary -- and until now a compare that arrived was clean about all of it. + if (readable(fleet, kFleetGuardSize)) { + trace::Region g; + g.name = "fleet"; + g.ptr = fleet; + g.size = kFleetGuardSize; + g.kind = trace::Region::Kind::Guard; + out.push_back(g); + } } StrategyServerMoveFleetHook::Args StrategyServerMoveFleetHook::rebind(trace::Scratch& s, diff --git a/src/shim/hooks/fleet_movement.h b/src/shim/hooks/fleet_movement.h index 5a881a6..f92ca34 100644 --- a/src/shim/hooks/fleet_movement.h +++ b/src/shim/hooks/fleet_movement.h @@ -46,6 +46,34 @@ struct StrategyServerMoveFleetHook { static Args rebind(trace::Scratch& s, void* self, void* fleet, float dt); static bool ours(void* self, void* fleet, float dt); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("on arrival: dispatches SEFleetArrived and runs one of three arrival " + "handlers by destination kind (enter system / join fleet / stop at point)", + trace::Risk::High, + "declared input boundary -- an arriving call is expected to differ in all of " + "it, and none of it is declared, so the compare says nothing about arrivals", + "guard:fleet sees the fleet's own words; the event and the system do not"); + c.unmodelled("on departure: cancels every still-acting ship (with a log line each) and " + "calls ServerSystem::FleetDeparts, which rewrites the system's ownership bits", + trace::Risk::High, "writes through pointers to ships and to the system"); + c.unmodelled("the tanker top-up refuels other ships in the fleet", + trace::Risk::Medium, + "the per-ship range regions would show it, but ours does not model it, so a " + "fleet with a tanker diverges for a known reason"); + c.unmodelled("a node-line waypoint's step comes from the stutter profile", + trace::Risk::Medium, + "NodeLineStep / BuildStutterSegments are written and unit-tested but not " + "wired in; the hook steps every waypoint type as speed x dt, so a node-line " + "leg is knowingly mis-stepped and only its type is recorded", + "declared gap: docs/B4.md"); + c.unmodelled("a missed probabilistic jump scatters the fleet in a random direction", + trace::Risk::Medium, + "the direction is a second draw whose mapping is not modelled; ours leaves " + "the position alone and reports the scatter distance, so the generator " + "region diverges by one word on a miss"); + c.unmodelled("the route revalidation and the waypoint list itself", + trace::Risk::Medium, "declared input boundary; the waypoint vector is not a region"); + } }; struct StrategyServerProcessFleetMovementHook { @@ -59,6 +87,37 @@ struct StrategyServerProcessFleetMovementHook { static Args rebind(trace::Scratch& s, void* self); static void ours(void* self); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("`ours` re-reads the LIVE fleet list after the original has run", + trace::Risk::High, + "the gate-traffic total is computed by the original at the very end of the " + "pass, so a pre-call snapshot would diverge for the wrong reason. It breaks " + "the compare invariant that ours never touches live memory, and it makes " + "this hook's verdict partly self-fulfilling: the input to our arithmetic is " + "the original's own post-move state"); + c.unmodelled("drives MoveFleet up to five times per fleet", + trace::Risk::High, + "every undeclared effect of MoveFleet happens inside this call too; the pass " + "schedule is recorded in the arguments but never compared"); + c.unmodelled("writes FPdpos into every fleet and clears flags 0x2 and 0x100 on every fleet", + trace::Risk::High, + "no region covers the fleets, only the players' gate-traffic words"); + c.unmodelled("OnFleetArrived posts EVENT_FLEET_ARRIVED", + trace::Risk::High, + "the same class of write as B3's defect, and there is no replace mode for " + "this hook, so nothing behind the compare could catch it either"); + c.unmodelled("the original accumulates by player->index but writes back by the player's " + "position in the server vector, into a fixed 32-int array with no bounds " + "check", + trace::Risk::Medium, + "a real latent bug in the original that our side reproduces only while " + "index == position; the reference save never separates them"); + c.unmodelled("PassSchedule() is never called by the hook, and FleetSummary::targetFleetId " + "/ relation are never filled", + trace::Risk::Medium, + "the header claims ours predicts the call order for a trace to check; that " + "prediction is not actually emitted"); + } }; void init_fleet_movement(std::uintptr_t exe_base, void (*log_line)(const char* line)); diff --git a/src/shim/hooks/global_consts.h b/src/shim/hooks/global_consts.h index 5b8b8f9..1be4d1b 100644 --- a/src/shim/hooks/global_consts.h +++ b/src/shim/hooks/global_consts.h @@ -34,6 +34,25 @@ struct GlobalConstsLoadFileHook { static Args rebind(trace::Scratch& s, const char* file, void* consts); static void ours(const char* file, void* consts); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("erases each consumed key from the caller's std::map (FUN_008b8d20)", + trace::Risk::Medium, + "the map is a LoadAll temporary; declaring a red-black tree as a region is " + "not possible before the call. First-occurrence-wins is reproduced in " + "game::config::apply instead, so the *effect* is modelled, the container is " + "not", + "LoadAll's post-state would have to be hooked to see it"); + c.unmodelled("writes three kinds of line to the game log (unrecognised key, applied " + "key, expected-but-not-found)", + trace::Risk::Low, "log text is not part of the simulation state"); + c.unmodelled("opens the file through the VFS and allocates/releases a refcounted buffer", + trace::Risk::Low, + "ours performs the same two calls, so allocation behaviour matches by " + "construction rather than by comparison"); + c.unmodelled("String slots assign through the engine's own std::string, leaking one heap " + "block per long string in compare mode", + trace::Risk::Low, "start-up only; documented in docs/M1.md"); + } }; // Process facts the hook needs: the exe's load address (parser identification, the scale diff --git a/src/shim/hooks/research.cpp b/src/shim/hooks/research.cpp index b0efe55..1e36260 100644 --- a/src/shim/hooks/research.cpp +++ b/src/shim/hooks/research.cpp @@ -30,6 +30,20 @@ constexpr std::size_t kNodeSize = A::TechNode_size; // 0x34 constexpr std::size_t kRngSize = A::RNG_size; // 0x9cc constexpr std::size_t kTreeHeadSize = A::TechTree_off_Nodes + 0xc; // owner + the node vector constexpr std::size_t kMaxNodes = 8192; // loop guard for a garbage vector header + +// Not in the generated header (it is recovered from the save schema, not from an instruction): +// ServerPlayer+0x29c is an inline Game::EventStorage, 0x1c bytes, holding a +// vector at +4 and the next-event id EvNxID at +0x14. Posting an event bumps +// EvNxID and grows the turn's list -- the write B3's compare could not see. +// Source: sots-re/findings/objects/struct-recovery.md, ServerPlayer table row 0x29c. +constexpr std::size_t kPlayerEventsOff = 0x29c; +constexpr std::size_t kEventStorageSize = 0x1c; +constexpr std::size_t kEventsVecOff = 0x04; +constexpr std::size_t kEventsNextIdOff = 0x14; +// Whole-object guard spans. ServerPlayer is 0x3e0 and the TechTree header we care about ends +// at the order counter (+0x20). Source: findings/control-flow/turn-spine.md object table. +constexpr std::size_t kPlayerSize = 0x3e0; +constexpr std::size_t kTreeGuardSize = 0x24; constexpr std::size_t kMaxAlloc = 1024; constexpr int kMtWords = mars::rng::MT19937::N; @@ -119,6 +133,7 @@ struct CallState { std::uintptr_t rng_base = 0; // the LIVE generator address, for next-pointer math std::size_t rng_region = 0; std::size_t ob_region = 1; + int events_region = -1; // the owner's EventStorage, -1 when the owner is unreadable std::vector node_region; // node index -> region index, -1 when the slot is null std::vector names; // stable storage for Region::name std::vector scratch_nodes; @@ -222,6 +237,23 @@ Tv describe_i32(const void* p, std::size_t, unsigned) { return s; } +// The owner's inline EventStorage. The three vector words are heap pointers (ignored by the +// default policy), so what the diff actually compares is `turns` -- how many TurnEvents the +// list holds -- and `next_id`, the counter every posted event bumps. Posting the +// EVENT_RESEARCH_OVERBUDGET message moves `next_id`, so a run where ours does not post it now +// diverges here instead of passing silently. That is the whole point of this region. +Tv describe_events(const void* p, std::size_t, unsigned) { + Tv s = tv::struct_(); + const char* begin = static_cast(ptr_at(p, kEventsVecOff)); + const char* end = static_cast(ptr_at(p, kEventsVecOff + 4)); + s.add("turns_bytes", tv::i32(end >= begin ? static_cast(end - begin) : -1)); + s.add("next_id", tv::i32(word_at(p, kEventsNextIdOff))); + s.add("vec_begin", tv::ptr(ptr_at(p, kEventsVecOff))); + s.add("vec_end", tv::ptr(ptr_at(p, kEventsVecOff + 4))); + s.add("vec_cap", tv::ptr(ptr_at(p, kEventsVecOff + 8))); + return s; +} + // ---- the generator seen by ours ---------------------------------------------------------- struct ShimRandom final : sots::sim::IRandom { @@ -323,6 +355,46 @@ void TechTreeProcessResearchHook::regions(std::vector& out, void* n.describe = &describe_node; out.push_back(n); } + + // The owner's event storage. This is the region whose absence made B3's compare clean while + // replace mode diverged: the over-budget branch posts EVENT_RESEARCH_OVERBUDGET, which bumps + // EvNxID, and nothing declared here could see it. `ours` still does not post the event, so + // the divergence this region now reports is real and expected -- visible instead of silent. + void* owner = readable(tree, kTreeHeadSize) ? ptr_at(tree, A::TechTree_off_Owner) : nullptr; + g_call.events_region = -1; + if (readable(owner, kPlayerEventsOff + kEventStorageSize)) { + g_call.events_region = static_cast(out.size()); + trace::Region ev; + ev.name = "events"; + ev.ptr = static_cast(owner) + kPlayerEventsOff; + ev.size = kEventStorageSize; + ev.describe = &describe_events; + out.push_back(ev); + } + + // ---- guards: pushed LAST so every recorded index above is also a Scratch index ---------- + // + // Coarse spans that no reimplementation writes. Anything the ORIGINAL moves inside them and + // outside every Result region above is reported as an undeclared write -- which is how the + // next B3 gets found before a save-hash oracle has to find it. + if (readable(owner, kPlayerSize)) { + trace::Region g; + g.name = "player"; + g.ptr = owner; + g.size = kPlayerSize; + g.kind = trace::Region::Kind::Guard; + out.push_back(g); + } + if (readable(tree, kTreeGuardSize)) { + // Catches the completion-order counter at TechTree+0x20, which SetResearched bumps and + // which the per-node `order` word is read from. + trace::Region g; + g.name = "tree_header"; + g.ptr = tree; + g.size = kTreeGuardSize; + g.kind = trace::Region::Kind::Guard; + out.push_back(g); + } } TechTreeProcessResearchHook::Args TechTreeProcessResearchHook::rebind(trace::Scratch& s, void* tree, diff --git a/src/shim/hooks/research.h b/src/shim/hooks/research.h index bef46d1..5b6621a 100644 --- a/src/shim/hooks/research.h +++ b/src/shim/hooks/research.h @@ -53,6 +53,38 @@ struct TechTreeProcessResearchHook { static Args rebind(trace::Scratch& s, void* tree, void* rng, void* alloc, int* overbudget); static void ours(void* tree, void* rng, void* alloc, int* overbudget); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + // THE B3 DEFECT. Kept as a note even though `events` is now a declared region, because + // ours still does not post the event -- the region makes the gap loud, it does not close + // it. See docs/harness-audit.md. + c.unmodelled("posts EVENT_RESEARCH_OVERBUDGET on the owner's EventStorage in the same " + "branch that sets node.flag = 2", + trace::Risk::High, + "the message text is composed from the tech name, so ours cannot synthesise " + "it; it would have to be posted through the game's own event API. This is " + "the defect that made a clean compare false: replace mode's autosave " + "differed from the oracle by exactly this one event", + "region:events (EvNxID now diverges instead of passing silently)"); + c.unmodelled("posts EVENT_TECHS_UNLOCKED for nodes that became available this turn", + trace::Risk::Medium, + "the trailing unlock loop makes no draw and writes no node, but it does " + "build a names list and post an event", + "region:events"); + c.unmodelled("TechTree::SetResearched on completion: the turn/order stamps, the child " + "unlock cascade, the recursive research of zero-cost children, and the " + "owner's OnTechResearched callback", + trace::Risk::High, + "its own milestone (B2); the callback writes live player state that compare " + "mode must not touch, and it consumes one extra RNG word", + "guard:player, guard:tree_header"); + c.unmodelled("bumps the tree's completion-order counter (TechTree+0x20)", + trace::Risk::Medium, + "part of SetResearched; the per-node `order` word is compared but the " + "counter it comes from was not a region", + "guard:tree_header"); + c.unmodelled("writes a completion line to the game log", + trace::Risk::Low, "log text is not simulation state"); + } }; // Process facts the hook needs (exe base for the RVAs, a line logger). Call once before diff --git a/src/shim/hooks/tech_effects.cpp b/src/shim/hooks/tech_effects.cpp index 19b7175..10feaf3 100644 --- a/src/shim/hooks/tech_effects.cpp +++ b/src/shim/hooks/tech_effects.cpp @@ -31,6 +31,11 @@ namespace { namespace A = sots::addr; +// Whole-object guard span. ServerPlayer is 0x3e0 bytes; its inline EventStorage (the completion +// events) sits at +0x29c. Neither is in the generated header -- both come from the recovered +// object table in sots-re/findings/ (turn-spine.md and objects/struct-recovery.md). +constexpr std::size_t kPlayerGuardSize = 0x3e0; + // The two game entry points `ours` leans on. Both are verified, read-only reads of the // tech tree: the same delegation B3 makes to TechTree::Cost. using IsTechFn = bool(SHIM_THISCALL*)(void* master, void* def, int techId); @@ -226,6 +231,22 @@ void ServerPlayerOnTechResearchedHook::regions(std::vector& out, g_call.region_of[i] = static_cast(out.size()); out.push_back(r); } + + // ---- guard: pushed LAST so every region_of[] index above is also a Scratch index -------- + // + // The whole ServerPlayer. The thirteen Result regions above are masked out of it, so what + // this reports is exactly the writes this milestone knows it does not model and could not + // otherwise see: the inline EventStorage at +0x29c that the three completion events bump, + // and anything else the callback touches that the region table missed. + // ServerPlayer is 0x3e0 bytes (findings/control-flow/turn-spine.md object table). + if (readable(self, kPlayerGuardSize)) { + trace::Region g; + g.name = "player"; + g.ptr = self; + g.size = kPlayerGuardSize; + g.kind = trace::Region::Kind::Guard; + out.push_back(g); + } } ServerPlayerOnTechResearchedHook::Args ServerPlayerOnTechResearchedHook::rebind(trace::Scratch& s, diff --git a/src/shim/hooks/tech_effects.h b/src/shim/hooks/tech_effects.h index 0239793..c05929a 100644 --- a/src/shim/hooks/tech_effects.h +++ b/src/shim/hooks/tech_effects.h @@ -52,6 +52,35 @@ struct ServerPlayerOnTechResearchedHook { static Args rebind(trace::Scratch& s, void* self, void* def, bool silent); static void ours(void* self, void* def, bool silent); static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("posts EVENT_RESEARCH_COMPLETE / _UNDERBUDGET / _TEMPERANCE on the owner's " + "EventStorage when !silent", + trace::Risk::High, + "the same class of write as B3's defect, and this hook has no replace-mode " + "oracle that could catch it: gotcha 4 in docs/B2.md says a changed save hash " + "on a completion turn is expected and therefore not a finding", + "guard:player (EventStorage is inline at ServerPlayer+0x29c)"); + c.unmodelled("writes every owned system's AI flag (CCC_AIVrus / CCC_AISlv), re-evaluates " + "the arcology civilian cap, cures addiction and clears plague across systems " + "AND ships", + trace::Risk::High, + "writes through pointers to other objects; compare mode must not touch live " + "state, and no region reaches them"); + c.unmodelled("the pending plague-cure roll (ServerPlayer::RollResearchEvent)", + trace::Risk::High, + "it draws exactly one word from the strategic generator unconditionally; " + "running it in compare mode would consume real randomness. The two words it " + "guards are still cleared and the record says whether it would have fired", + "this is the extra draw B3 observed on a completion"); + c.unmodelled("TechTree::SetResearched for the Zuul boarding-pod grant", + trace::Risk::Medium, "it would mutate the live tree, and it recurses"); + c.unmodelled("allocates or frees the node-bore block at ServerPlayer+0x308", + trace::Risk::Medium, + "ours has no allocator the game's runtime could free, so replace mode calls " + "the game's own updater -- which means replace mode never exercises our " + "node-bore selection at all", + "region:node_bore, declared only when the block already exists"); + } }; // Process facts the hook needs (exe base for the RVAs, a line logger). Call once before diff --git a/src/shim/main.cpp b/src/shim/main.cpp index 6a00597..22f9975 100644 --- a/src/shim/main.cpp +++ b/src/shim/main.cpp @@ -253,6 +253,11 @@ void Shim_Init(HMODULE self) { ColonyTurnHook::register_policy(tracer); MoveFleetHook::register_policy(tracer); FleetMovementHook::register_policy(tracer); + // A hook that never stated what it does not check is a defect, not a detail: say so in + // shim.log as well as in the trace's meta line (docs/harness-audit.md). + for (const std::string& h : tracer.unstated_hooks()) + Log("COVERAGE: hook %s registered with no coverage statement -- its compare results are " + "not trustworthy", h.c_str()); char exeSha[65] = {}; if (!shim::trace::sha256_file(exePath, exeSha)) Log("trace: could not hash %s", exePath); if (tracer.open(SHIM_BUILD_ID, exeSha)) { diff --git a/src/shim/trace/emitter.cpp b/src/shim/trace/emitter.cpp index 1e9264a..04fea3d 100644 --- a/src/shim/trace/emitter.cpp +++ b/src/shim/trace/emitter.cpp @@ -342,6 +342,39 @@ std::string canonical(const Tv& v) { return b.str(); } +// ---- coverage -------------------------------------------------------------------------------- + +const char* risk_name(Risk r) { + switch (r) { + case Risk::Low: return "low"; + case Risk::High: return "high"; + case Risk::Medium: break; + } + return "medium"; +} + +Coverage& Coverage::unmodelled(const char* what, Risk risk, const char* why, const char* mitigation) { + CoverageNote n; + n.what = what ? what : ""; + n.why = why ? why : ""; + n.risk = risk; + n.mitigation = mitigation ? mitigation : ""; + notes_.push_back(std::move(n)); + return *this; +} + +Coverage& Coverage::complete(const char* why) { + complete_ = true; + why_ = why ? why : ""; + return *this; +} + +const char* Coverage::state() const { + if (!notes_.empty()) return "partial"; + if (complete_) return "complete"; + return "unstated"; +} + // ---- records ---------------------------------------------------------------------------------------- namespace { @@ -439,6 +472,23 @@ void emit_record(Buf& out, const Record& r) { out.puts(",\"diff\":"); emit_diff(out, r.diff); } + if (r.has_coverage) { + out.puts(",\"coverage\":{\"guards\":["); + for (std::size_t i = 0; i < r.guards.size(); ++i) { + if (i) out.put(','); + emit_esc(out, r.guards[i]); + } + out.puts("],\"undeclared\":["); + for (std::size_t i = 0; i < r.undeclared.size(); ++i) { + const UndeclaredWrite& u = r.undeclared[i]; + if (i) out.put(','); + out.puts("{\"region\":"); + emit_esc(out, u.region); + out.printf(",\"off\":%llu,\"len\":%llu}", static_cast(u.offset), + static_cast(u.length)); + } + out.printf("],\"n\":%llu}", static_cast(r.undeclared_total)); + } if (r.err) { out.puts(",\"err\":"); emit_esc(out, *r.err); @@ -460,7 +510,8 @@ void emit_meta(Buf& out, const Meta& m) { emit_esc(out, m.started); out.printf(",\"inline_max\":%u,\"hooks\":{", m.inline_max); for (std::size_t i = 0; i < m.hooks.size(); ++i) { - const HookPolicy& p = m.hooks[i].second; + const HookPolicy& p = m.hooks[i].second.policy; + const Coverage& c = m.hooks[i].second.coverage; if (i) out.put(','); emit_esc(out, m.hooks[i].first); out.puts(":{\"ftol\":"); @@ -474,7 +525,21 @@ void emit_meta(Buf& out, const Meta& m) { } out.put(']'); } - out.put('}'); + out.printf(",\"coverage\":{\"state\":\"%s\",\"why\":", c.state()); + emit_esc(out, c.complete_why()); + out.puts(",\"unmodelled\":["); + for (std::size_t j = 0; j < c.notes().size(); ++j) { + const CoverageNote& n = c.notes()[j]; + if (j) out.put(','); + out.puts("{\"what\":"); + emit_esc(out, n.what); + out.printf(",\"risk\":\"%s\",\"why\":", risk_name(n.risk)); + emit_esc(out, n.why); + out.puts(",\"mitigation\":"); + emit_esc(out, n.mitigation); + out.put('}'); + } + out.puts("]}}"); } out.puts("}}}\n"); } diff --git a/src/shim/trace/emitter.h b/src/shim/trace/emitter.h index d5d2f7b..cbb0b80 100644 --- a/src/shim/trace/emitter.h +++ b/src/shim/trace/emitter.h @@ -86,8 +86,56 @@ Tv struct_(); Tv json(const char* canonical_text); // caller guarantees section-7 canonical JSON } // namespace tv +// ---- coverage ------------------------------------------------------------------------------- +// +// Every hook descriptor must say, in code, what the original does that the diff cannot see. +// A declared region is a *check*; a coverage note is an *admission*. `meta.hooks[H].coverage` +// carries the admissions into the log so tracecmp.py can print them next to a clean run -- +// a passing compare then always states what it did not compare. +// +// See docs/harness-audit.md. The three states: +// complete the descriptor claims its Result regions cover every write the original makes +// partial the descriptor lists writes it does not model (the honest normal case) +// unstated neither -- a defect: the hook has never been audited +enum class Risk : std::uint8_t { Low, Medium, High }; +const char* risk_name(Risk r); + +struct CoverageNote { + std::string what; // the write the original performs + std::string why; // why it is not declared / not modelled + Risk risk = Risk::Medium; + std::string mitigation; // "" | "guard:" | "region:" | free text +}; + +class Coverage { +public: + // The original does `what`, and no Result region covers it. + Coverage& unmodelled(const char* what, Risk risk, const char* why, const char* mitigation = ""); + // Explicit claim: the declared Result regions cover every write the original performs. + // A Guard region that fires on a hook in this state is a contradiction, and loud. + Coverage& complete(const char* why); + + bool claims_complete() const { return complete_; } + const std::string& complete_why() const { return why_; } + const std::vector& notes() const { return notes_; } + const char* state() const; // "complete" | "partial" | "unstated" + +private: + std::vector notes_; + bool complete_ = false; + std::string why_; +}; + // ---- records ------------------------------------------------------------------------------------ +// A byte run inside a Guard region that the original (or, in replace mode, `ours`) changed and +// that no Result region covers: a side effect the descriptor forgot to declare. +struct UndeclaredWrite { + std::string region; + std::size_t offset = 0; + std::size_t length = 0; +}; + struct SideEntry { std::string name; enum Before : std::uint8_t { Absent, IsNull, Value } before_kind = Absent; @@ -127,6 +175,12 @@ struct Record { int diverged = -1; // <0 omit, 0 false, 1 true bool has_diff = false; std::vector diff; + // Guard coverage. `has_coverage` = at least one Guard region was declared and evaluated for + // this call, so an empty `undeclared` genuinely means "nothing moved outside the checks". + bool has_coverage = false; + std::vector guards; + std::vector undeclared; // capped; `undeclared_total` is the true count + std::size_t undeclared_total = 0; std::optional err; std::optional note; }; @@ -139,12 +193,18 @@ struct HookPolicy { std::vector unordered; // list paths to treat as sets }; +// Everything meta.hooks[] carries: the diff policy and the coverage admission. +struct HookMeta { + HookPolicy policy; + Coverage coverage; +}; + struct Meta { std::string build; std::string exe_sha256; std::string started; unsigned inline_max = 256; - std::vector> hooks; + std::vector> hooks; }; // ---- output buffer -------------------------------------------------------------------------------- diff --git a/src/shim/trace/hook.h b/src/shim/trace/hook.h index c9b7ff0..07aa430 100644 --- a/src/shim/trace/hook.h +++ b/src/shim/trace/hook.h @@ -13,6 +13,7 @@ // static Args rebind(Scratch& s, Table* t, const char* k, int v); // args for `ours` // static int ours(Table* t, const char* k, int v); // the reimplementation // static HookPolicy policy(); // meta.hooks entry +// static void coverage(Coverage& c); // REQUIRED: what it misses // }; // // Hook::detour() is the function to install (MinHook target -> detour) and @@ -24,7 +25,14 @@ // Compare snapshot, call original, snapshot; copy the *before* snapshots into scratch // memory, rebind the args onto the copies, call ours, snapshot the copies, diff, // emit {.., ours, diverged, diff}. The original's result is what the caller gets. -// Replace call ours only; nothing is emitted (nothing to diff against). +// Replace snapshot regions, call ours only, snapshot again, emit {args, ret, side, coverage}. +// There is nothing to diff against, so no `ours`/`diverged`/`diff` — but the record +// exists, which is what lets a replace run be inspected instead of only hashed. +// +// Region kinds (tracer.h): Result regions are the verdict; Guard regions are a coarse span +// watched around the original that reports any write no Result region covers. Every descriptor +// must also define `static void coverage(Coverage&)` naming what it knowingly does not check -- +// it is a compile error not to. See docs/harness-audit.md. // // Nothing escapes the hook boundary: every capture step is wrapped, a throw from ours or // from a describe/regions/rebind callback becomes `err` in the record, and the original is @@ -39,6 +47,7 @@ #include #include +#include #include #include #include @@ -94,6 +103,14 @@ struct RetSlot { std::optional describe() const { return std::nullopt; } }; +// Every descriptor must define `static void coverage(Coverage&)`. This is the compile-time +// half of docs/harness-audit.md: a hook cannot exist without saying what it does not check. +template +struct has_coverage : std::false_type {}; +template +struct has_coverage()))>> + : std::true_type {}; + inline std::string what(const char* stage) { std::string s = stage; try { @@ -114,17 +131,30 @@ class Hook; template class Hook> { + static_assert(detail::has_coverage::value, + "hook descriptor must define `static void coverage(shim::trace::Coverage&)`: " + "list every write the original performs that no declared Result region covers, " + "or call c.complete(\"why\") to claim there are none. See docs/harness-audit.md."); + public: using Ret = typename D::Ret; using Fn = typename FnPtr::type; + // Undeclared-write spans reported per call before the list is truncated (the true count + // still travels in the record's `coverage.n`). + static constexpr std::size_t kMaxUndeclaredSpans = 16; + // Trampoline to the original (MinHook fills it) — or, in host tests, the real function. inline static Fn original = nullptr; inline static Mode mode = Mode::Off; // Reads the mode for D::name from the tracer (Off when the tracer is not open). static void configure(Tracer& t) { mode = t.mode_for(D::name); } - static void register_policy(Tracer& t) { t.register_hook(D::name, D::policy()); } + static void register_policy(Tracer& t) { + Coverage c; + D::coverage(c); + t.register_hook(D::name, D::policy(), c); + } static Fn detour() { if constexpr (D::conv == CallConv::Cdecl) return &detour_cdecl; @@ -135,7 +165,7 @@ public: static Ret dispatch(A... a) { switch (mode) { case Mode::Off: return original(a...); - case Mode::Replace: return D::ours(a...); + case Mode::Replace: return run_replace(a...); case Mode::Trace: return run(a...); case Mode::Compare: return run(a...); } @@ -162,6 +192,9 @@ private: rec.hook = D::name; D::describe_args(rec.args, a...); D::regions(regions, a...); + if (!regions_well_ordered(regions)) + throw std::runtime_error("descriptor bug: a Result region is declared after a " + "Guard region, so scratch indices no longer line up"); for (const Region& r : regions) before.push_back(Snapshot::capture(r)); captured = true; } catch (...) { @@ -174,15 +207,28 @@ private: try { if (captured) { rec.ret = orig_ret.template describe(); + // Result regions become `side` entries (and the state ours runs on); Guard + // regions never leave the hook -- they only answer "did the original write + // anywhere this descriptor did not declare?". + std::vector result_before; for (std::size_t i = 0; i < regions.size(); ++i) { + const Snapshot after = before[i].recapture(regions[i]); + if (regions[i].kind == Region::Kind::Guard) { + rec.has_coverage = true; + rec.guards.push_back(before[i].name); + undeclared_writes(regions[i], before[i], after, regions, rec.undeclared, + rec.undeclared_total, kMaxUndeclaredSpans); + continue; + } SideEntry e; e.name = before[i].name; e.before_kind = SideEntry::Value; e.before = before[i].to_tv(inline_max); - e.after = before[i].recapture(regions[i]).to_tv(inline_max); + e.after = after.to_tv(inline_max); rec.side.push_back(std::move(e)); + result_before.push_back(before[i]); } - if (kCompare) compare(rec, before, inline_max, a...); + if (kCompare) compare(rec, result_before, inline_max, a...); } } catch (...) { rec.err = detail::what("capture after original"); @@ -195,6 +241,77 @@ private: return orig_ret.get(); } + // Replace: `ours` alone, on the live arguments. Historically this emitted nothing, which is + // how B3's replace-mode divergence stayed invisible until the save-file oracle caught it. + // Now, whenever the tracer is open, a replace call records what `ours` wrote into the + // declared regions and what the Guard regions saw it move outside them -- so a replace log + // can be read (and --replay'd) against a compare/trace golden instead of only a save hash. + static Ret run_replace(A... a) { + Tracer& tr = Tracer::instance(); + if (!tr.is_open()) return D::ours(a...); // nothing to record: stay a pure passthrough + + const unsigned inline_max = tr.inline_max(); + Record rec; + std::vector regions; + std::vector before; + bool captured = false; + rec.mode = Mode::Replace; + rec.call_id = tr.next_call_id(); + rec.depth = tr.enter(); + try { + rec.hook = D::name; + D::describe_args(rec.args, a...); + D::regions(regions, a...); + if (!regions_well_ordered(regions)) + throw std::runtime_error("descriptor bug: a Result region is declared after a " + "Guard region, so scratch indices no longer line up"); + for (const Region& r : regions) before.push_back(Snapshot::capture(r)); + captured = true; + } catch (...) { + rec.err = detail::what("capture before ours"); + } + + detail::RetSlot ours_ret; + std::exception_ptr threw; + try { + ours_ret.invoke(D::ours, a...); + } catch (...) { + rec.err = detail::what("ours"); + threw = std::current_exception(); + } + + try { + if (captured) { + if (!threw) rec.ret = ours_ret.template describe(); + for (std::size_t i = 0; i < regions.size(); ++i) { + const Snapshot after = before[i].recapture(regions[i]); + if (regions[i].kind == Region::Kind::Guard) { + rec.has_coverage = true; + rec.guards.push_back(before[i].name); + undeclared_writes(regions[i], before[i], after, regions, rec.undeclared, + rec.undeclared_total, kMaxUndeclaredSpans); + continue; + } + SideEntry e; + e.name = before[i].name; + e.before_kind = SideEntry::Value; + e.before = before[i].to_tv(inline_max); + e.after = after.to_tv(inline_max); + rec.side.push_back(std::move(e)); + } + } + } catch (...) { + rec.err = detail::what("capture after ours"); + } + try { + tr.write(rec); + } catch (...) { + } + tr.leave(); + if (threw) std::rethrow_exception(threw); // replace has no original to fall back on + return ours_ret.get(); + } + static void compare(Record& rec, const std::vector& before, unsigned inline_max, A... a) { Scratch scratch(before); detail::RetSlot ours_ret; diff --git a/src/shim/trace/selftest.cpp b/src/shim/trace/selftest.cpp index 7e0f033..38f9e0b 100644 --- a/src/shim/trace/selftest.cpp +++ b/src/shim/trace/selftest.cpp @@ -38,6 +38,19 @@ std::uint32_t SHIM_CDECL FillThrows(std::uint8_t*, std::uint32_t, std::uint32_t) throw std::runtime_error("selftest: deliberate throw"); } +// Fills the blob's buffer AND bumps a counter the hook's Result region does not cover. +std::uint32_t SHIM_CDECL FillCounted(Blob* b, std::uint32_t n, std::uint32_t seed) { + if (!b) return 0; + ++b->calls; + return Fill(b->buf, n, seed); +} + +// The faithful-looking reimplementation: correct buffer, no counter. This is B3 in miniature. +std::uint32_t SHIM_CDECL FillCountedOurs(Blob* b, std::uint32_t n, std::uint32_t seed) { + if (!b) return 0; + return FillOurs(b->buf, n, seed); +} + void FillHook::describe_args(std::vector& out, std::uint8_t* buf, std::uint32_t n, std::uint32_t seed) { out.push_back(trace::tv::ptr(buf).named("buf")); out.push_back(trace::tv::u32(n).named("n")); @@ -58,6 +71,38 @@ FillHook::Args FillHook::rebind(trace::Scratch& s, std::uint8_t*, std::uint32_t return Args(s.as(0), n, seed); } +void FillGuardHook::describe_args(std::vector& out, Blob* b, std::uint32_t n, std::uint32_t seed) { + out.push_back(trace::tv::ptr(b).named("blob")); + out.push_back(trace::tv::u32(n).named("n")); + out.push_back(trace::tv::u32(seed).named("seed")); +} + +Tv FillGuardHook::describe_ret(std::uint32_t r) { return trace::tv::u32(r); } + +void FillGuardHook::regions(std::vector& out, Blob* b, std::uint32_t n, std::uint32_t) { + trace::Region r; + r.name = "buf"; + r.ptr = b ? b->buf : nullptr; + r.size = n; + out.push_back(r); + + // The whole object. Nothing our side writes lands here; anything the ORIGINAL moves outside + // `buf` comes back as an undeclared write. + trace::Region g; + g.name = "blob"; + g.ptr = b; + g.size = sizeof(Blob); + g.kind = trace::Region::Kind::Guard; + out.push_back(g); +} + +FillGuardHook::Args FillGuardHook::rebind(trace::Scratch& s, Blob*, std::uint32_t n, std::uint32_t seed) { + // Scratch holds Result regions only, so index 0 is `buf` even though a guard was declared. + // `buf` is at offset 0 of a Blob and `ours` writes nothing else, so the copy stands in for + // the object. + return Args(reinterpret_cast(s.ptr(0)), n, seed); +} + std::uint32_t run_once(trace::Mode mode) { using H = trace::Hook; H::original = &Fill; diff --git a/src/shim/trace/selftest.h b/src/shim/trace/selftest.h index 8aac6f6..01b882e 100644 --- a/src/shim/trace/selftest.h +++ b/src/shim/trace/selftest.h @@ -17,6 +17,18 @@ namespace shim::selftest { +// A "game object": the buffer the hook declares, plus a counter next to it that the original +// bumps and that no Result region covers -- the shape of B3's event-list defect, in miniature. +// `buf` first so the declared Result region starts at offset 0: `ours` can then run straight on +// the scratch copy, and `calls` sits at offset 64 -- outside every Result region. +struct Blob { + std::uint8_t buf[64]; + std::uint32_t calls; +}; + +std::uint32_t SHIM_CDECL FillCounted(Blob* b, std::uint32_t n, std::uint32_t seed); +std::uint32_t SHIM_CDECL FillCountedOurs(Blob* b, std::uint32_t n, std::uint32_t seed); + std::uint32_t SHIM_CDECL Fill(std::uint8_t* buf, std::uint32_t n, std::uint32_t seed); std::uint32_t SHIM_CDECL FillOurs(std::uint8_t* buf, std::uint32_t n, std::uint32_t seed); std::uint32_t SHIM_CDECL FillWrong(std::uint8_t* buf, std::uint32_t n, std::uint32_t seed); @@ -34,6 +46,9 @@ struct FillHook { static Args rebind(trace::Scratch& s, std::uint8_t* buf, std::uint32_t n, std::uint32_t seed); static std::uint32_t ours(std::uint8_t* buf, std::uint32_t n, std::uint32_t seed) { return FillOurs(buf, n, seed); } static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.complete("Fill writes buf[0..n) and nothing else; the whole range is a declared region"); + } }; struct FillWrongHook : FillHook { @@ -46,6 +61,33 @@ struct FillThrowsHook : FillHook { static std::uint32_t ours(std::uint8_t* buf, std::uint32_t n, std::uint32_t seed) { return FillThrows(buf, n, seed); } }; +// The B3 shape as a unit-testable fixture: `ours` reproduces the declared region exactly, so the +// diff is clean -- but the original also bumps `Blob::calls`, which only the Guard region sees. +struct FillGuardHook { + static constexpr const char* name = "Shim::SelfTest::FillGuard"; + static constexpr trace::CallConv conv = trace::CallConv::Cdecl; + using Ret = std::uint32_t; + using Args = std::tuple; + + static void describe_args(std::vector& out, Blob* b, std::uint32_t n, std::uint32_t seed); + static trace::Tv describe_ret(std::uint32_t r); + static void regions(std::vector& out, Blob* b, std::uint32_t n, std::uint32_t seed); + static Args rebind(trace::Scratch& s, Blob* b, std::uint32_t n, std::uint32_t seed); + static std::uint32_t ours(Blob* b, std::uint32_t n, std::uint32_t seed) { return FillCountedOurs(b, n, seed); } + static trace::HookPolicy policy() { return trace::HookPolicy{}; } + static void coverage(trace::Coverage& c) { + c.unmodelled("FillCounted bumps Blob::calls", trace::Risk::Medium, + "the fixture exists to prove the guard reports it", "guard:blob"); + } +}; + +// Same fixture, but the descriptor lies: it claims complete coverage while the original still +// bumps the counter. The guard turns that claim into a loud contradiction. +struct FillGuardLyingHook : FillGuardHook { + static constexpr const char* name = "Shim::SelfTest::FillGuardLying"; + static void coverage(trace::Coverage& c) { c.complete("nothing outside buf is written"); } +}; + // Runs FillHook once through its detour (original = Fill) with a 64-byte buffer. // Returns the checksum. `mode` is applied to the hook first. std::uint32_t run_once(trace::Mode mode); diff --git a/src/shim/trace/tracer.cpp b/src/shim/trace/tracer.cpp index 5038f19..33c4784 100644 --- a/src/shim/trace/tracer.cpp +++ b/src/shim/trace/tracer.cpp @@ -89,6 +89,53 @@ Tv Snapshot::to_tv(unsigned inline_max) const { Scratch::Scratch(const std::vector& before) : copies_(before) {} +// ---- guard coverage ---------------------------------------------------------------------------- + +bool regions_well_ordered(const std::vector& regions) { + bool seen_guard = false; + for (const Region& r : regions) { + if (r.kind == Region::Kind::Guard) seen_guard = true; + else if (seen_guard) return false; + } + return true; +} + +void undeclared_writes(const Region& guard, const Snapshot& before, const Snapshot& after, + const std::vector& all, std::vector& out, + std::size_t& total, std::size_t max_spans) { + const std::size_t n = before.data.size() < after.data.size() ? before.data.size() : after.data.size(); + if (!n || !guard.ptr) return; + const std::uintptr_t base = reinterpret_cast(guard.ptr); + + // Bytes of the guard that a Result region already checks: those are declared, not lost. + std::vector covered(n, false); + for (const Region& r : all) { + if (r.kind != Region::Kind::Result || !r.ptr || !r.size) continue; + const std::uintptr_t rb = reinterpret_cast(r.ptr); + const std::uintptr_t lo = rb > base ? rb - base : 0; + if (rb + r.size <= base) continue; + const std::uintptr_t hi = rb + r.size - base; + for (std::size_t i = static_cast(lo); i < n && i < hi; ++i) covered[i] = true; + } + + for (std::size_t i = 0; i < n;) { + if (before.data[i] == after.data[i] || covered[i]) { + ++i; + continue; + } + const std::size_t start = i; + while (i < n && before.data[i] != after.data[i] && !covered[i]) ++i; + ++total; + if (out.size() < max_spans) { + UndeclaredWrite w; + w.region = guard.name; + w.offset = start; + w.length = i - start; + out.push_back(std::move(w)); + } + } +} + // ---- diff ---------------------------------------------------------------------------------------------- namespace { @@ -305,14 +352,24 @@ Tracer& Tracer::instance() { void Tracer::configure(const Config& cfg) { cfg_ = cfg; } -void Tracer::register_hook(const char* name, const HookPolicy& policy) { +void Tracer::register_hook(const char* name, const HookPolicy& policy, const Coverage& coverage) { + HookMeta m; + m.policy = policy; + m.coverage = coverage; for (auto& h : hooks_) { if (h.first == name) { - h.second = policy; + h.second = std::move(m); return; } } - hooks_.emplace_back(name, policy); + hooks_.emplace_back(name, std::move(m)); +} + +std::vector Tracer::unstated_hooks() const { + std::vector out; + for (const auto& h : hooks_) + if (std::strcmp(h.second.coverage.state(), "unstated") == 0) out.push_back(h.first); + return out; } bool Tracer::open(const char* build_id, const char* exe_sha256) { diff --git a/src/shim/trace/tracer.h b/src/shim/trace/tracer.h index 4dd8255..99712fe 100644 --- a/src/shim/trace/tracer.h +++ b/src/shim/trace/tracer.h @@ -43,11 +43,30 @@ struct Config { // A declared region of memory the hooked function may write. `describe` turns the captured // bytes into a structured Tv (so diffs name a field); nullptr = plain `bytes`. +// +// Two kinds, and the difference is the whole point of docs/harness-audit.md: +// +// Result the classic region. Snapshotted before the original, copied into Scratch for +// `ours`, and diffed afterwards. This is a *check*: it is part of the verdict. +// Guard a coarse span (typically the whole object) that is snapshotted before and after +// the ORIGINAL only. It is never copied into Scratch, never handed to `ours` and +// never diffed against it. Any byte inside it that the original moved and that no +// Result region covers is an **undeclared write**: a side effect the descriptor +// forgot to declare, which a clean compare would otherwise never mention. +// (This is what would have caught B3's event-list append.) +// +// Scratch is built from the Result regions in declaration order, so a Guard never occupies a +// Scratch slot. Descriptors that remember a region's index as `out.size()` at push time +// therefore MUST push every Guard after every Result -- `regions_well_ordered()` checks it on +// each call and turns a violation into an `err` rather than a silently shifted scratch mapping. struct Region { + enum class Kind : std::uint8_t { Result, Guard }; + const char* name = ""; const void* ptr = nullptr; std::size_t size = 0; Tv (*describe)(const void* data, std::size_t size, unsigned inline_max) = nullptr; + Kind kind = Kind::Result; }; struct Snapshot { @@ -76,6 +95,20 @@ private: std::vector copies_; }; +// ---- guard coverage -------------------------------------------------------------------------- + +// False when a Result region is declared after a Guard region: the descriptor's recorded +// indices no longer line up with Scratch. See the Region comment above. +bool regions_well_ordered(const std::vector& regions); + +// +// Byte runs inside `guard` that changed between `before` and `after` and that no Region of +// kind Result in `all` covers. Appends at most `max_spans` entries to `out`; `total` is +// incremented by every span found, so a truncated list still reports an honest count. +void undeclared_writes(const Region& guard, const Snapshot& before, const Snapshot& after, + const std::vector& all, std::vector& out, + std::size_t& total, std::size_t max_spans = 16); + // ---- diff ------------------------------------------------------------------------------------------ // Appends diff entries for `a` (original) vs `b` (ours) at `path`; returns true if any. @@ -93,10 +126,13 @@ class Tracer { public: static Tracer& instance(); - // All three before open(): policy goes into the meta record. + // All three before open(): policy and coverage go into the meta record. void configure(const Config& cfg); - void register_hook(const char* name, const HookPolicy& policy); + void register_hook(const char* name, const HookPolicy& policy, const Coverage& coverage); const Config& config() const { return cfg_; } + // Hooks registered without a stated coverage (state() == "unstated"). A non-empty list is + // a structural defect: main.cpp logs it and tracecmp.py reports it on every run. + std::vector unstated_hooks() const; // Opens cfg.path ("w"), writes the meta line. false = could not open (tracing stays off). bool open(const char* build_id, const char* exe_sha256); @@ -119,7 +155,7 @@ public: private: Tracer() = default; Config cfg_; - std::vector> hooks_; + std::vector> hooks_; std::FILE* file_ = nullptr; std::int64_t t0_ = 0; Mutex mu_; diff --git a/tests/shim_trace/CMakeLists.txt b/tests/shim_trace/CMakeLists.txt index 519c02a..786806f 100644 --- a/tests/shim_trace/CMakeLists.txt +++ b/tests/shim_trace/CMakeLists.txt @@ -7,7 +7,7 @@ set(SOTS_TRACECMP_DIR "$ENV{HOME}/sots-re/verify/harness/compare" CACHE PATH set(_out ${CMAKE_CURRENT_BINARY_DIR}/out) file(MAKE_DIRECTORY ${_out}) -foreach(_t sha256 emitter diff hook) +foreach(_t sha256 emitter diff hook coverage) add_executable(shim_trace_test_${_t} test_${_t}.cpp) target_link_libraries(shim_trace_test_${_t} PRIVATE shim_trace) target_compile_options(shim_trace_test_${_t} PRIVATE -Wall -Wextra -Werror) @@ -19,4 +19,8 @@ add_test(NAME shim_trace_sha256 COMMAND shim_trace_test_sha256) add_test(NAME shim_trace_emitter COMMAND shim_trace_test_emitter ${_out}/emitter_oracle.jsonl ${CMAKE_CURRENT_SOURCE_DIR}/oracle_emit.py) add_test(NAME shim_trace_diff COMMAND shim_trace_test_diff) +# Every hook descriptor's coverage declaration, checked on the host (the descriptors themselves +# only compile on the MinGW cross build, so this is where an unaudited hook gets caught). +target_link_libraries(shim_trace_test_coverage PRIVATE shim_techfx) +add_test(NAME shim_trace_coverage COMMAND shim_trace_test_coverage) add_test(NAME shim_trace_hook COMMAND shim_trace_test_hook ${_out}) diff --git a/tests/shim_trace/oracle_emit.py b/tests/shim_trace/oracle_emit.py index ad793ad..12f76c3 100644 --- a/tests/shim_trace/oracle_emit.py +++ b/tests/shim_trace/oracle_emit.py @@ -13,9 +13,18 @@ def build(mk) -> str: inf, nan = float("inf"), float("nan") meta = {"format": 1, "build": "shim-trace test", "exe_sha256": "0" * 64, "started": "2026-09-07T00:00:00Z", "inline_max": 256, - "hooks": {"Hook::A": {"ftol": 0.0, "ftol_kind": "abs", "ptr": "ignore"}, + "hooks": {"Hook::A": {"ftol": 0.0, "ftol_kind": "abs", "ptr": "ignore", + "coverage": {"state": "complete", + "why": "nothing outside the declared regions", + "unmodelled": []}}, "Mars::ParseBlock": {"ftol": 1e-6, "ftol_kind": "rel", "ptr": "exact", - "unordered": ["ret.v.items"]}}} + "unordered": ["ret.v.items"], + "coverage": {"state": "partial", "why": "", + "unmodelled": [ + {"what": "appends to the owner's event list", + "risk": "high", + "why": "no region reaches it", + "mitigation": "guard:player"}]}}}} fwd = bytes(range(16)) recs = [ {"ts": 1000, "hook": "CfgVar_RegisterKey", "mode": "trace", "call_id": 0, "thread": 4120, "depth": 0, @@ -63,7 +72,14 @@ def build(mk) -> str: "diverged": True, "diff": [{"path": "call", "why": "err", "orig": None, "ours": "ours threw"}], "err": "ours threw"}, {"ts": 1222, "hook": "Y", "mode": "replace", "call_id": 6, "thread": 2, - "args": [mk.boolean(False, "flag")], "ret": mk.ptr(0), "side": {}}, + "args": [mk.boolean(False, "flag")], "ret": mk.ptr(0), "side": {}, + "coverage": {"guards": ["player"], "undeclared": [], "n": 0}}, + {"ts": 1259, "hook": "Y", "mode": "compare", "call_id": 7, "thread": 2, + "args": [], "ret": mk.null(), "side": {}, "ours": {"ret": mk.null(), "side": {}}, + "diverged": False, "diff": [], + "coverage": {"guards": ["player", "tree_header"], + "undeclared": [{"region": "player", "off": 672, "len": 12}, + {"region": "player", "off": 688, "len": 4}], "n": 3}}, ] return mk.emit_meta(meta) + "".join(mk.emit_record(r) for r in recs) diff --git a/tests/shim_trace/test_coverage.cpp b/tests/shim_trace/test_coverage.cpp new file mode 100644 index 0000000..f92d3de --- /dev/null +++ b/tests/shim_trace/test_coverage.cpp @@ -0,0 +1,170 @@ +// The harness's own coverage gate (docs/harness-audit.md). +// +// The shim's hook descriptors only reach a compiler on the MinGW cross build, so the +// `static_assert` in Hook<> alone would not stop an unaudited hook from landing on a host-only +// CI run. This test instantiates every descriptor's Hook<> on the host -- which fires that +// static_assert -- and then checks at run time that what each one declared is actually usable: +// +// * every hook states its coverage (never "unstated"); +// * every unmodelled note carries a `what` and a `why` (an empty admission is not one); +// * a note tagged `region:X` / `guard:X` names something, so the mitigation can be checked; +// * the guard machinery reports a write outside the declared regions, and stays silent when +// there is none. +// +// Add a `take<>()` line in collect() when you add a hook; the count assertion in +// game_hooks_admit_their_boundary() fails until you do. +#include +#include +#include +#include +#include + +#include "check.h" +#include "shim/hooks/colony_turn.h" +#include "shim/hooks/compute_budget.h" +#include "shim/hooks/dictionaries.h" +#include "shim/hooks/fleet_movement.h" +#include "shim/hooks/global_consts.h" +#include "shim/hooks/research.h" +#include "shim/hooks/tech_effects.h" +#include "shim/trace/hook.h" +#include "shim/trace/selftest.h" + +using namespace shim::trace; + +namespace { + +struct Row { + const char* name; + Coverage cov; +}; + +std::vector rows; + +// Instantiating Hook is what fires the descriptor's compile-time coverage requirement. +template +void take() { + static_assert(sizeof(Hook) > 0, "descriptor must be usable with Hook<>"); + Row r; + r.name = D::name; + D::coverage(r.cov); + rows.push_back(std::move(r)); +} + +void collect() { + take(); + take(); + take(); + take(); + take(); + take(); + take(); + take(); + take(); + take(); + take(); +} + +void every_hook_states_its_coverage() { + for (const Row& r : rows) { + const std::string state = r.cov.state(); + if (state == "unstated") { + std::printf("FAIL %s: coverage() is empty -- add notes or call complete()\n", r.name); + CHECK(false); + } + if (state == "complete") CHECK(!r.cov.complete_why().empty()); + for (const CoverageNote& n : r.cov.notes()) { + if (n.what.empty() || n.why.empty()) { + std::printf("FAIL %s: a coverage note has an empty what/why\n", r.name); + CHECK(false); + } + // A mitigation that names a region or guard must actually name one. + const bool tagged = n.mitigation.rfind("region:", 0) == 0 || n.mitigation.rfind("guard:", 0) == 0; + if (tagged) CHECK(n.mitigation.find(':') + 1 < n.mitigation.size()); + } + } +} + +// The nine game hooks are all partial by construction: each one models a slice. If one ever +// claims completeness, that is a claim a Guard region has to back up -- say so loudly here. +void game_hooks_admit_their_boundary() { + int partial = 0; + for (const Row& r : rows) { + if (std::strncmp(r.name, "Shim::SelfTest::", 16) == 0) continue; + CHECK(std::strcmp(r.cov.state(), "partial") == 0); + CHECK(!r.cov.notes().empty()); + ++partial; + } + CHECK_EQ(partial, 9); // bump this when a hook is added, and audit it first +} + +// The B3 shape: a Result region that ours reproduces exactly, next to a word only the original +// writes. The diff is clean; the guard is not. +void guard_reports_the_undeclared_write() { + using H = Hook; + H::original = &shim::selftest::FillCounted; + + shim::selftest::Blob live{}; + std::vector regions; + shim::selftest::FillGuardHook::regions(regions, &live, 16, 5); + CHECK_EQ(regions.size(), static_cast(2)); + CHECK(regions[0].kind == Region::Kind::Result); + CHECK(regions[1].kind == Region::Kind::Guard); + + const Snapshot before = Snapshot::capture(regions[1]); + shim::selftest::FillCounted(&live, 16, 5); + const Snapshot after = Snapshot::capture(regions[1]); + + std::vector out; + std::size_t total = 0; + undeclared_writes(regions[1], before, after, regions, out, total); + // buf[0..16) is a declared Result region and is masked out; `calls` at offset 64 is not. + CHECK_EQ(total, static_cast(1)); + CHECK_EQ(out.size(), static_cast(1)); + CHECK_STR(out[0].region, "blob"); + CHECK_EQ(out[0].offset, offsetof(shim::selftest::Blob, calls)); + CHECK(out[0].length >= 1 && out[0].length <= 4); + + // No movement at all -> nothing reported (a guard must not cry wolf). + std::vector quiet; + std::size_t none = 0; + undeclared_writes(regions[1], after, after, regions, quiet, none); + CHECK_EQ(none, static_cast(0)); + CHECK(quiet.empty()); +} + +// Truncation still reports an honest total. +void guard_truncates_but_counts() { + std::uint8_t a[64] = {}; + std::uint8_t b[64] = {}; + for (int i = 0; i < 64; i += 2) b[i] = 1; // 32 one-byte runs + + Region g; + g.name = "span"; + g.ptr = a; + g.size = sizeof a; + g.kind = Region::Kind::Guard; + Snapshot before = Snapshot::capture(g); + Region gb = g; + gb.ptr = b; + Snapshot after = Snapshot::capture(gb); + after.name = before.name; + + std::vector all{g}; + std::vector out; + std::size_t total = 0; + undeclared_writes(g, before, after, all, out, total, 4); + CHECK_EQ(out.size(), static_cast(4)); + CHECK_EQ(total, static_cast(32)); +} + +} // namespace + +int main() { + collect(); + every_hook_states_its_coverage(); + game_hooks_admit_their_boundary(); + guard_reports_the_undeclared_write(); + guard_truncates_but_counts(); + return tracetest::finish("shim_trace_coverage"); +} diff --git a/tests/shim_trace/test_emitter.cpp b/tests/shim_trace/test_emitter.cpp index e4c39e0..49cd0cf 100644 --- a/tests/shim_trace/test_emitter.cpp +++ b/tests/shim_trace/test_emitter.cpp @@ -209,19 +209,50 @@ static void golden_records() { pb2.ftol_kind = "rel"; pb2.ptr_exact = true; pb2.unordered = {"ret.v.items"}; - m.hooks.emplace_back("Hook::A", pa); - m.hooks.emplace_back("Mars::ParseBlock", pb2); + HookMeta ha; + ha.policy = pa; + ha.coverage.complete("nothing outside the declared regions"); + HookMeta hb; + hb.policy = pb2; + hb.coverage.unmodelled("appends to the owner's event list", Risk::High, "no region reaches it", + "guard:player"); + m.hooks.emplace_back("Hook::A", ha); + m.hooks.emplace_back("Mars::ParseBlock", hb); Buf mb; emit_meta(mb, m); CHECK_STR(mb.str(), "{\"meta\":{\"format\":1,\"build\":\"sots-engine test\",\"exe_sha256\":\"" + std::string(64, '0') + "\",\"started\":\"2026-09-07T00:00:00Z\",\"inline_max\":256,\"hooks\":{" - "\"Hook::A\":{\"ftol\":0,\"ftol_kind\":\"abs\",\"ptr\":\"ignore\"}," - "\"Mars::ParseBlock\":{\"ftol\":9.9999999999999995e-07,\"ftol_kind\":\"rel\",\"ptr\":\"exact\",\"unordered\":[\"ret.v.items\"]}}}}\n"); + "\"Hook::A\":{\"ftol\":0,\"ftol_kind\":\"abs\",\"ptr\":\"ignore\"," + "\"coverage\":{\"state\":\"complete\",\"why\":\"nothing outside the declared regions\"," + "\"unmodelled\":[]}}," + "\"Mars::ParseBlock\":{\"ftol\":9.9999999999999995e-07,\"ftol_kind\":\"rel\",\"ptr\":\"exact\",\"unordered\":[\"ret.v.items\"]," + "\"coverage\":{\"state\":\"partial\",\"why\":\"\",\"unmodelled\":[{" + "\"what\":\"appends to the owner's event list\",\"risk\":\"high\"," + "\"why\":\"no region reaches it\",\"mitigation\":\"guard:player\"}]}}}}}\n"); Meta m0; Buf m0b; emit_meta(m0b, m0); CHECK_STR(m0b.str(), "{\"meta\":{\"format\":1,\"build\":\"\",\"exe_sha256\":\"\",\"started\":\"\",\"inline_max\":256,\"hooks\":{}}}\n"); + + // The coverage block on a record: guards present, one undeclared span, honest total. + Record cv; + cv.hook = "Hook::A"; + cv.mode = Mode::Compare; + cv.call_id = 9; + cv.thread = 3; + cv.ts = 30; + cv.has_coverage = true; + cv.guards.push_back("player"); + cv.undeclared.push_back(UndeclaredWrite{"player", 0x2b0, 4}); + cv.undeclared_total = 2; + Buf cvb; + emit_record(cvb, cv); + CHECK_STR(cvb.str(), + "{\"ts\":30,\"hook\":\"Hook::A\",\"mode\":\"compare\",\"call_id\":9,\"thread\":3," + "\"args\":[],\"ret\":null,\"side\":{}," + "\"coverage\":{\"guards\":[\"player\"]," + "\"undeclared\":[{\"region\":\"player\",\"off\":688,\"len\":4}],\"n\":2}}\n"); } static void buf_growth() { @@ -262,8 +293,15 @@ static void write_oracle_fixture(const std::string& path) { pb.ftol_kind = "rel"; pb.ptr_exact = true; pb.unordered = {"ret.v.items"}; - m.hooks.emplace_back("Hook::A", pa); - m.hooks.emplace_back("Mars::ParseBlock", pb); + HookMeta ha; + ha.policy = pa; + ha.coverage.complete("nothing outside the declared regions"); + HookMeta hb; + hb.policy = pb; + hb.coverage.unmodelled("appends to the owner's event list", Risk::High, "no region reaches it", + "guard:player"); + m.hooks.emplace_back("Hook::A", ha); + m.hooks.emplace_back("Mars::ParseBlock", hb); emit_meta(out, m); { // rec0: str/i32/ptr args, bool ret, inline bytes region before/after @@ -384,11 +422,29 @@ static void write_oracle_fixture(const std::string& path) { r.err = "ours threw"; emit_record(out, r); } - { // rec6: replace + { // rec6: replace, now with a coverage block (guards seen, nothing undeclared moved) Record r; r.ts = 1222; r.hook = "Y"; r.mode = Mode::Replace; r.call_id = 6; r.thread = 2; r.args.push_back(tv::boolean(false).named("flag")); r.ret = tv::ptr(static_cast(0)); + r.has_coverage = true; + r.guards.push_back("player"); + emit_record(out, r); + } + { // rec7: compare with undeclared writes the guard caught + Record r; + r.ts = 1259; r.hook = "Y"; r.mode = Mode::Compare; r.call_id = 7; r.thread = 2; + r.ret = tv::null(); + r.has_ours = true; + r.ours_ret = tv::null(); + r.diverged = 0; + r.has_diff = true; + r.has_coverage = true; + r.guards.push_back("player"); + r.guards.push_back("tree_header"); + r.undeclared.push_back(UndeclaredWrite{"player", 672, 12}); + r.undeclared.push_back(UndeclaredWrite{"player", 688, 4}); + r.undeclared_total = 3; emit_record(out, r); } CHECK(out.ok()); diff --git a/tests/shim_trace/test_hook.cpp b/tests/shim_trace/test_hook.cpp index ed3f284..ea479e1 100644 --- a/tests/shim_trace/test_hook.cpp +++ b/tests/shim_trace/test_hook.cpp @@ -97,7 +97,7 @@ static void clean_log(const std::string& path) { CHECK_EQ(HFill::detour()(buf, 64, 10), reference(64, 10)); CHECK_EQ(HFill::detour()(buf, 0, 11), reference(0, 11)); // empty region HFill::mode = Mode::Replace; - CHECK_EQ(HFill::detour()(buf, 16, 12), reference(16, 12)); // replace: nothing emitted + CHECK_EQ(HFill::detour()(buf, 16, 12), reference(16, 12)); // replace: ours' own record // call ids are process-global and increase across threads; depth stays 0 per thread std::thread t([&] { @@ -107,13 +107,17 @@ static void clean_log(const std::string& path) { }); t.join(); tr.close(); - CHECK_EQ(tr.records_written(), 6u); + CHECK_EQ(tr.records_written(), 7u); const std::string text = slurp(path); const auto ls = lines(text); - CHECK_EQ(ls.size(), static_cast(7)); + CHECK_EQ(ls.size(), static_cast(8)); CHECK(has(ls[0], "{\"meta\":{\"format\":1,\"build\":\"shim_trace_test\",\"exe_sha256\":\"aaaa")); - CHECK(has(ls[0], "\"inline_max\":32,\"hooks\":{\"Shim::SelfTest::Fill\":{\"ftol\":0,\"ftol_kind\":\"abs\",\"ptr\":\"ignore\"},\"Shim::SelfTest::FillWrong\":{")); + CHECK(has(ls[0], "\"inline_max\":32,\"hooks\":{\"Shim::SelfTest::Fill\":{\"ftol\":0,\"ftol_kind\":\"abs\",\"ptr\":\"ignore\"," + "\"coverage\":{\"state\":\"complete\",\"why\":\"Fill writes buf[0..n) and nothing else; " + "the whole range is a declared region\",\"unmodelled\":[]}},\"Shim::SelfTest::FillWrong\":{")); + // every registered hook states its coverage, and none is left unstated + CHECK(tr.unstated_hooks().empty()); CHECK(has(ls[1], "\"hook\":\"Shim::SelfTest::Fill\",\"mode\":\"trace\",\"call_id\":")); CHECK(has(ls[1], "\"depth\":0,\"args\":[{\"t\":\"ptr\",\"v\":\"0x")); CHECK(has(ls[1], "\"n\":\"buf\"},{\"t\":\"u32\",\"v\":16,\"n\":\"n\"},{\"t\":\"u32\",\"v\":7,\"n\":\"seed\"}],\"ret\":{\"t\":\"u32\",\"v\":")); @@ -126,10 +130,15 @@ static void clean_log(const std::string& path) { CHECK(has(ls[3], "\"ours\":{\"ret\":{\"t\":\"u32\",\"v\":")); CHECK(has(ls[3], "\"diverged\":false,\"diff\":[]}")); CHECK(has(ls[5], "\"n\":0,\"sha256\":\"e3b0c442")); - CHECK(!has(text, "\"mode\":\"replace\"")); + // replace now records what ours wrote: side present, no ours/diverged/diff to compare against + CHECK(has(ls[6], "\"mode\":\"replace\"")); + CHECK(has(ls[6], "\"v\":12,\"n\":\"seed\"}")); + CHECK(has(ls[6], "\"side\":{\"buf\":{\"before\":{\"t\":\"bytes\"")); + CHECK(!has(ls[6], "\"ours\":")); + CHECK(!has(ls[6], "\"diverged\":")); // the thread's record: different thread id, depth 0, a later call_id than the main thread's - CHECK(has(ls[6], "\"mode\":\"trace\"")); - CHECK(has(ls[6], "\"v\":13,\"n\":\"seed\"}")); + CHECK(has(ls[7], "\"mode\":\"trace\"")); + CHECK(has(ls[7], "\"v\":13,\"n\":\"seed\"}")); // ASCII only, LF only for (char c : text) CHECK(static_cast(c) < 0x80 && c != '\r');