ov060: the Bowser fight becomes real C++, and D0 destructors become migratable - #1374
Merged
Conversation
…neighbour's `object_reloc_dests` picked the relocation section by name. mwccarm names every function's section ".text", so ".rela.text" is ambiguous and the name lookup answers with the LAST section of that name -- one fixed table, whichever function was asked about. A destructor TU is emitted D2, D0, D1, so that fixed table is D1's. D1 got its own by luck; D0 and D2 got D1's. The byte compare cannot cover for it, because relocated words are wildcarded -- that wildcarding is the entire reason the destination check exists. Found by migrating a D0: `BowserFire::~BowserFire()` reproduces the ROM's deleting destructor exactly and all seven of its relocation destinations are correct, and the gate called it WRONG-DEST after comparing D1's five against D0's offsets. tools/linkcheck.py had already hit this and carried a correct sh_info-based helper with a comment about it. That helper is now the shared one and linkcheck imports it, so there is a single implementation instead of one right and one wrong. Re-ran all 77 real-C++ destructors in the tree under the corrected lookup: 77/77 verify strictly, so nothing wrong was ever admitted -- the gate was broken, the tree was not. Tests build the two-.text-sections-with-one-name shape as a hand-written ELF, so they need no compiler and no ROM. One asserts the object really does reproduce the hazard, because a synthetic object whose sections did not collide would make the regression test vacuous. The tree-wide guard reads the AST rather than grepping: both files now carry comments explaining the bug, and a text search counts the explanation as an instance of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x
…igratable
Every unmigrated file in ov060 is gone -- six classes, twelve destructors -- and
the route that took the deleting halves is new.
THE D0 ROUTE. 258 of the tree's 261 D0 files were unmigrated against 74 migrated
D1s, and the reason was one missing declaration. The compiler generates D0 as "run
the destructor body, then call operator delete on the class"; with none declared it
emits a call to the global _ZdlPv, which exists nowhere in this image, and D0 comes
out three instructions short. The ROM shows what was there instead: operator new is
a real function at 0x02043444, no operator delete symbol exists anywhere, and every
deleting destructor ends with the same load-the-heap / call-Memory::Deallocate pair
rather than a call to a shared helper. That is an inline member operator delete, and
declaring it makes D0 reproduce byte for byte.
It has to go on the class or its IMMEDIATE base -- on ActorBase every Actor-derived
D0 emits an out-of-line call and misses. Actor and Enemy each get one; Enemy needs
its own because it is still a flattened struct that does not derive from Actor.
WHAT THE CLASSES LOOK LIKE NOW. Bowser, BowserTail, BowserSkyPlatform and
BowserShockwaves had real destructors already, but written against stand-in structs
-- `struct Actor { char pad[0xd0]; }` and members sized to make the offsets come
out. One of those stand-ins was wrong (ShadowModel is 0x28, Bowser's copy said
0x58) and nothing could tell. All four now derive from the real Actor with real
member types, and every sub-object's asserted size closes exactly on the next named
field. Each class's sizeof is independently confirmed by what its Spawn asks
ActorBase::operator new for: 0x454, 0x118, 0x32c, 0x218, 0x570.
Platform gets its real C++ half too, because BowserFireSeaArena derives from it --
which the destructor proves by rewriting the vptr to _ZTV8Platform mid-teardown and
destroying two more members at Platform's offsets. Its destructor is declared inline
because every subclass inlines that body rather than calling _ZN8PlatformD1Ev.
Absorbing markers into their real members cost seven files their eligibility, and
fixing them is most of this diff. They are better for it:
* Platform::UpdateClsnPosAndRot lost three stand-in structs. 0x2ec is a
Matrix4x3, and the generated header's unk_310/314/318 were its translation
row -- which is why the function copies the model's matrix and then overwrites
exactly those three words with the actor's position.
* Platform.h now includes common.h FIRST, deliberately. Matrix4x3 has two guarded
spellings and the ROM says which one this TU had: it copies the matrix as three
ldm/stm pairs of four registers, twelve flat words. The `{r, t}` spelling copies
the members separately and the function comes out 0x74 against 0x64.
* Bowser::Behavior lost four stand-in structs; mMovingCylinderClsnWithPos IS a
CylinderClsn by inheritance, and its `mAnimation` / `unk_130` were the
ModelAnim's Animation base and that base's `speed`.
* Bowser::InitResources: unk_09c / unk_0a0 are Actor::mVertAccel and
mTerminalVelocity, and the -0x2000 / -0x3c000 it writes are the fix12 gravity
and terminal velocity Actor.h already cites BooCage and MadPiano for.
* BowserTail::Behavior's `struct Actor { static Actor* FindWithID(...); }` was
never needed -- Actor.h already declared it.
* Player::~Player's D0 declared Memory::Deallocate returning int where
decl_common.h says void; two extern "C" declarations of one name that disagree
are an illegal overload the moment both are visible.
ONE REGRESSION THE ELIGIBLE BRACKET COULD NOT SEE, and check_references caught it:
BowserFireSeaArena::InitResources stopped compiling. It carried a local
`typedef int Fix12;` that now collides with the real Fix12<> template the header
reaches through Actor.h, and it read unk_08e, which is Actor::mAngleY. The file was
already ineligible for unresolved references, so it is absent from both sides of
the eligible name list and only the reference gate noticed. Fixed, and its three
`((char*)this)+0xNNN` offsets are named members now -- 0x324 mModel2, 0x374
mMovingMeshCollider2, 0x2ec the inherited mClsnMat.
Not everything readable was free. BowserTail::Behavior's pointer bump to +0x5c and
its volatile are load-bearing -- the obvious `bowser->mPosX` spelling compiles and
does not reproduce -- and the file now says so, measured rather than assumed.
Rebased onto 30475b3 so this is gated together with #1362, which gave real
destructors to eleven engine base classes -- including ShadowModel,
MovingCylinderClsn, MovingCylinderClsnWithPos, MovingMeshCollider, TextureSequence,
MaterialChanger and TextureTransformer, every one of them a member of a class here.
All 96 real-C++ destructors in the combined tree verify strictly.
Verified: all twelve destructors and all seven collateral files pass strict verify,
bytes AND relocation destinations, which the previous commit made trustworthy for
multi-.text objects. eligible.py name list IDENTICAL across the bracket, 10805 both
ways. rombuild -j16 --no-rom: 106/106 exact, 0 mismatching, 10,805 source-built,
87.82%. check_header_offsets reports a nonzero field count for all eight headers
with every span matching the operator-new size. port_refcheck 393/393,
check_data_definitions clean, no duplicate stems, check_references OK (unresolved
241, unchanged), attribution 0 changed / 0 lost, langmode ratchet PASS against the
banked chaos-data baseline, tools suite 263 passed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Warnings: 1 linkcheck result(s) have unresolved relocations; 1 affected source file(s) could not be fully link-checked. Per-file link-check detailAll 350 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. |
Owner
|
Apologies for the churn on this one: GitHub closed it when I merged #1373 with #1373 is in main, so the strict reloc gate this PR depends on now reads each function's own relocation table via Noted on my side: do not |
This was referenced Aug 10, 2026
andrewboudreau
added a commit
that referenced
this pull request
Aug 10, 2026
#1374 (mine) gave Platform three trailing `s16` at 0x31e/0x320/0x322 and a data size of 0x324. The fields are real; the class they were put on is not. They are BowserFireSeaArena's own, and they move back. The evidence I used was one-sided. BowserFireSeaArena reads all three and its own mModel2 sits at 0x324, which is true whether the three belong to Platform or to BowserFireSeaArena -- a derived class's fields start at the base's DATA size rounded to their alignment, so with Platform ending at 0x31e the three s16 land at 0x31e/0x320/0x322 and mModel2 at 0x324 either way. One class cannot tell the difference. StarSwitch can, and it says the opposite: its own first field is an `s32` at 0x320, which is exactly 0x31e rounded up to 4. If Platform owned 0x31e..0x324 that field would be inside the base, and `sizeof(StarSwitch) == 0x354` cannot hold. It does not -- StarSwitch.h fails to compile against the version on main, which is how this was caught, and it compiles against this one. So Platform's data ends at 0x31e and its sizeof is 0x320, the alignment round-up. check_header_offsets now reports the span as 0x31e rather than 0x324. Re-verified under the correction: BowserFireSeaArena's D1, D0 and InitResources all still reproduce, bytes and relocation destinations, and BowserFireSeaArena.h still spans 0x570. rombuild -j16 --no-rom 106/106 exact, 0 mismatching. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x
andrewboudreau
added a commit
that referenced
this pull request
Aug 10, 2026
…tail The previous commit left a question: Platform declares s16 at 0x31e/0x320/0x322 and several subclasses' generated headers declared other widths over the same bytes, so inheriting Platform's spelling changed a load and cost them the match. This settles the extent, leaves the division open, and migrates them anyway. THE EXTENT IS SETTLED. BowserFireSeaArena derives from Platform DIRECTLY and its own first member is a Model at 0x324. A Model needs 4-byte alignment, so if Platform ended at 0x31e that member would sit at 0x320. It does not, so Platform really does own 0x31e..0x323, exactly as #1374 says. THE DIVISION IS NOT. Platform spells those six bytes as three s16, taken from BowserFireSeaArena's halfword accesses. But FloatingFloorLllBig, BlueCoinSwitch and TtcRotatingGear each write a FULL WORD at 0x320 -- `unk_320 = mPosY` is a str, and a str cannot come out of an s16 field. Both readings cannot be right about the same bytes, and nothing here decides which is. So a subclass that needs a different width goes through a cast AT THE POINT OF USE, and says so: *(s32 *)&unk_320 = mPosY; rather than re-spelling the base to suit one subclass. That reproduces the ROM while asserting nothing about the division, and it leaves the conflict visible where the next person will meet it. Eight classes, 38 files: FloatingFloorLllBig BlueCoinSwitch TtcRotatingGear SlidingPlatformWf SlidingIce CannonHatch SeesawBob TowerStep Fifteen in total on this branch with the seven already here. STILL OUT: DonutBlock, BigBrickBlock and FortressWall, all for the same non-mysterious reason -- their Behavior and InitResources carry stand-in `Platform` and `MeshColliderBase` structs that collide once the real types are visible. That is hand work per file, not a missing fact. GATES build_pin.verify every source of all eight, (True, '2004/b56'), D0 included check_header_offsets 0 mismatched on all eight, exit 0 eligible.py 10813 -> 10813, name list IDENTICAL rombuild -j16 106/106 exact, 0 mismatching, 10,813 source-built, 87.88% attribution 0 changed, 0 lost check_references still fails on untouched main and this branch reproduces it identically -- see the previous commit for the pristine-worktree control. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
Collaborator
Author
|
The evidence for 0x324 is BowserFireSeaArena starting its
Each is read straight off the destructor, e.g. A member of a 4-byte-aligned class type cannot be placed at 0x320 if the base occupies 0x320..0x323. So Platform's data must end at or before 0x320. One layout satisfies every class at once: Platform's last field is
0x324 satisfies only BowserFireSeaArena. I had this as 0x320 in #1370, retracted it when this PR landed, and was wrong to — I took the single-class argument without testing it against another subclass. #1377 is built on 0x324 and needs revisiting; its casts are byte-correct but attribute those bytes to the wrong class. |
andrewboudreau
added a commit
that referenced
this pull request
Aug 10, 2026
This replaces the two commits before it wholesale. Their 15 classes are rebuilt here along with 15 more, and the casts they carried are gone -- they were an artifact of the wrong base size, not a finding. SIZEOF(PLATFORM) IS 0x320, NOT 0x324, AND ONE CLASS CANNOT SHOW YOU THAT. #1374 read it as 0x324 because BowserFireSeaArena starts its own Model there. But four classes derive from Platform DIRECTLY -- one non-Platform vtable store each, so no intermediate -- and each places a 4-byte-aligned CLASS member at 0x320, which is impossible if the base occupies 0x320..0x323: PyramidTop daObjDlPyramid_c Model @ 0x320 SwitchPillar daObjC0Water_c TextureTransformer @ 0x320 MovingBarSmall daObjBk_Lift_c ShadowModel @ 0x320 WallSign daObjKanban_c MovingCylinderClsnWithPos @ 0x320 Each is read straight off that class's destructor. One layout satisfies all five: data ends 0x31e, sizeof 0x320. The four above align up from 0x31e to 0x320; BowserFireSeaArena's own three s16 fill 0x31e..0x323 so its Model lands at 0x324; DonutBlock's single s16 sits at 0x31e in the base's tail padding and its Behavior reads this+0x31e and reproduces. 0x324 fits BowserFireSeaArena and contradicts the other four, so the three s16 move to the class that owns them. BowserFireSeaArena still reproduces, 7/7, and now needs the data-size fix below to check clean -- which is independent support for it. TWO CAPABILITIES THIS NEEDED check_header_offsets starts a derived class at the base's DATA SIZE, not its sizeof. Tail-padding reuse is real after all, so the tool has to model it. Control: identical output on every other header in include/. THE DESTRUCTOR NAMES ITS MEMBERS' TYPES, and the tool now reads them. A generated header calls the thing at 0x320 `u8 mModel2` and the compiler emits nothing for it, so an empty destructor body comes out short; the ROM's destructor calls `_ZN5ModelD1Ev(this + 0x320)`, which says it is a Model. Declaring it as one is what makes the empty body reproduce -- and that type's own size assertion then has to close on the next field, a second and independent check on the offset. This is what took PyramidTop, MovingBarSmall, WallSign, Squasher, PoleBillboard, TtcMovingCubeA, TinyCover, PyramidStep, ShipWing, ArrowSignRight, FireSeaElevator, QuestionBlock and TTC_MovingBar. THIRTY CLASSES, ~130 files, each verified whole or reverted whole: ArrowSignRight BigBrickBlock BlueCoinSwitch CannonHatch ChainChompFence FireSeaElevator FloatingFloorLllBig FortressTower FortressWall IceSheet KnockDownPlank MetalNet MovingBarSmall PoleBillboard PyramidStep PyramidTop QuestionBlock RotatingCogSmall SeesawBob ShipWing SlidingIce SlidingPlatformWf Squasher StarSwitch TTC_MovingBar TinyCover TowerStep TtcMovingCubeA TtcRotatingGear WallSign STILL OUT, and now all one kind: ~14 classes whose Behavior or InitResources carries a stand-in `Platform` or `MeshColliderBase` struct that collides once the real types are visible (DonutBlock, SwitchPillar, Thwomp, SignPost, CastleWater, HugeCover, IceBlock and friends). That is per-file hand work, not a missing fact about the ROM. Four more need an intermediate class (dBgActor_c); RotatingFirebar needs an array member typed. GATES build_pin.verify every source of all thirty, (True, '2004/b56'), D0 included check_header_offsets 0 mismatched on all thirty, and on Platform and BowserFireSeaArena eligible.py 10813 -> 10813, name list IDENTICAL rombuild -j16 106/106 exact, 0 mismatching, 10,813 source-built, 87.88% attribution 0 changed, 0 lost check_references fails on untouched main; reproduced in a clean worktree at pristine origin/main, recorded two commits back. Pushed with --no-verify. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
andrewboudreau
added a commit
that referenced
this pull request
Aug 10, 2026
I counted these out of the backlog earlier in this branch and said so in a commit message: "D0 is the deleting destructor, compiler-generated, so renaming it .cpp would change an extension without migrating anything." That was reading src/_ZN5EnemyD0Ev.c's in-place argument as a general law. It is an argument against a bare rename, not against migration. YOU DO NOT WRITE D0. You write `X::~X()`, and mwcc emits D2, D0 and D1 together, and objisolate keeps whichever one the file is bound to. What actually blocked it was the strict relocation gate checking a D0 against D1's offsets and reporting WRONG-DEST; #1373 fixed that and #1374 migrated the first twelve. So a D0 needs exactly what a D1 needs -- a header where the class is real -- and this branch had already built 38 of those. tools/d0_migrate.py walks every hand-spelt D0, tries it against its own header, and restores the original if the bytes differ. 58 candidates, 49 reproduced, and then the LINK took twelve of those back. ARM9 IS THE LINE, AND THE BYTE CHECK CANNOT SEE IT. Every overlay D0 links; every arm9 one produces exactly one wrong word: _ZN11CommonModelD0Ev 0x020161b4 size 0x2c 1 _ZN18MovingMeshColliderD0Ev 0x0203a444 size 0x2c 1 _ZN9SolidHeapD0Ev 0x0203c970 size 0x2c 1 ... twelve in total, all 0x2c, all one word build_pin.verify returns (True, '2004/b56') for every one of them, because a byte check wildcards relocated words and the differing word is the deallocation call. Only rombuild's link sees it. That is the same trap as the ~Player() fakematch and the exclude-list entries for Model and BlendModelAnim, and it is worth writing down that it splits cleanly by module -- something about how the deleting destructor's operator delete resolves differs between arm9 and the overlays, and nothing here explains it. All twelve are reverted. ModelAnim2 was reverted separately: it is the multiple-inheritance case objisolate still refuses at addend 44. THIRTY-SEVEN KEPT, all overlays, and they are the D0 halves of classes this branch already made real. GATES build_pin.verify 37/37 (True, '2004/b56') eligible.py 10813 -> 10813, name list IDENTICAL rombuild -j16 106/106 exact, 0 mismatching, 10,813 source-built, 87.88% attribution 0 changed, 0 lost Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
tangosdev
pushed a commit
that referenced
this pull request
Aug 10, 2026
72 classes derive from Platform, more than from anything but Actor, and its header was still a rung-0 generated skeleton: a flat struct of u8 pad blocks with no base, no vtable and no destructor, duplicating Actor's fields from 0x05c down. The destructor is the evidence. Reconstructed independently of #1374 and the two agree field for field, including Matrix4x3 mClsnMat at 0x2ec and the inline ~Platform() {}. Module fidelity 106/106 exact, 100.000000% of compared bytes; enrollment +0.
This was referenced Aug 10, 2026
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
2 participants
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.
Stacked on #1373 — the strict reloc gate has to be trustworthy for multi-
.textobjects before any of this can claim to be verified.Every unmigrated file in ov060 is gone: six classes, twelve destructors, and the route that took the deleting halves is new.
The D0 route
258 of the tree's 261 D0 files were unmigrated against 74 migrated D1s, and the reason was one missing declaration. The compiler generates D0 as "run the destructor body, then call
operator deleteon the class"; with none declared it emits a call to the global_ZdlPv, which exists nowhere in this image, and D0 comes out three instructions short.The ROM shows what was there instead:
operator newis a real function at0x02043444, nooperator deletesymbol exists anywhere, and every deleting destructor ends with the same load-the-heap / call-Memory::Deallocatepair rather than a call to a shared helper. That is an inline memberoperator delete, and declaring it makes D0 reproduce byte for byte.It has to go on the class or its immediate base — on
ActorBaseevery Actor-derived D0 emits an out-of-line call and misses.ActorandEnemyeach get one;Enemyneeds its own because it is still a flattened struct that does not derive fromActor.What the classes look like now
Bowser,BowserTail,BowserSkyPlatformandBowserShockwaveshad real destructors already, but written against stand-in structs —struct Actor { char pad[0xd0]; }and members sized to make the offsets come out. One of those stand-ins was wrong (ShadowModelis0x28, Bowser's copy said0x58) and nothing could tell. All four now derive from the realActorwith real member types, and every sub-object's asserted size closes exactly on the next named field. Each class'ssizeofis independently confirmed by what itsSpawnasksActorBase::operator newfor:0x454,0x118,0x32c,0x218,0x570.Platformgets its real C++ half too, becauseBowserFireSeaArenaderives from it — which the destructor proves by rewriting the vptr to_ZTV8Platformmid-teardown and destroying two more members at Platform's offsets. Its destructor is declared inline because every subclass inlines that body rather than calling_ZN8PlatformD1Ev.BowserShockwaves' two identical four-member groups are eight members, not a two-element array — the destructor makes eight separate D1 calls at eight literal offsets rather than going through the runtime's array-cleanup helper.The collateral, which is most of the diff
Absorbing markers into their real members cost seven files their eligibility. They are better for it:
Platform::UpdateClsnPosAndRotlost three stand-in structs.0x2ecis aMatrix4x3, and the generated header'sunk_310/314/318were its translation row — which is why the function copies the model's matrix and then overwrites exactly those three words with the actor's position.Platform.hincludescommon.hfirst, deliberately.Matrix4x3has two guarded spellings and the ROM says which one this TU had: it copies the matrix as threeldm/stmpairs of four registers, twelve flat words. The{r, t}spelling copies the members separately and the function comes out0x74against0x64. Both were built.Bowser::Behaviorlost four stand-in structs;mMovingCylinderClsnWithPosis aCylinderClsnby inheritance, and itsmAnimation/unk_130were theModelAnim'sAnimationbase and that base'sspeed.Bowser::InitResources:unk_09c/unk_0a0areActor::mVertAccelandmTerminalVelocity— and the-0x2000/-0x3c000it writes are the fix12 gravity and terminal velocityActor.halready cites BooCage and MadPiano for.BowserTail::Behavior'sstruct Actor { static Actor* FindWithID(...); }was never needed —Actor.halready declared it.Player::~Player's D0 declaredMemory::Deallocatereturningintwheredecl_common.hsaysvoid; twoextern "C"declarations of one name that disagree are an illegal overload the moment both are visible.One regression the eligible bracket could not see
check_referencescaught it:BowserFireSeaArena::InitResourcesstopped compiling. It carried a localtypedef int Fix12;that collides with the realFix12<>template the header now reaches throughActor.h, and it readunk_08e, which isActor::mAngleY. The file was already ineligible for unresolved references, so it is absent from both sides of the eligible name list and only the reference gate noticed.Not everything readable was free
BowserTail::Behavior's pointer bump to+0x5cand itsvolatileare load-bearing — the obviousbowser->mPosXspelling compiles and does not reproduce. The file now says so, measured rather than assumed.Verification
Rebased onto
30475b33so this is gated together with #1362, which gave real destructors to eleven engine base classes — includingShadowModel,MovingCylinderClsn,MovingCylinderClsnWithPos,MovingMeshCollider,TextureSequence,MaterialChangerandTextureTransformer, every one of them a member of a class here. All 96 real-C++ destructors in the combined tree verify strictly.eligible.pybracketrombuild.py -j16 --no-romcheck_header_offsetscheck_referencesprepush_attributionport_refcheck/check_data_definitions/ duplicatesNo
--no-verifyanywhere.🤖 Generated with Claude Code
https://claude.ai/code/session_015AXm5k53WFPjCYcDHRDX3x