fix(codegen): per-process temp object paths, and make emission determinism a hard gate (#7131) - #7140
fix(codegen): per-process temp object paths, and make emission determinism a hard gate (#7131)#7140proggeramlug wants to merge 8 commits into
Conversation
) The knob-isolation gate detected a nondeterministic host and SKIPPED its emission half rather than reporting phantom diffs. That workaround existed because objects were nondeterministic on ELF (#7131); #7135 fixed the cause, so the skip now hides a regression instead of a known defect. - nondeterminism is a hard failure, on every host, before any knob is judged - --require-emission removed: it selected between a real check and a skip, and only one of those is still a decision worth having - new census-determinism subcommand + self-test: the standalone instrument the issue asks for, so any agent can establish object-hash validity on the host it is measuring on
…x caveat (#7131) - linker.rs: a measured table of which paths clang/ld actually record in an ELF object (only the .ll BASENAME does), so a future temp-path change can be reviewed without re-deriving it, plus the PERRY_DEBUG_SYMBOLS caveat - census README: replace 'it is not deterministic on aarch64 Linux' with the check that establishes whether it is - CI: run the determinism check on the ubuntu-latest repsel-census job — the ELF host that can actually observe a relapse — and both verdict self-tests - correct the two unreleased changelog fragments that still describe the skip and the removed --require-emission flag
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…path (#7131) #7135 content-addressed both temp names. The .ll had to become a pure function of the IR — clang records a unit's source basename into the ELF object, which was #7131. The .o did not, and lost the pid it used to carry. TEMP_NONCE_COUNTER is per-process state and every process starts it at 0, so two 'perry' processes compiling identical IR both chose perry_llvm_<hash>_0.o. compile_ll_to_object deletes the object once it has read it, so they deleted it out from under each other. Measured on macOS at a3b31c0: 8 of 12 concurrent same-source compiles failed with Failed to read clang output at .../perry_llvm_eee31bbdd9dc24a5_0.o This is #509 again, one scope out — the pid that used to prevent it was removed as collateral. The output path is recorded nowhere (measured: clang records only the .ll BASENAME, as an STT_FILE symbol; not the directory, not the CWD, not -o, and ld -r records neither its inputs nor its output), so uniquifiers are free on the .o and mandatory. Same for the atomic-write staging .tmp, which two processes also reached with the same hash and the same counter. cargo fmt: linker.rs was left unformatted by #7135, so lint is red on main.
…surements disproved - knob-isolation and the determinism check had identical private _digest implementations; share one so 'the objects are the same' cannot drift - the module docstring said output names 'may keep their uniquifying counter'. The counter alone is not enough — it is per-process state every process starts at 0 — which is the collision this check then found - Mach-O does not keep the .ll basename in the debug map, it does not record it at all (verified directly); the byte delta is ~10, measured 12
The 'debug map' explanation came from the issue and is not what I observed: Mach-O does not record the .ll basename in the .o at all (no -g, no debug map). Also the real byte delta and the concurrency note.
proggeramlug
changed the title
1 participant
Closes the loop on #7131. #7135 fixed the cause but its Linux verification box
was never ticked, and the workaround the defect forced on the census gate was
still in place. Verifying it on the Pi found a second defect that #7135 shipped.
1. What was embedded where
Measured, not assumed (aarch64 Debian clang 19.1.7, ELF, no
-g):.o?.llbasenameSTT_FILEin.symtab.lldirectory, process CWD-g)-ooutput pathld -rinput / output pathsSo the
.llname must be a pure function of the IR — that is #7131 — anduniquifiers are free on every output name. The table is now a comment on
TEMP_NONCE_COUNTER, because both halves have now been got wrong once.On Mach-O the
.llbasename is not recorded at all (verified directly, notinferred): that is why macOS looked clean while carrying the same defect, and
why a macOS-only reviewer could not tell a fixed compiler from a broken one.
2. A regression #7135 shipped, found by verifying it
#7135 content-addressed both temp names. The
.odid not need it and lost thepid it used to carry.
TEMP_NONCE_COUNTERis per-process state that everyprocess starts at
0, so twoperryprocesses compiling identical IR both choseperry_llvm_<hash>_0.o— andcompile_ll_to_objectdeletes the object once ithas read it.
Four concurrent compiles of one census fixture:
a3b31c0d8(main)This is #509 again, one scope out. It breaks any parallel build of the same
input — including every A/B harness in
scripts/compiler_output_harness/. Fixedby putting the pid back on the
.oand on the atomic-write staging.tmp,which had the same defect. The
.llstays content-addressed.It was found because the new check runs its repeats concurrently: identical
IR now shares one
.ll, so racing it is part of the subject. A serial checknever opens the window — and indeed the Pi's serial arms were all green.
3. Red then green
Three arms on a Raspberry Pi 5, built sequentially from one target dir (same
toolchain by construction), three distinct binary hashes:
census-determinism, 26 workloads22367565f(before #7135)a3b31c0d8(#7135 as merged)--repeat 3 --jobs 3)suite_01_startuptwice, serially, with the mechanism visible:macOS: 26/26 workloads x 3 compiles -> one hash each. The #7039
closure-iteration-order fix has not regressed.
4. No behavioural change
Stronger than an oracle diff, and available because of the very instrument this
PR is about: across all 26 census workloads the objects emitted by
a3b31c0d8and by this branch are byte-identical, 0/26 differences. This change renames
temp files and nothing else, and the emitted bytes say so.
Why that is the better instrument here, not a substitute for one: an oracle
diff can only observe behaviour the corpus happens to exercise, whereas
identical object bytes leave no room for a behavioural difference at all. The
property is also true by construction — this change alters only the names of
files in the temp directory, and section 1 measures that output names are
recorded nowhere in the object.
And the oracle diff, on the Pi against the pinned Node 26.5.1:
FAIL=0 — the acceptance criterion (the script exits non-zero iff
FAIL != 0).The 119
UNVERare arms that were inert on that row: the corpus did not makethem collect or move anything, which the matrix reports as not-green by design
(#6942/#6946/#6950) rather than counting as a pass. That ratio is a property of
the corpus, not of this change. The Pi needed node 26.5.1 installed
user-locally, because the script refuses an unpinned oracle — correctly.
5. The payoff: the workaround is gone
census-knob-isolationdetected a nondeterministic host and skipped itsemission half — so the half that caught the
PERRY_CANONICAL_STR_LOCALSleakcould not run on Linux at all, the host where object-hash A/B is most useful (it
is the one with an unprivileged instruction-retired counter). Removed:
knob is judged;
--require-emissionis deleted — it selected between a real check and a skip,and only one of those is still a decision worth having;
census-determinism(new) is the standalone instrument: establish thatobject-hash evidence is valid on the host you are measuring on before trusting
a day of A/B. The whole corpus twice is 7 s on an M1 with an optimized
compiler, so CI runs the full 26 rather than a subset — cheap enough that
there is no reason to weaken it;
ubuntu-latestrepsel-censusjob — the ELF host that canactually observe a relapse — alongside both verdict self-tests.
The gate's subject is live on the CI host's architecture, not just on the Pi:
compiling one
.llunder two different basenames produces different objects for--target x86_64-unknown-linux-gnuas well asaarch64, with the basenamevisible in
strings. The embedding is a property of the ELF writer, not of thetarget arch — so
ubuntu-latest(x86_64) can see the defect if it returns.Full
census-knob-isolationre-run on macOS with these changes: OK, emissionhalf exercised on every knob.
6. Sabotage-verified, both directions
Every check here was made to fail on purpose before being believed:
.oname (the #7135 shape)object_temp_name...RED.tmpnameobject_temp_name...RED.llname (re-break #7131).lltests RED7. What I could not verify, and other findings
the floor I was given, and a disk-full sweep corrupts every other agent's run
on that box. The Pi needed node 26.5.1 installed user-locally
(
~/node-26.5.1, PATH-scoped) because its system node is v22 — the matrixrefuses to run against an unpinned oracle, correctly.
mainis red independently of this PR, and has been for five nights.The nightly
Testsworkflow fails oncompiler-output-regression,api-docs-drift,cargo-test,lintand bothWarningsjobs.lint'sreported failure is
unclassified workspace crates: perry-ext-node-forge,unrelated to anything here. Separately and verifiably,
origin/main'scrates/perry-codegen/src/linker.rsdoes not passrustfmt --check— fix(codegen): content-address LLVM temp IR names (#7131) #7135landed it unformatted — and
scripts/check_file_size.shlists nine over-capfiles, none of them touched here. I fixed the formatting of the file I am
editing; the rest is not mine to smuggle into this PR. Judge this PR's CI
against
main's, not against green.the content of emitted objects without a version bump (both
22367565fanda3b31c0d8are0.5.1265), and the build cache keys onperry_version, so acache populated by a pre-fix compiler still validates against a post-fix one at
the same version. The census never sees it (
--no-cacheplusPERRY_NO_AUTO_OPTIMIZE=1), and the maintainer's bump at merge invalidates it.No cache-key change here: nothing in this PR alters cache identity in either
direction.
.llfiles are never deleted. fix(codegen): content-address LLVM temp IR names (#7131) #7135 stoppedunlinking them (a sibling worker may share the path), so the temp dir now
accumulates one file per distinct IR ever compiled — measured 1627 files /
951.8 MB on one dev box after a day. Not a correctness bug, and it deserves its
own change rather than riding on a determinism fix.
byte-identical-objects result in section 4 is the direct evidence.
one
CARGO_TARGET_DIR, not one per arm — the pattern the repo's ownbuild_arms.shuses, which makes the rustc/LLVM toolchain identical acrossarms by construction. The failure mode that costs (cargo not rebuilding, so
two "arms" are the same binary) is caught directly: all three
perryhashesdiffer, and each arm's artifacts were copied out with mtimes after its own
build started. Package set was
-p perry -p perry-runtime-static -p perry-stdlib-staticon every hop.--profile perry-dev, not--release. The property undertest is the compiler's output, which the compiler's own optimization level
does not reach;
--releaseon a contended 4-core Pi shared with another agentwas not a good trade.