Correct the ov006 near-miss records: a defective draft faked a zero-word floor - #2701
andrewboudreau wants to merge 6 commits into
Conversation
Three in-tree records about ov006 near-misses were wrong. Each is re-measured here rather than carried forward from the stale number. 1. func_ov006_020d27dc: the banked source's defect, and a floor mark. The banked c_source spells `*(s32 *)((int)p + 0x5374)` at line 241. The ROM addresses that site as `add r0,sb,#0x5000 / ldr r1,[r0,#0x374]`; the cast instead forces `ldr r0,[pc,#0x8c4] / ldr r0,[sb,r0]` plus a literal-pool word. Dropping that one cast gives 3652 bytes, md5 8462f11dba034fd64356bc06233a65cf, byte-equal to the ROM outside a single word with the whole literal pool identical. The only residue is the ROM's `blt` at +0x564, so the budget is exactly +1 word, not zero. The corrected source is deliberately NOT banked. evaluate_full sentinels its size mismatch at 999 and the strictly-improving upsert refuses it against the stored 17, and the stored 17 is size-exact only by virtue of that spurious pool word. The row therefore keeps divergences 17 and its c_source unchanged, and carries a floor mark (class cond_opt) recording the mechanism, the measurement and the refutations. A 14-divergence size-exact variant exists but stores to p+0x46d4, which the ROM's low arm does not do; it is logically wrong and is not banked either. 2. func_ov006_020e5450: the 29 was never a comparable number. Five in-tree records claimed a best attempt of 29 divergences. That attempt's own note records "size 0x52c vs 0x560": it was scored against a candidate 52 bytes shorter than the target, so the 29 is a truncated verdict over a short window, not a divergence count for this function. It is not comparable with any size-exact score, and it made the function look far closer than it is. Re-measured at the exact size 0x560 (1376 bytes, 344 words): 191 divergent words of 344, which is what the bank already held; the bank row is unchanged. The stale 29 is corrected in the TU banner, the TU manifest, the class-facts writer guidance (which also carried a stale "banked draft is at 324"), and the attempt row itself. That row's divergences field is cleared rather than rescored: the attempt's source no longer exists in the tree, so nothing can re-score it, and a cleared field is the schema's own shape for an attempt without a score. The honest re-measurement is logged as its own attempt row instead of being attributed to someone else's run. 3. Two wrong parameter lists. src/func_02012718.c defined `void func_02012718(void *a, int b)`. The ROM and include/decl_common.h say (int, int), and the first parameter is an integer sound ID, not a pointer. The definition was the wrong side, but the agreement gate keys declarations against the definition, so correcting it alone would have flipped twelve agreeing declarations into new disagreements and reddened every push in the repo. Corrected across the whole set instead: the definition, thirteen pointer-spelled mirror declarations and their sixteen call sites (each passing the ID as `(void *)0x...`), plus one (int, u32) spelling. func_02012718 now has twenty-four declarations and zero disagreements, down from fourteen. src/func_ov006_020e5b70.c declared `func_ov006_020e5450(void)`. The target's prologue is `mov sb,r1; mov sl,r0`, which proves two parameters, and src/func_ov006_020e5b7c.c already spelled (char *, int). The veneer forwards r0/r1 untouched, so its own parameter list is the target's; it is now spelled that way and still assembles to the same twelve bytes. Verification: rombuild --no-rom reports 11,204 of 11,204 source-built functions reproducing, 0 mismatching, module fidelity 106/106 exact -- identical to the pre-change baseline, so every retype is byte-inert. check_src_tu_compiles 177/177. check_decl_agreement --check reports no new disagreements, with fourteen banked disagreements gone and none added, so the baseline is shrunk by exactly those fourteen (deletions only) rather than left stale to mask a future re-break at the same sites. Also: the cast lever's stated bound in notes/mwccarm-codegen.md was wrong. It claimed that on a `char *` base both spellings compile to the same 3652 bytes, md5 8462f11d... That md5 is the cast-dropped object alone; the banked source produces 163fe692... at 3656. Of the four removable `(int)p` sites in that source, three are byte-inert and the fourth is worth four bytes, so a byte-strided base does not make the cast inert, it makes it inert at most sites. The cast object emits exactly one register-offset load in the whole 914-instruction function, at the site whose value feeds the if-converted compare. The passage now says to probe the site that feeds the divergence rather than a convenient one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Per-file link-check detailAll 17 changed file(s) compile to the ROM byte-for-byte with correct relocation targets.
The private worker commits a test merge, builds the stock ROM profile, compares every executable module, measures matched and source-built code, checks contributor lineage, and verifies affected relocations. The mod profile is opt-in and is not part of this merge gate. |
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
Humanizer/integrator review of c74a9b9e0b2607157b88feed1e7798b686134498.
I read the sound-ID contract and tail-veneer changes, including their affected callers and the Curling2 evidence corrections. The sound call sites now pass numeric IDs directly, and the veneer explicitly forwards its two arguments. I independently ran prepush_linkcheck.py over this head's merge-base range: 47 checked, 47 VERIFIED, zero warnings and zero blocking results, including the complete touched Curling2 TU. This proves the scoped byte/relocation result, not complete source acceptance.
SOUND-2701-01 (correctness of the declared interface), src/func_02012718.c:2-4: the correction stops one call short. src/func_020127ec.c actually defines six int parameters, but the changed wrapper still declares its second parameter void* and now casts the sound ID back to (void*)a. Replace that declaration with extern void func_020127ec(int, int, int, int, int, int); and pass a directly. I tested exactly this two-line substitution in a scratch .c under 2004/b56: emitted function bytes and relocation records are unchanged, and func_02012718 at arm9 0x02012718, size 0x3c, is VERIFIED, diffs: [], blind: 0. This removes the contradictory interface rather than moving the integer-to-pointer cast into the callee wrapper.
Fix this finding and refresh proof before source acceptance. The live source gate also requires this head to contain the current reviewable main base and have independently published review evidence for that composition. Preserve the distinction between the obsolete attempt's cleared metric and the new measurement; do not turn the historical 29 into an attributed 191. I have not merged, rewritten, or registered a source-review pass for this PR.
`src/func_ov006_020fce04.c` conflicted only because the two sides edited
ADJACENT declaration lines, not the same one. Both edits are kept:
this branch func_02012718's first parameter, void * -> int, and the
matching `(void *)` cast dropped from the call
main data_ov006_0213d954 respelled from a scalar to `[]`, and the
use respelled to `[0]`
The body had already merged to exactly that combination; only the two-line
declaration block needed resolving, and neither side touched the other's line.
`nearmiss/db.jsonl` merged through the union driver: 29 rows, no duplicate
`(module, addr)` keys, key set exactly main's union this branch's.
`config/match_attempts.jsonl` has no byte-identical duplicated lines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
Second merge of main, needed because #2715 landed between the first one and now: it replaced `tools/check_decl_agreement.py` and regenerated the whole `config/decl-agreement-baseline.json`, so the first merge's baseline was banked by a gate revision that no longer exists. Baseline REGENERATED, not hand-resolved: main's copy taken first, then `--update`. The six contradictions it banks are this branch's own churn -- five are `func_02012718`'s remaining `void *` declarations against the `void (int, int)` definition this branch retypes, and one is `func_ov006_020e5450` declared `char *` against the `dScMgCurling2_c *` definition this branch introduces. The byte gate proves the retype: 106/106 exact, mismatching 0, ROM-build PASS, 409 real compiles. `nearmiss/db.jsonl` doubled two rows through the union driver. Resolved by provenance against merge-base fe05a37, not by comparing `divergences`: ov074 0x02121380 only main edited `floor` -- kept main's ov006 0x020d27dc only this branch edited `floor` -- kept this branch's 29 rows, no duplicate keys, key set exactly main's union this branch's. `config/match_attempts.jsonl` has no byte-identical duplicated lines. Follow-up, NOT done here: the five `void *` declarations of `func_02012718` are now banked as disagreeing with the definition. Correcting them to `int` is a separate change that has to be rebuilt, since a declaration edit can move instruction selection at the call site. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
SOUND-2701-01. The previous correction stopped one call short: func_02012718's
first parameter became int, but the wrapper still declared func_020127ec's
second parameter void* and cast the sound ID back with (void *)a -- moving the
integer-to-pointer contradiction into the callee's declaration instead of
removing it.
The tree already agrees on int, and this file was the only dissenter:
src/func_020127ec.c:6 void func_020127ec(int a0, int a1, int a2,
int a3, int a4, int a5)
src/actors/dScMgD3DBase_c.cpp:142 void func_020127ec(int a0, int a1, int a2,
int a3, int a4, int a5)
src/func_02012718.c:2 (was) extern void func_020127ec(int, void*, int,
int, int, int)
So the callee's own definition and the one other prototyped declaration both
say six ints.
Reverified: 8759 reused from cache, 1 compiled, mismatching 0,
106/106 exact, ROM-build analysis PASS.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
|
SOUND-2701-01: fixed in -extern void func_020127ec(int, void*, int, int, int, int);
+extern void func_020127ec(int, int, int, int, int, int);
void func_02012718(int a, int b){
- func_020127ec(2, (void *)a, 4, 0, 0, func_020126e8(b));
+ func_020127ec(2, a, 4, 0, 0, func_020126e8(b));
}I censused the tree before changing it, since the last round's lesson was that the callee's own file can be the odd one out. Here it isn't — this wrapper was:
( Full build, not a scoped linkcheck: On your other point — noted, and I won't restate it as a general rule. The claim I'd defend is only the measured one: this pointer/int correction, at this arity, was byte-inert here, and that says nothing about parameter or return types in general, which can and do change argument passing, conversions and caller code. The historical 🤖 Generated with Claude Code |
func_ov006_020e5b70 is a tail-call veneer, so its parameter list is its target's. Fixing the arity from 0 to 2 was right -- the target's prologue `mov sb,r1; mov sl,r0` proves two parameters, and the veneer forwards r0/r1 untouched. But spelling the first one `char *` asserts a pointee type the definition contradicts: src/func_ov006_020e5450.cpp:24 defines it `void func_ov006_020e5450(dScMgCurling2_c *self, int idx)`. That turned a banked `arity:0` disagreement into a banked `param:#1 char *` one -- debt traded, not paid. `void *` pays it. It carries the arity, which is the byte-relevant part, and check_decl_agreement's _declines_to_answer rule treats `void *` against any object pointer as the absence of a claim rather than a wrong one, so the symbol needs no baseline entry at all. A C shard cannot name a C++ class, so `void *` is also the only honest spelling available here. check_decl_agreement: 18,226 -> 18,225 disagreements (param 7,699 -> 7,698), no new ones; the baseline shrinks by removals only. rombuild -j16 --no-rom: 1 compiled, 11,206 reproducing / 0 mismatching, module fidelity 106/106 exact, ROM-build analysis PASS. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
Independent recheck at 3242291da1aa8cdbb86884b9f42384a1fc365f94: SOUND-2701-01 is fixed. The wrapper declares func_020127ec with six integer parameters and forwards the numeric sound ID directly. Fresh strict linkcheck of func_02012718, arm9 0x02012718, size 0x3c, returns VERIFIED, diffs: [], blind: 0.
This closes that narrow interface finding. The PR still includes source and manifest changes in addition to ledger corrections, so its remaining source review and current-main composition requirements continue to apply. I am coordinating integration and have not treated the repaired wrapper or green private validation as a whole-candidate Source review pass.
1 participant
Four record corrections around
func_ov006_020d27dcandfunc_ov006_020e5450. No byte changes anywhere:rombuild --no-romreads 11,205/11,205 reproducing, 0 mismatching, 106/106 exact, PASS — identical to the pre-change baseline, so every retype here is byte-inert.1. The
func_ov006_020d27dcfloor was resting on a defective draftThe banked source spelled one site
*(s32 *)((int)p + 0x5374). Disassembly shows that cast is the first mismatch:That one spurious literal-pool word is the entire 3656-vs-3652 delta — the row looks size-exact only because of the defect. The real budget is +1 word, not zero.
The corrected source could not be banked as a replacement: at 3652 bytes
evaluate_fullsentinels the size mismatch at 999, and the strictly-improving upsert correctly refuses that against 17. So the row keeps its source and gains a floor mark carrying the mechanism, the md5 pair, the +1-word budget and the refutations. The size-exact 14-divergence variant stays unbanked — it stores top+0x46d4, which the ROM's low arm does not do.A correction to the standing note: I predicted all five
(int)psites were load-bearing. Three of the four removable ones are byte-inert — only the site feeding the if-converted compare pays. The old bound ("byte-strided base ⇒ inert") is wrong; the rule is probe the site that feeds the divergence, not a convenient one.2. The bogus "29" was in five places, not two
func_ov006_020e5450had been recorded at 29 from an attempt whose own note sayssize 0x52c vs 0x560. Beyond the TU banner andconfig/match_attempts.jsonl, it was also inconfig/tu_manifest.d/ov006/dScMgCurling2_c.jsonand two sites innotes/data/class-facts/dScMgCurling2_c.json— the latter also carrying a stale "banked draft is at 324" that was actively steering future work off the row. Re-measured: 191/344 at exactly 0x560.The attempt row's
divergencesis cleared to null rather than overwritten with 191. That attempt's source is gone and cannot be rescored, and writing my measurement into another author's run would misattribute it.nullis the schema's own shape — absent on 889 of 2,073 rows — and the honest 191/344 is logged as its own row vialog_attempt.py.3.
func_02012718— the decl-agreement gate had 14 baselined disagreementsThe definition was the wrong side, but fixing it alone would have flipped 12 agreeing declarations into new red. Fixed the whole set instead: definition, 13 pointer mirrors, 16 call sites and one
u32. Now 24 declarations, 0 disagreements.Baseline churn is 14 removed, 0 added — the baseline shrinks, deletions only. That is deliberate: leaving stale entries would mask a future re-break at those same sites.
4.
func_ov006_020e5450— a(void)veneer the gate could not seeIt reported 0 disagreements only because nothing defines the symbol, so the veneer's
(void)was invisible. Nowvoid (char *, int), agreeing withfunc_ov006_020e5b7c.c, still 0. The veneer was byte-gated individually, positive control first: it matched before and after.Gates
check_src_tu_compiles: 177/177check_decl_agreement --check: no new disagreementsrombuild --no-rom: PASS, 11,205/11,205, 106/106 exact — unchanged from baselineRebased onto
ca1511f81:origin/mainmoved mid-session (1547d5fbf→ca1511f81) because sibling worktrees share the ref store. Every gate above was re-run on the new base.What is deliberately not here
A structural note on the
func_ov006_020e5450ordering rules was written and then dropped. Section 6cz, landed on main while this branch was in flight, derives the same two rules, names this branch's exact 193/db5e3dcfcell, and carries the line 191 → 187. Publishing a competing "191 is the honest measurement" beside it — under a section letter 6cz had just taken — would have been noise. The prose and the attempt row point at 6cz instead.Plain English
Four bookkeeping fixes, no change to any compiled byte. The main one: a reconstruction we had recorded as "perfect size, 17 details wrong" was only the right size because of a mistake in it — remove the mistake and it's a word too short, which means the real gap is bigger than the record claimed. Separately, a wrong figure from an old run had been copied into five different files, one of which was steering people toward a draft that doesn't exist. And a function's declared argument types disagreed with its definition in fourteen places; fixing only the obvious one would have broken twelve others, so all twenty-four now agree.
🤖 Generated with Claude Code
https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA