307 lines
23 KiB
Markdown
307 lines
23 KiB
Markdown
# 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<D>` carries a `static_assert` on it, so **a hook without a coverage statement does not
|
||
compile**. The notes go into `meta.hooks.<H>.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.<H>.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.<H>.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.
|