Skip to content

Grow ELF string tables for partition aliases - #2126

Merged
andrewboudreau merged 1 commit into
mainfrom
tools/objisolate-strtab-growth
Sep 1, 2026
Merged

Grow ELF string tables for partition aliases#2126
andrewboudreau merged 1 commit into
mainfrom
tools/objisolate-strtab-growth

Conversation

@andrewboudreau

Copy link
Copy Markdown
Collaborator

Stacked on #2103 and intentionally separate from the FlyGuy source promotion.\n\nExtends objisolate's fail-closed vtable partition-symbol rewrite when a required neutral symbol name is longer than its exact dead donor's string slot. Fitting names keep the existing in-place path; longer names are appended to the linked ELF32 ET_REL .strtab with NUL padding that preserves later-section alignment. Only later sh_offset values and e_shoff move. Section/symbol indices, links, relocations, and non-string payloads remain unchanged and are reparsed/rechecked before success.\n\nAll existing donor provenance, uniqueness, dead/unreferenced state, boundary, shared-slot, and suffix-overlap checks remain mandatory. Nonstandard headers, program headers, extended indices, malformed/overlapping layouts, and bad string-table boundaries refuse.\n\nValidation at c2f3dc5:\n- independent wired objisolate/tu_production/tubuild suites: 119/119 passed\n- focused regression uses both real 19-character names data_ov070_02123184 and data_ov070_021231a0 with distinct safe 18-character donors\n- exact FlyGuy integration: 27/27 TU text contributions; full production 11,088/11,088 functions, 3/3 source-owned data claims, 106/106 modules, zero mismatches, exact stock ROM\n- strict control: 11,038/11,038, exact stock ROM, same nine pre-existing symbol diagnostics\n- port references 405/405; diff clean\n\nOnly tools/objisolate.py and tools/test_objisolate.py change. No source, claims, attempt-ledger, or dashboard artifacts are included.

@andrewboudreau andrewboudreau added the attribution-override Maintainer accepts this PR's contributor-credit changes; validation reports them as warnings label Sep 1, 2026
@tangos-validator

tangos-validator Bot commented Sep 1, 2026

Copy link
Copy Markdown

✅ PR validation — Passed

noverify: no source/build-data changes in this PR

