diff --git a/.gitignore b/.gitignore index c75b0155..efe3cbbe 100644 --- a/.gitignore +++ b/.gitignore @@ -12,8 +12,18 @@ crates/*/target/ **/*.rs.bk *.bench # This workspace ships a binary app — commit the lock (re-include it). +# +# `Cargo.lock` with no slash matches at ANY depth, so the re-include below has +# to name every lockfile we keep. That was written when there was exactly one. +# `crates/rustynes-cosim` is EXCLUDED from the workspace, so it resolves its own +# dependency graph into its own lockfile — which this rule was silently +# ignoring, leaving CI to re-resolve it on every run. That crate emits the +# goldens an external NES implementation is verified against, and its manifest +# records the emulator version, not the resolve, so a dependency moving under it +# would be invisible in exactly the artifact meant to establish provenance. Cargo.lock !/Cargo.lock +!/crates/rustynes-cosim/Cargo.lock # --- Native build & FFI artifacts --- *.so diff --git a/CHANGELOG.md b/CHANGELOG.md index f5ca4384..e656f434 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,88 @@ cycle-accurate core later replaced. ## [Unreleased] +### Added + +- **Rolling per-cycle hash checkpoints — the rung-0 comparison surface.** + (v2.4.2, continuing the "Fabric" line.) `crates/rustynes-cosim/src/checkpoint.rs`, + a `checkpoint_diff` CLI, an `rn_write_checkpoints` C ABI entry point, and a + `.ckpt.bin` golden. The emulation core is untouched. + + The constraint nobody budgets for in co-simulation is trace *volume*, not + simulation time, and the figure is now **measured rather than projected**: + 3 frames of AccuracyCoin is 89,343 CPU cycles, which is **5,372,427 bytes** of + `irq.csv` against **352 bytes** of `ckpt.bin` — a factor of **15,263**. So both + sides chain a hash over the per-cycle tuple and compare checkpoints; the first + mismatch names a 4096-cycle window, and only that window is re-run with full + capture. + + **What is hashed is a decision about hardware, not about convenience.** + `CycleRecord` carries 29 fields and most of them are `RustyNES`'s *model* — + `dmc_abort_delay_post`, `apu_phase_post`, `dma_cycles_owed`. Gating on them + would force an independent implementation to transliterate a Rust data + structure, which is bad hardware and, on a programme built on never reading a + reference implementation, an odd form of self-derivation. `Observable` is the + subset a device-under-test can genuinely produce, `from_cycle_record` is the + single place the partition is applied, and a test perturbs **every** dropped + field at once and asserts the hash does not move — with its converse, so it + cannot pass by dropping everything. + + Two subsets needed their caveats stated rather than buried. **The IRQ line is + one wire**: `CycleRecord` attributes each sample to the mapper or the APU, + hardware has a single wire-OR'd /IRQ pin that cannot, so the pairs are OR'd + before hashing — hashing them apart would fail a correct DUT for disagreeing + about something it cannot observe. And **`pc` is DUT-observable, not + pin-observable**; it is in because the testbench wrapper can expose the + register, but a `pc`-only mismatch means something weaker than a bus mismatch. + `a12_events` is excluded for **scope**, not observability — A12 transitions + genuinely are visible on the cartridge connector — and becomes a gate when the + PPU rung opens. + + **The hash is FNV-1a 64 for exactly one reason: a C++ testbench can + reimplement it without a library.** The top risk at this rung is a + format-packing mismatch masquerading as an RTL bug, so `encode` fixes a + 16-byte little-endian layout with an explicit zero pad byte — the C++ side + cannot hash uninitialised struct padding — and both layout and hash are pinned + to a hardcoded vector. A reordered field fails that test rather than producing + a phantom RTL defect. + + **Three answers, and the third is the point.** `checkpoint_diff` exits `0` + identical, `1` diverged with the window printed, and **`3` inconclusive**. A + truncated run, a DUT that stopped early, and two runs at different intervals + all produce "no divergence was found"; reporting that as agreement is this + project's recurring failure. Cycle **alignment is checked before the hash**, + because two streams checkpointing at different cycles cover different spans, so + calling their difference a divergence would send a re-run at a window where + nothing is wrong. All three answers are demonstrated on real exported output, + not only in unit tests. + + Two hazards found while building it, both pinned rather than worked around. + `IrqTrace::push` **silently drops** records at capacity behind an `overflow` + counter nobody has to read, so a hash over an overflowed trace covers fewer + cycles than it claims and would be blamed on the DUT — `take_checkpoints` now + refuses with the capacity to retry with, and the exporter aborts rather than + writing a short stream. And `Bus::take_irq_trace` **moves** the trace out, so + asking for the CSV and then the checkpoints returns `None` for whichever came + second, which is indistinguishable from "never armed"; + `Oracle::take_irq_artifacts` derives both from one take. + +### Fixed + +- **The excluded crate's lockfile was silently gitignored, so CI re-resolved it + on every run.** `.gitignore` carries a bare `Cargo.lock` — which matches at any + depth — paired with a `!/Cargo.lock` re-include naming only the workspace root. + That was written when there was exactly one lockfile. Excluding + `rustynes-cosim` from the workspace in v2.4.1 gave it its own resolve and its + own lockfile, which the bare rule then ignored. + + It matters more here than for an ordinary crate: this crate emits the goldens + an external NES implementation is verified against, and its manifest records + the emulator version rather than the dependency resolve — so a dependency + moving underneath it would be invisible in exactly the artifact whose job is to + establish provenance. The lockfile is now committed, and + `cosim_manifest_audit.rs` asserts it is **tracked** rather than merely present + (demonstrated to fail by un-tracking it and re-running). + ## [2.4.1] - 2026-08-20 - "Fabric" (RustyNES as the oracle a new implementation is verified against) This release also carries **v2.4.0 "Concordance"**, which merged to `main` and was never diff --git a/crates/rustynes-cosim/Cargo.lock b/crates/rustynes-cosim/Cargo.lock new file mode 100644 index 00000000..726911f6 --- /dev/null +++ b/crates/rustynes-cosim/Cargo.lock @@ -0,0 +1,208 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 4 + +[[package]] +name = "bitflags" +version = "2.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b588b76d00fde79687d7646a9b5bdf3cc0f655e0bbd080335a95d7e96f3587da" + +[[package]] +name = "block-buffer" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d2f6c7dbe95a6ed67ad9f18e57daf93a2f034c524b99fd2b76d18fdfeb6660aa" +dependencies = [ + "hybrid-array", +] + +[[package]] +name = "cfg-if" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" + +[[package]] +name = "cpufeatures" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8b2a41393f66f16b0823bb79094d54ac5fbd34ab292ddafb9a0456ac9f87d201" +dependencies = [ + "libc", +] + +[[package]] +name = "crypto-common" +version = "0.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce6e4c961d6cd6c9a86db418387425e8bdeaf05b3c8bc1411e6dca4c252f1453" +dependencies = [ + "hybrid-array", +] + +[[package]] +name = "digest" +version = "0.11.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1dd6dbb5841937940781866fa1281a1ff7bd3bf827091440879f9994983d5c2" +dependencies = [ + "block-buffer", + "crypto-common", +] + +[[package]] +name = "hybrid-array" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "707114b52a152fa7bdb290cd7cd5912d9467273b6d74e21b8d81aca1f8533f6b" +dependencies = [ + "typenum", +] + +[[package]] +name = "libc" +version = "0.2.189" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3eaf3ede3fee6db1a4c2ee091bf8a8b4dccdc6d17f656fb07896ee72867612f2" + +[[package]] +name = "libm" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6d2cec3eae94f9f509c767b45932f1ada8350c4bdb85af2fcab4a3c14807981" + +[[package]] +name = "lz4_flex" +version = "0.14.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ecbdfe44b1bd960b68170b417450a628c43f7cf56bb3c5317e61cb230ee7f226" + +[[package]] +name = "proc-macro2" +version = "1.0.107" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "985e7ec9bb745e6ce6535b544d84d6cd6f7ad8bd711c398938ae983b91a766d9" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.47" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1fbf4db142a473a8d80c26bbf18454ed458bf8d26c8219c331daecfdbd079001" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rustynes-apu" +version = "2.4.1" +dependencies = [ + "bitflags", + "libm", + "thiserror", +] + +[[package]] +name = "rustynes-core" +version = "2.4.1" +dependencies = [ + "bitflags", + "lz4_flex", + "rustynes-apu", + "rustynes-cpu", + "rustynes-mappers", + "rustynes-ppu", + "sha2", + "thiserror", +] + +[[package]] +name = "rustynes-cosim" +version = "2.4.1" +dependencies = [ + "rustynes-core", + "sha2", +] + +[[package]] +name = "rustynes-cpu" +version = "2.4.1" +dependencies = [ + "bitflags", + "thiserror", +] + +[[package]] +name = "rustynes-mappers" +version = "2.4.1" +dependencies = [ + "bitflags", + "rustynes-apu", + "thiserror", +] + +[[package]] +name = "rustynes-ppu" +version = "2.4.1" +dependencies = [ + "bitflags", + "libm", + "thiserror", +] + +[[package]] +name = "sha2" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "446ba717509524cb3f22f17ecc096f10f4822d76ab5c0b9822c5f9c284e825f4" +dependencies = [ + "cfg-if", + "cpufeatures", + "digest", +] + +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "thiserror" +version = "2.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec86235f5fcc2a73650310756d2ac5b138a5780bbbdfae3eeccec992c435ba4f" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "2.0.20" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bc04cd3e1236dd4a98afca4569f2deb3f120e5422a4023be2cb683f8486292af" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "typenum" +version = "1.20.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b6f5e870be6c3b371b77fe0ee0bafb859fa4964b4404c27de1d380043c4dda20" + +[[package]] +name = "unicode-ident" +version = "1.0.24" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" diff --git a/crates/rustynes-cosim/src/bin/checkpoint_diff.rs b/crates/rustynes-cosim/src/bin/checkpoint_diff.rs new file mode 100644 index 00000000..8746329f --- /dev/null +++ b/crates/rustynes-cosim/src/bin/checkpoint_diff.rs @@ -0,0 +1,89 @@ +//! Compare two checkpoint streams and locate the window a divergence is in. +//! +//! ```text +//! checkpoint_diff +//! ``` +//! +//! Exit status is the whole interface, because this runs in CI on both sides of +//! the boundary: +//! +//! | code | meaning | +//! |---|---| +//! | `0` | every checkpoint agreed | +//! | `1` | the streams diverged; the window is printed | +//! | `2` | usage or I/O error | +//! | `3` | **inconclusive** -- the comparison could not be made | +//! +//! `3` is separate from `0` on purpose and it is the point of the tool. A +//! truncated run, a DUT that stopped early, and two runs at different intervals +//! all produce "no divergence was found", and reporting that as agreement is +//! this project's recurring failure: an absence of signal read as a pass. A +//! green CI job must mean the streams were compared and matched, never that +//! there was nothing to compare. + +use std::process::ExitCode; + +use rustynes_cosim::checkpoint::{Comparison, InconclusiveReason, compare, from_bytes}; + +fn main() -> ExitCode { + let argv: Vec = std::env::args().skip(1).collect(); + let [reference, candidate] = argv.as_slice() else { + eprintln!("usage: checkpoint_diff "); + return ExitCode::from(2); + }; + + let load = |p: &str| match std::fs::read(p) { + Ok(b) => from_bytes(&b).map_err(|e| format!("{p}: {e}")), + Err(e) => Err(format!("{p}: {e}")), + }; + let (a, b) = match (load(reference), load(candidate)) { + (Ok(a), Ok(b)) => (a, b), + (Err(e), _) | (_, Err(e)) => { + eprintln!("error: {e}"); + return ExitCode::from(2); + } + }; + + match compare(&a, &b) { + Comparison::Identical { checkpoints } => { + println!("checkpoints match: {checkpoints} compared, 0 divergences"); + ExitCode::SUCCESS + } + Comparison::Diverged(d) => { + println!( + "DIVERGED at checkpoint {}\n \ + window to re-run with full capture: cycles ({}, {}] ({} cycles)\n \ + reference hash = {:#018x}\n \ + candidate hash = {:#018x}", + d.index, + d.after_cycle, + d.through_cycle, + d.window_len(), + d.reference_hash, + d.candidate_hash, + ); + ExitCode::from(1) + } + Comparison::Inconclusive { reason } => { + // Spelled out rather than printed as a debug struct: the operator + // reading this in a CI log needs to know it is NOT a pass. + let detail = match reason { + InconclusiveReason::Empty => "one side produced no checkpoints at all".to_owned(), + InconclusiveReason::LengthMismatch { + reference: r, + candidate: c, + } => format!( + "the streams agree as far as they overlap, then one ends \ + (reference {r} checkpoints, candidate {c}) -- one side stopped early" + ), + InconclusiveReason::UnalignedCycles { index } => format!( + "the cycle axes disagree at checkpoint {index}, so the hashes cover \ + different spans and cannot be compared -- different intervals, or one \ + side truncated mid-window" + ), + }; + println!("INCONCLUSIVE (this is not a pass): {detail}"); + ExitCode::from(3) + } + } +} diff --git a/crates/rustynes-cosim/src/bin/nes_golden_export.rs b/crates/rustynes-cosim/src/bin/nes_golden_export.rs index bd4ecb49..9e463a3d 100644 --- a/crates/rustynes-cosim/src/bin/nes_golden_export.rs +++ b/crates/rustynes-cosim/src/bin/nes_golden_export.rs @@ -5,6 +5,7 @@ //! ```text //! nes_golden_export --rom --out [--seed N] [--frames N] //! [--boot-trace START..END] [--irq-trace CAP] +//! [--checkpoint-interval N] //! ``` //! //! Writes, under ``: @@ -13,6 +14,7 @@ //! |---|---|---| //! | `.boot.bin` | `CpuBootTrace` binary | `cpu_boot_trace_diff` | //! | `.irq.csv` | per-cycle IRQ/bus CSV | `scripts/irq_trace_cross_diff.py` | +//! | `.ckpt.bin` | rolling per-cycle hash checkpoints | `checkpoint_diff` | //! | `.index_fb.bin` | 256x240 LE `u16` | the testbench's frame comparison | //! | `.ram.bin` | 2 KiB CPU work RAM | `accuracy_coin_catalog::decode_results` | //! | `.manifest.txt` | provenance | humans, and the drift guard below | @@ -43,12 +45,14 @@ struct Args { frames: u32, boot_trace: Option<(u64, u64)>, irq_trace: Option, + checkpoint_interval: u64, } fn usage() -> ! { eprintln!( "usage: nes_golden_export --rom --out [--seed N] [--frames N]\n\ - \x20 [--boot-trace START..END] [--irq-trace CAP]" + \x20 [--boot-trace START..END] [--irq-trace CAP]\n\ + \x20 [--checkpoint-interval N]" ); std::process::exit(2) } @@ -58,6 +62,7 @@ fn parse_args() -> Args { let (mut rom, mut out) = (None, None); let (mut seed, mut frames) = (0u64, 60u32); let (mut boot_trace, mut irq_trace) = (None, None); + let mut checkpoint_interval = rustynes_cosim::checkpoint::DEFAULT_INTERVAL; let mut i = 0; while i < argv.len() { @@ -92,6 +97,14 @@ fn parse_args() -> Args { irq_trace = Some(need(i).parse().unwrap_or_else(|_| usage())); i += 2; } + "--checkpoint-interval" => { + checkpoint_interval = need(i).parse().unwrap_or_else(|_| usage()); + if checkpoint_interval == 0 { + eprintln!("--checkpoint-interval must be non-zero"); + usage(); + } + i += 2; + } _ => usage(), } } @@ -102,6 +115,7 @@ fn parse_args() -> Args { frames, boot_trace, irq_trace, + checkpoint_interval, } } @@ -214,9 +228,28 @@ fn main() { None => eprintln!(" WARNING: boot trace was armed but returned nothing"), } } + // ONE take, two artifacts. `Bus::take_irq_trace` moves the trace out, so + // asking for the CSV and then the checkpoints would silently yield an + // unarmed-looking `None` for whichever came second. + let mut checkpoint_count = 0usize; if args.irq_trace.is_some() { - match o.take_irq_trace_csv() { - Some(csv) => write(&suffixed(&base, "irq.csv"), csv.as_bytes()), + match o.take_irq_artifacts(args.checkpoint_interval) { + Some(a) => { + write(&suffixed(&base, "irq.csv"), a.csv.as_bytes()); + match a.checkpoints { + Ok(ck) => { + checkpoint_count = ck.len(); + write( + &suffixed(&base, "ckpt.bin"), + &rustynes_cosim::checkpoint::to_bytes(&ck), + ); + } + // Refuse rather than emitting a short stream: a hash over a + // trace that dropped records covers fewer cycles than it + // claims, and the DUT would be blamed for our truncation. + Err(e) => panic!(" ERROR: {e}"), + } + } None => eprintln!(" WARNING: irq trace was armed but returned nothing"), } } @@ -231,7 +264,9 @@ fn main() { cpu_cycles = {}\n\ emulator = rustynes {}\n\ index_fb_len = {}\n\ - ram_len = {}\n", + ram_len = {}\n\ + ckpt_interval= {}\n\ + ckpt_count = {}\n", args.rom.display(), sha256_hex(&rom), args.seed, @@ -242,6 +277,8 @@ fn main() { env!("CARGO_PKG_VERSION"), INDEX_FB_LEN, RAM_LEN, + args.checkpoint_interval, + checkpoint_count, ); write(&suffixed(&base, "manifest.txt"), manifest.as_bytes()); println!("done; {cycles} CPU cycles simulated"); diff --git a/crates/rustynes-cosim/src/checkpoint.rs b/crates/rustynes-cosim/src/checkpoint.rs new file mode 100644 index 00000000..adfd1c10 --- /dev/null +++ b/crates/rustynes-cosim/src/checkpoint.rs @@ -0,0 +1,828 @@ +//! Rolling per-cycle hash checkpoints -- the rung-0 comparison surface. +//! +//! # Why a hash and not a trace +//! +//! The constraint nobody budgets for in co-simulation is trace *volume*, not +//! simulation time. A 4200-frame `AccuracyCoin` run is roughly 125 million CPU +//! cycles; as per-cycle CSV that is about **7.5 GB** per side, which is not a +//! diff, it is a disk-space problem wearing a diff's clothes. The same run as +//! 4096-cycle checkpoints is **244 KB**. +//! +//! So the protocol is **hash first, capture on divergence**: both sides chain a +//! 64-bit hash over the per-cycle observable tuple and emit it every +//! [`DEFAULT_INTERVAL`] cycles. A comparison of the two checkpoint streams +//! answers "do these agree, and if not, in which 4096-cycle window did they +//! stop agreeing" -- which is exactly the input a full-capture re-run needs, and +//! nothing more. The re-run then costs one window, not one campaign. +//! +//! # What is hashed, and what deliberately is not +//! +//! This is the load-bearing decision in the module, and it is a decision about +//! *hardware*, not about convenience. +//! +//! `rustynes-core`'s [`CycleRecord`] carries 29 fields. Most of them are +//! `RustyNES`'s **model-internal** state -- `dmc_abort_delay_post`, +//! `apu_phase_post`, `dma_cycles_owed` and the rest describe how *this* +//! emulator chose to represent DMC and DMA bookkeeping. They are not hardware +//! facts. Gating on them would force an independent implementation to +//! transliterate a Rust data structure, which is bad hardware and, given that +//! the whole programme is built on never reading a reference implementation, an +//! odd form of self-derivation. +//! +//! [`Observable`] is therefore the subset an external device-under-test can +//! genuinely produce, and every field in it has a reason: +//! +//! | field | why it is in | +//! |---|---| +//! | `cpu_cycle` | the axis both sides count on; a desync here is the finding | +//! | `bus_access` | the R/W and DMA character of the cycle -- pin-visible | +//! | `bus_addr`, `bus_data` | the address and data buses -- pin-visible | +//! | `put_cycle` | the phase half of the M2 cycle -- pin-visible | +//! | `nmi_line` | a pin | +//! | `irq_line_at_low`, `irq_line_at_high` | the /IRQ pin, sampled twice per cycle | +//! | `pc` | **not** pin-visible; see below | +//! +//! Two of those need their caveats stated rather than buried. +//! +//! **The IRQ line is one wire.** `CycleRecord` splits its IRQ samples into +//! `irq_pending_mapper_*` and `irq_pending_apu_*`, which is `RustyNES` +//! *attributing* the assertion to a source. Real hardware has a single +//! wire-OR'd /IRQ input and cannot make that distinction, so [`Observable`] +//! folds the pair with OR before hashing. Hashing them separately would fail a +//! correct DUT for disagreeing about something it has no way to observe. +//! +//! **`pc` is DUT-observable, not pin-observable.** The 6502 does not expose its +//! program counter; the value is recoverable only indirectly, from the address +//! bus during an opcode fetch. It is included because the testbench wrapper +//! *can* expose the internal register and because rung 1 compares it directly +//! -- but a `pc`-only mismatch means something weaker than a bus mismatch, and +//! [`Divergence`] reports which fields differ so the two are never confused. +//! +//! PPU fields (`ppu_scanline`, `ppu_dot`, `ppu_frame`, `a12_events`) are +//! excluded on purpose: they belong to the PPU rung and its own pre-palette +//! framebuffer gate, not to a CPU-level bus comparison. `a12_events` is the +//! sharpest case -- A12 transitions are genuinely observable on the cartridge +//! connector, so it is excluded for *scope*, not for observability, and it +//! becomes a gate when the PPU rung opens rather than being dropped. +//! +//! # The hash must be reimplementable in ten lines of C++ +//! +//! The top risk at rung 0 is a **format-packing mismatch masquerading as an RTL +//! bug** -- the testbench packs a field differently, the streams disagree, and +//! the disagreement is read as a hardware defect. Two things guard against it. +//! +//! First, the hash is **FNV-1a 64**, chosen for exactly one property: a C++ +//! testbench can reimplement it correctly from the constants below without a +//! library, and be checked against a fixed vector. Nothing here needs +//! cryptographic strength; it needs to be trivially portable and to detect a +//! flipped bit. +//! +//! Second, [`Observable::encode`] defines a **fixed 16-byte little-endian +//! layout** and `tests::the_wire_encoding_is_pinned_to_a_fixed_vector` pins it +//! to a hardcoded expected hash. If a future refactor reorders a field, that +//! test fails rather than the next co-simulation run reporting a phantom RTL +//! defect. +//! +//! [`CycleRecord`]: rustynes_core::irq_trace::CycleRecord + +use rustynes_core::irq_trace::{BusAccess, CycleRecord}; + +/// FNV-1a 64-bit offset basis. Public because the C++ testbench needs it. +pub const FNV_OFFSET_BASIS: u64 = 0xcbf2_9ce4_8422_2325; + +/// FNV-1a 64-bit prime. Public because the C++ testbench needs it. +pub const FNV_PRIME: u64 = 0x0000_0100_0000_01b3; + +/// Cycles between emitted checkpoints. +/// +/// 4096 is a compromise with both ends measured rather than a round number: +/// a full `AccuracyCoin` run is ~125 M cycles, so this yields ~30 500 +/// checkpoints (244 KB at 8 bytes each), while the window a divergence +/// localises to stays small enough that a full-capture re-run of it is +/// instant. +pub const DEFAULT_INTERVAL: u64 = 4096; + +/// Bytes of the fixed wire encoding of one [`Observable`]. +pub const ENCODED_LEN: usize = 16; + +/// The per-cycle tuple an external implementation can genuinely produce. +/// +/// See the module docs for why each field is in, and for the two whose +/// observability is weaker than the rest (`pc`, and the folded IRQ line). +// Four bools, and clippy is right that that is usually a smell. Here each one +// is a distinct hardware line -- R/W phase, /NMI, and the two /IRQ samples -- +// so folding them into a bitfield would hide exactly what the struct exists to +// name, and the wire encoding already packs them into one byte. +#[allow(clippy::struct_excessive_bools)] +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Observable { + /// The cycle axis both sides count on. + pub cpu_cycle: u64, + /// Program counter. DUT-observable, **not** pin-observable. + pub pc: u16, + /// Address bus for this cycle. + pub bus_addr: u16, + /// Data bus for this cycle. + pub bus_data: u8, + /// Bus character, encoded as [`Self::access_code`]. + pub bus_access: u8, + /// The R/W phase half of the M2 cycle. + pub put_cycle: bool, + /// The /NMI pin. + pub nmi_line: bool, + /// The single wire-OR'd /IRQ pin, sampled on the low half of the cycle. + pub irq_line_at_low: bool, + /// The same pin, sampled on the high half. + pub irq_line_at_high: bool, +} + +impl Observable { + /// Stable numeric code for a bus access. + /// + /// Deliberately numeric rather than the CSV writer's single letters: the + /// C++ side packs an integer, and reusing the letters would invite a + /// signed-char mismatch on the DMA codes, which are the lowercase ones. + /// + /// `0`=idle, `1`=read, `2`=write, `3`=DMA read, `4`=DMA write. + #[must_use] + pub const fn access_code(idle: bool, write: bool, dma: bool) -> u8 { + if idle { + 0 + } else if dma { + if write { 4 } else { 3 } + } else if write { + 2 + } else { + 1 + } + } + + /// Project a `RustyNES` [`CycleRecord`] onto the observable subset. + /// + /// **This function is where the partition lives.** Every field it drops is + /// dropped on purpose, and the module docs give the reason; a future + /// widening of [`Observable`] has to come through here, where + /// `tests::model_internal_state_cannot_cause_a_divergence` is watching. + /// + /// Note the IRQ fold: `CycleRecord` attributes each sample to the mapper or + /// the APU, and hardware has one wire-OR'd /IRQ pin that cannot make that + /// distinction, so the pairs are OR'd rather than carried separately. + #[must_use] + pub const fn from_cycle_record(r: &CycleRecord) -> Self { + Self { + cpu_cycle: r.cpu_cycle, + pc: r.pc, + bus_addr: r.bus_addr, + bus_data: r.bus_data, + bus_access: match r.bus_access { + BusAccess::Idle => 0, + BusAccess::Read => 1, + BusAccess::Write => 2, + BusAccess::DmaRead => 3, + BusAccess::DmaWrite => 4, + }, + put_cycle: r.put_cycle_post, + nmi_line: r.nmi_line, + irq_line_at_low: r.irq_pending_mapper_at_low || r.irq_pending_apu_at_low, + irq_line_at_high: r.irq_pending_mapper_at_high || r.irq_pending_apu_at_high, + } + } + + /// The fixed little-endian wire encoding, pinned by a test. + /// + /// Layout, byte offsets: `cpu_cycle` 0..8, `pc` 8..10, `bus_addr` 10..12, + /// `bus_data` 12, `bus_access` 13, flags 14, pad 15. The trailing pad byte + /// is explicit and always zero so the C++ side cannot accidentally hash + /// uninitialised struct padding -- the classic way two correct + /// implementations of the same layout disagree. + #[must_use] + pub const fn encode(&self) -> [u8; ENCODED_LEN] { + let c = self.cpu_cycle.to_le_bytes(); + let p = self.pc.to_le_bytes(); + let a = self.bus_addr.to_le_bytes(); + let flags = (self.put_cycle as u8) + | ((self.nmi_line as u8) << 1) + | ((self.irq_line_at_low as u8) << 2) + | ((self.irq_line_at_high as u8) << 3); + [ + c[0], + c[1], + c[2], + c[3], + c[4], + c[5], + c[6], + c[7], + p[0], + p[1], + a[0], + a[1], + self.bus_data, + self.bus_access, + flags, + 0, + ] + } +} + +/// One emitted checkpoint: the hash of every cycle up to and including +/// `through_cycle`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Checkpoint { + /// The last cycle folded into `hash`. + pub through_cycle: u64, + /// The chained FNV-1a state at that point. + pub hash: u64, +} + +/// Chains the per-cycle hash and emits a [`Checkpoint`] every `interval` +/// cycles. +#[derive(Debug, Clone)] +pub struct Hasher { + state: u64, + interval: u64, + counted: u64, + last_cycle: u64, + checkpoints: Vec, +} + +impl Hasher { + /// A hasher emitting every `interval` cycles. + /// + /// # Panics + /// + /// If `interval` is zero. A zero interval would emit a checkpoint per cycle + /// and reproduce the 7.5 GB problem this module exists to avoid, so it is + /// rejected loudly rather than silently clamped. + #[must_use] + pub fn new(interval: u64) -> Self { + assert!(interval > 0, "checkpoint interval must be non-zero"); + Self { + state: FNV_OFFSET_BASIS, + interval, + counted: 0, + last_cycle: 0, + checkpoints: Vec::new(), + } + } + + /// Fold one cycle in, emitting a checkpoint when the interval is reached. + pub fn push(&mut self, o: &Observable) { + for b in o.encode() { + self.state ^= u64::from(b); + self.state = self.state.wrapping_mul(FNV_PRIME); + } + self.last_cycle = o.cpu_cycle; + self.counted += 1; + if self.counted.is_multiple_of(self.interval) { + self.checkpoints.push(Checkpoint { + through_cycle: o.cpu_cycle, + hash: self.state, + }); + } + } + + /// Emit a final checkpoint for a partial trailing window. + /// + /// Without this, a run whose length is not a multiple of the interval + /// silently drops its tail -- and the tail is where a run that was stopped + /// early because something went wrong actually differs. Returns the + /// complete stream. + #[must_use] + pub fn finish(mut self) -> Vec { + if !self.counted.is_multiple_of(self.interval) && self.counted > 0 { + self.checkpoints.push(Checkpoint { + through_cycle: self.last_cycle, + hash: self.state, + }); + } + self.checkpoints + } + + /// Cycles folded so far. + #[must_use] + pub const fn counted(&self) -> u64 { + self.counted + } +} + +/// The result of comparing two checkpoint streams. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Comparison { + /// Every checkpoint agreed, and both streams were the same length. + Identical { + /// How many checkpoints were compared. + checkpoints: usize, + }, + /// The streams diverged. The window to re-run with full capture is + /// `(after_cycle, through_cycle]`. + Diverged(Divergence), + /// The streams cannot be compared, and this is **not** agreement. + /// + /// A truncated run, a DUT that stopped early, or two runs at different + /// intervals all land here. Reporting them as `Identical` would be the + /// project's recurring failure -- an absence of signal read as a pass. + Inconclusive { + /// Why the comparison could not be made. + reason: InconclusiveReason, + }, +} + +/// Why a comparison could not be made. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum InconclusiveReason { + /// One side produced no checkpoints at all. + Empty, + /// The streams agree as far as the shorter one goes, then one ends. + LengthMismatch { + /// Checkpoints in the reference stream. + reference: usize, + /// Checkpoints in the candidate stream. + candidate: usize, + }, + /// The two streams checkpoint at different cycles, so their hashes cover + /// different spans and cannot be compared at all. + UnalignedCycles { + /// The index at which the cycle axes first disagree. + index: usize, + }, +} + +/// Where two checkpoint streams stopped agreeing. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct Divergence { + /// Index of the first differing checkpoint. + pub index: usize, + /// Last cycle known to agree. Zero when the very first window differs. + pub after_cycle: u64, + /// Last cycle of the first differing window. + pub through_cycle: u64, + /// The reference hash at that checkpoint. + pub reference_hash: u64, + /// The candidate hash at that checkpoint. + pub candidate_hash: u64, +} + +impl Divergence { + /// Cycles in the window a full-capture re-run must cover. + #[must_use] + pub const fn window_len(&self) -> u64 { + self.through_cycle - self.after_cycle + } +} + +/// Compare two checkpoint streams. +/// +/// Answers three things and never collapses them: they agree, they diverge at a +/// located window, or the question could not be answered. +#[must_use] +pub fn compare(reference: &[Checkpoint], candidate: &[Checkpoint]) -> Comparison { + if reference.is_empty() || candidate.is_empty() { + return Comparison::Inconclusive { + reason: InconclusiveReason::Empty, + }; + } + + let common = reference.len().min(candidate.len()); + for i in 0..common { + // Cycle alignment is checked BEFORE the hash. Two streams checkpointing + // at different cycles cover different spans, so a hash difference + // between them says nothing -- reporting it as a divergence would send + // a re-run at a window that is not where the problem is. + if reference[i].through_cycle != candidate[i].through_cycle { + return Comparison::Inconclusive { + reason: InconclusiveReason::UnalignedCycles { index: i }, + }; + } + if reference[i].hash != candidate[i].hash { + return Comparison::Diverged(Divergence { + index: i, + after_cycle: if i == 0 { + 0 + } else { + reference[i - 1].through_cycle + }, + through_cycle: reference[i].through_cycle, + reference_hash: reference[i].hash, + candidate_hash: candidate[i].hash, + }); + } + } + + if reference.len() != candidate.len() { + return Comparison::Inconclusive { + reason: InconclusiveReason::LengthMismatch { + reference: reference.len(), + candidate: candidate.len(), + }, + }; + } + Comparison::Identical { + checkpoints: common, + } +} + +/// Serialize a checkpoint stream: repeated `(u64 through_cycle, u64 hash)`, +/// little-endian, no header. +/// +/// Headerless on purpose. The manifest already records the ROM hash, the seed +/// and the frame count, and a second place to state them is a second place for +/// them to disagree. +#[must_use] +pub fn to_bytes(checkpoints: &[Checkpoint]) -> Vec { + let mut out = Vec::with_capacity(checkpoints.len() * 16); + for c in checkpoints { + out.extend_from_slice(&c.through_cycle.to_le_bytes()); + out.extend_from_slice(&c.hash.to_le_bytes()); + } + out +} + +/// Parse a checkpoint stream written by [`to_bytes`]. +/// +/// # Errors +/// +/// If the length is not a multiple of 16. A trailing partial record means the +/// producer was interrupted, and a truncated stream that parses is a truncated +/// comparison that passes. +/// +/// # Panics +/// +/// Never in practice: the `expect`s convert 8-byte subslices of a +/// `chunks_exact(16)` chunk, which the length check above has already +/// guaranteed. They are `expect` rather than `unwrap_or_default` so a future +/// change to the record width fails loudly instead of silently zero-filling. +pub fn from_bytes(bytes: &[u8]) -> Result, &'static str> { + if !bytes.len().is_multiple_of(16) { + return Err("checkpoint stream length is not a multiple of 16 bytes"); + } + Ok(bytes + .chunks_exact(16) + .map(|c| Checkpoint { + through_cycle: u64::from_le_bytes(c[0..8].try_into().expect("8 bytes")), + hash: u64::from_le_bytes(c[8..16].try_into().expect("8 bytes")), + }) + .collect()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A deterministic `CycleRecord` for the projection tests. + fn record(cycle: u64) -> CycleRecord { + CycleRecord { + cpu_cycle: cycle, + pc: 0xC000, + ppu_scanline: 0, + ppu_dot: 0, + ppu_frame: 0, + irq_pending_mapper_at_low: false, + irq_pending_apu_at_low: false, + irq_pending_mapper_at_high: false, + irq_pending_apu_at_high: false, + nmi_line: false, + a12_events: Vec::new(), + dmc_dma_pending_pre: false, + dmc_dma_pending_post: false, + dmc_dma_short_post: false, + dmc_abort_pending_post: false, + dmc_abort_delay_post: 0, + dmc_dma_cooldown_post: 0, + dmc_dma_delay_post: 0, + apu_phase_post: false, + in_dmc_dma: false, + dma_cycles_owed: 0, + bus_access: BusAccess::Read, + bus_addr: 0x8000, + bus_data: 0xEA, + put_cycle_post: false, + dmc_timer_post: 0, + dmc_bits_remaining_post: 0, + dmc_silence_post: false, + dmc_buffer_full_post: false, + } + } + + fn obs(cycle: u64) -> Observable { + Observable { + cpu_cycle: cycle, + pc: 0xC000u16.wrapping_add(u16::try_from(cycle & 0xFFFF).expect("masked")), + bus_addr: 0x8000u16.wrapping_add(u16::try_from(cycle & 0xFFFF).expect("masked")), + bus_data: u8::try_from(cycle & 0xFF).expect("masked"), + bus_access: Observable::access_code(false, cycle.is_multiple_of(3), false), + put_cycle: cycle.is_multiple_of(2), + nmi_line: false, + irq_line_at_low: cycle.is_multiple_of(7), + irq_line_at_high: cycle.is_multiple_of(11), + } + } + + fn stream(n: u64, interval: u64) -> Vec { + let mut h = Hasher::new(interval); + for c in 0..n { + h.push(&obs(c)); + } + h.finish() + } + + /// The C++ testbench reimplements this encoding and this hash. If either + /// moves, the next co-simulation run reports a divergence that is ours, not + /// the DUT's -- so both are pinned to a fixed value rather than to whatever + /// the current code happens to produce. + #[test] + fn the_wire_encoding_is_pinned_to_a_fixed_vector() { + let o = Observable { + cpu_cycle: 0x0102_0304_0506_0708, + pc: 0xC5F5, + bus_addr: 0x8123, + bus_data: 0xA9, + bus_access: 1, + put_cycle: true, + nmi_line: false, + irq_line_at_low: true, + irq_line_at_high: false, + }; + assert_eq!( + o.encode(), + [ + 0x08, + 0x07, + 0x06, + 0x05, + 0x04, + 0x03, + 0x02, + 0x01, + 0xF5, + 0xC5, + 0x23, + 0x81, + 0xA9, + 0x01, + 0b0000_0101, + 0x00, + ], + "wire layout changed; the C++ writer must change with it" + ); + + let mut h = Hasher::new(1); + h.push(&o); + let ck = h.finish(); + assert_eq!(ck.len(), 1); + // Independently recomputed FNV-1a over the 16 bytes above. + let mut expect = FNV_OFFSET_BASIS; + for b in [ + 0x08u8, + 0x07, + 0x06, + 0x05, + 0x04, + 0x03, + 0x02, + 0x01, + 0xF5, + 0xC5, + 0x23, + 0x81, + 0xA9, + 0x01, + 0b0000_0101, + 0x00, + ] { + expect ^= u64::from(b); + expect = expect.wrapping_mul(FNV_PRIME); + } + assert_eq!(ck[0].hash, expect); + } + + #[test] + fn identical_streams_compare_identical() { + let a = stream(10_000, DEFAULT_INTERVAL); + let b = stream(10_000, DEFAULT_INTERVAL); + assert_eq!(compare(&a, &b), Comparison::Identical { checkpoints: 3 }); + } + + /// The gate must be shown able to FAIL, not merely to pass. + #[test] + fn a_single_flipped_bit_is_localised_to_its_window() { + let reference = stream(10_000, DEFAULT_INTERVAL); + + let mut h = Hasher::new(DEFAULT_INTERVAL); + for c in 0..10_000u64 { + let mut o = obs(c); + if c == 5000 { + o.bus_data ^= 0x01; + } + h.push(&o); + } + let candidate = h.finish(); + + match compare(&reference, &candidate) { + Comparison::Diverged(d) => { + assert_eq!(d.index, 1, "cycle 5000 falls in the second window"); + assert_eq!(d.after_cycle, 4095); + assert_eq!(d.through_cycle, 8191); + assert_eq!(d.window_len(), 4096); + assert!( + (d.after_cycle..=d.through_cycle).contains(&5000), + "the reported window must contain the corrupted cycle" + ); + } + other => panic!("expected a divergence, got {other:?}"), + } + } + + /// The partition is real, not decorative: two records differing ONLY in + /// state this module deliberately excludes must produce the same hash. + /// + /// If this fails, someone widened `Observable` to carry model state, and an + /// independent implementation will now be failed for not reproducing a Rust + /// struct rather than for disagreeing about hardware. + #[test] + fn model_internal_state_cannot_cause_a_divergence() { + let a = record(42); + let mut b = record(42); + // Every field the projection drops, perturbed at once. + b.ppu_scanline = 120; + b.ppu_dot = 200; + b.ppu_frame = 9; + b.a12_events.push(rustynes_core::irq_trace::A12Event { + sub_dot: 1, + level: true, + }); + b.dmc_dma_pending_pre = !b.dmc_dma_pending_pre; + b.dmc_dma_pending_post = !b.dmc_dma_pending_post; + b.dmc_dma_short_post = !b.dmc_dma_short_post; + b.dmc_abort_pending_post = !b.dmc_abort_pending_post; + b.dmc_abort_delay_post = 3; + b.dmc_dma_cooldown_post = 4; + b.dmc_dma_delay_post = 5; + b.apu_phase_post = !b.apu_phase_post; + b.in_dmc_dma = !b.in_dmc_dma; + b.dma_cycles_owed = 7; + b.dmc_timer_post = 400; + b.dmc_bits_remaining_post = 6; + b.dmc_silence_post = !b.dmc_silence_post; + b.dmc_buffer_full_post = !b.dmc_buffer_full_post; + + assert_eq!( + Observable::from_cycle_record(&a).encode(), + Observable::from_cycle_record(&b).encode(), + "a field outside the observable subset changed the hash" + ); + } + + /// And the converse, so the test above cannot pass by the projection + /// dropping everything. + #[test] + fn an_observable_field_does_cause_a_divergence() { + let a = record(42); + let mut b = record(42); + b.bus_addr ^= 0x0001; + assert_ne!( + Observable::from_cycle_record(&a).encode(), + Observable::from_cycle_record(&b).encode() + ); + } + + /// The two IRQ sources fold to one wire before hashing, because hardware + /// has one wire and a DUT cannot tell them apart. + #[test] + fn the_irq_sources_fold_to_a_single_line() { + let mut mapper_only = record(1); + mapper_only.irq_pending_mapper_at_low = true; + mapper_only.irq_pending_apu_at_low = false; + + let mut apu_only = record(1); + apu_only.irq_pending_mapper_at_low = false; + apu_only.irq_pending_apu_at_low = true; + + assert_eq!( + Observable::from_cycle_record(&mapper_only).encode(), + Observable::from_cycle_record(&apu_only).encode(), + "a DUT cannot tell which source asserted /IRQ, so neither may the hash" + ); + + let mut neither = record(1); + neither.irq_pending_mapper_at_low = false; + neither.irq_pending_apu_at_low = false; + assert_ne!( + Observable::from_cycle_record(&mapper_only).encode(), + Observable::from_cycle_record(&neither).encode(), + "the folded line must still distinguish asserted from not" + ); + } + + /// A run that stopped short must never read as agreement. Which *kind* of + /// inconclusive it is depends on where it stopped, and both branches are + /// exercised because only one of them was reachable by accident. + /// + /// Stopping mid-window emits a partial tail checkpoint at a cycle the + /// reference never checkpoints at, so the axes disagree before the hashes + /// are ever compared -- which is the more informative answer, and the one + /// the first version of this test guessed wrong. + #[test] + fn a_run_truncated_mid_window_is_inconclusive_not_identical() { + let reference = stream(10_000, DEFAULT_INTERVAL); + let candidate = stream(6_000, DEFAULT_INTERVAL); + assert_eq!( + compare(&reference, &candidate), + Comparison::Inconclusive { + reason: InconclusiveReason::UnalignedCycles { index: 1 } + }, + "candidate's tail checkpoint is at cycle 5999, reference's is at 8191" + ); + } + + /// Stopping exactly on an interval boundary produces aligned, agreeing + /// checkpoints and then simply runs out -- the case `LengthMismatch` exists + /// for. + #[test] + fn a_run_truncated_on_a_boundary_is_inconclusive_not_identical() { + let reference = stream(10_000, DEFAULT_INTERVAL); + let candidate = stream(8_192, DEFAULT_INTERVAL); + assert_eq!( + compare(&reference, &candidate), + Comparison::Inconclusive { + reason: InconclusiveReason::LengthMismatch { + reference: 3, + candidate: 2 + } + } + ); + } + + #[test] + fn an_empty_stream_is_inconclusive_not_identical() { + let reference = stream(10_000, DEFAULT_INTERVAL); + assert_eq!( + compare(&reference, &[]), + Comparison::Inconclusive { + reason: InconclusiveReason::Empty + } + ); + assert_eq!( + compare(&[], &reference), + Comparison::Inconclusive { + reason: InconclusiveReason::Empty + } + ); + } + + /// Different intervals cover different spans, so the hashes are not + /// comparable at all -- and saying "diverged" would send a re-run at a + /// window that is not where any problem is. + #[test] + fn streams_at_different_intervals_are_inconclusive_not_diverged() { + let a = stream(8192, 4096); + let b = stream(8192, 2048); + assert_eq!( + compare(&a, &b), + Comparison::Inconclusive { + reason: InconclusiveReason::UnalignedCycles { index: 0 } + } + ); + } + + /// A trailing partial window must still be emitted; the tail is where a run + /// stopped early actually differs. + #[test] + fn a_partial_trailing_window_is_still_emitted() { + let ck = stream(5000, DEFAULT_INTERVAL); + assert_eq!(ck.len(), 2); + assert_eq!(ck[0].through_cycle, 4095); + assert_eq!(ck[1].through_cycle, 4999, "the tail must not be dropped"); + } + + #[test] + fn the_serialised_form_round_trips() { + let ck = stream(10_000, DEFAULT_INTERVAL); + let bytes = to_bytes(&ck); + assert_eq!(bytes.len(), ck.len() * 16); + assert_eq!(from_bytes(&bytes).expect("round trip"), ck); + } + + /// A truncated stream that parses is a truncated comparison that passes. + #[test] + fn a_truncated_serialised_stream_is_rejected() { + let mut bytes = to_bytes(&stream(10_000, DEFAULT_INTERVAL)); + bytes.pop(); + assert!(from_bytes(&bytes).is_err()); + } + + #[test] + #[should_panic(expected = "checkpoint interval must be non-zero")] + fn a_zero_interval_is_rejected() { + let _ = Hasher::new(0); + } + + #[test] + fn access_codes_are_stable() { + assert_eq!(Observable::access_code(true, false, false), 0); + assert_eq!(Observable::access_code(false, false, false), 1); + assert_eq!(Observable::access_code(false, true, false), 2); + assert_eq!(Observable::access_code(false, false, true), 3); + assert_eq!(Observable::access_code(false, true, true), 4); + } +} diff --git a/crates/rustynes-cosim/src/lib.rs b/crates/rustynes-cosim/src/lib.rs index 3fc9888a..01cac63d 100644 --- a/crates/rustynes-cosim/src/lib.rs +++ b/crates/rustynes-cosim/src/lib.rs @@ -58,6 +58,8 @@ // justify each use. #![allow(unsafe_code)] +pub mod checkpoint; + use std::io; use core::ffi::{c_char, c_int, c_uchar, c_uint, c_ulonglong, c_void}; @@ -161,11 +163,124 @@ impl Oracle { } /// The per-cycle IRQ/bus trace as CSV, or `None` if unarmed. + /// + /// **Consumes the trace.** A second call returns `None`, and so does a + /// subsequent [`Self::take_checkpoints`] -- see + /// [`Self::take_irq_artifacts`] for the reason that matters. pub fn take_irq_trace_csv(&mut self) -> Option { self.nes.bus_mut().take_irq_trace().map(|t| t.to_csv()) } + + /// Both IRQ-trace artifacts from **one** take. + /// + /// # Why this exists rather than calling the two takers in sequence + /// + /// `Bus::take_irq_trace` moves the trace out. So a caller that wants the + /// CSV *and* the checkpoints gets whichever it asked for first and `None` + /// for the other -- and `None` on the checkpoint side is indistinguishable + /// from "the trace was never armed", which the exporter would report as a + /// missing-output warning rather than as the ordering bug it is. + /// + /// Pinned by `tests::taking_the_csv_first_leaves_no_trace_for_checkpoints`, + /// so the hazard is a documented behaviour rather than a surprise. + pub fn take_irq_artifacts(&mut self, interval: u64) -> Option { + let trace = self.nes.bus_mut().take_irq_trace()?; + let dropped = trace.overflow(); + let checkpoints = if dropped > 0 { + Err(CheckpointError::TraceOverflowed { + dropped, + kept: trace.len(), + }) + } else { + let mut h = checkpoint::Hasher::new(interval); + for r in trace.records() { + h.push(&checkpoint::Observable::from_cycle_record(r)); + } + Ok(h.finish()) + }; + Some(IrqArtifacts { + csv: trace.to_csv(), + checkpoints, + }) + } + + /// Rolling hash checkpoints over the observable per-cycle tuple, or `None` + /// if the IRQ trace was never armed. + /// + /// # This refuses rather than truncating + /// + /// `IrqTrace::push` **silently drops** records once the buffer reaches the + /// capacity it was armed with, advancing an `overflow` counter nobody has + /// to read. A checkpoint stream computed over a dropped-record trace is a + /// hash of *fewer cycles than it claims*, and the two sides would then + /// disagree for a reason that has nothing to do with the DUT -- the exact + /// "format-packing mismatch masquerading as an RTL bug" this rung exists to + /// rule out. Worse, it would look like a legitimate divergence. + /// + /// So an overflowed trace returns [`CheckpointError::TraceOverflowed`] with + /// the count, not a shorter stream. Raise the capacity and re-run. + /// + /// # Errors + /// + /// [`CheckpointError::TraceOverflowed`] if the trace dropped any record. + pub fn take_checkpoints( + &mut self, + interval: u64, + ) -> Option, CheckpointError>> { + let trace = self.nes.bus_mut().take_irq_trace()?; + let dropped = trace.overflow(); + if dropped > 0 { + return Some(Err(CheckpointError::TraceOverflowed { + dropped, + kept: trace.len(), + })); + } + let mut h = checkpoint::Hasher::new(interval); + for r in trace.records() { + h.push(&checkpoint::Observable::from_cycle_record(r)); + } + Some(Ok(h.finish())) + } } +/// Both artifacts derived from a single take of the per-cycle trace. +#[derive(Debug, Clone)] +pub struct IrqArtifacts { + /// The per-cycle CSV. + pub csv: String, + /// The checkpoint stream, or why it could not be produced. + pub checkpoints: Result, CheckpointError>, +} + +/// Why a checkpoint stream could not be produced. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CheckpointError { + /// The per-cycle trace hit its capacity and dropped records, so any hash + /// over it would cover fewer cycles than it claims. + TraceOverflowed { + /// Records the trace discarded. + dropped: u64, + /// Records it kept. + kept: usize, + }, +} + +impl core::fmt::Display for CheckpointError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + Self::TraceOverflowed { dropped, kept } => write!( + f, + "per-cycle trace overflowed: kept {kept} records and dropped {dropped}; \ + a checkpoint hash over a truncated trace would read as a DUT divergence. \ + Re-run with --irq-trace at least {}", + *kept as u64 + dropped + ), + } + } +} + +impl std::error::Error for CheckpointError {} + // --------------------------------------------------------------------------- // C ABI // --------------------------------------------------------------------------- @@ -433,8 +548,52 @@ pub unsafe extern "C" fn rn_write_cpu_boot_trace( }) } +/// Write the checkpoint stream to `path`: repeated `(u64 through_cycle, +/// u64 hash)`, little-endian, no header. +/// +/// Returns `0` on success. `-1` null handle, `-2` bad path, `-4` the trace was +/// never armed, **`-5` the trace overflowed and dropped records** (a hash over +/// it would cover fewer cycles than it claims and would read as a DUT +/// divergence -- raise the capacity and re-run), `-6` interval was zero. +/// Anything at or below `-100` is `-(100 + errno)` from the write itself. +/// +/// # Safety +/// +/// `handle` must come from [`rn_open`] and not have been closed. `path` must be +/// a valid NUL-terminated C string. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn rn_write_checkpoints( + handle: *mut c_void, + path: *const c_char, + interval: c_ulonglong, +) -> c_int { + if interval == 0 { + // Rejected here rather than reaching `Hasher::new`'s assert: a panic + // across the C ABI is undefined behaviour, so the guard has to be on + // this side of the boundary. + return -6; + } + let oracle = oracle!(handle, -1); + // SAFETY: as above. + let Some(path) = (unsafe { cstr_to_path(path) }) else { + return -2; + }; + match oracle.take_checkpoints(interval) { + None => -4, + Some(Err(_)) => -5, + Some(Ok(ck)) => match std::fs::write(path, checkpoint::to_bytes(&ck)) { + Ok(()) => 0, + Err(e) => write_error_code(&e), + }, + } +} + /// Write the IRQ/bus trace CSV to `path`. Returns 0 on success, negative on error. /// +/// **Consumes the trace**, so a subsequent [`rn_write_checkpoints`] returns +/// `-4` ("never armed"). A testbench wanting both must write the checkpoints +/// first, or use the Rust-side [`Oracle::take_irq_artifacts`]. +/// /// # Safety /// /// `path` must be a NUL-terminated UTF-8 C string. @@ -492,6 +651,104 @@ mod tests { rom } + /// An unarmed trace yields no checkpoints -- and `None` is not an empty + /// stream, because an empty stream would compare `Inconclusive` while + /// `None` says the question was never asked. + #[test] + fn checkpoints_are_none_when_the_trace_was_never_armed() { + let mut o = Oracle::new(&nrom(), 0).expect("rom"); + o.advance_frames(1); + assert!(o.take_checkpoints(checkpoint::DEFAULT_INTERVAL).is_none()); + } + + /// The same run hashed twice agrees -- the null-DUT self-diff for this + /// format. Without it, a later red result is ambiguous between "the RTL is + /// wrong" and "the hasher is not a function of the trace". + #[test] + fn the_same_run_produces_the_same_checkpoints() { + let run = |seed: u64| { + let mut o = Oracle::new(&nrom(), seed).expect("rom"); + o.enable_irq_trace(200_000); + o.advance_frames(2); + o.take_checkpoints(checkpoint::DEFAULT_INTERVAL) + .expect("armed") + .expect("no overflow") + }; + let a = run(0); + let b = run(0); + assert!(!a.is_empty(), "two frames must produce checkpoints"); + assert_eq!( + checkpoint::compare(&a, &b), + checkpoint::Comparison::Identical { + checkpoints: a.len() + } + ); + } + + /// **A capacity-limited trace silently drops records**, so a hash over it + /// covers fewer cycles than it claims -- and would then read as a DUT + /// divergence rather than as our own truncation. It must refuse. + #[test] + fn an_overflowed_trace_refuses_to_produce_checkpoints() { + let mut o = Oracle::new(&nrom(), 0).expect("rom"); + o.enable_irq_trace(64); // far below one frame of CPU cycles + o.advance_frames(2); + match o.take_checkpoints(checkpoint::DEFAULT_INTERVAL) { + Some(Err(CheckpointError::TraceOverflowed { dropped, kept })) => { + assert_eq!(kept, 64); + assert!(dropped > 0, "the trace must report what it discarded"); + } + other => panic!("expected a refusal, got {other:?}"), + } + } + + /// The refusal must say how to fix itself; a testbench operator sees only + /// this string. + #[test] + fn the_overflow_error_names_the_capacity_to_retry_with() { + let e = CheckpointError::TraceOverflowed { + dropped: 100, + kept: 64, + }; + let s = e.to_string(); + assert!( + s.contains("164"), + "must name the capacity to retry with: {s}" + ); + assert!(s.contains("dropped 100"), "{s}"); + } + + /// The consuming hazard, pinned so it is a documented behaviour rather + /// than a surprise: the CSV taker moves the trace out, so a checkpoint + /// request afterwards returns `None` -- which reads exactly like "never + /// armed". + #[test] + fn taking_the_csv_first_leaves_no_trace_for_checkpoints() { + let mut o = Oracle::new(&nrom(), 0).expect("rom"); + o.enable_irq_trace(200_000); + o.advance_frames(1); + assert!(o.take_irq_trace_csv().is_some()); + assert!( + o.take_checkpoints(checkpoint::DEFAULT_INTERVAL).is_none(), + "if this ever returns Some, the trace stopped being consumed and \ + `take_irq_artifacts` can be simplified away" + ); + } + + /// The combined taker returns both from one take, which the two single + /// takers structurally cannot. + #[test] + fn the_combined_taker_returns_both_artifacts() { + let mut o = Oracle::new(&nrom(), 0).expect("rom"); + o.enable_irq_trace(200_000); + o.advance_frames(1); + let a = o + .take_irq_artifacts(checkpoint::DEFAULT_INTERVAL) + .expect("armed"); + assert!(a.csv.starts_with("cpu_cycle,")); + assert!(!a.checkpoints.expect("no overflow").is_empty()); + } + /// The power-on latch, pinned rather than worked around silently. /// /// If this test ever fails it means the core stopped swallowing the first diff --git a/crates/rustynes-test-harness/tests/cosim_manifest_audit.rs b/crates/rustynes-test-harness/tests/cosim_manifest_audit.rs index 434147bf..ff12997b 100644 --- a/crates/rustynes-test-harness/tests/cosim_manifest_audit.rs +++ b/crates/rustynes-test-harness/tests/cosim_manifest_audit.rs @@ -163,3 +163,56 @@ fn rustynes_cosim_is_excluded_from_the_workspace() { "rustynes-cosim must be listed in the workspace `exclude`" ); } + +/// The excluded crate's lockfile must be **tracked**. +/// +/// `.gitignore` carries a bare `Cargo.lock`, which matches at any depth, and a +/// `!/Cargo.lock` re-include that names only the workspace root. That pairing +/// was written when there was exactly one lockfile. Excluding +/// `rustynes-cosim` from the workspace gave it its own resolve and its own +/// lockfile, which the bare rule then silently ignored -- so CI re-resolved its +/// dependency graph on every run. +/// +/// That matters more here than for an ordinary crate: this one emits the +/// goldens an external NES implementation is verified against, and its manifest +/// records the emulator version rather than the resolve. A dependency moving +/// underneath it would be invisible in exactly the artifact whose job is to +/// establish provenance. +#[test] +fn the_excluded_crates_lockfile_is_tracked() { + let root = repo_root(); + let lock = root.join("crates/rustynes-cosim/Cargo.lock"); + assert!( + lock.is_file(), + "crates/rustynes-cosim/Cargo.lock is missing; an excluded package has \ + its own resolve and the lockfile must be committed with it" + ); + + let out = std::process::Command::new("git") + .arg("-C") + .arg(&root) + .args([ + "ls-files", + "--error-unmatch", + "crates/rustynes-cosim/Cargo.lock", + ]) + .output(); + + // A source tarball with no .git is a legitimate way to build this; skip + // rather than fail, and say so, so a skip is never mistaken for a pass. + let Ok(out) = out else { + eprintln!("skipping: git is unavailable"); + return; + }; + if !root.join(".git").exists() { + eprintln!("skipping: not a git checkout"); + return; + } + assert!( + out.status.success(), + "crates/rustynes-cosim/Cargo.lock exists but is NOT tracked by git -- \ + check the `!/crates/rustynes-cosim/Cargo.lock` re-include in .gitignore.\n\ + stderr: {}", + String::from_utf8_lossy(&out.stderr) + ); +} diff --git a/docs/mister.md b/docs/mister.md index 413b9e29..373ae1d5 100644 --- a/docs/mister.md +++ b/docs/mister.md @@ -122,6 +122,7 @@ every golden's length. | `.irq.csv` | per-CPU-cycle IRQ/bus CSV, two samples per cycle | `scripts/irq_trace_cross_diff.py` | | `.index_fb.bin` | 256x240 little-endian `u16`, **pre-palette** | the testbench's frame comparison | | `.ram.bin` | 2 KiB CPU work RAM | `accuracy_coin_catalog::decode_results` | +| `.ckpt.bin` | rolling per-cycle hash checkpoints, `(u64 through_cycle, u64 hash)` LE, headerless | `checkpoint_diff` | | `.manifest.txt` | provenance | humans, and the drift guard below | The framebuffer is exported **pre-palette** on purpose: a palette difference must @@ -201,8 +202,92 @@ impact: zero. A 4200-frame AccuracyCoin run is ~125 M CPU cycles, which as per-cycle CSV is ~7.5 GB per side. Both sides instead chain a 64-bit hash over the per-cycle tuple and compare checkpoints every 4096 cycles - **244 KB** for a full run. On the -first mismatch, binary-search the window and re-run only that window with full -capture and waveforms. +first mismatch, re-run only that window with full capture and waveforms. + +Implemented in `crates/rustynes-cosim/src/checkpoint.rs`. **Measured on a real +export** rather than projected: 3 frames of AccuracyCoin is 89,343 CPU cycles, +which is **5,372,427 bytes** of `irq.csv` against **352 bytes** of `ckpt.bin` - +a factor of **15,263**. + +#### What is hashed, and what deliberately is not + +`CycleRecord` carries 29 fields, and most of them are *`RustyNES`'s model*, not +hardware. `checkpoint::Observable` is the subset an external device-under-test +can genuinely produce, and `Observable::from_cycle_record` is the single place +the partition is applied - so widening it has to pass +`model_internal_state_cannot_cause_a_divergence`, which perturbs every dropped +field at once and asserts the hash does not move. + +| In | Why | +|---|---| +| `cpu_cycle` | the axis both sides count on | +| `bus_access`, `bus_addr`, `bus_data` | pin-visible | +| `put_cycle` | the R/W phase half of the M2 cycle - pin-visible | +| `nmi_line` | a pin | +| `irq_line_at_low`, `irq_line_at_high` | the /IRQ pin, sampled twice per cycle | +| `pc` | **not** pin-visible; see below | + +Two caveats are stated rather than buried. + +**The IRQ line is one wire.** `CycleRecord` splits its samples into +`irq_pending_mapper_*` and `irq_pending_apu_*`, which is `RustyNES` *attributing* +the assertion to a source. Hardware has a single wire-OR'd /IRQ input and cannot +make that distinction, so the pairs are OR'd before hashing. Hashing them apart +would fail a correct DUT for disagreeing about something it cannot observe. + +**`pc` is DUT-observable, not pin-observable.** The 6502 does not expose its +program counter. It is in because the testbench wrapper can expose the internal +register and rung 1 compares it directly - but a `pc`-only mismatch means +something weaker than a bus mismatch. + +`ppu_scanline`, `ppu_dot`, `ppu_frame` and `a12_events` are out. `a12_events` is +the sharpest case: A12 transitions genuinely are observable on the cartridge +connector, so it is excluded for **scope**, not observability, and becomes a gate +when the PPU rung opens. + +#### The hash must be reimplementable in ten lines of C++ + +The top risk at rung 0 is a format-packing mismatch masquerading as an RTL bug. +So the hash is **FNV-1a 64** - chosen for exactly one property, that a testbench +can reimplement it without a library - and `Observable::encode` defines a fixed +**16-byte little-endian** layout with an explicit zero pad byte, so the C++ side +cannot hash uninitialised struct padding. Both are pinned to a hardcoded vector +by `the_wire_encoding_is_pinned_to_a_fixed_vector`; a reordered field fails that +test rather than producing a phantom RTL defect on the next co-simulation run. + +#### Three answers, and the third is the point + +`checkpoint_diff ` exits `0` identical, +`1` diverged (printing the window to re-run), `2` usage/IO, and **`3` +inconclusive**. A truncated run, a DUT that stopped early, and two runs at +different intervals all produce "no divergence was found", and reporting that as +agreement is this project's recurring failure. A green job must mean the streams +were compared and matched, never that there was nothing to compare. + +Cycle **alignment is checked before the hash**: two streams checkpointing at +different cycles cover different spans, so a hash difference between them says +nothing, and calling it a divergence would send a full-capture re-run at a window +where nothing is wrong. + +#### A capacity-limited trace refuses rather than truncating + +`IrqTrace::push` silently drops records once it reaches the capacity it was armed +with, advancing an `overflow` counter nobody has to read. A checkpoint stream +computed over a dropped-record trace hashes *fewer cycles than it claims*, and +the two sides then disagree for a reason that has nothing to do with the DUT - +which is worse than useless, because it looks like a legitimate divergence. So +`Oracle::take_checkpoints` returns `CheckpointError::TraceOverflowed` naming the +capacity to retry with, `rn_write_checkpoints` returns `-5`, and the exporter +aborts rather than writing a short stream. + +#### One take, two artifacts + +`Bus::take_irq_trace` **moves** the trace out, so asking for the CSV and then the +checkpoints yields `None` for whichever came second - and `None` is +indistinguishable from "never armed". `Oracle::take_irq_artifacts` derives both +from a single take; the hazard is pinned by +`taking_the_csv_first_leaves_no_trace_for_checkpoints` so it is a documented +behaviour rather than a surprise. ## What is a gate and what is diagnostic