tools: let a policy license the class data a promoted TU emits - #1986
Closed
andrewboudreau wants to merge 1 commit into
Closed
tools: let a policy license the class data a promoted TU emits#1986andrewboudreau wants to merge 1 commit into
andrewboudreau wants to merge 1 commit into
Conversation
Promoting a TU consolidates N one-function objects into one, which takes objisolate's intact multi-symbol path: nothing may survive that the manifest has not licensed, and `deadstrip_plan` accepted STT_FUNC only. A class's `_ZTV`/`_ZTI`/`_ZTS` are STT_OBJECT, so the only way to promote was to strip the destructor that makes mwcc emit them -- silently trading away the one thing that makes a vtable comparable to the cartridge at all. Measured on ov100/daObjPathLift_c: the promoted TU went from 14 data records to 0. Three pieces: * `deadstrip_plan` accepts STT_OBJECT. Data goes to `externalise`, not `dead` -- the destructor must go on storing the vptr -- so a surviving section may reference one by name, but only for the RTTI trio whose relocation shapes are surveyed, and never through an unnamed section symbol. mwcc's `_ZTV<C>` addresses the vtable object; symbols.txt's addresses the slots, so the plan names the code sections whose vptr stores still carry the preamble skip in a new `rebase` key and `_apply` corrects them. Existing plans carry no such key and keep the old rule exactly. * A new `deadstrip-data` disposition in rombuild/tubuild, licensed on an address argument: the home lies outside every range the entry claims, so dsd delinks it from the cartridge either way and discarding this object's copy cannot cost the image a byte. A home INSIDE a claimed range is refused -- that is content the source must build. Soundness is not enough, though: a vtable whose slots disagree means the class model is wrong, so `_data_body_reasons` makes romdata_check's relocation-resolved comparison binding for exactly the symbols a policy row names, before the surgery zeroes them. An entry with no module of its own now fails closed rather than answering "no claimed range" for every address. * `deadstrip-duplicate`'s body proof compared raw section bytes, which can only be right for a body with no relocations -- a 4-byte `bx lr` was the only one that had ever taken this path. Every `bl` and vptr store holds an addend here and a linked address in the cartridge. objisolate now compares the words no relocation covers, and `rombuild._duplicate_body_reasons` links the body through linkcheck and compares the ones it masked. Both halves of every duplicate body are checked now, where before one flat case passed and anything real would have been rejected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
✅ PR validation — Passednoverify: no source/build-data changes in this PR Each changed |
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
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.
Part 3 of the #1878 split, after #1977 (the duplicate-body policy) and #1979 (the
promotion that needed it). Tools only — no manifest row uses the new disposition yet,
so this build is expected to be byte-for-byte identical to main.
The problem
Promoting a TU consolidates N one-function objects into one, which takes objisolate's
intact multi-symbol path: nothing may survive that the manifest has not licensed,
and
deadstrip_planacceptedSTT_FUNConly. A class's_ZTV/_ZTI/_ZTSareSTT_OBJECT, so the only way to promote a TU was to strip the destructor that makesmwcc emit them — silently trading away the one thing that makes a vtable comparable to
the cartridge at all.
Measured on
ov100/daObjPathLift_c(the TU #1979 just promoted): 14 ROM-data recordsto 0. No gate reports this.
rombuild's 106/106 is blind to vtables andromdata_checkis advisory, so every promotion so far has traded vtable coverage forconsolidation without anyone noticing —
ActorBase_SceneNode, the first promoted TU,emits no ROM-data records either.
Three pieces
1.
deadstrip_planacceptsSTT_OBJECT. Data goes toexternalise, notdead—the destructor must go on storing the vptr — so a surviving section may reference one by
name, but only for the RTTI trio whose relocation shapes are surveyed, and never through
an unnamed section symbol.
mwcc's
_ZTV<C>addresses the vtable object;symbols.txt's is the slot array.So a vptr store carries either the 8-byte preamble skip (a definition in this object —
correct it) or nothing at all (the source declared the ROM symbol directly, as
daObjPathLift_c.cppdoes today withextern int _ZTV10dBgActor_c[];— leave it).Anything strictly between the two is refused rather than guessed at. The plan names the
sections needing the correction in a new
rebasekey; plans without one keep the oldrule exactly.
2. A new
deadstrip-datadisposition, licensed on an address argument ratherthan a byte-identity one. A promoted entry claims exact ranges; a data home outside
every one of them is content dsd delinks from the cartridge whatever this object emits,
so discarding the copy cannot cost the image a byte. A home inside a claimed range is
refused — that is a range the source undertook to build — and so is an entry that
declares no module of its own, which would otherwise clear the range test vacuously for
every address in the ROM.
Sound is not the same as right: a vtable whose slots disagree with the cartridge means
the class model is wrong, and quietly dropping it would bury that evidence in the very
file that produced it. So
romdata_check's relocation-resolved, word-by-word comparisonis made binding for exactly the symbols a policy row names, and run before the
surgery zeroes them. It stays advisory everywhere else.
PARTIALis the normal verdictfor a vtable — the ROM extent is distance-to-next-symbol, so a trailing alignment gap
that belongs to nobody shortens a fully-matching body — and only
DIFFERSrefuses.3.
deadstrip-duplicate's body proof was wrong, and this fixes it. It compared rawsection bytes against the cartridge, which can only be right for a body with no
relocations: every
bland every vptr store holds an addend in the object and alinked address in the image. A 4-byte
bx lr(_ZN7Vector3D1Ev) is the only bodyflat enough ever to have passed it; anything real would have been rejected. objisolate
now compares the words no relocation covers, and
rombuild._duplicate_body_reasonslinks the body through
linkcheckand compares the ones it masked. Both halves arechecked now.
Verification
pytest tools/test_objisolate.py tools/test_rombuild.py tools/test_tubuild.py—98 passed, +12 new cases covering both accepted vptr spellings, the shape between
them, the non-RTTI import refusal, function-only duplicate evidence, and all four
deadstrip-datalicence conditions.python tools/rombuild.py -j 16— 106/106 exact, 100.000000% of compared bytes,PASS; 446 verified / 239 partial / 15 differ from 7,798 object records, identical
to main.
What comes next
The manifest PR that uses it restores
ov100/daObjPathLift_c's real destructor andtakes it from 0 to 14 ROM-data records — 12 named and checked against the cartridge,
360 bytes compared, 3 VERIFIED / 9 PARTIAL / 0 DIFFERS, including this class's own
128-byte
_ZTV15daObjPathLift_catov100:0x0214857c. It has to be a separate PRbecause the validator restores all of
tools/from base, so no PR can test its owntool change.
🤖 Generated with Claude Code