Accept partitioned TUs without vtable splits - #2117
Merged
Merged
Conversation
andrewboudreau
added
the
attribution-override
✅ PR validation — PassedCommitted merge passes; 6 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (6)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (6 changed, 0 lost -- src/_ZN11dScMgBase_c24OnHitByCannonBlastedCharEv.c: tangosdev -> andrewboudreau; src/_ZN14dScMgD3DBase_c15OnHitByMegaCharEv.c: ruspecial -> andrewboudreau; src/_ZN14dScMgD3DBase_c24OnHitByCannonBlastedCharEv.c: tangosdev -> andrewboudreau; +3 more); 5 linkcheck result(s) have unresolved relocations. Per-file link-check detailAll 1201 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. |
Base automatically changed from
cpp/kaitendai-intact-tu
to
tools/tu-span-text-sections
September 1, 2026 01:39
andrewboudreau
force-pushed
the
tools/partitioned-no-split-alias
branch
from
September 1, 2026 01:47
0390b24 to
7194700
Compare
andrewboudreau
commented
Sep 1, 2026
andrewboudreau
left a comment
andrewboudreau
left a comment
Collaborator
Author
There was a problem hiding this comment.
Hold this one — its base branch is no longer going anywhere
#2117 targets tools/tu-span-text-sections. That is #2109's branch, and #2109
was closed as superseded once #2121 re-cut the same content onto main (merged
01:39:08Z). The branch still exists, so this PR still reads CLEAN and is still
mergeable — but merging it would deliver the change into an orphaned branch, not into
main.
Measured just now:
git merge-base --is-ancestor origin/tools/tu-span-text-sections origin/main
-> NO
This already happened once tonight. #2111 (cpp/kaitendai-intact-tu) merged into that
same base at 01:39:53Z. It reports MERGED in every UI and its content is not on
main; because the repo auto-deletes head branches, that orphaned branch is now the only
surviving copy of the kaitendai TU. That one is on me — I asked for the #2109 re-cut
without checking what was stacked on it.
This is a three-PR chain, not one PR
#2117 base = tools/tu-span-text-sections <- dead
#2119 base = tools/partitioned-no-split-alias
#2120 base = cpp/kuruma-intact-tu
#2119 stacks on #2117 and #2120 stacks on #2119, so all three are rooted on the
dead branch. Merging any of them in order lands all three off-main.
The fix
Retarget #2117 to main. Its content is independent of the tu-span change (that is
already on main via #2121), so this should be a base change and not a rebase — but
please confirm the diff against main is what you intend before merging, since the
current diff is computed against the dead base.
#2119 and #2120 can keep their stacked bases; once #2117 points at main the whole
chain roots correctly again.
Worth adopting generally
MERGED is not the same as on main. For any stacked PR:
gh pr view <n> --json baseRefName
git merge-base --is-ancestor origin/<base> origin/main
And before closing or superseding any PR, retarget whatever is stacked on it first —
closing a base silently orphans everything above it.
Collaborator
Author
|
Agreed that this chain must be rooted back on main. I checked the requested base change before applying it: a plain retarget of the current head produces a 150-file main diff that also sweeps in open #2096/#2103/#2104 and orphaned #2111, including the blocked FlyGuy/RcCarpet work and dashboard artifacts. That is not the intended #2117 diff. I am therefore replaying only #2117's independent two-file tools commit onto current main, then retargeting the PR to main. After its focused gates pass, I will replay #2119 and #2120 above that clean root and validate whether they have any real dependency on the orphaned chain. |
andrewboudreau
force-pushed
the
tools/partitioned-no-split-alias
branch
from
September 1, 2026 01:57
7194700 to
254fe53
Compare
andrewboudreau
changed the base branch from
tools/tu-span-text-sections
to
main
September 1, 2026 01:58
Collaborator
Author
|
Fixed. #2117 is now based directly on main at head 254fe53: exactly one commit and exactly two changed files (tools/tubuild.py and tools/test_tubuild.py). I replayed only the original 27-insertion/4-deletion patch, excluding the 150-file orphaned ancestry. Validation: 57/57 tubuild tests, Python compile, 405/405 port refs, no new unresolved references, and clean pre-push gates. I also tested the next stack member rather than assuming it follows: Kuruma genuinely depends on the #2103 nontextSectionOrder machinery, so #2119 and #2120 are now draft until that prerequisite reaches main. |
andrewboudreau
commented
Sep 1, 2026
andrewboudreau
left a comment
andrewboudreau
left a comment
Collaborator
Author
There was a problem hiding this comment.
Approving and merging. This is a real bugfix, and I found a stronger justification for it than the PR description claims — the shape it rescues is not an edge case, it is the only shape any manifest in this repo currently has.
The delta, from the control flow
partitioned is a CLI flag (tubuild.py:4298), not a manifest key — partitioned_link in the manifest is the recorded output of _record_partitioned, not an input. So the two arms are:
any(storageAlias or partitionSymbols) |
old | new |
|---|---|---|
| True | has_vtable_splits=True → audit runs → overwrites storage_aliases_ok |
identical |
| False | no audit; storage_aliases_ok keeps not partitioned |
no audit; storage_aliases_ok=True |
The second row differs only when partitioned=True. There, the old code left storage_aliases_ok=False permanently — no audit to overwrite it, no diagnostic, and nothing an author could add short of fabricating a split that does not exist. partitioned_link_ready then returns verified=False forever. An unfixable false refusal.
The positive control is live, and it is repo-wide
I executed partition_vtable_rebiases against every manifest on main (e1b878601) rather than grepping for the key:
manifests scanned : 97
claims refused : 0
produced >=1 policy : 3 ov047/daObjKm3_Kaitendai_c, ov047/daObjKm3_Kurumajiku_c, ov070/daBrq_c
any(alias|partition) : 0
Zero of ninety-seven. Under the old code, a --partitioned linkcheck could not set storage_aliases_ok true for any TU in the tree. The partitioned path was refusing everything.
That is not an accident of neglect — it is the shape main deliberately standardized on yesterday. dedaed495 ("Promote daObjKm3_Kaitendai_c intact translation unit") removed the storage_alias block from ov047/daObjKm3_Kaitendai_c.json and replaced it with emitted_storage_address + address_point_bias, with this evidence string:
compiler storage includes the two-word vtable preamble at 0x02112374; the configured public address point is eight bytes later and all 32 slot relocations match without inventing a second name for the ABI preamble
So the repo decided to stop naming the preamble, and cmd_linkcheck was still gating on that name being present. This PR closes exactly that gap.
A trap for anyone auditing this after me: Kaitendai's manifest still records partitioned_link.lastAttempt.linkedStorageAliases with a real, exact alias (data_ov047_02112374, donor _ZN20daObjKm3_Kaitendai_cD2Ev). I read that first and briefly concluded the opposite of the truth. It is stale recorded output from before dedaed495; the input row it was derived from no longer exists. Run the function, don't read the record.
Byte evidence
A change to tubuild.py needs a ROM, not just static gates — and this one earns it, because cmd_linkcheck is on the production build path: tu_production.py:121 shells out to tubuild.py linkcheck --baseline, which is why a rombuild log opens with tubuild's [1/8]…[8/8] before its own [1/6].
Full build of the merge tree bac18aa51dfc (main + this PR):
source-built functions: 11,088 (2,067,148 / 2,211,124 code bytes, 93.49%)
reproducing: 11,088 mismatching: 0
source-owned data claims: 3 (reproducing 3, mismatching 0)
module fidelity: 106/106 exact, 100.000000% of compared bytes
ROM-build analysis: PASS
ROM data from source: 529 verified, 191 partial, 6 differ, 522 unnamed
ROM sha256: d1506e90efae5e2d2cf119926a4ac2a291bd5ca78349d09d5024e1a918c478e8
Byte-identical to the main-only baseline on every metric. Note this build is a regression control, not a delta control: the production invocation passes --baseline and never --partitioned, so partitioned=False there and the two versions are provably identical. The delta's control is the manifest census above.
Byte correctness never rested on this flag in any case — partitioned_link_ready still requires all of equivalent, data_ok, artifacts_ok, module_ok, modules_check_ok, symbols_ok, rom_ok is True, rom_identical, no_stray_outputs alongside it. This loosens a gate that was refusing valid input; it does not loosen the ROM comparison.
Static gates, premerge_check.py 2117 on the merge tree: 8/8 pass → pass, nothing green → red. pytest tools/test_tubuild.py tools/test_tubuild_vtable_partitions.py → 66 passed.
Two non-blocking notes
1. The test does not cover the change. test_partitioned_storage_alias_gate_is_vacuously_exact_without_splits exercises initial_storage_alias_verdict in isolation. That helper is a pure predicate that was never wrong; the bug was in cmd_linkcheck's control flow, and no test touches it. The regression this PR fixes could be reintroduced tomorrow with all 66 tests still green. Worth a follow-up that drives cmd_linkcheck with partitioned=True and an alias-free policy set and asserts storage_aliases_ok.
2. The surviving initializer is dead, and now fails open. storage_aliases_ok = True at 4390 is unconditionally reassigned at ~4769 before any read (4943/4947/4978). So it is dead either way — but = not partitioned at least failed closed if control flow ever changed, and = True fails open. Since the value is dead, False costs nothing and keeps the safe failure direction. Not worth holding the PR for; worth taking if you touch the function again.
Merging with a merge commit rather than a squash, so #2119 retargets cleanly.
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.
Partitioned TU linkcheck now treats the absence of a vtable split policy as vacuously satisfied. Manifests that declare storageAlias or partitionSymbols still fail closed until the post-link symbol audit verifies them. Adds focused regression coverage for ordinary address points and both split-policy forms. Verified with the canonical objisolate/tubuild suite: 102 passed.