Prepare is static, Whomp's address was double-mangled, and the reference ratchet is unstuck - #1397
Merged
Conversation
The reference ratchet is stuck on main, and has been since the merges that moved it. `--update` refuses whenever `bad` is non-empty, and main's own drift lands there, so no PR can bank progress past it. The failure reproduces byte-identically on a clean `origin/main` checkout with nothing applied. Regenerated from a clean main. Ten lines change, and each is main's own state: eligible 10805 -> 10814 #1382, #1391 and #1393 all added functions. func_02057410 recorded as missing `_ll_mod`; it actually misses `_ll_udiv` and `_ull_mod`. Those are mwccarm runtime helper names -- the ROM implements them as __aeabi_uldiv / __aeabi_ulmod in ITCM at 0x01ffa9dc and 0x01ffa9e8 -- and there is no alias mechanism, so the entry is permanent rather than a to-do. _Z13func_020072c0v gone: #1382 renamed that symbol to _ZN7Vector3D1Ev. ov079 Whomp::InitResources ADDED. It is unresolvable on main today and was simply missing from the baseline. The next commit fixes half of it; the other half, func_021135d4, resolves to 15 overlays sharing base 0x021111a0, so no single name is correct. ov091 func_ov091_021339fc gone: a merged PR converted it to a hand-written asm hatch, which removes it from the candidate set. That reclassification is indistinguishable from a regression to the reason-switching guard -- which is exactly what jammed the ratchet. Its own commit, before any content, so the next commits' effect on the backlog reads as a diff against an honest starting point instead of being buried in a rewrite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
Both were declared as ordinary members and defined as forwarding
(this, &model, &file). They take two arguments and no this. Four
independent lines of evidence agree:
- func_02046d50 and func_020470e8, the real bodies behind the two 0xc
veneers, read only r0 and r1. r2 is never touched, which rules out a
third argument.
- the offsets r1 is read at are exactly the animation file's fields
(BTP_File: 0x2, 0x4, 0x8, 0xc, 0x1c, 0x20), so r1 is the SECOND
argument, not the third.
- both matched implementations, src/func_02046d50.c and
src/func_020470e8.c, already take two parameters.
- every call site in the tree passes two.
The veneer bodies are pure tail jumps (ldr ip,[pc]; bx ip), so they
reproduce under either signature. That is how the wrong reading survived:
the definition files never constrained it, and the callee and callers --
which do -- were not consulted. A static member mangles identically, so no
symbol changes and no config moves.
TextureTransformer, the third sibling, was already corrected. Its header
names these two veneers as "called the same way" and notes that the old
reading "cost the PC port a wrong-register hunt"; the follow-up never
happened. This is it.
Twenty-two call sites drop the mangled extern for the member spelling.
Several of those externs were a bare `extern` on a mangled name inside a
//cpp file, which mangles a second time, so removing them retires that
latent defect as well. Eyerok's linkcheck improves BLIND-39 -> BLIND-35 as
four call destinations become resolvable. SnowmanHead and InvisibleSecret
declare the class locally rather than including TextureSequence.h: both
carry `typedef int Fix12;`, which collides with the real math/Fix12.h
template.
The six .c callers keep the mangled extern -- C has no ::.
Gates: rombuild 106/106 exact, 10,813 reproducing, 0 mismatching; eligible
name list byte-identical to main at 10813; linkcheck 23 checked, 21
verified, 0 blocking; attribution 0 changed / 0 lost; port_refcheck 393
resolve; header offsets, data definitions and duplicate sources clean.
Goomba::InitResources and MrI::InitResources do not reproduce -- before or
after. Both fail identically on origin/main (999 and 7 words), and their
compiled bytes are unchanged by this commit: 916 -> 916 and 664 -> 664,
identical in both cases.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
_ZN5Whomp13InitResourcesEv.cpp declares _ZN16MeshColliderBase16UpdatePosAndAngs... with a bare `extern`, and that declaration sits OUTSIDE the extern "C" block above it. In a //cpp file that mangles the already-mangled name a second time, so the address taken at the func_020393d4 call points at a _Z88_ZN16... that does not exist. The byte gate cannot see this -- match.py compares relocated words as wildcards, so the function reproduces either way. check_references and prepush_linkcheck are the only gates that look at what a reference targets, and both flagged it: linkcheck goes BLIND-1 -> VERIFIED with the fix, and the function still reproduces. Found while running the gates for the Prepare change, in a file that change already touches. The other entry in the same check_references report, func_02057410's _ll_udiv/_ull_mod, is unrelated to this branch and reproduces identically on origin/main. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
Downward only, and both deletions are attributable to a commit above:
_Z49_ZN15TextureSequence7PrepareER8BMD_FileR8BTP_Fileii
resolved by making Prepare static -- the two trailing 'ii' were a
shadow declaration disagreeing with the real two-argument signature.
_Z88_ZN16MeshColliderBase16UpdatePosAndAngsERS_P5ActorR10ClsnResultR7Vector3P10Vector3_16S8_v
resolved by spelling Whomp's extern the way decl_common.h does, so the
already-mangled name is not mangled a second time.
Nothing is added. Whomp keeps its entry for func_021135d4, which resolves to 15
overlays sharing base 0x021111a0 -- no single name is correct, so it stays.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
The Prepare change first spelled the call surface inline at each of the 11 call
sites:
struct TextureSequence { static void Prepare(BMD_File &, BTP_File &); };
That compiles and byte-matches, but it is a shadow declaration, and the langmode
ratchet counts those: shadow_decls.local_body_no_include 1507 -> 1510, with nothing
falling to offset it. The workflow has an escape hatch for a deliberate rise, but
using it here would mean adding debt in exactly the direction the plan is draining.
The headers cost nothing, so they win:
shadow_decls.local_body_no_include 1507 -> 1503 (-4)
A 7-point swing, and the ROM does not move -- the call is a direct bl either way.
Two files needed the Fix12 shadow removed first. `typedef int Fix12;` is harmless
while a file includes nothing, but the real Fix12 is a TEMPLATE, so pulling in a
real header turns every use into "template argument list expected" rather than a
plain redefinition. SnowmanHead and InvisibleSecret now take those extern
parameters as plain int, which is what BowserFire, BowserTail, KoopaShell,
LavaBubble and BabyPenguin already do. That trap is why the local-declaration route
was taken in the first place; it is not specific to those two files.
VERIFIED rombuild -j16: 106/106 exact, 10,814 source-built, 0 mismatching, 87.96%
langmode ratchet PASS
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Warnings: 3 linkcheck result(s) have unresolved relocations; 2 affected source file(s) could not be fully link-checked. Per-file link-check detail2 of 310 changed file(s) do not match the ROM (NO-SYM).
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #1390, rebuilt on current main. Same finding, better evidence, and two
things #1390 got wrong.
Prepare is static
TextureSequence::PrepareandMaterialChanger::Preparetake nothis. Fourindependent lines say so, and the ROM body is a
0xclong-call veneer(
ldr ip,[pc]; bx ip) that shuffles no arguments — so the real body's register useis the call surface:
func_02046d50reads onlyr0andr1.r2is never touched, which rulesout three arguments.
r1is read at —+2,+4,+8,+0xc,+0x1c,+0x20— areexactly
BTP_File's fields.TextureTransformer.hhad already documented this for both siblings; the follow-upnever happened. A static member mangles identically to a non-static one, so the
symbol does not change — only the honesty of the declaration does.
Whomp: an address that was double-mangled
_ZN5Whomp13InitResourcesEvtook the address of an already-mangled name declaredwith a bare
externoutside the file'sextern "C"block, so C++ mangled it asecond time into
_Z88_ZN16MeshColliderBase16UpdatePosAndAngs...v— which existsnowhere. The byte gate cannot see this:
match.pywildcards every relocated word andnever looks at what a call targets. Only
check_references.pydoes.The reference ratchet was stuck on main
--updaterefuses wheneverbadis non-empty, and main's own drift lands there — sono PR could bank progress past it. Reproduced byte-identically on a clean
origin/maincheckout with nothing applied.Commit 1 re-pins the baseline from clean main, as its own commit before any content,
so the rest of the PR reads as a diff against an honest starting point. Ten lines
change and each is main's state, not mine — the
eligiblecount,func_02057410'sreal missing names, a symbol #1382 renamed, Whomp (genuinely unresolvable and simply
absent), and
func_ov091_021339fc, which a merged PR converted to a hand-written asmhatch. That last one is what jammed the ratchet: a deliberate reclassification is
indistinguishable from a regression to the reason-switching guard.
Commit 4 then banks downward-only what this PR resolves — two double-mangled names,
one per content commit, nothing added.
What #1390 got wrong
It spelled the call surface inline at all 11 raw call sites:
That byte-matches, but it is a shadow declaration, and the langmode ratchet counts
them:
shadow_decls.local_body_no_include1507 → 1510, with nothing falling tooffset it. There is an escape hatch for a deliberate rise; using it here would have
banked debt in the exact direction the plan is draining, in a PR whose point is
making a call surface more honest. The headers cost nothing, so:
shadow_decls.local_body_no_includeA 7-point swing, and the ROM does not move — the call is a direct
bleither way.Two files needed a
typedef int Fix12;shadow removed first. It is invisible while afile includes nothing, but the real
Fix12is a template, so a real header turnsevery use into
template argument list expectedrather than a plain redefinition.That trap is why the local-declaration route was taken, and it is not specific to
those two files — anything taking the header route will hit it.
Verified
rombuild -j16check_referencesunresolved references OK— ran the check, did not skip itcheck_header_offsetsprepush_attributioncheck_data_definitionscheck_duplicate_sourcesport_refcheck🤖 Generated with Claude Code
https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe