Repository navigation
Commit 955d2fd
v2.9.0 "Survey" -- every audit re-checked, and the SuperStation One surveyed (#561)
* ci,docs: run the av-record tests; schedule the mobile checklist at v2.9.3
Two loose ends carried into v2.9.0 (maintainer instruction).
CI never ran the `av-record` tests. The `full` clippy step in `lint` has
always LINTED that feature, but no test job enabled it, and
`crates/rustynes-frontend/src/av_record.rs` is compiled only with it
(`pub mod av_record` under the feature in lib.rs). So its 13 tests, and
the av-record-gated paths in app.rs, emu.rs and ui_shell.rs, were
invisible to every test job -- the same shape as the mapper-audio-off
gap the step above it closed in v2.3.7. The `test` job gains a
Linux-only `cargo test -p rustynes-frontend --lib --features av-record`,
the whole lib rather than the one module because the gated code spans
four files. Measured first: 614 lib tests pass with the feature on
(9.2 s once built); the av_record module alone is 13 of 13. actionlint
passes.
The v2.7.4 mobile device checklist moves to a release of its own:
v2.9.3 (maintainer decision, 2026-09-26). The v2.9.x plan's table gains
that row, and "whatever the board finds" becomes v2.9.4+. VERSION-PLAN's
v2.9.x row names it. The checklist itself is unchanged; this repository
still records no run of it.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* docs(adr): v3.0.0 removes the v2.7.5 deprecations and the dead NMI detector
ADR 0041 reserved one decision for v2.9.0: whether v3.0.0 removes the
items v2.7.5 deprecated. The maintainer decided yes, and further than the
question asked -- the NMI edge detector goes too. ADR 0042 records it.
Why the detector matters: sample_nmi_edge runs in the live PPU catch-up
loop but feeds only the deprecated poll_nmi, and its two fields
(nmi_edge_latch, last_nmi_level) are serialised in the .rns BUS section
(BUS_SECTION_VERSION 1). Removing the methods alone would not touch the
save-state layout; removing the detector does. ADR 0041 item 4 said "No
save-state break is planned", so it gets a dated amendment note pointing
here rather than a silent rewrite.
Facts measured for the decision (v2.9.0 scratch survey):
- 19 items, all since = "2.7.5": 18 rustynes_cpu::Bus methods + ApuBus.
- Zero callers outside the trait's own default bodies; ApuBus has no
implementor; nothing is published to crates.io or reachable through
libretro, UniFFI, Lua or wasm.
- A trial removal in a scratch copy compiled and passed clippy, no_std,
the chip tests, AccuracyCoin, nestest, the DMA pins, save-state and
visual-regression suites and the snapshot-schema audit. The full
test-roms suite, frontend feature clippy set and wasm gates were NOT
run on it; the ADR says so.
- The removal also exposes dead code that was never deprecated
(oam_dma_overlap_cycle, dmc_step_was_get, dma_total), so v2.7.5's
"18 dead methods" was a lower bound.
Deliberately NOT decided here: how v3.0.0 loads a version-1 BUS section.
The ADR puts the relevant fact on the table (the two fields feed only the
removed code, so read-and-discard loses nothing) beside ADR 0003/0028's
clean rejection, and leaves the choice to v3.0.0. Nothing is removed now:
v2.9.x is MINOR and removal is an API break.
Also: the v2.9.0 "Survey" plan (decisions table, deliverables, gate), and
the plans index marks v2.7.x/v2.8.x shipped and v2.9.x in progress.
Verified: markdownlint (pre-commit pin) passes on all four files.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(hdpack,patch): three v2.9.0 re-audit findings -- crafted packs and patches
The v2.9.0 frontend re-audit reproduced three input-boundary defects that
the v2.7.3 fixes (SEC-01, the PNG and pack budgets) sat right beside. Each
is pinned by a test that failed on the unfixed tree.
NF-06 (High): a two-number <overscan> rule killed the app.
`<overscan>` was parsed as four unbounded u32s and kept as given.
HdCompositor::new clamped the crop to one pixel (`saturating_sub(..)
.max(1)`) but kept the ORIGIN, so `<overscan>300,0,0,0` iterated rows
300..301 and indexed byte 307,200 of a 245,760-byte framebuffer:
"range start index 307200 out of range" at hdpack.rs:1896 on the first
composited frame. Desktop release is panic = "abort", so that is a crash;
on mobile composite_hd_frame is not a contained_frame method, so it
poisons the bridge mutex. `0,4294967295,0,1` also overflowed `left +
right` (a debug-build panic, a silent wrap in release).
Fix: one rule, overscan_is_valid (checked adds; top+bottom < 240,
left+right < 256), applied at parse time AND again in
HdCompositor::new, because `overscan` is a plain field and parsing is not
the only way to set it. An invalid crop is treated as no crop. Under the
rule crop_w/crop_h >= 1 and origin + crop never passes the frame edge, so
the subtraction no longer needs saturating. Two tests, one per line of
defence; each check was reverted as a mutant and each test caught its own.
NF-04 (Medium): hires.txt was read with no cap.
Images were capped per entry (64 MiB) and per pack (1 GiB) but the rule
file was read with read_to_string in both loaders; the audit's 509 KiB
zip inflating to 512 MiB peaked at ~533 MiB. read_hires() now reads
through take(MAX_HIRES_BYTES + 1) and REFUSES an over-cap file (32 MiB,
about half a million rule lines) rather than truncating it, since a
truncated rule file would load as a different, partial pack. The test
pads a valid pack one byte past the cap with a comment line the parser
ignores, so an uncapped loader still loads it: red on the uncapped
reader for the zip path, and the folder path is asserted the same way,
each beside a control at exactly the cap. The bomb zip is under 256 KiB.
NF-07 (Medium): a BPS TargetCopy grew the output without bound.
The declared target size was capped (MAX_PATCH_OUTPUT) and compared only
after the action stream. TargetCopy's own check (`cursor >= out.len()`)
can never fire while the output grows under the cursor, so a 27-byte
patch declaring a 16-byte target reached 1 GiB before failing, and a
2^40 length would run until OOM. Such a patch is auto-applied when it
sits beside the ROM it was built for (the source-CRC check is the only
precondition). One check before every action, `length > target_size -
out.len()`, keeps out.len() <= target_size as a loop invariant. The test
observes WHERE it fails: OffsetOutOfRange at the action (fixed) versus
"wrong target size" at the end after building 64 MiB (unfixed; red run
shown), so reverting the check is caught.
Trap re-hit while mutating: restoring a file with `cp -p` gives it an
mtime OLDER than the mutant's build, so cargo reuses the mutant and the
"restored" tree still fails. cmp + touch before the confirming run.
Verified: rustynes-hdpack lib 77 passed / 0 failed / 1 ignored;
rustynes-frontend patch:: 15 passed; cargo fmt --all --check; clippy -D
warnings on both crates, all targets. The remaining frontend findings
(NF-01/02/03/05/08/09, the Lua sandbox and HD audio) are separate
changes.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(core): a Power Cycle no longer erases the battery save
Found while implementing the maintainer's v2.9.0 decision that power-on
movies start from cleared save RAM, and it inverts the premise that
decision was taken on.
The defect. Bus::power_cycle rebuilds the mapper from the ROM bytes so a
cold boot is a true cold boot, and the rebuild zeroed ALL cartridge RAM,
battery-backed included -- "a battery-pull", its comment said, adding
"RustyNES does not persist standard battery saves to disk, so nothing
on-disk is lost". That stopped being true at v2.7.3, which made the
desktop write Nes::sram() to <data_dir>/battery/<sha>.sav whenever the
live bytes differ from the last write. So: F3 or the menu's Power Cycle
zeroes the RAM, the once-a-second comparison sees a change, and writes
the zeros over the player's save. Mobile has persisted saves since
v2.7.4 and shares the core, so it had the same defect.
Red first: battery_save's new `a_power_cycle_keeps_the_save` runs a
battery cart, flushes its save (0xA5 at $6000), power-cycles, flushes
again, and asserts the file is unchanged. On the old core it failed at
the RAM check: left 0, right 165.
The fix keeps battery-backed RAM across the rebuild (copied out before,
back in after, guarded on equal length) when the header's battery bit is
set, and still clears volatile PRG-RAM and CHR-RAM, which is what a
console does.
The movie decision, which this makes necessary rather than redundant.
With the old power_cycle a power-on movie ALWAYS started from cleared
cartridge RAM, because the power cycle cleared it. v2.7.3's
docs/frontend.md recorded the opposite as a known limitation ("power-
cycles the console, which does not clear cartridge RAM") -- wrong in the
other direction, and corrected there with the old sentence quoted. Now
that power_cycle keeps the save, the movie must clear it explicitly:
new rustynes_core::power_on_for_movie (power cycle + sram zeroed, the
state a fresh load with no .sav holds) is called by Movie::seek_to_start
for PowerOn playback and by both recording entry points, desktop
movie_ui and mobile movie_record_from_power_on, so recording and replay
reconstruct the same state.
Mutations, all caught once the tests used a BATTERY cart: seek_to_start
back to power_cycle; power_on_for_movie without the clear; the desktop
recorder back to power_cycle. The first run of the two core mutants came
back NOT CAUGHT -- the tests used a no-battery ROM, whose RAM the
rebuild clears anyway, so the movie's clear was unobservable. That is
what exposed the power_cycle behaviour in the first place.
Byte-identity: the full `cargo test --workspace --features test-roms`
suite on this change is 2,764 passed / 0 failed / 20 ignored (v2.8.4:
2,756; the difference is new tests), AccuracyCoin and nestest included,
so no test ROM's outcome depends on battery RAM being cleared by a
power cycle. The mobile recording path has no test that reaches it.
Not changed: TAStudio. Its editor anchors frame 0 on the RUNNING game
and exports as a PowerOn movie -- a separate, older inconsistency,
recorded for the maintainer (the fix changes what opening TAStudio
does).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(hdpack): bound HD-audio decoding and decode each file once
v2.9.0 frontend re-audit NF-05 (Medium). decode_ogg_to_mono pushed every
decoded sample with no length limit, resample_linear then built a second
full copy at the output rate, and decode_tracks_from_folder decoded a
file once PER DECLARATION. The audit's probe: one 315 KiB OGG of an hour
of 8 kHz silence, declared four times, held 2,636 MiB of f32 PCM
(VmHWM 2.8 GB), decoded on the winit thread at pack load. v2.7.3's
SEC-01 bounded the pack's IMAGES and CON-03 the rate ratio; neither
touched a track's length.
Three bounds:
- MAX_TRACK_SECONDS (15 min of source audio). A longer track is REFUSED
(rule inert), not truncated mid-piece. Checked per decoded packet, so
an over-long stream stops decoding as soon as it passes the ceiling.
- MAX_PACK_AUDIO_SAMPLES (512 MiB of f32, ~46 min at 48 kHz) across the
whole pack, charged per DISTINCT file. The remaining budget is passed
into the decoder as an output-sample ceiling and converted to a source
ceiling at the stream's OWN rate. (A first version converted at a
fixed 8 kHz ratio, which would have refused a 48 kHz track that fits;
the test now checks the conversion at two output rates.)
- Dedupe: HdAudioTrack.pcm becomes Arc<[f32]>, shared by every
declaration naming the same file; a failed or refused file is
remembered too, so it is not decoded again. The field is only read
through deref, so no caller outside the crate changes.
Tests (a 2.7 KiB fixture: one second of 8 kHz mono silence, generated
with ffmpeg anullsrc, -fflags +bitexact, no content to license):
a_track_over_the_duration_cap_is_refused and
a_file_declared_twice_is_decoded_once_and_the_pack_is_bounded
(Arc::ptr_eq for the sharing; a budget of exactly one track leaves the
second file inert). They could not compile against the old API (no
capped entry points); three mutations each caught: the length check
removed, the dedupe removed (a key per declaration), the budget ignored.
Not done: decoding still runs on the winit thread at pack load. It is
now bounded (at most one 512 MiB budget's worth, once per file) rather
than moved; moving it off-thread is a responsiveness change, not a
memory-safety one.
Verified: rustynes-hdpack lib 79 passed / 1 ignored; clippy -D warnings
on rustynes-hdpack and on rustynes-frontend with scripting,hd-pack; fmt.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(tastudio): anchor a new project on power-on, the state it exports from
New finding NF-10, found while fixing power-on movies; fixed per the
maintainer's decision of 2026-09-26 ("anchor on power-on").
TasEditor::new pins whatever state it is handed as frame 0, and its own
contract says that state "must already be at the project's start state
(typically a fresh power_cycle)". App::ensure_tas_editor handed it the
RUNNING game ("non-destructive: we do not power-cycle the running
game"), while TasEditor::to_movie always exports StartPoint::PowerOn. So
a project opened mid-game exported a movie -- .rnm, and via it .fm2 and
.bk2 -- that replayed from power-on rather than from what was edited:
a desync from frame 0.
Fix: TasEditor::new_from_power_on runs rustynes_core::power_on_for_movie
(the same power cycle + cleared cartridge RAM a power-on recording and
Movie::seek_to_start use) and anchors there; the app opens TAStudio
through it, and does what a cold boot does elsewhere -- clears the
rewind history and lag tally, resets frame pacing, re-pushes the
per-channel mute mask and gain the power cycle reset (as do_power_cycle
does; output-only, defaults byte-identical), resets the RetroAchievements
session -- and logs that the console restarted. The cost is the
maintainer's accepted one: opening the panel restarts the game.
Test: the_frame0_anchor_is_the_state_the_exported_movie_starts_from runs
a game 30 frames, opens an editor, exports its movie, seeks a SECOND
console with a different 7-frame history to the movie's start, and
asserts its snapshot equals the editor's frame-0 anchor byte for byte.
Mutating new_from_power_on to skip the power-on -- exactly the old app
behaviour -- fails it. The app call site needs a window and is not under
test; clippy passes on the default set and with retroachievements.
Verified: tastudio 36 passed; fmt; clippy -D warnings (default and
--features retroachievements).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* docs(audits): the v2.9.0 re-audit reports, the RTL ledger, three decisions
The four v2.9.0 re-audit reports (maintainer decision: Claude subagents,
one per scope, read-only) are committed beside the originals, VERBATIM,
as docs/audits/v2.9.0-<scope>-reaudit.md. Like the first four they state
findings; the ledgers record what was done. They are added to
.markdownlintignore rather than lint-fixed, because a lint fix is an
edit to the record (the originals happened to pass; these do not: table
cells with unescaped pipes, a bare fence, trailing spaces).
Totals: 139 existing ledger rows re-verified across the four scopes,
0 regressed; 39 new findings (core 8, frontend 9, libretro 10, RTL 12).
docs/audits/README.md gains the table.
rtl-disposition.md gains its v2.9.0 section: NR-01..NR-12 plus the
SuperStation One presence-probe finding and the files.qip omission the
first compile caught, each with verdict, measured evidence and sibling
commit (62e3306, 9fa782c, 93071cd, 38d23d4, cbcd2e2). NR-08 (PRG and CHR
in one SDRAM bank) stays OPEN for v2.9.1's measured optimisation pass;
NR-07's PLL reset change needs a board observation at v2.9.2. The other
three ledgers get their sections as their fixes land.
Maintainer decisions of 2026-09-26, recorded:
- ADR 0042: v3.0.0 also renames LockstepBus (the name describes the
retired dot-lockstep scheduler); the title and consequences say so.
- core-disposition §6.2: the three v2.7.1 Provenance headers were
reviewed and accepted -- recorded on the maintainer's word, dated,
and marked as such, since nothing in the tree could show it.
- The v2.9.0 plan's decisions table gains those two, NF-02's stepped
Rust matcher, NF-10's power-on anchor, and the MARM outcome.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(libretro): save RAM only with a battery; legal PRG-RAM descriptors
libretro v2.9.0 re-audit NL-02 (Medium) and NL-06 (Low).
NL-02 -- a cartridge without a battery got a .srm.
`retro_get_memory_data/size(RETRO_MEMORY_SAVE_RAM)` returned `Nes::sram()`
whenever it was non-empty. Several boards expose RAM through `sram()`
whatever the header says (NROM always 8 KiB; MMC1/MMC3 allocate by
default), and the core's own doc on `Nes::has_battery` says a host that
persists save RAM must gate on it. The desktop's `.sav` does
(battery_save.rs); the libretro core did not. RetroArch writes SAVE_RAM to a
`.srm` and loads it back after `retro_load_game` (libretro.h's documented
use of SAVE_RAM; RetroArch's source was not traced), so 581 images of the
local corpus without a battery got a save file the hardware never had, and
their work RAM came back on the next boot. Both the legacy pointer/size and
the `RETRO_MEMDESC_SAVE_RAM` flag now follow `has_battery()`. Volatile RAM is
still described to the memory map, with no flag, so cheat search and
RetroAchievements keep seeing `$6000-$7FFF`. An FDS image has no battery,
so its 32 KiB adapter RAM stops being a `.srm` too (its real saves live on
the disk image; see NL-03).
NL-06 -- the PRG-RAM descriptor broke libretro.h's shape rules.
It was `{start: $6000, select: 0, len: sram_len}` for every size. libretro.h
(`retro_memory_descriptor::select`): with select 0 "each byte is assumed
mapped exactly once. In this case, 'len' must be a power of two." So a
64 KiB buffer claimed `$6000-$15FFF` (past the 16-bit bus), 32 KiB claimed
PRG-ROM at `$8000-$DFFF`, and 73,728 bytes (three commercial MMC1 dumps) was
not a power of two at all. `register_memory_maps` now calls a pure
`memory_descriptors(&mut Nes)`, which describes:
- the CPU window at `$6000`: `min(sram_len, 8 KiB)` (the FDS RAM adapter's
real 32 KiB to `$DFFF`);
- when the buffer is larger, the whole buffer in a named "SRAM" address
space, byte n at address n (libretro.h's naming rule: S/R/M are not hex
digits, so it cannot read as a blank-space address);
- both cut by `push_aligned` into power-of-two pieces that start on a
multiple of their own length (8 KiB at $6000 + 16 KiB at $8000 + 8 KiB at
$C000 for the FDS; 64 KiB + 8 KiB for 73,728), every piece pointing at the
buffer base with an `offset` ("the same byte, the same pointer").
Alignment is stricter than libretro.h states; with select 0 the frontend
must derive the claimed range from start and len alone, and an aligned
power of two is unambiguous under any derivation. A descriptor cannot follow
a bank switch, so a banked board shows bank 0 in the window, as before.
Red first (C-ABI harness, which now copies every SET_MEMORY_MAPS
descriptor):
- save_ram_is_exposed_only_for_a_battery_backed_cartridge: nestest (no
battery bit) "left: 8192 right: 0"; with the bit set, 8192 and a
SAVE_RAM-flagged window.
- save_ram_descriptors_have_the_shape_libretro_h_requires: synthetic MMC1
NES 2.0 images with 73,728 / 64 KiB / 32 KiB: "select 0 needs a
power-of-two len: ... start: 24576, select: 0, len: 73728".
Plus tests::every_image_gets_legal_prg_ram_descriptors, a sweep of every
`.nes` under tests/roms through `memory_descriptors` (committed CC0 in CI;
run locally also over the gitignored external dumps, green) and the
synthetic FDS adapter; its first run caught the unaligned 32 KiB FDS window
in this commit's own first draft.
Mutations (each restored with cp -p + cmp + touch):
- SAVE_RAM pointer/size ungated -> the battery test fails;
- descriptor flags always SAVE_RAM -> the battery test and the sweep fail;
- `push_aligned` emitting one unsplit piece -> the shape test and the sweep
fail.
Docs: advanced_features.md:11 ("battery-backed carts only", false while
every non-empty buffer was flagged) and its SRAM section rewritten;
CHANGELOG [Unreleased] notes that an existing `.srm` for a battery-less game
is no longer read or written (it stays on disk), and that a header wrongly
missing the battery bit stops saving, as on the desktop.
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 28/28;
libretro_info_audit 3/3, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(libretro): refuse a serialize buffer below retro_serialize_size
libretro v2.9.0 re-audit NL-10 (Info).
libretro.h, `retro_serialize`: "If failed, or size is lower than
retro_serialize_size(), it should return false, true otherwise."
`on_serialize` returned false only when the state itself did not fit
(`split_at_mut_checked`). The reported size is the state at load plus
`SAVE_STATE_DEVICE_HEADROOM` (26 bytes, v2.8.0 L-2.1), so a buffer up to 26
bytes short of what the core asked for succeeded. Harmless with RetroArch,
which passes the reported size, but a contract violation, and a frontend
that sized from an older answer would have been told a truncated-headroom
save worked.
The check is `slice.len() < core.serialize_size`, made before the snapshot
so a refused call does no work. It runs inside `contained` like the rest.
Red first: abi_tests::serialize_refuses_a_buffer_smaller_than_it_asked_for
loads nestest, runs a frame, and serializes into `size - 1` bytes (must be
false) and `size` bytes (must be true). Before the fix: "a buffer below
retro_serialize_size must be refused".
Mutation: `<` -> `<=` (refuse the exact size) fails this test and four
others that serialize into exactly `retro_serialize_size()` bytes. Restored
with cp -p + cmp + touch.
Docs: advanced_features.md serialization bullet; CHANGELOG [Unreleased].
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 29/29;
libretro_info_audit 3/3, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(libretro): declare core options once per init cycle
libretro v2.9.0 re-audit NL-05 (Low).
`rust-libretro` 0.3.2's `retro_set_environment` calls
`CoreOptions::set_core_options` only when `initial`, i.e. on the first
environment callback the process-global instance ever receives, and that
instance is never reset (libretro audit L-1.4). The core declared its one
option (`rustynes_four_score`, `SET_VARIABLES`) there. A frontend that keeps
the library loaded and runs `retro_deinit` -> `retro_set_environment` ->
`retro_init` (a static build, or any host that does not `dlclose`) therefore
got no second declaration; the log interface, pixel format, descriptors and
controller info were re-sent, because they live in `on_set_environment`. If
such a frontend drops its option set at deinit, `GET_VARIABLE` fails and the
Four Score stays off with no menu entry. That frontend behaviour is inferred,
not traced in RetroArch's source; the missing declaration is measured at the
C ABI.
Fix: `set_core_options` is now an intentional no-op (declaring there too
would declare twice on the first call), and `on_set_environment` declares
through `declare_core_options` whenever a new `options_declared` flag is
clear. `on_deinit` clears it, so each init cycle declares exactly once, from
its first `retro_set_environment`. libretro.h permits repeated
`SET_VARIABLES` provided the option count does not change; this core keeps
repeats to one per cycle rather than one per environment call, which is the
more conservative reading.
Red first: abi_tests::core_options_are_declared_again_after_deinit_and_init
(the harness now counts SET_VARIABLES calls) runs deinit, set_environment,
init, then a second set_environment. Before: "a new init cycle must declare
the core options, left: 1, right: 2".
Mutations (restored with cp -p + cmp + touch):
- not clearing `options_declared` in `on_deinit` -> the test fails;
- declaring on every `retro_set_environment` -> the "repeated set_environment
within one cycle declares nothing new" assertion fails.
Docs: advanced_features.md (Four Score bullet); CHANGELOG [Unreleased].
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 30/30;
libretro_info_audit 3/3, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* feat(libretro): extended disk-control interface with side labels
libretro v2.9.0 re-audit NL-07 (Low).
`on_set_environment` called `enable_disk_control_interface()`, which sends
the v0 `SET_DISK_CONTROL_INTERFACE`. The v0 struct (libretro.h
`retro_disk_control_callback`) has no `get_image_label`, so the core's
"Side A" / "Side B" labels (`on_get_image_label`) were dead code and
RetroArch's Disk Control menu showed bare indices, contrary to
docs/libretro/advanced_features.md. Measured by the audit at the C ABI:
v0 x1, EXT x0, with the frontend reporting interface version 1.
Fix: a new `register_disk_control(cb)` asks
`GET_DISK_CONTROL_INTERFACE_VERSION`; at version >= 1 it sends
`SET_DISK_CONTROL_EXT_INTERFACE`, falling back to v0 when the version is 0
or the frontend refuses the ext struct. libretro.h (command 57) allows either
at version >= 1.
Why not `rust-libretro`'s `enable_extended_disk_control_interface`: its
`retro_get_image_label_callback` copies the label with no terminating NUL and
no null check on `label` (libretro audit L-1.5, ACCEPTED because the path was
unreachable while only v0 was registered). Registering ext makes that path
reachable, so the label entry is the core's own `extern "C"
disk_image_label` instead: it refuses a null or zero-length buffer, refuses
`index >= get_num_images()` (libretro.h: "Returns 'false' if index is
invalid"), truncates to `len - 1` bytes and always writes the NUL. It asks
`rust_libretro::retro_get_num_images_callback` for the count, i.e. the same
trampoline registered beside it. Every other entry is `rust-libretro`'s
trampoline, as v0 registers it, so no vendoring of `rust-libretro` was
needed. The label text moved into a pure `side_label` (same bounded
"Side A".."Side Z", then "Side 27"...), and the now-unregistered
`on_get_image_path` / `on_get_image_label` trait impls are removed.
`set_initial_image` and `get_image_path` are NULL ("Optional - may be
NULL"). libretro.h: the frontend records/restores the last disk index only
if both are implemented. An FDS game boots from side A, and a single `.fds`
container has no per-side path to verify a restore request against, so the
core declines that feature rather than booting a remembered side B.
The harness gained a system directory (a synthetic 8 KiB BIOS written to a
per-process temp dir, as crates/rustynes-test-harness/tests/fds.rs does; the
real disksys.rom is never committed), a synthetic two-sided fwNES disk, and
answers for GET_DISK_CONTROL_INTERFACE_VERSION / SET_DISK_CONTROL_INTERFACE
/ SET_DISK_CONTROL_EXT_INTERFACE.
Red first: abi_tests::disk_labels_reach_a_frontend_with_the_extended_interface
("version 1: the extended interface, left: 0, right: 1"). It then loads the
two-sided disk through the C ABI and calls the registered `get_image_label`
into 0xFF-filled buffers: "Side A", "Side B", "Sid\0" in 4 bytes, false for
index 2, a zero length and a null buffer; and checks a version-0 frontend
gets v0.
Mutations (restored with cp -p + cmp + touch):
- label copy without the terminator and without `len - 1` (the upstream
copy's behaviour) -> fails ("left: None", no NUL in the buffer);
- index bound loosened -> "index 2 of a two-sided disk is invalid" fails;
- registering `rust_libretro::retro_get_image_label_callback` -> fails (it
calls the removed trait method, so no label at all).
Docs: advanced_features.md "Multi-side disk swap" rewritten (it claimed the
labels surfaced); CHANGELOG [Unreleased].
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 31/31;
libretro_info_audit 3/3, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* feat(libretro): keep FDS disk writes in the save directory
libretro v2.9.0 re-audit NL-03 (Medium).
An FDS game saves by writing to the disk in the drive (the FDS mapper's
`store_byte` mirrors the BIOS write stream into the raw side and sets
`disk_dirty`). Nothing in the libretro crate called `Nes::disk_is_dirty` /
`disk_image_bytes` / `clear_disk_dirty`, so every in-game save was lost when
content closed; only save states survived. The desktop frontend has
persisted the disk since v2.2.0 (`emu.rs` `flush_fds_save`,
`<data dir>/fds-saves/<hex>.fds.sav`).
Design choice: a sidecar in the frontend's save directory, not
RETRO_MEMORY_SAVE_RAM.
- libretro.h, RETRO_MEMORY_SAVE_RAM: "If save game data is too complex for a
single memory buffer, the SAVE_DIRECTORY (preferably) or SYSTEM_DIRECTORY
environment callback can be used." GET_SAVE_DIRECTORY (31): the save
directory "should be used to store SRAM, memory cards, high scores, etc,
if the libretro core cannot use the regular memory interface".
- The disk is not a live buffer: `disk_image_bytes` builds it on demand from
the drive state. As SAVE_RAM it would need a mirror buffer re-synced every
frame, and the frontend fills SAVE_RAM only after `retro_load_game`
returns, so the core would have to rebuild the console on the first
`retro_run`, after the memory maps (raw pointers into the old console)
were handed out. Reading the sidecar before the console is built has
neither problem.
- Same name and bytes as the desktop's `.fds.sav` (the headerless image,
keyed by the SHA-256 of the ORIGINAL image as loaded), so a save moves
between the two frontends; the key survives the image changing.
- The file lives at `<save dir>/RustyNES/<hex>.fds.sav`.
Mechanism:
- `load_fds` (factored out of `load_game`) builds the pristine console, then
`restore_fds_disk` boots `Nes::from_disk(saved)` when the file exists. A
file that no longer parses is logged and the pristine disk boots; the next
write replaces it. No save directory: logged at load, nothing persisted.
- `persist_fds_disk` writes the image when `disk_is_dirty()`, through a
`.tmp` file and a rename (`write_file_atomically`), clearing the dirty
flag only after the rename succeeds, so a failed write retries. It reads
the disk under its own `catch_unwind` and runs even on a poisoned core,
matching the maintainer's L-1.1 decision that a contained panic must not
cost the player's save.
- When: `tick_fds_flush` in `run_single`, FDS_FLUSH_FRAMES (60) frames after
the disk first goes dirty (a write sequence spans many frames; writing per
dirty frame would rewrite the image dozens of times per save), plus
`retro_unload_game`, `retro_deinit`, and the start of the next
`retro_load_game` for a frontend that skipped the unload.
- Cost on the hot path: one `disk_is_dirty()` per frame for an FDS game,
nothing for a cartridge (`fds_save_path` is None). The image allocation
happens only when dirty.
Test (C ABI): the harness now answers GET_SYSTEM_DIRECTORY /
GET_SAVE_DIRECTORY with per-process dirs under the OS temp dir
(`RustyNES/libretro-abi-<pid>`), and its synthetic 8 KiB BIOS now SAVES TO
THE DISK the way a game does, through $4023/$4024/$4025 and the $4030
byte-transfer flag: 200 gap bytes, then side A's 56-byte disk-info block
with the signature intact and a marker after it. The real disksys.rom is
never committed. abi_tests::fds_disk_writes_survive_closing_the_game:
clean boot has no marker in the save state (the FDS snapshot carries the
disk); 5 frames later it does (the BIOS wrote it) and no file exists yet;
unload writes `<save>/RustyNES/<sha>.fds.sav` with the marker at raw 15;
with the file removed, 70 frames without unload write it again; the next
load of the ORIGINAL image boots the saved disk (marker in the state with
no frame run).
Red: the test run against HEAD's lib.rs (this commit's tests, the previous
commit's core; lib.rs copied aside and restored with cp -p + cmp + touch):
"unloading must save the written disk to .../RustyNES/4b15e5...fds.sav".
Mutations (restored the same way):
- periodic flush never firing -> "a written disk must be saved within a
second";
- the saved image not used at load -> "the saved disk must be the one in
the drive".
Not done: no GET_SAVE_DIRECTORY fallback to the content or system
directory (libretro.h permits a heuristic; a frontend with no save
directory is logged instead). A frontend's own cloud sync of the saves
folder is inferred to pick the file up, not traced in RetroArch.
Docs: advanced_features.md FDS section ("In-game saves"); crate preamble;
CHANGELOG [Unreleased].
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 32/32;
libretro_info_audit 3/3, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* feat(libretro): Vs. System palette, DIP defaults, coins and service
libretro v2.9.0 re-audit NL-08 (Medium).
The libretro load path was `Emu::from_rom` alone, which consults the
SHA-keyed `vs_db` only to recognise a DualSystem cabinet. The desktop's
`apply_vs_db` additionally applies the entry's PPU type (the colour LUT) and
DIP default, and binds `insert_coin` / `set_vs_service` to keys. The libretro
crate called none of `insert_coin`, `clear_coin`, `set_vs_service`,
`set_vs_dip`, `set_vs_ppu_type`. The re-audit measured, on 7 local Vs.
UniSystem dumps in the DB: the DB palette changes the picture in 7/7 (iNES
1.0 dumps default to the 2C03 LUT, which vs_db.rs calls the wrong colours),
and a coin changes WRAM in 6/7 with no libretro input able to produce one.
Palette + DIP: `apply_vs_database(&mut Nes)` = `vs_db::lookup(rom_sha256)`
then `apply_vs_entry`, which sets `set_vs_ppu_type(entry.vs_ppu_type)` and
`set_vs_dip(entry.vs_dip)`. Called on the single console after load and on
BOTH consoles of a cabinet, as the desktop does. Both setters are no-ops on a
non-Vs. cart, and the PPU type changes only the output LUT. The desktop's DIP
precedence is config > DB > 0; this core has no config equivalent, so DB > 0.
Coins + service (`VsPanel`): read from the same one-call RetroPad bitmask
the pads already use (`read_joypad`; `joypad_to_buttons` is now
`nes_buttons(read_joypad(..))`), so a Vs. cart adds no `input_state` call
(the L-2.5 counting test is unchanged).
- L on port n drops a coin in acceptor n: single cart ports 1-2 ->
acceptors 1-2; cabinet ports 1-2 -> main 1-2, ports 3-4 -> sub 1-2
(`VsDualSystem::insert_coin` 0-3).
- R on port 1 holds the service button (cabinet: port 3 = sub panel).
- A coin is an edge-triggered PULSE of VS_COIN_HOLD_FRAMES = 3 frames (the
desktop's constant; 50 ms against the 40-70 ms the core documents for the
switch), not a level, so a held L is one coin. A new coin re-arms the
pulse; `clear_coin` releases all latched coins when it expires.
- Only when `Nes::is_vs_system()` (single) or a cabinet is loaded; other
carts take no Vs. path at all.
L and R are unused by a NES pad and keep each player's coin on their own
controller; the audit suggested Select/L/R, but Select is a real NES button.
Descriptors: two more `'static` tables (`VS_SINGLE_DESCRIPTORS`, 35 + NULL;
`VS_DUAL_DESCRIPTORS`, 38 + NULL) built by a generalised const
`port_descriptors::<N>(extras)`, whose `assert!(N == 32 + extras + 1)` makes
a mis-sized table a compile error. Sent at load for a Vs. cart (libretro.h:
SET_INPUT_DESCRIPTORS "can be called at any time"), and the standard table
is sent back at unload or at the next non-Vs. load (`describe_inputs`).
Tests, red first where reachable:
- abi_tests::vs_system_coins_and_service_reach_the_game: a synthetic 16 KiB
NES 2.0 Vs. NROM whose program copies $4016/$4017 into WRAM $00/$01, driven
through the C ABI with a fake RetroPad bitmask. Before the fix: "port 1 L
drops a coin in acceptor 1: 0x00". Asserts coin bit 5, pulse not level,
acceptor 2 bit 6, service bit 2 held and released, the L/R descriptors
while loaded and gone after unload.
- abi_tests::vs_dual_system_coins_reach_the_main_console (same ROM with
byte 13 = 0x50): red "port 1 L: main acceptor 1: 0x00"; also a port-3 coin
does not reach the main console.
- tests::a_vs_database_entry_sets_the_palette_and_the_dips: nestest
re-headed as a Vs. cart with a hand-made 2C04 entry: DIP 0xA5, picture
changed; no entry, no change. (New helper, so no red is possible.)
- abi_tests::a_vs_dump_in_the_database_renders_with_its_palette and
tests::database_listed_vs_dumps_get_their_entry: LOCAL ONLY. The committed
corpus has no Vs. dump; in CI both find none and print a skip. Locally
(the gitignored external dumps) the first DB-listed single cart, "Stroke &
Match Golf (VS)", is loaded through the C ABI and its 300th presented
frame equals a console with the DB entry applied and differs from one
without; 23 DB-listed dumps get their DIP entry.
Mutations (restored with cp -p + cmp + touch):
- DB not applied at load -> the local palette ABI test fails ("the core must
present the database's palette");
- coin as a level (no edge detection) -> "a coin is a pulse, not a level:
0x20";
- Vs. descriptors not withdrawn at unload -> the coin test fails.
Not done: user-set DIP switches (a core option). Eight switches do not fit
the legacy one-list-per-option SET_VARIABLES without eight options or a
256-value list; that wants core-options v2, which this core does not use.
Written up in advanced_features.md. The DB default is applied.
Also: corrects the NL-02 sweep test's doc comment, which claimed it covered
the synthetic MMC1 sizes (the C-ABI test does).
Docs: advanced_features.md new "Vs. System palette, DIP switches, coins and
service" section with the mapping table; CHANGELOG [Unreleased].
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 37/37 (local, with
the external dumps linked in; the two local-only tests skip in CI);
libretro_info_audit 3/3, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(libretro): .info core_options and description; audit the flags
libretro v2.9.0 re-audit NL-04 (Low).
`crates/rustynes-libretro/rustynes_libretro.info` said
`core_options = "false"` although the core has declared the
`rustynes_four_score` option through SET_VARIABLES since v2.8.1 (L-S1), and
its `description` still said "a strict, lockstep master-clock scheduler at
PPU-dot resolution", the scheduler v2.0.0 "Timebase" replaced (ADR 0002 /
0029) and the same claim L-3.5b removed from architecture.md. UPSTREAM_SYNC.md
already recorded this exact class once (`disk_control` false while disk
control was wired, fixed in v2.2.4) and listed the capability flags as
"assert by hand"; `libretro_info_audit` checked only licence, version and
extensions.
Fix, LOCAL `.info` only (the libretro-super copy RetroArch downloads follows
at the v3.0.0 sync, per ADR 0041; nothing upstream was touched):
- `core_options = "true"`;
- the scheduler sentence now describes the one-clock, two-halves-per-cycle
model; the feature sentence adds what v2.9.0 made true (in-game FDS disk
saves, labelled disk swap, Four Score, Vs. palettes + coin/service) and
drops "true RGB Vs. System arcade boards", which overstated a core that
rendered listed Vs. dumps in the 2C03 palette until NL-08.
Audit (crates/rustynes-test-harness/tests/libretro_info_audit.rs):
- libretro_info_capability_flags_match_the_core: `core_options`,
`disk_control`, `memory_descriptors`, `input_descriptors` must each be
"true" exactly when the environment call implementing it appears in
rustynes-libretro/src/lib.rs (`set_variables(`/`set_core_options_v2(`,
`set_disk_control[_ext]_interface(`, `set_memory_maps(`,
`set_input_descriptors(`). Source-derived, like the extensions check,
because the crate is cdylib+staticlib and cannot be linked from here.
- libretro_info_description_names_no_retired_design: rejects "lockstep",
"PPU-dot resolution", "dot-lockstep".
Red first: both new tests failed on the old `.info` ("core_options =
\"false\", but the core implements it"; "description still says
\"lockstep\""). Mutation: `disk_control = "false"` -> the flag test fails
(restored with cp -p + cmp + touch).
Docs: UPSTREAM_SYNC.md field table (four flags now audited, description
partly) and the second occurrence of the drift recorded; CHANGELOG
[Unreleased].
Gates: cargo fmt --all --check; cargo clippy -p rustynes-libretro
--all-targets -D warnings; cargo test -p rustynes-libretro 37/37;
libretro_info_audit 5/5, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* docs(libretro): record the dual-cabinet serialize cost and libnx status
libretro v2.9.0 re-audit NL-09 (Low/Info) and the L-3.2 libnx side note.
Documentation only; no code changed.
NL-09 -- measured, not changed. A Vs. DualSystem cabinet's
`retro_serialize` uses `VsDualSystem::snapshot`: both consoles' full
`Nes::snapshot` including the THM thumbnail, into a fresh ~645 KB Vec per
call, where a single console uses a pooled `snapshot_core_into`.
Probe (session scratchpad, release, against rustynes-core): the synthetic
cabinet (nestest, NES 2.0 byte 13 = 0x50), 60 frames, then six interleaved
rounds of 400 calls each of the current `dual.snapshot()`, a candidate that
writes both consoles' `snapshot_core_into` into pooled buffers and assembles
the container into a pooled Vec (latch byte a placeholder: a cost model,
not a restorable blob), and one single `snapshot_core_into`. Serialize
only; restore is NL-01's. Host loaded by other sessions (load average 9-15).
- run 1: current 428-1,630 us (645,529 B); candidate 54-78 us (522,615 B);
single 18-24 us (261,300 B);
- run 2: current 314-465 us; candidate 46-50 us; single 15-17 us.
Same sign in all 12 rounds, a factor of 6 to 20.
Why no code: `scripts/perf/ab_check.sh` adjudicates only the `full_frame`
workloads, which a serialize-only change does not move, so the project's
adoption rule cannot be applied as written and is not claimed to be met.
The fix belongs in rustynes-core (`VsDualSystem::snapshot_into`, no THM,
caller's buffer) on the same type whose `restore` NL-01 is making atomic,
with a restore round-trip test and a serialize workload added to the A/B
tooling. Recorded in docs/performance.md (new v2.9.0 section). It affects
the four DualSystem titles only.
The audit's documentation half was true and is fixed: architecture.md said
the buffers are "sized once so the frame loop does not allocate". It now
states that `retro_run` does not allocate in steady state and lists the
three exceptions: the cabinet serialize (every call), the single serialize
buffer's first growth, and the FDS disk image built once a second after a
game writes its disk (NL-03). Its "battery RAM" memory-map bullet is
corrected for NL-02.
libnx -- recorded OPEN, not built. The crate Makefile maps `platform=libnx`
to `aarch64-nintendo-switch-freestanding`: `rustc --print target-list`
lists it, `rustup target list` offers no rust-std for it (tier 3), and a
`*-freestanding` target has no std while the wrapper needs it (`std::fs`,
`CString`, `catch_unwind`). A nightly `-Zbuild-std` attempt was not made.
The buildbot never reads the Makefile's platform table and has no Rust
Switch template (.gitlab-ci.yml header), so nothing shipped is affected;
the mapping and the buildbot matrix are unchanged. Written into
architecture.md's compile-targets list.
CHANGELOG [Unreleased] carries one bullet for both.
Gates: cargo fmt --all --check; cargo test -p rustynes-libretro 37/37;
libretro_info_audit 5/5, libretro_makefile_audit 5/5,
snapshot_schema_audit 8/8; markdownlint via pre-commit.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* docs(audits): the libretro ledger's v2.9.0 section
Records the re-verification (23 rows, all hold) and a disposition for
NL-01..NL-10 with the cherry-picked commits (3153f170..1aee5565).
NL-01 is tracked with the core finding NC-03 (same root cause); NL-09
is recorded, not changed, for v2.9.1; the libnx mapping stays open.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(core,libretro): make every failed restore leave the machine untouched
v2.9.0 re-audit NC-03 + NL-01 (same root cause) and NC-05.
Mechanism. `Nes::apply_snapshot` mutates in stages: the BUS, PPU, APU
and MAP sections through `LockstepBus::restore`, then the CPU section,
then the cross-section CPU/PPU clock check (F-05). A rejection at any
later stage returns with the earlier stages applied. v2.7.4 (MOB-08)
fixed that with a snapshot-then-rollback in `restore_inner`, but only
when `clear_rewind` was set, i.e. only on the LOUD `restore`. The
comment justified skipping it on `restore_quiet` with a premise: quiet
restores "only ever restore snapshots this core just wrote, which cannot
fail these checks".
Root cause. The premise was false. `rustynes-libretro`'s
`on_unserialize` calls `restore_quiet` for EVERY `retro_unserialize`: a
user's Load State from disk, a state written by an older core (every
section version is compared with `!=`), a netplay peer's state, a
truncated or corrupt file. The quiet/loud split is about the rewind
ring (same timeline or not); nothing about it says the bytes are
trusted. The re-audit's `partial` probe showed a rejected CPU section
leaving the bus, PPU, APU, mapper and WRAM from a frame-3000 file under
a frame-5 CPU, with the F-05 clock check never reached; RetroArch then
kept emulating that machine after reporting the load as failed.
Fix. `restore_inner` now takes the backup on both paths. It is the
THM-less `snapshot_core_into` (the thumbnail is ignored on restore, so
the backup restores exactly what the loud path's full `snapshot()` did)
into a new `restore_backup: Vec<u8>` pooled on the `Nes`, so the hot
path (run-ahead's and netplay's per-frame rollbacks) allocates nothing
for it in steady state. The loud path also stops building a 61 KiB
thumbnail it only ever threw away. The clock check needs no separate
wiring: it is the last stage of `apply_snapshot` on both paths, and a
failure at it is now rolled back like any other.
Cost, measured roughly (release, nestest, a machine at load average ~36
from other sessions, so NOT an A/B under the ab_check rule): the added
`snapshot_core_into` is ~22 us against ~850 us for `restore_quiet`
itself and ~14.5 ms for a frame on that loaded box, i.e. well under 1%
of a frame. Recorded in docs/frontend.md.
Vs. DualSystem (NC-05). `VsDualSystem::restore` did
`self.main.restore(main)?; self.sub.restore(sub)?;`. Each console rolls
itself back, but a valid main block followed by a rejected sub block
left main on the file's timeline and sub on the running one, sharing a
WRAM re-converged only on the success path. Main is now snapshotted
first and put back with `restore_quiet` if the sub block fails (quiet
because the loud restore that succeeded already bumped the timeline
generation and cleared main's ring; neither is emulated state). libretro
reaches this through `dual.restore` for the four DualSystem boards.
The false comment at nes.rs (the v2.7.4 premise) is replaced by the
account above; the old sentence is quoted in docs/frontend.md next to
the correction, per the project's rule of keeping disproved prose
visible.
ROM hash tag (NL-01's note) -- recorded as a decision, not added. The
header tag is the first bytes of `sha256_of(bytes)` over the WHOLE file
including its iNES header (`Nes::from_rom`), so checking it on restore
would reject every legitimate state after a header-only fix of the same
dump (corrected mapper number, mirroring or battery bit), and
`Nes::restore`'s rustdoc has always documented loading from a different
ROM as allowed. The decision and its reason are on `Nes::restore` and
in docs/frontend.md; a PRG+CHR-keyed tag would make the check possible
and is a format change for the maintainer to weigh.
Red evidence (each run on the unfixed code, then green after):
- rustynes-core `nes::tests::a_failed_quiet_restore_leaves_the_machine_untouched`
(new): FAILED "stale CPU version: a failed quiet restore changed the
machine". Covers two rejection depths: the CPU section version (after
all four bus sections applied) and the clock check (after the CPU
applied too), and asserts the rewind ring is untouched.
- rustynes-test-harness `vs_dualsystem_synth::a_dual_restore_with_a_rejected_sub_block_changes_neither_console`
(new): FAILED "a rejected dual restore changed the cabinet (main
148905 / sub 446711)" -- main at the EARLY state's cycle.
- rustynes-libretro `abi_tests::a_rejected_unserialize_leaves_the_machine_as_it_was`
(new, through the C ABI: serialize, run 50 frames, unserialize the
early state with its last CPU section's version bumped, serialize
again): single and dual.
Mutations (each restored with cp -p + cmp + touch afterwards):
- M1, rollback only when `clear_rewind` (the pre-fix behaviour):
CAUGHT by the core quiet-restore test and by the libretro test
("a rejected unserialize changed the console"); the loud MOB-08 test
still passes, as it should.
- M2, the dual rollback replaced by `Ok(())`: CAUGHT by the dual synth
test and the libretro test ("a rejected dual unserialize changed the
cabinet").
Gates: cargo fmt --check; cargo clippy --workspace --all-targets -D
warnings (clean); thumbv7em no_std build; cargo test -p rustynes-core
(211 + 0/2 ignored), -p rustynes-libretro (26), --test
vs_dualsystem_synth (4); markdownlint on the two docs.
Not done: the dual container still allocates its main backup (and, per
NL-09, its whole 645 KB snapshot) per call; pooling the dual path is
NL-09's subject and was left to it.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(core): bound restored master clocks absolutely, not only their skew
v2.9.0 re-audit NC-04.
Mechanism. `LockstepBus::check_restored_clocks` (v2.7.0, F-05) rejected
a restored CPU `master_clock` and bus `ppu_clock` more than
RESTORED_CLOCK_SKEW_MAX (1,024) master clocks apart, because a far-apart
pair made `run_ppu_to` tick billions of dots or never tick at all. It
bounded only the difference. The CPU advances its clock with
`wrapping_add` (cpu.rs:446/707/751) and the PPU catch-up loop is
`while self.ppu_clock + ppu_div <= target`, so a crafted state with
BOTH clocks ~20,000 master clocks below 2^64 passed the skew check,
the CPU clock wrapped to a small value within a frame, the PPU clock
stayed near 2^64, and the loop condition never held again: the frame
counter froze and NMI never fired. That is exactly the state F-05 set
out to reject. `run_frame` still returned (on its CPU-cycle budget), so
it read as a frozen picture, not a hang.
Fix. A second bound, RESTORED_CLOCK_MAX = 2^62, applied to the larger
of the two clocks (the smaller is then bounded too, being within the
skew). Derivation, recorded on the constant: the fastest master clock
is NTSC/Dendy's ~21.477 MHz, so 2^62 master clocks is ~2.15e11 s, about
6,800 years of continuous emulation, which no genuine state can reach;
the headroom above it is 3 x 2^62, about 20,000 more years, so nothing
from an accepted state can wrap or overflow the catch-up addition.
Red evidence. New `nes::tests::restored_clocks_near_the_top_of_u64_are_rejected`
shifts BOTH clocks by the same amount (the PPU through the existing test
seam, the CPU's by patching its unique 8 bytes inside the CPU section),
restores into a fresh machine, and for accepted states requires the
frame counter to advance over two frames. Run with the constant present
but the check absent, it FAILED: "at the ceiling: the PPU stopped" (the
ceiling then being u64::MAX, i.e. the re-audit's demonstration). Cases:
unshifted and a century of emulation (accepted and running), exactly at
2^62 (accepted and running), 2^62 + 1 and 20,000 below 2^64 (rejected).
Mutation. `if false && highest > RESTORED_CLOCK_MAX`: CAUGHT
("past the ceiling: Ok(())"); the existing skew test still passes.
Restored by reversing the edit, touched, re-run green.
Output unchanged for any genuine state: the bound is 6,800 years out.
docs/frontend.md (the save-state section) and CHANGELOG updated.
Gates: fmt --check; clippy -p rustynes-core --all-targets -D warnings;
thumbv7em no_std build; cargo test -p rustynes-core (212 passed).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(mappers): mirror an undersized PRG-ROM in the multicart 32K window
v2.9.0 re-audit NC-01.
Mechanism. Seven discrete multicart boards in multicart_discrete.rs
index a 32 KiB PRG window: Multicart61 and Multicart62 (their 32 KiB
path), Multicart202 and Multicart212 (`prg_32k_mode`), Multicart58
(`prg32_mode`), and `DiscreteMapper::prg_32k`, which serves mappers 46
and 57. Each computed a bank count, reduced the bank modulo it, and
indexed `prg_rom[bank * 32K + off]` with `off` up to $7FFF:
- a 16 KiB image clamps the 32 KiB count to 1 via `.max(1)`; bank 0's
window then runs 16 KiB past the end;
- 202 and 212 count 16 KiB banks and keep an even `lo16` for 32 KiB
mode, so an odd count can put the second half past the end (212 at
48 KiB and 80 KiB; 202 at 80 KiB -- at 48 KiB 202's decode only
reaches `lo16` 0 or 1, which fits, because its 32K-mode bit forces
page bit 2).
The constructors accept these sizes, so an undersized dump or a header
typo crashed the emulator thread (native), the tab (wasm32), or the
core (libretro, contained) on the first fetch from $C000-$FFFF after a
write selected 32 KiB mode.
Fix. Each of the six index expressions is taken modulo
`prg_rom.len()`, as `m004_mmc3.rs` already does. That is the hardware
answer (a smaller part in a larger window leaves address lines
unconnected and mirrors) and it is the identity whenever the index is
already in bounds, so every image that ran before produces identical
output; only accesses that previously panicked change. The 16 KiB and
8 KiB helpers are untouched: iNES sizes are multiples of 16 KiB, and
they were not in the finding. Bounds only; the file's Provenance header
(Mesen2, for other boards in the file) is untouched and no attribution
comment was altered.
Red evidence. New crates/rustynes-mappers/tests/undersized_prg.rs: for
each of the seven mappers at 16/48/80 KiB of PRG (CHR-RAM for 61, whose
constructor requires it), every write address $6000-$FFFF once, then
20,000 xorshift writes, each followed by reads across the window, under
catch_unwind, failures collected. On the unfixed tree it FAILED with
exactly the re-audit's set: 46, 57, 58, 61, 62 at 16 KiB; 202 at 16
and 80 KiB; 212 at 16, 48 and 80 KiB. A size a board REFUSES is
reported as a failure too, so the matrix cannot shrink silently (the
first draft hit that for 61 with CHR-ROM, which is why it is CHR-RAM).
Mutation. Reverting the modulo at two sites (58, 212): CAUGHT, and
precisely -- "mapper 58, 16 KiB PRG", "mapper 212, 16/48/80 KiB PRG".
Restored by reversing the edits, touched, green.
Gates: fmt --check; clippy -p rustynes-mappers --all-targets -D
warnings; thumbv7em no_std build; cargo test -p rustynes-mappers (lib
769 + all integration suites green); markdownlint. docs/mappers.md
gains edge case 11; CHANGELOG [Unreleased] gains a Mappers entry.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(mappers): refuse a GTROM save state whose banks the board lacks
v2.9.0 re-audit NC-02.
Mechanism. `Gtrom111::load_state` (mapper 111, GTROM/Cheapocabra)
assigned `prg_bank`, `chr_bank` and `nt_bank` raw from the blob after
checking only its length and version. The fetch paths use them
unmasked: `cpu_read` indexes `prg_rom[prg_bank * 32K + ..]`,
`chr_offset` `chr_bank * 8K + ..` into 16 KiB of CHR-RAM, `nt_offset`
`nt_bank * 4K + ..` into 8 KiB of nametable RAM. So a corrupt or edited
`.rns` (or RetroArch `.state`) for a GTROM game restored with `Ok(())`
and the next CPU fetch from $8000 panicked -- the IMP-01 class (a
deferred out-of-bounds read) inside a mapper section. The v2.7.4
rollback cannot help, because nothing fails during the restore. The
re-audit's whole-machine probe hit it through the LOUD `Nes::restore`
("index out of bounds: the len is 32768 but the index is 8372319"),
and the mapper-level probe at bytes 2 and 3 as well; no other board of
the 296 configurations swept produced an index panic.
Fix. Validate before assigning anything: reject a PRG bank at or above
the 32 KiB bank count and a CHR or nametable bank above 1, as
`MapperError::Invalid` naming all three values. Those are exactly the
values `update_register` can produce ((value & 0x0F) % count, and one
bit each), so every genuine state still loads and emulation output is
unchanged.
Red evidence (both on the unfixed code):
- rustynes-mappers `homebrew_boards::tests::m111_load_state_rejects_banks_the_board_cannot_hold`
(new): FAILED "byte 1 = 0x08 must be rejected". It also loads every
value the register CAN produce (PRG 0..7, CHR 0..1, NT 0..1 on a
256 KiB board) and fetches at the top of each window, so the check
cannot over-reject.
- rustynes-core `nes::tests::a_gtrom_state_with_an_impossible_bank_is_refused_and_the_game_runs_on`
(new, whole machine through `Nes::restore` on a synthetic mapper-111
image, MAP body bytes 1/2/3 = 0xFF): FAILED "MAP byte 1 = 0xff must be
refused". It then requires the machine unchanged and two more frames
to run.
Mutation. The check reduced to `prg_bank > prg_banks` (off by one, and
the CHR/NT bounds dropped): CAUGHT by both tests ("byte 1 = 0x08 must
be rejected"; "MAP byte 2 = 0xff must be refused"). Restored by
reversing the edit.
Gates: fmt --check; clippy -p rustynes-mappers -p rustynes-core
--all-targets -D warnings; thumbv7em no_std build; cargo test -p
rustynes-mappers (lib 770 + integration suites), -p rustynes-core --lib
(213); markdownlint. docs/mappers.md's mapper-111 row and CHANGELOG
updated. No Provenance header in homebrew_boards.rs.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* fix(mappers): bound the BS-5 DIP and FDS audio prescaler on restore
v2.9.0 re-audit NC-06 and NC-07: two mapper save-state fields restored
raw and then used in arithmetic that cannot take every byte value.
NC-06, mapper 286 (Waixing BS-5). `SimpleBmc::load_state` did
`self.dip = data[c + 3]` unbounded, and the `$A000` PRG-select decode
evaluates `addr & (1u16 << (self.dip + 4))`. A restored dip >= 12
overflowed the shift ("attempt to shift left with overflow" from 0x80
or 0x41) and 0xFF overflowed the add, in every build with overflow
checks (dev, test, fuzz); in release the shift amount wraps and the
decode is silently wrong. The board has four DIP settings: nesdev's
mapper 286 page (fetched for this fix; it is not in the vendored wiki)
says "The AND mask to which the PRG-ROM select responds is determined
in bits 4-7 by a DIP switch", settings 01/02/04/08, i.e. dip 0..=3 in
this decode. `load_state` now rejects dip > 3 as MapperError::Invalid,
checked before any field is assigned. No code sets the DIP to anything
but 0 and no other SimpleBmc board reads it, so no genuine state is
affected.
NC-07, FDS audio. `FdsAudio::read_tail` restored `cycle_prescaler =
src[off]` raw; `clock` does `+= 1` then resets at `>= 16`. 0xFF
overflowed the add (the re-audit's fdsprobe: "fds.rs:1021 :: attempt
to add with overflow", state offset 41108), and in release any 16..=254
delayed one wave/modulation clock by up to 240 CPU cycles. `read_tail`
now rejects a prescaler >= 16 before assigning anything (the byte sits
at TAIL_LEN - 3, before the two flag bytes). fds.rs carries a
Provenance header (puNES, for the drive-timing table); it is untouched
and this is a bounds check only.
Red evidence (both on the unfixed code):
- `bmc_simple::tests::bs5_load_state_rejects_a_dip_the_board_cannot_have`:
FAILED "DIP 0x04 must be rejected". It also loads dip 0..=3 and
writes every $A000-$AFFF address under each.
- `fds::tests::a_restored_audio_prescaler_outside_0_to_15_is_rejected`,
through the whole device's `load_state` at the byte's real offset
(the audio tail is located in the blob by a distinctive wavetable and
must occur exactly once): FAILED "prescaler 0x10 must be rejected".
It also loads 0..=15 and runs 64 cycles.
Neither red run reproduced the debug-build panic itself (the tests
assert rejection at load time); that demonstration is the re-audit's.
Mutations (each reversed afterwards): DIP bound `> 3` -> `> 11` (the
loosest bound that avoids the panic but admits impossible settings):
CAUGHT "DIP 0x04 must be rejected". Prescaler `>= 16` -> `> 16`:
CAUGHT "prescaler 0x10 must be rejected".
Gates: fmt --check; clippy -p rustynes-mappers --all-targets -D
warnings; tests with and without `mapper-audio`; thumbv7em no_std build;
cargo test -p rustynes-mappers (lib 772 + integration suites);
markdownlint. docs/mappers.md edge case 12 states the rule these and
NC-02 share; CHANGELOG updated.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* docs(frontend): record the Zapper light model as default-on since v2.3.6
v2.9.0 re-audit NC-08.
docs/frontend.md's "Zapper light-timing" passage still described the
beam-relative temporal model as "(opt-in)", "**default off**", and
closed with "It stays opt-in because there is no pass/fail light-gun
test ROM to adjudicate it". The model has been on by default since
v2.3.6. Verified in code before editing: `LockstepBus::new`
initialises `zapper_temporal_light: true` (bus.rs, "v2.3.6: ON by
default"), it is the only constructor site, the frontend never calls
`set_zapper_temporal_light`, and both rustdocs (`Nes::` and
`LockstepBus::set_zapper_temporal_light`) say "Default ON since
v2.3.6". The core audit's §4.7 fixed those rustdocs and missed this
subsystem doc.
The passage now states the promotion and its measured reason (Duck
Hunt: the frame model answers frame N's reads with frame N-1, the
inverse of what the game needs, so nothing could be hit; score 000000
vs 000500 on the same ROM/aim/inputs), taken from the rustdoc that
records it. Per the project's rule for disproved prose, the old
"stays opt-in" sentence is quoted beside the correction rather than
deleted, and the lead-in's "supported titles re-poll every frame, so
frame-granular sampling suffices" -- the premise the promotion refuted
-- is replaced.
Docs only; no code or output change. markdownlint passes.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014qfTKi2M3swo7qnwvYCkDj
* test(fuzz): let the save_state target reach eight mappers' state
v2.9.0 re-audit, follow-up to NC-02: the re-audit noted the
`save_state` fuzz target could not have found the GTROM bank defect
because its only base machine was NROM (fuzz_targets/save_state.rs),
whose `MAP ` section is nearly empty.
Change. `synth_nrom()` becomes `synth(mapper, prg16, chr8)`, which
writes the same program and vectors into EVERY 16 KiB bank (so it runs
whatever a mapper powers up with at $C000-$FFFF) and uses a NES 2.0
header only for mapper…1 parent b2d5da8 commit 955d2fd
78 files changed
Lines changed: 8895 additions & 438 deletions
File tree
- .github
- release-notes
- workflows
- crates
- rustynes-core/src
- rustynes-cosim
- rustynes-frontend/src
- tastudio
- rustynes-hdpack
- src
- tests/fixtures
- rustynes-libretro
- src
- rustynes-mappers
- src
- tests
- rustynes-mobile/src
- rustynes-script/src
- rustynes-test-harness/tests
- docs
- adr
- agents
- audits
- libretro
- fuzz/fuzz_targets
- to-dos
- plans
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
545 | 545 | | |
546 | 546 | | |
547 | 547 | | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
548 | 557 | | |
549 | 558 | | |
550 | 559 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
0 commit comments