check_header_offsets: a derived class starts at the base's DATA SIZE, and three classes come back - #1375
Merged
Merged
Conversation
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Per-file link-check detailAll 13 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. |
Scene::~Scene stores TWO vptrs and then calls ActorBase's destructor:
str r2, [r4] ; _ZTV5Scene
str r1, [r4] ; _ZTV12ActorDerived <- ActorDerived's D2, INLINED
bl ActorBase::~ActorBase
A merely declared `virtual ~ActorDerived();` cannot produce that. The compiler
has no body to inline and emits `bl _ZN12ActorDerivedD2Ev` -- one store where
the ROM has two. Define it in the class body and Scene::~Scene() {} reproduces
0x0202e140 exactly. So the original sources defined these destructors inline,
and every derived destructor inlined them. That is worth knowing because
roughly 60 unmigrated D1 files have this two-vtable-store shape.
Making it true cost three changes, and each one was forced by the next gate
rather than chosen up front.
1. objisolate: CORRECT AN UNDEF VTABLE REFERENCE, NOT JUST AN EXTERNALISED ONE
An inlined base destructor stores a vptr for a class this object never
defines, so `_ZTV12ActorDerived` is UNDEF from the start with addend 8 --
never a candidate for externalisation, so the existing correction never
looked at it, and the guard refused the file.
objisolate PREDICTED THIS. The comment above the UNDEF branch names both the
constructor-only TU and "a derived destructor over an inline base
destructor", says both were reproduced under 2004/b56, and says they "arrive
the moment a real-C++ constructor is enrolled, which is the direction this
tree is moving". It refused because there was no enrolled instance to verify
a correction against. Scene is that instance.
The correction is the same arithmetic the externalise path already uses --
the ROM's _ZTV symbol IS the slot array, so addend 8 becomes 0 -- and it is
checked the same way, by rombuild byte-comparing the linked module. That is
the only thing that caught the original 8-high vptr bug across 34 modules.
ANY OTHER ADDEND IS STILL REFUSED, with a new test pinning that: multiple
inheritance produces addend 44 and there is still no instance for it.
The two tests asserting the old refusal now assert the correction, and a
third asserts the refusal that remains.
2. eligible.py: STB_LOPROC IS A DEFINITION
An inline function's out-of-line copy is emitted under mwcc's COMDAT
binding, STB_LOPROC. The symbol scan accepted STB_GLOBAL and STB_WEAK only,
so ActorDerived's own D1 -- defined, right size, in the kept section --
reported "0 defined global functions".
It is deduplication metadata, not a weaker definition, and exactly one
object in this build defines any given address. Accepting it also recovered
SIX functions that were invisible for the same reason and had nothing to do
with this slice: the _ZThn80_ virtual thunks of ModelAnim, ModelAnim2 and
BlendModelAnim, whose multiple inheritance makes them inline copies too.
3. ActorDerived's D1 file carries a FORCING TU
With the definition in the header that file cannot define it again, and a TU
that merely includes the header emits nothing. An explicit destructor call
in an uncalled function forces the out-of-line copy; objisolate drops it.
RESULT
eligible.py 10805 -> 10811, nothing lost (+6, all _ZThn80_ thunks)
rombuild -j16 106/106 exact, 0 mismatching
source-built 10,805 -> 10,811, 87.82% -> 87.83%
test_objisolate 6 passed
port_refcheck 393 references, 0 stale
langmode ratchet PASS
no --no-verify
NOT DONE HERE. Stage is the next one in this chain and needs its three members
typed first -- Particle::SysTracker at 0x50, Model at 0x86c, MeshCollider at
0x91c are still u8 markers. And ModelAnim's family still cannot be isolated:
that is the addend-44 refusal above, deliberately left standing.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
72 classes derive from Platform -- more than from anything but Actor itself --
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, and it closes on three boundaries at once:
t[0] = _ZTV8Platform
MovingMeshCollider::~MovingMeshCollider(this + 0x124)
Model::~Model(this + 0xd4)
Actor::~Actor(this)
Actor is 0xd0, so Platform's own data starts there. Model is 0x50, so a Model
at 0xd4 ends at exactly 0x124 -- where the destructor puts the collider.
MovingMeshCollider is 0x1c8, so it ends at 0x2ec. Three sizes, each asserted by
its own header, each landing on an offset the destructor names independently.
WHAT THE MARKERS TURNED OUT TO BE. Four of the generated fields were never
Platform's: unk_0f0 is the Model's own mat4x3 (0xd4 + 0x1c), and unk_114/118/
11c are that matrix's translation row (+0x24/+0x28/+0x2c). UpdateModelPosAndRotY
was writing the model's position through raw offsets and now says so, with the
`this + 0xf0` magic address gone.
AND 0x2ec IS A SECOND MATRIX, on the same kind of arithmetic. The generated
header had 0x24 of padding there and three s32 at 0x310/0x314/0x318 -- but
0x2ec + 0x24 IS 0x310, so those three are that span's last row and 0x2ec..0x31c
is one Matrix4x3. UpdateClsnPosAndRot settles it: it copies the model's matrix
in and then writes the actor's position into 0x310/0x314/0x318. A matrix and its
translation, not four unrelated fields. It is what the mesh collider is
transformed by, so it is mClsnMat.
Also migrated: IsClsnInRange and IsClsnInRangeOnScreen (unk_0b0 was
Actor::mFlags). Five sources, three shadow structs deleted, and the two
remaining `Obj`/`MMC` stand-ins are gone entirely.
ONE IDIOM DELIBERATELY KEPT. UpdateClsnPosAndRot still casts to a flat 12-word
struct for the matrix copy. `mClsnMat = mModel.mat4x3` is the readable spelling
and is +0x10 bytes: Matrix4x3 is {Matrix3x3 r; Vector3 t;}, so member-wise
assignment copies 9 words then 3 (ldm/stm 4+4+1, then a separate 3) where the
ROM copies 12 flat as 4+4+4. include/MovingMeshCollider.h already records this
obstacle for SetFile and Transform and asks for one shared flat-copy helper in
a pass of its own. This is not that pass.
The destructor is defined INLINE, per the convention this branch establishes --
all 72 subclasses inline its vptr store rather than calling it, so the body has
to be visible. Its D1 file carries the forcing TU.
GATES
build_pin.verify 5/5 (True, '2004/b56')
eligible.py 10811 -> 10811, name list IDENTICAL
rombuild -j16 106/106 exact, 0 mismatching, 10,811 source-built, 87.83%
check_header_offsets 8 commented fields, 0 mismatched, 0 unparsed, 0x31e
no --no-verify
A first eligible run reported func_ov006_020c3ad8 as "compile failed" and it is
`void func_ov006_020c3ad8(void) {}` with no includes -- it cannot see this
header, and it compiles standalone. Transient build contention; a serial re-run
returned an identical name list. Recorded because the reflex on a broken
neighbour should be to re-run before believing it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
#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
…ings that stop the other 41 First subclasses on top of #1370. Both headers were generated skeletons restating all of Actor and Platform; they are `struct X : Platform` now, their destructors are real methods, and five other sources came along because they had to compile against the new header. Nothing below 0x320 is declared twice any more. The renames are all Actor's real names: mActorID/mActorId -> actorID, mParam -> param1, and a local `typedef int Fix12` deleted from three files -- it shadowed the real Fix12 template the moment Platform.h made it visible. Raw offsets became members: `this + 0x124` is mMeshCollider, `this + 0x2ec` is mClsnMat, `this + 0xd4` is mModel. I tried nine subclasses and kept two. The seven that came back out are the useful part of this commit, because each names a different blocker: 1. FOUR HAVE AN INTERMEDIATE CLASS. ShutterBob, ShutterHmc, FallBlockLll and MetalNetLift store THREE vtables, not two -- e.g. FallBlockLll writes _ZTV15daObjFlMaruta_c, then _ZTV10dBgActor_c, then _ZTV8Platform. There is a class between them and Platform (dBgActor_c, and an unnamed one at data_ov002_021099e4 for the Shutters). It has to be reconstructed first. 2. THREE USE PLATFORM'S TAIL PADDING, AND check_header_offsets CANNOT MODEL IT. DonutBlock, FortressWall and BigBrickBlock put fields at 0x31e/0x31f. Platform's last field ends at 0x31e and its size rounds to 0x320, and the Itanium ABI lets a derived class use a non-POD base's tail padding -- the bytes confirm it, DonutBlock::Behavior reads this+0x31e and reproduces. check_header_offsets starts a derived class's fields at the base's SIZEOF rather than its DATA SIZE, so it reports every field mismatched and EXITS 1. All three verified (DonutBlock: five functions, including two rewritten off their shadow structs) and all three are reverted, because shipping headers that make a gate hard-fail is worse than shipping fewer. The tool wants the base's data size; that is the next change, and it unblocks all three plus every other subclass with a field below 0x320. 3. ONE GREW ITS .text SECTION. BigMovingIceBlock::Behavior byte-verified and then went ineligible: `.text section 0x258 != declared 0x234`. The TU emits something beyond the declared function once Platform.h is visible -- the inline destructor's COMDAT copy is the obvious suspect and is unproven. Worth knowing because ENROLL.PY PRESERVES `complete` BY SYMBOL NAME, so the now-ineligible file stayed enrolled, compiled anyway, and took ov056 down with 461 mismatching words. An eligibility loss is not self-correcting; check the bracket before trusting the build. Two field-placement notes for whoever does the next batch: - Do NOT pad from 0x31e. The base's asserted size already covers 0x31e..0x31f, so a `u8 pad_31e[2]` shifts every later field by 2 and reports as a mismatch. Start padding at 0x320. - `mAngleY += x` and `mAngleY = mAngleY + x` are NOT interchangeable, and which one matches is per-SITE. DonutBlock::Behavior needs the compound form (the ROM CSEs the field address); Player::St_WallJump_Init needs the other. Same for a bool temp: `int b = (x & 8) != 0; if (b)` emits movne/moveq where a direct `if` folds them. GATES build_pin.verify 7/7 (True, '2004/b56') eligible.py 10811 -> 10811, name list IDENTICAL rombuild -j16 106/106 exact, 0 mismatching, 10,811 source-built, 87.83% check_header_offsets StarSwitch 16 fields 0 mismatched; FloatingFloorLllBig 2/0 no --no-verify Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
… and three classes come back The Itanium ABI lets a derived class place members in a non-POD base's TAIL PADDING, and this tree's classes do it. Platform's last field ends at 0x31e and its size rounds to 0x320, so DonutBlock's s16 lands at 0x31e -- confirmed by the bytes, since DonutBlock::Behavior reads this+0x31e and reproduces the ROM. check_header_offsets started a derived class's fields at the base's SIZEOF. For those classes it reported every field mismatched and EXITED 1, which is why #1372 shipped without them. THE FIX IS NOT A TAIL-PADDING SPECIAL CASE. Starting at the data size is simply the correct offset that sizeof was approximating, and it is right for both kinds of class, because the walk already aligns each field: a derived class whose first own field is a 4-byte value still gets 0x31e rounded up to 0x320. The data size is read off the LAST COMMENTED FIELD -- the ROM evidence the tool already checks against -- rather than recomputed, so it cannot drift from the walk it feeds. CONTROL: every header in include/ was run through the tool before and after. The output is byte-identical -- no existing verdict moves, and the only headers whose result changes are the ones that could not be written until now. So DonutBlock, FortressWall and BigBrickBlock come back, with the source rewrites #1372 had to revert with them: DonutBlock 5 functions. Behavior loses a `typedef int Fix12` and stand-in MeshColliderBase and Platform structs; the `t[0x1e/2]` read off `this + 0x300` was its own unk_31e reached the long way round. InitResources loses three raw offsets -- 0xd4, 0x124 and 0x2ec are mModel, mMeshCollider and mClsnMat. FortressWall 4 functions BigBrickBlock 4 functions Two spellings in DonutBlock::Behavior are load-bearing and were measured, not chosen: `mAngleY += unk_31e` (the ROM CSEs the field address; the expanded form is the matching one in Player::St_WallJump_Init, so this is per-SITE), and a bool materialised into an int temp (the ROM emits movne/moveq and then tests, where a direct `if` folds them). GATES build_pin.verify 13/13 (True, '2004/b56') across the three classes check_header_offsets DonutBlock 1/0, FortressWall 4/0, BigBrickBlock 4/0 and identical output on every other header in include/ eligible.py 10811 -> 10811, name list IDENTICAL rombuild -j16 106/106 exact, 0 mismatching, 10,811 source-built no --no-verify Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
Collaborator
Author
|
Do not merge as argued — the premise is falsified. This PR's motivating case was that DonutBlock, FortressWall and BigBrickBlock place fields in Platform's tail padding at 0x31e/0x31f. #1374 landed Platform independently with better evidence: So no class in this family reuses tail padding, and the three classes this PR restores should not be declaring those fields at all. The code change — starting a derived class at the base's data size rather than its |
andrewboudreau
force-pushed
the
cpp/platform-subclasses
branch
from
August 10, 2026 08:48
dcf05f7 to
6eff8f3
Compare
andrewboudreau
force-pushed
the
cpp/header-offsets-datasize
branch
from
August 10, 2026 08:48
1938144 to
37e9d3f
Compare
Collaborator
Author
|
Rebased onto the restacked #1372; builds clean. The base moved twice under this PR: #1374 landed the Rebuilt at the top of the stack, which exercises all four PRs together: 106/106 exact, 0 mismatching, 10,812 source-built, 87.86%. Worth noting that your premise is what resolved the #1372 breakage independently. |
Collaborator
Author
|
Retracting my own retraction — this PR was right. See #1374 for the evidence: four subclasses place a class member at 0x320, so Platform's data ends at 0x31e and So the fix here is correct and has a live motivating case after all. My earlier comment saying the premise was falsified is withdrawn. |
Collaborator
Author
|
Re-checked: green, zero commits behind main, no conflict and no rebase needed. But it is now fully contained in #1377 — do not merge both. #1377 grew while this was open and its file set is a strict superset of this PR's: every path here, including It also arrives at the same layout conclusion independently — its The hazard is that both are green. They each carry the I have verified #1377 by hand (its validator refused it as too large): full ROM 106/106 exact, eligible bracket loses nothing, all 40 changed headers clean. Details on that PR. Nothing to fix here — flagging the duplication rather than closing someone else's PR. |
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.
check_header_offsets: a derived class starts at the base's DATA SIZE, and three classes come back
The Itanium ABI lets a derived class place members in a non-POD base's TAIL
PADDING, and this tree's classes do it. Platform's last field ends at 0x31e and
its size rounds to 0x320, so DonutBlock's s16 lands at 0x31e -- confirmed by the
bytes, since DonutBlock::Behavior reads this+0x31e and reproduces the ROM.
check_header_offsets started a derived class's fields at the base's SIZEOF. For
those classes it reported every field mismatched and EXITED 1, which is why
#1372 shipped without them.
THE FIX IS NOT A TAIL-PADDING SPECIAL CASE. Starting at the data size is simply
the correct offset that sizeof was approximating, and it is right for both
kinds of class, because the walk already aligns each field: a derived class
whose first own field is a 4-byte value still gets 0x31e rounded up to 0x320.
The data size is read off the LAST COMMENTED FIELD -- the ROM evidence the tool
already checks against -- rather than recomputed, so it cannot drift from the
walk it feeds.
CONTROL: every header in include/ was run through the tool before and after.
The output is byte-identical -- no existing verdict moves, and the only headers
whose result changes are the ones that could not be written until now.
So DonutBlock, FortressWall and BigBrickBlock come back, with the source
rewrites #1372 had to revert with them:
DonutBlock 5 functions. Behavior loses a
typedef int Fix12andstand-in MeshColliderBase and Platform structs; the
t[0x1e/2]read offthis + 0x300was its own unk_31ereached the long way round. InitResources loses three raw
offsets -- 0xd4, 0x124 and 0x2ec are mModel, mMeshCollider
and mClsnMat.
FortressWall 4 functions
BigBrickBlock 4 functions
Two spellings in DonutBlock::Behavior are load-bearing and were measured, not
chosen:
mAngleY += unk_31e(the ROM CSEs the field address; the expanded formis the matching one in Player::St_WallJump_Init, so this is per-SITE), and a
bool materialised into an int temp (the ROM emits movne/moveq and then tests,
where a direct
iffolds them).GATES
build_pin.verify 13/13 (True, '2004/b56') across the three classes
check_header_offsets DonutBlock 1/0, FortressWall 4/0, BigBrickBlock 4/0
and identical output on every other header in include/
eligible.py 10811 -> 10811, name list IDENTICAL
rombuild -j16 106/106 exact, 0 mismatching, 10,811 source-built
no --no-verify
Stacked on #1372.
🤖 Generated with Claude Code