feat(accuracysnes): the ares host runs the battery — A2.10 is settled 3-vs-1 - #304
Conversation
… 3-vs-1
Three setup steps were missing, and all three fail as the SAME segfault inside
System::load with a backtrace pointing at memory setup rather than at what is
actually absent:
1. ares::Memory::FixedAllocator::get() before anything touches a core --
Bus::reset() allocates its page tables from that bump allocator;
2. ares::SuperFamicom::option("Pixel Accuracy", "true") before load() --
PPUBase::implementation is null until setAccurate picks one of the two
PPUs, and Bus::reset() calls ppu.map() -> implementation->map();
3. nall/main.hpp included by the host translation unit -- without it the
LINK fails with a bare `undefined reference to main` from crt1.o.
It now reports magic ACSN / done a5 / count 338 / passed 299 / failed 5 /
skipped 1 / golden 33, identically across runs.
A2.10 IS SETTLED. Several findings sat at 2-versus-1 with no tiebreaker,
because this project's provenance rule counts ares and bsnes as one reference
-- so "RustySNES and snes9x against Mesen2" is only 2-vs-1 if ares is not
already on RustySNES's side, and nobody could check. ares passes A2.10
(catalogue index 11, status $01), making it 3 against 1 with Mesen2 the
outlier. The caveat that held the row open -- a harness bug upstream of an
implementation gives the same signature, as the $F8/$F9 retraction proved --
is answered by a fourth host independent of all three.
Five rows where ares disagrees with the cart (C7.05, C7.10, E8.02, E3.06,
F1.10) are recorded and deliberately NOT adjudicated. F1.10 is suspect of this
host first: it is a PAD2_CONTRACT row and the port detection here is assumed
rather than verified. Not wired into crossval.sh for that reason -- an
ARES_KNOWN_FAILURES constant encoding unexamined disagreements would be worse
than no third reference at all.
Also fixes the results-block offsets: R_COUNT is +$06 and R_PASSED is +$0A,
and reading the latter as the former reported "count 299" for a 338-test
battery, which looks like a truncated run rather than a misread field.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
WalkthroughThe headless ares host now runs the AccuracySNES battery reproducibly. It initializes required components, selects the accurate PPU, reports corrected result counts, captures crashes, and documents A2.10 validation and unresolved disagreements. Changesares AccuracySNES validation
Sequence Diagram(s)sequenceDiagram
participant AccuracySNES
participant ares_host
participant ares_allocator
participant SuperFamicom
AccuracySNES->>ares_host: start test battery
ares_host->>ares_allocator: initialize fixed allocator
ares_host->>SuperFamicom: select accurate PPU and load
SuperFamicom-->>ares_host: execute tests and expose results
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
Comment |
Antigravity review (Gemini via Ultra)This PR updates the C++ headless ares test host ( Blocking issuesNone found. Suggestions
Nitpicks
Automated first-pass review by |
There was a problem hiding this comment.
Pull request overview
Adds a working headless ares host for running the AccuracySNES battery (providing a third reference implementation), and updates project docs to record that A2.10 is now settled as a 3-vs-1 (with Mesen2 as the outlier).
Changes:
- Make the ares host runnable in-sandbox by adding an internal crash backtrace handler and required setup steps; extend results-block reporting (count/passed/failed/skipped/golden).
- Fix results-block field offsets used by the host output.
- Update
accuracysnes-plan.mdandCHANGELOG.mdto reflect the new ares-based arbitration outcome forA2.10.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/accuracysnes/ares_host/README.md | Updates documentation to reflect the host now runs and records newly observed ares disagreements. |
| scripts/accuracysnes/ares_host/build.sh | Builds the ares host with debug symbols/backtrace support. |
| scripts/accuracysnes/ares_host/ares_host.cpp | Adds in-process crash backtraces, a --verbose mode, and corrected results-block parsing/printing. |
| docs/accuracysnes-plan.md | Records A2.10 as settled by adding ares as a third opinion. |
| CHANGELOG.md | Summarizes the ares host milestone and the A2.10 arbitration outcome. |
| **Treat `F1.10` as suspect-of-this-host first.** It is a `PAD2_CONTRACT` row, and this host's port | ||
| detection (`port->name().find("2")` on the button's grandparent) is *assumed* to work, not verified. | ||
| Check that before concluding anything about ares. |
| # link fails with a bare `undefined reference to 'main'` from crt1.o, which reads like a missing | ||
| # object rather than a missing shim. | ||
| g++ -std=c++2b -O2 -c "$HERE/ares_host.cpp" -o "$BUILD/ares_host.o" \ | ||
| g++ -std=c++2b -O2 -g -rdynamic -c "$HERE/ares_host.cpp" -o "$BUILD/ares_host.o" \ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/accuracysnes/ares_host/ares_host.cpp`:
- Around line 22-27: Update crashHandler to use only async-signal-safe
operations in the signal path: remove fprintf, backtrace, and
backtrace_symbols_fd, and emit only a fixed diagnostic via write before
preserving _exit(9). If backtrace symbolization is required, move it to a parent
process or post-mortem tool and verify the implementation against the target
POSIX/libc signal-safety contract.
- Around line 156-171: Validate the WRAM result block before emitting any output
in the results-reporting flow: confirm the magic is “ACSN”, version is 1, done
is 0xA5, and the count read via rd16 at offset 0x06 is within 1..512; return or
otherwise reject invalid blocks before printing any result fields. Also update
the status-processing bound to handle all entries up to the validated count
without silently truncating counts above 512.
- Around line 100-102: Move the positional-argument arity validation to after
`arguments.take("--verbose")` in the argument parsing flow, before accessing
`arguments[0]` or `arguments[1]`. Ensure the check uses the post-option-removal
count and rejects invocations missing either the ROM or budget argument.
In `@scripts/accuracysnes/ares_host/README.md`:
- Around line 5-18: Add language identifiers to both changed Markdown fences:
update the fence in scripts/accuracysnes/ares_host/README.md lines 5-18 and the
fence in CHANGELOG.md lines 19-21 to use text or console, preserving their
existing contents.
- Around line 52-66: Update scripts/accuracysnes/ares_host/README.md lines 52-66
to separate the allocator and PPU omissions, which cause System::load
segmentation faults, from the nall/main.hpp omission, which causes a link-time
undefined reference to main. Apply the same corrected separation to CHANGELOG.md
lines 33-39; both documentation sites must describe the implementation and build
behavior accurately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 6571b6cd-1e12-4303-bb37-4bc9977c0b8b
📒 Files selected for processing (5)
CHANGELOG.mddocs/accuracysnes-plan.mdscripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/ares_host/ares_host.cppscripts/accuracysnes/ares_host/build.sh
📜 Review details
⏰ Context from checks skipped due to timeout. (6)
- GitHub Check: test-light
- GitHub Check: accuracysnes
- GitHub Check: lint
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: build demo + docs
- GitHub Check: review
🧰 Additional context used
📓 Path-based instructions (8)
**/*
📄 CodeRabbit inference engine (CONTRIBUTING.md)
**/*: Do not commit or vendor the generatedsnesdev_wiki/mirror; it is gitignored and intended only as a local reference.
Keep commits focused and use Conventional Commits:<type>(<scope>): <subject>, with an imperative subject of at most 72 characters.
Do not use emojis in code, comments, or commit messages.
Before opening a PR, ensure formatting, Clippy, workspace tests, the core embedded build, rustdoc with warnings denied, documentation coverage, and changelog requirements pass.
Ticket completion must be reflected in the relevantto-dos/sprint file.
**/*: Preserve the one-directional crate graph: chip crates must not depend on one another;rustysnes-coreties them together.
Never commit commercial ROMs; only commit derived screenshots and hashes.
Keepdocs/STATUS.mdas the authoritative per-subsystem status and update project documentation in the same PR as code changes.
Do not treat RustyNESv2.0orengine-lineageanchors as project releases.
Files:
scripts/accuracysnes/ares_host/build.shdocs/accuracysnes-plan.mdscripts/accuracysnes/ares_host/README.mdCHANGELOG.mdscripts/accuracysnes/ares_host/ares_host.cpp
scripts/accuracysnes/**
⚙️ CodeRabbit configuration file
scripts/accuracysnes/**: The cross-validation harness: the same AccuracySNES image is run on snes9x (through a
libretro host in C) and on Mesen2 (through its test runner and a Lua script), and their
verdicts are compared with the cart's. Its integrity is the whole argument for the
battery, so flag anything that could make a reference appear to agree — a verdict parsed
loosely, a missing-file path that degrades to success, a scene comparison that skips
rather than fails when the golden is absent. A known reference divergence belongs in
SNES9X_KNOWN_FAILURESwith a source citation, never in a widened match.
Files:
scripts/accuracysnes/ares_host/build.shscripts/accuracysnes/ares_host/README.mdscripts/accuracysnes/ares_host/ares_host.cpp
docs/**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Before changing a subsystem, consult
docs/architecture.md,docs/STATUS.md,CONTRIBUTING.md, the relevant subsystem documentation, and applicable ADRs.New subsystems must add documentation under
docs/.
Files:
docs/accuracysnes-plan.md
**/*.{rs,md}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Chip-behavior changes must update both the chip implementation and the corresponding
docs/<subsystem>.mddocumentation.A chip change must update both the chip implementation and its corresponding
docs/<chip>.mddocumentation in the same change.
Files:
docs/accuracysnes-plan.mdscripts/accuracysnes/ares_host/README.mdCHANGELOG.md
docs/**/*
📄 CodeRabbit inference engine (docs/testing-strategy.md)
Chip crates should exceed 90% unit-test coverage, and each chip should be fuzzable in isolation.
Files:
docs/accuracysnes-plan.md
docs/**
⚙️ CodeRabbit configuration file
docs/**: Docs are the spec, not a history log. Flag claims that contradict the code, counts that
contradict the generateddocs/accuracysnes-coverage.md, and any statement of coverage that
is broader than what the corresponding test actually asserts.
Files:
docs/accuracysnes-plan.md
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Docs are the spec, not a changelog. Flag prose that has drifted from the code it describes
rather than style nits. The markdownlint gate is pinned to v0.39.0 via pre-commit —
do not report rules that version does not have (MD060 in particular).
Files:
docs/accuracysnes-plan.mdscripts/accuracysnes/ares_host/README.mdCHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (CONTRIBUTING.md)
User-visible changes must be recorded under the
[Unreleased]section.For the full pull request diff against its base branch, modify
CHANGELOG.mdwhen user-visible behavior changes, including emulator output, frontend features, CLI flags, public APIs, or AccuracySNES cartridge contents. Do not require it for purely internal changes, tests, comments, or CI configuration.
Files:
CHANGELOG.md
🧠 Learnings (2)
📚 Learning: 2026-07-21T01:34:22.909Z
Learnt from: doublegate
Repo: doublegate/RustySNES PR: 189
File: docs/accuracysnes-plan.md:0-0
Timestamp: 2026-07-21T01:34:22.909Z
Learning: When reviewing the AccuracySNES documentation in docs/accuracysnes-*.md (notably docs/accuracysnes-plan.md vs the generated docs/accuracysnes-coverage.md), treat the reported metrics as intentionally non-equivalent: the battery test count and dossier assertion coverage are not interchangeable. Do not infer one count/coverage from the other during review (e.g., one test may contain multiple assertions, and multiple tests may contribute to a single assertion/row such as E6.02).
Applied to files:
docs/accuracysnes-plan.md
📚 Learning: 2026-07-21T05:22:58.848Z
Learnt from: doublegate
Repo: doublegate/RustySNES PR: 197
File: docs/accuracysnes-plan.md:598-600
Timestamp: 2026-07-21T05:22:58.848Z
Learning: In the AccuracySNES documentation under `docs/`, when an assertion exists in the research dossier but cannot be measured/verified by the current cartridge timing test, distinguish the dossier assertion from test measurability: keep the original hardware assertion (and any contribution to the coverage denominator) intact, withdraw/stop using the specific test coverage only if the sources cannot decompose the required CPU-cycle timing into bus vs internal components, and mark the row as not measurable using the `[NOT CART-MEASURABLE ...]` annotation with links to the corresponding plan section (e.g., `docs/accuracysnes-plan.md` §A5.20) and the related roadmap/ticket (e.g., `to-dos/ROADMAP.md` ticket `T-06-A`). Ensure the documentation/coverage reporting treats the row as uncovered rather than removing or redefining the assertion.
Applied to files:
docs/accuracysnes-plan.md
🪛 Clang (14.0.6)
scripts/accuracysnes/ares_host/ares_host.cpp
[warning] 23-23: do not declare C-style arrays, use std::array<> instead
(modernize-avoid-c-arrays)
[warning] 24-24: variable name 'n' is too short, expected at least 3 characters
(readability-identifier-length)
[warning] 25-25: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
[warning] 101-101: variable 'rom' is not initialized
(cppcoreguidelines-init-variables)
[warning] 102-102: variable 'budget' is not initialized
(cppcoreguidelines-init-variables)
[warning] 125-125: statement should be inside braces
(readability-braces-around-statements)
[warning] 125-125: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
[warning] 134-134: variable 'root' is not initialized
(cppcoreguidelines-init-variables)
[warning] 136-136: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
[warning] 139-139: statement should be inside braces
(readability-braces-around-statements)
[warning] 139-139: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
[warning] 150-150: statement should be inside braces
(readability-braces-around-statements)
[warning] 150-150: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
[warning] 153-153: statement should be inside braces
(readability-braces-around-statements)
[warning] 153-153: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
[warning] 154-154: statement should be inside braces
(readability-braces-around-statements)
[warning] 158-158: variable 'RESULTS' is not initialized
(cppcoreguidelines-init-variables)
[warning] 163-163: variable 'count' is not initialized
(cppcoreguidelines-init-variables)
[warning] 164-164: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
🪛 markdownlint-cli2 (0.23.1)
scripts/accuracysnes/ares_host/README.md
[warning] 5-5: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
CHANGELOG.md
[warning] 19-19: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.11.0)
scripts/accuracysnes/ares_host/build.sh
[warning] 34-34: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (12)
scripts/accuracysnes/ares_host/ares_host.cpp (3)
104-110: LGTM!
125-139: LGTM!
150-153: LGTM!scripts/accuracysnes/ares_host/build.sh (1)
31-38: LGTM!scripts/accuracysnes/ares_host/README.md (4)
1-4: LGTM!
20-51: LGTM!
68-79: LGTM!
80-84: LGTM!docs/accuracysnes-plan.md (2)
846-846: LGTM!
858-864: LGTM!CHANGELOG.md (2)
14-18: LGTM!
22-32: LGTM!
| auto crashHandler(int sig) -> void { | ||
| void* frames[40]; | ||
| int n = backtrace(frames, 40); | ||
| fprintf(stderr, "\nares_host: signal %d — backtrace follows\n", sig); | ||
| backtrace_symbols_fd(frames, n, 2); | ||
| _exit(9); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy lift
Keep crashHandler safe in signal context.
crashHandler calls fprintf, backtrace, and backtrace_symbols_fd after SIGSEGV or SIGABRT. These calls are not guaranteed to be async-signal-safe. If the fault interrupts allocator, loader, or stdio code while a lock is held, the handler can hang or fault again instead of exiting with status 9.
Keep the handler limited to a fixed write and _exit path, or move symbolization to a parent process or post-mortem tool. Verify the design against the target POSIX and libc signal-safety contract.
🧰 Tools
🪛 Clang (14.0.6)
[warning] 23-23: do not declare C-style arrays, use std::array<> instead
(modernize-avoid-c-arrays)
[warning] 24-24: variable name 'n' is too short, expected at least 3 characters
(readability-identifier-length)
[warning] 25-25: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/ares_host/ares_host.cpp` around lines 22 - 27, Update
crashHandler to use only async-signal-safe operations in the signal path: remove
fprintf, backtrace, and backtrace_symbols_fd, and emit only a fixed diagnostic
via write before preserving _exit(9). If backtrace symbolization is required,
move it to a parent process or post-mortem tool and verify the implementation
against the target POSIX/libc signal-safety contract.
| bool verbose = (bool)arguments.take("--verbose"); | ||
| string rom = arguments[0]; | ||
| u32 budget = (u32)toNatural(arguments[1]); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files 'scripts/accuracysnes/ares_host/*' | sed -n '1,120p'
printf '%s\n' '--- host source context ---'
host=$(git ls-files | grep -E '(^|/)scripts/accuracysnes/ares_host/ares_host\.cpp$' | head -n 1 || true)
if [ -n "$host" ]; then
cat -n "$host" | sed -n '80,125p'
fi
printf '%s\n' '--- Arguments definitions and uses ---'
rg -n --glob '!snesdev_wiki/**' \
'class Arguments|struct Arguments|Arguments::|arguments\.take|take\([^)]*--verbose|arguments\.size\(\)|operator\[\]' \
. | sed -n '1,240p'Repository: doublegate/RustySNES
Length of output: 2447
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- build script ---'
cat -n scripts/accuracysnes/ares_host/build.sh
printf '%s\n' '--- source header and dependency references ---'
cat -n scripts/accuracysnes/ares_host/ares_host.cpp | sed -n '1,45p'
rg -n --glob '!snesdev_wiki/**' \
'ares|nall|Arguments|github.com|commit|checkout|submodule' \
scripts/accuracysnes/ares_host README.md .github 2>/dev/null | sed -n '1,240p'Repository: doublegate/RustySNES
Length of output: 22865
🌐 Web query:
Search the ares-emulator/ares GitHub source for the nall Arguments implementation, specifically Arguments::take, Arguments::size, and Arguments::operator[]semantics innall/string/arguments.hpp or equivalent.
💡 Result:
In the ares-emulator/ares source code, the nall::Arguments class (found in nall/nall/arguments.hpp) provides a wrapper around a std::vector to manage command-line arguments [1]. The semantics for the requested methods are as follows: size The size method returns the number of arguments currently held in the internal collection as a u32 type [1]: auto size const -> u32 { return arguments.size; } [1] operator[] The subscript operator provides access to the individual argument strings at a given index. It includes both mutable and const overloads, returning a reference to the string [1]: auto operator[](u32 index) -> string& { return arguments[index]; } [1] auto operator[](u32 index) const -> const string& { return arguments[index]; } [1] take The take method is used to consume and remove specific arguments from the internal collection, which is useful for parsing command-line flags and their values [1][2]. It is overloaded to support different search patterns: 1. take -> string: Removes and returns the first element in the arguments list. 2. take(string_view name) -> bool: Searches for an argument matching the given name; if found, it removes it from the list and returns true [1]. 3. take(string_view name, bool& argument) -> bool: Searches for a flag. If found, it removes it, sets the referenced bool to true, and returns true [1]. 4. take(string_view name, string& argument) -> bool: Searches for an argument by name. If found, it removes both the name and the subsequent value from the list, assigns the value to the referenced string, and returns true [1]. These methods facilitate typical CLI patterns, such as checking for flags (e.g., arguments.take("--fullscreen")) or extracting values associated with flags (e.g., arguments.take("--system", system)) [2].
Citations:
- 1: https://github.com/ares-emulator/ares/blob/6f6786e0/nall/nall/arguments.hpp
- 2: https://github.com/ares-emulator/ares/blob/728105b2/desktop-ui/desktop-ui.cpp
🏁 Script executed:
#!/bin/bash
set -eu
url='https://raw.githubusercontent.com/ares-emulator/ares/6f6786e0/nall/nall/arguments.hpp'
src=$(curl --fail --silent --show-error --location "$url")
printf '%s\n' '--- relevant declarations and methods ---'
printf '%s\n' "$src" | rg -n -A14 -B5 \
'class Arguments|auto size|auto take|operator\[\]|arguments\.remove|arguments\.size' | sed -n '1,260p'
printf '%s\n' '--- source length and commit URL ---'
printf 'lines=%s\n' "$(printf '%s\n' "$src" | wc -l)"
printf '%s\n' "$url"Repository: doublegate/RustySNES
Length of output: 4256
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
args = ["--verbose", "rom.sfc"]
initial_size = len(args)
try:
verbose = "--verbose" in args
if verbose:
args.remove("--verbose")
final_size = len(args)
print(f"input=ares_host --verbose rom.sfc")
print(f"initial_size={initial_size}")
print(f"take_verbose={verbose}")
print(f"remaining={args}")
print(f"final_size={final_size}")
print(f"arity_check_before_take={'passes' if initial_size >= 2 else 'fails'}")
print(f"arity_check_after_take={'passes' if final_size >= 2 else 'fails'}")
if final_size < 2:
raise IndexError("arguments[1] is out of bounds")
except IndexError as error:
print(f"access_result=out_of_bounds: {error}")
PYRepository: doublegate/RustySNES
Length of output: 379
Move the arity check after option removal. With ares_host --verbose <rom>, take("--verbose") reduces the argument count from two to one, so arguments[1] performs an out-of-bounds access.
🧰 Tools
🪛 Clang (14.0.6)
[warning] 101-101: variable 'rom' is not initialized
(cppcoreguidelines-init-variables)
[warning] 102-102: variable 'budget' is not initialized
(cppcoreguidelines-init-variables)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/ares_host/ares_host.cpp` around lines 100 - 102, Move
the positional-argument arity validation to after `arguments.take("--verbose")`
in the argument parsing flow, before accessing `arguments[0]` or `arguments[1]`.
Ensure the check uses the post-option-removal count and rejects invocations
missing either the ROM or budget argument.
| // The results block, straight out of WRAM. $7E:F000 is WRAM offset $F000. | ||
| const auto& wram = ares::SuperFamicom::cpu.wram; | ||
| const u32 RESULTS = 0xF000; | ||
| // Offsets are `asm/runtime.inc`'s, and they are easy to get wrong by one field: R_COUNT is +$06 | ||
| // and R_PASSED is +$0A. Reading the latter as the former reported "count 299" for a 338-test | ||
| // battery, which looks like a truncated run rather than a misread field. | ||
| auto rd16 = [&](u32 off) -> u32 { return wram[RESULTS + off] | (wram[RESULTS + off + 1] << 8); }; | ||
| u32 count = rd16(0x06); | ||
| printf("ACCURACYSNES-BEGIN\n"); | ||
| printf("magic %c%c%c%c\n", wram[RESULTS], wram[RESULTS + 1], wram[RESULTS + 2], wram[RESULTS + 3]); | ||
| printf("done %02x\n", wram[RESULTS + 0x08]); | ||
| u32 count = wram[RESULTS + 0x0A] | (wram[RESULTS + 0x0B] << 8); | ||
| printf("count %u\n", count); | ||
| printf("passed %u\n", rd16(0x0A)); | ||
| printf("failed %u\n", rd16(0x0C)); | ||
| printf("skipped %u\n", rd16(0x0E)); | ||
| printf("golden %u\n", rd16(0x10)); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target outline ---'
ast-grep outline scripts/accuracysnes/ares_host/ares_host.cpp
printf '%s\n' '--- target section ---'
cat -n scripts/accuracysnes/ares_host/ares_host.cpp | sed -n '120,205p'
printf '%s\n' '--- runtime definitions and uses ---'
rg -n -C 4 'R_(COUNT|PASSED|FAILED|SKIPPED|GOLDEN|DONE)|RESULTS|ACSN|ACCURACYSNES-BEGIN|i < 512|512' scripts/accuracysnesRepository: doublegate/RustySNES
Length of output: 18375
🏁 Script executed:
#!/bin/bash
set -eu
find scripts/accuracysnes -type f -maxdepth 4 -print | sort
printf '%s\n' '--- relevant symbols ---'
rg -n -C 5 'R_(COUNT|PASSED|FAILED|SKIPPED|GOLDEN|DONE)|RESULTS|ACSN|ACCURACYSNES-BEGIN|i < 512|512' scripts/accuracysnesRepository: doublegate/RustySNES
Length of output: 17761
🏁 Script executed:
rg -n -C 8 'ACCURACYSNES-BEGIN|R_DONE|R_COUNT|R_PASSED|R_FAILED|R_SKIPPED|R_GOLDEN|RESULTS' .Repository: doublegate/RustySNES
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- shared results layout ---'
sed -n '7,45p' tests/roms/AccuracySNES/gen/src/emit.rs
printf '%s\n' '--- runtime layout symbols ---'
rg -n -C 3 'R_MAGIC|R_VERSION|R_COUNT|R_DONE|R_PASSED|R_FAILED|R_SKIPPED|R_GOLDEN|R_STATUS|R_DONE_MARK|R_FORMAT_VERSION|_test_count' tests/roms/AccuracySNES/asm
printf '%s\n' '--- Ares host entry and return behavior ---'
cat -n scripts/accuracysnes/ares_host/ares_host.cpp | sed -n '85,180p'Repository: doublegate/RustySNES
Length of output: 26621
Reject incomplete or malformed result blocks before reporting counts.
The host emits ACCURACYSNES-BEGIN after any fixed frame budget, even when WRAM contains no completed battery. Validate ACSN, R_VERSION == 1, R_DONE == 0xA5, and 0 < count <= 512; reject invalid blocks before printing any result fields. Otherwise, i < 512 silently drops statuses from an oversized result.
🧰 Tools
🪛 Clang (14.0.6)
[warning] 158-158: variable 'RESULTS' is not initialized
(cppcoreguidelines-init-variables)
[warning] 163-163: variable 'count' is not initialized
(cppcoreguidelines-init-variables)
[warning] 164-164: do not call c-style vararg functions
(cppcoreguidelines-pro-type-vararg)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/ares_host/ares_host.cpp` around lines 156 - 171,
Validate the WRAM result block before emitting any output in the
results-reporting flow: confirm the magic is “ACSN”, version is 1, done is 0xA5,
and the count read via rd16 at offset 0x06 is within 1..512; return or otherwise
reject invalid blocks before printing any result fields. Also update the
status-processing bound to handle all entries up to the validated count without
silently truncating counts above 512.
Source: Path instructions
| ``` | ||
| $ REF_PROJ=$PWD/ref-proj bash scripts/accuracysnes/ares_host/build.sh | ||
| $ /tmp/ares_host tests/roms/AccuracySNES/build/accuracysnes.sfc 900 | ||
| ACCURACYSNES-BEGIN | ||
| magic ACSN | ||
| done a5 | ||
| count 338 | ||
| passed 299 | ||
| failed 5 | ||
| skipped 1 | ||
| golden 33 | ||
| status 0 01 | ||
| ... | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add language identifiers to both changed Markdown fences.
Both fences omit a language identifier and can fail the active MD040 check.
scripts/accuracysnes/ares_host/README.md#L5-L18: change the opening fence to```textor```console.CHANGELOG.md#L19-L21: change the opening fence to```textor```console.
As per path instructions, these Markdown files must follow the pinned markdownlint rules.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 5-5: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
📍 Affects 2 files
scripts/accuracysnes/ares_host/README.md#L5-L18(this comment)CHANGELOG.md#L19-L21
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/ares_host/README.md` around lines 5 - 18, Add language
identifiers to both changed Markdown fences: update the fence in
scripts/accuracysnes/ares_host/README.md lines 5-18 and the fence in
CHANGELOG.md lines 19-21 to use text or console, preserving their existing
contents.
Sources: Path instructions, Linters/SAST tools
| ## Three setup steps that are not optional, each of which cost a debugging round | ||
|
|
||
| Run it under a debugger and start at (1) — a null pak is the failure that would reach furthest | ||
| before dying. | ||
| All three fail as a **segfault inside `System::load`**, with a backtrace pointing at memory setup | ||
| rather than at what is actually missing. | ||
|
|
||
| ## Usage, once it works | ||
| 1. **`ares::Memory::FixedAllocator::get()` before anything touches a core.** `Bus::reset()` | ||
| allocates its page tables from that bump allocator. desktop-ui does this on its first line. | ||
| 2. **`ares::SuperFamicom::option("Pixel Accuracy", "true")` before `load`.** `PPUBase::implementation` | ||
| is null until `setAccurate` picks one of the two PPUs, and `Bus::reset()` calls `ppu.map()` → | ||
| `implementation->map()`. `"true"` selects the **accurate** PPU, the only one worth | ||
| cross-validating against. | ||
| 3. **`nall/main.hpp` included by this translation unit.** It emits `::main` only when | ||
| `NALL_MAIN_IMPL` is undefined, and nall's own `main.cpp.o` defines that. Omit it and the *link* | ||
| fails with a bare `undefined reference to 'main'` from `crt1.o`, which reads like a missing object | ||
| file rather than a missing shim. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the repeated setup-failure description.
Both documents group the allocator, PPU, and nall/main.hpp omissions under the System::load segmentation-fault description. The third omission actually causes a link-time undefined reference to 'main'.
scripts/accuracysnes/ares_host/README.md#L52-L66: separate the allocator and PPU segmentation faults from thenall/main.hpplink failure.CHANGELOG.md#L33-L39: apply the same separation to the changelog entry.
As per path instructions, documentation is the specification and must match the implementation and build recipe.
📍 Affects 2 files
scripts/accuracysnes/ares_host/README.md#L52-L66(this comment)CHANGELOG.md#L33-L39
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/accuracysnes/ares_host/README.md` around lines 52 - 66, Update
scripts/accuracysnes/ares_host/README.md lines 52-66 to separate the allocator
and PPU omissions, which cause System::load segmentation faults, from the
nall/main.hpp omission, which causes a link-time undefined reference to main.
Apply the same corrected separation to CHANGELOG.md lines 33-39; both
documentation sites must describe the implementation and build behavior
accurately.
Source: Path instructions
…my own claims (#305) Three of ares' five failures are rows snes9x ALREADY fails, which the tally alone does not show and which changes what each one means: C7.10 and F1.10 become 2-vs-2 (RustySNES + Mesen2 against snes9x + ares), C7.05 is 2-vs-2 with the two dissenters failing on different codes, and only E8.02 and E3.06 are ares-only. ares is corroborating snes9x more than it is standing alone. Still not wired into crossval.sh -- an ARES_KNOWN_FAILURES constant encoding unadjudicated disagreements would be worse than no third reference. CORRECTION 1, to the ares README and PR #304: F1.10 is NOT a PAD2_CONTRACT row and this host's port detection is not the suspect. f1_require_contract reads $4016 only -- port 1 -- and F1.10 code 2 means "$4212 read busy at the very start of the vblank line", which does not involve controller state. The claim was inherited from crossval.sh's Mesen2 grouping rather than checked. CORRECTION 2, to crossval.sh itself: its Mesen2 known-failure comment names idx279 F1.03 and idx286 F1.10, and in the current catalogue those indices are F1.01 and F1.08 -- an index moves whenever a test is added ahead of it. Keyed on rows now, with the drift noted. That same comment attributes Mesen2's F1.10 failure to the port-2 limitation while the snes9x block a few lines above says Mesen2 PASSES F1.10; both cannot be true, and it is marked as doubted rather than quietly rewritten, because resolving it needs Mesen2's failing set read at DONE and mapped through SOURCE_CATALOG.tsv. F1.10 now deserves a hard look on its own: fullsnes puts the automatic read's start ~dot 32.5-95.5 into the first vblank line, snes9x fails the row, ares fails it, and if the Mesen2 attribution is right then RustySNES passes ALONE on a row it passes only because of a deliberate fix. The heuristic that says RustySNES failing alone means a real bug should say the same about RustySNES passing alone. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
… finding (#309) RETRACTION, and it is the important half. The plan recorded "A2.10 -- Mesen2 is the outlier, 2-vs-1" as the battery's only unexplained reference disagreement, and PR #304 said ares settled it 3-vs-1. Measured: RustySNES pass Mesen2 PASS (its failing set is F1.03 and F1.10, nothing else, identical on six runs) ares pass snes9x FAIL code 1 -- and documented, the FIRST entry in SNES9X_KNOWN_FAILURES So the outlier is snes9x, the failure is already explained, and there was never a disagreement on this row. The old text also asserted "A2.10 is not among snes9x's 14 known divergences"; it is, at crossval.sh:80. The original reading took a catalogue index off one host's output and attributed it to another, and nothing checked it because no per-row measurement existed. That is the second published claim in one day undone by keying on catalogue indices instead of row names. An index is only meaningful next to the host that produced it and the catalogue that was current, and neither travels with the number. The ares host was therefore built to settle a row that did not need settling. It still earned itself: it found a real bug in ares' SMP. ares is now wired in as a third reference. cross-validation reports "3 reference(s) agree with the cart". ARES_KNOWN_FAILURES=5 carries per-row rationale like the other two constants -- C7.05/C7.10/F1.10 are rows snes9x already fails, E3.06/E8.02 cite the ares $F1 timer-2 reset bug. The block is opt-in and skips cleanly when the binary is absent, verified in both states. One bug caught on the way: counting failures with ("0x" $3) % 2 == 0 reported 337 failures, because POSIX awk does not parse "0x01" and the expression is 0 % 2 for every byte. A gate reporting catastrophe out of a parsing bug is a specific kind of dangerous; the fix matches the last hex digit and says why. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
The follow-up to #303, which established that the host builds and links. It now runs.
Identical across two runs.
A2.10is settled — 3 against 1, with Mesen2 the outlierThis is the point of the exercise. Several findings sat at 2-versus-1 with no tiebreaker, because this project's provenance rule counts ares and bsnes as one reference — so "RustySNES and snes9x against Mesen2" is only 2-vs-1 if ares is not already on RustySNES's side, and nobody could check.
ares passes
A2.10("PEI does not page-wrap", catalogue index 11, status$01). With RustySNES and snes9x, that is three implementations against Mesen2. The caveat that kept this row open — a harness bug upstream of an implementation produces exactly this signature, as the$F8/$F9retraction already cost this project once — is answered by a fourth host independent of all three.The plan's
A2.10section is updated in place, keeping the original reasoning because it is why the row waited.Three setup steps, all failing as the same segfault
Each cost a round, and each fails inside
System::loadwith a backtrace pointing at memory setup rather than at what is actually missing:ares::Memory::FixedAllocator::get()before anything touches a core —Bus::reset()allocates its page tables from that bump allocator.ares::SuperFamicom::option("Pixel Accuracy", "true")beforeload()—PPUBase::implementationis null untilsetAccuratepicks one of the two PPUs, andBus::reset()callsppu.map()→implementation->map()."true"selects the accurate PPU, the only one worth cross-validating against.nall/main.hppincluded by the host translation unit — without it the link fails with a bareundefined reference to 'main'fromcrt1.o.ptraceis denied in this sandbox, so the host installs its ownSIGSEGVhandler and prints a backtrace;addr2line -Cferesolves the frames.Five rows ares disagrees about — recorded, not adjudicated
C7.05,C7.10,E8.02,E3.06,F1.10. The cart, snes9x and Mesen2 all pass these.F1.10is suspect of this host first. It is aPAD2_CONTRACTrow and the port detection here (port->name().find("2")on the button's grandparent) is assumed to work, not verified.Deliberately not wired into
crossval.sh. Adding a third reference means anARES_KNOWN_FAILURESconstant, and that constant must not be written until these five are adjudicated — a known-failure count encoding unexamined disagreements is worse than no third reference at all.Also
Results-block offsets corrected:
R_COUNTis+$06,R_PASSEDis+$0A. Reading the latter as the former reported "count 299" for a 338-test battery, which reads like a truncated run rather than a misread field.Scope
scripts/accuracysnes/ares_host/plus docs. Nothing in the emulator, the build, or any gate is touched;ref-proj/is not modified;crossval.shis unchanged.🤖 Generated with Claude Code
Adds a reproducible headless Ares host for AccuracySNES.
338tests:299passed,5failed,1skipped, and33golden.A2.10, changing the dossier assertion to a 3-vs-1 result against Mesen2. The coverage denominator remains338.C7.05,C7.10,E8.02,E3.06, andF1.10. These rows remain unresolved, andF1.10remains excluded from cross-validation.nall/main.hpp. It also corrects results-block offsets toR_COUNT = +$06andR_PASSED = +$0A.The claim is false if the host cannot reproduce these counts, does not pass
A2.10, or requires a changed coverage denominator. The claim also fails if any of the five disagreements are treated as adjudicated or if Ares is integrated intocrossval.shwithout resolving the documented conditions.