Each changed src/*.c|*.cpp is compiled and its relocated bytes compared to the binary data on a private build box. Passing requires every changed file to reproduce the ROM byte-for-byte with correct relocation targets — this catches WRONG-DEST relocations and non-reproducing near-misses that ledger-scoped linkcheck skips.

Base automatically changed from tools/intact-tu-section-order to main September 1, 2026 03:20
@andrewboudreau
andrewboudreau force-pushed the tools/objisolate-strtab-growth branch from c2f3dc5 to ab3956d Compare September 1, 2026 03:48

@andrewboudreau andrewboudreau left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gated to the standard I published on #2103: because this changes tools/objisolate.py — the code every promoted TU on main links through — static gates are not enough, so I built the ROM twice.

The pair

python tools/rombuild.py -j16 --no-rom, same worktree, same invocation, same shared objcache. Baseline is origin/main @ 9005acd67; the target is the real merge commit 7ad601f9a (= main + this PR), not the branch tip.

main 9005acd67 main + #2126 7ad601f9a
compile 10808 reused, 0 compiled 10808 reused, 0 compiled
intact TU gates dsd modules PASS, zero new symbol errors, storage aliases exact identical
source-built functions 11,088 11,088
reproducing / mismatching 11,088 / 0 11,088 / 0
source-owned data claims 3 (3 reproducing, 0 mismatching) 3 (3 reproducing, 0 mismatching)
module fidelity 106/106 exact, 100.000000% 106/106 exact, 100.000000%
ROM-build analysis PASS PASS
ROM data 529 verified, 191 partial, 6 differ, 522 unnamed (7,549 records) 529 / 191 / 6 / 522 (7,549)

Byte-identical on every reported metric. The compile step being fully cached is expected and is not a gap in coverage — rebias_object_symbols runs at link time via tu_production.py:64 and :357, which is uncached, and the manifest currently carries 30 promoted TUs (plus 3 link-verified), so the modified function was exercised thirty times per build in both runs.

Static: premerge_check.py 2126 on merge tree eea1820807d7, 8/8 pass->pass.

Why the risk is where the build actually looked

The in-place arm is untouched: len(new_name) <= len(donor_bytes) still takes byte-for-byte the pre-existing path, so nothing that works today can change behaviour. The only new arm is the one that used to return

"error": f"{label} {new_name} does not fit donor {donor_name}'s string-table slot"

i.e. a case that previously produced no object at all. A new arm that only fires where the old code refused cannot regress a passing build by construction.

That leaves the three new unconditional guards in rebias_object_symbols as the sole theoretical regression surface, since those run on every TU including the in-place ones:

  • symtab.sh_entsize != 16
  • symtab sh_link not a valid STRTAB
  • any st_shndx == SHN_XINDEX

These are exactly what a full build clears, and it did: 30 promoted TUs through the real path, zero refusals, storage aliases exact.

I also read the growth routine against its own claims. It refuses non-ELF32/non-ET_REL, program headers present, e_ehsize != 52, e_shentsize != 40, SHN_XINDEX shstrndx, a present SHT_SYMTAB_SHNDX, a bad or non-STRTAB link, non-byte or NUL-bearing names, a string table outside the file, missing boundary NULs, a malformed section-header table, insertion inside the section headers, non-power-of-two alignment, payload outside the file, overlap with the string table, a section growth would split, section/header overlap, and ELF32 limits — then re-parses and re-verifies section count, e_shoff, eight header keys, every offset and size, every non-NOBITS payload, and that the new string data is exactly old + addition. Fail-closed throughout, and the alignment is taken as the max sh_addralign over sections at or after the insertion point, so nothing downstream loses its alignment. The post-growth re-fetch of elf/symtab/string_table and the recomputation of every relocation_rewrites fileOffset as sh_offset + entryOffset + 8 matches main's RELA-addend convention at line 1243, and base = symtab.header["sh_offset"] is correctly read after the re-fetch.

Why this is worth landing rather than working around

This is the enabling half of a blocker I raised myself. On #2096 I asked for the coined FlyGuy_VT7/FlyGuy_VT14 aliases to be dropped in favour of the neutral exact config symbols — and the author did, to data_ov070_02123184 / data_ov070_021231a0. Those are 19 characters against 18-character donors, which is precisely the refusal this PR removes. Without it, the correct-naming fix is unimplementable and the pressure goes back onto inventing short names, which is the thing that produces coined names the ROM later contradicts.

One review note, non-blocking

The pre-existing suffix-overlap case (old line 789) changed from policy to long_alias, so the in-place path's string-table boundary refusal is no longer asserted anywhere — the new assertNotIn("string-table growth refused", ...) proves ordering instead, which is a different property. Worth restoring a small in-place boundary case at some point; not a reason to hold this.

The equivalence test itself is the right shape: it drives both paths on the same object and asserts identical relocations, identical symbol records except the two name offsets, identical section headers and payloads modulo the shift, and new_strings[:len(old_strings)] == old_strings.

Verdict

Clear to merge. Static 8/8, full ROM build byte-identical to main on every metric, the risky surface is exercised 30x per build, and it unblocks a naming correction I asked for. Merging now.

(For the record, per repo convention: I cannot press Approve — one shared account — so this comment is the sign-off.)

@andrewboudreau andrewboudreau left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record on the rebase: the head moved from c2f3dc580 to ab3956d6a between my gate run and this comment, so I re-checked rather than assume it was a no-op.

  • git diff c2f3dc580 ab3956d6a -- tools/ is emptyobjisolate.py and test_objisolate.py are byte-identical to what I read and built.
  • The whole old->new tree delta is the Kaitendai TU content, i.e. the base advancing over #2125, not new PR content. origin/main..ab3956d6a is one commit touching exactly the two tools/ files.
  • git merge-tree --write-tree origin/main ab3956d6a -> eea1820807d759e5c80ca6055e1645382f6207d1, which is bit-for-bit 7ad601f9a^{tree} — the tree I actually compiled. Same tree, same result.

So the measurements in my previous comment apply unchanged to the current head. origin/main is still 9005acd67, all seven checks green. Merging.

@andrewboudreau
andrewboudreau merged commit e1b8786 into main Sep 1, 2026
7 checks passed
@andrewboudreau
andrewboudreau deleted the tools/objisolate-strtab-growth branch September 1, 2026 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

attribution-override Maintainer accepts this PR's contributor-credit changes; validation reports them as warnings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant