diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d91f3d9239..94ed5eb98d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1218,6 +1218,7 @@ jobs: python3 scripts/compiler_output_regression.py census-self-test python3 scripts/compiler_output_regression.py census-knob-isolation-self-test python3 scripts/compiler_output_regression.py census-determinism-self-test + python3 scripts/compiler_output_regression.py census-temp-hygiene-self-test python3 -m unittest tests.test_repsel_census - name: Build compiler @@ -1235,6 +1236,21 @@ jobs: --repeat 2 \ --jobs 4 + # #7144. The other consequence of content-addressing the `.ll`: workers + # holding identical IR share the name, so #7135 stopped deleting it and + # nothing else did — one leftover per distinct IR ever compiled. CI never + # saw it (runner temp dirs are reclaimed) while developer machines + # reached 29 GB. This step compiles with `TMPDIR` pointed at an empty + # directory and asserts it is still empty afterwards; note that a + # repeat-and-compare check would NOT have caught it, because identical + # IR reuses the identical name. + - name: Temp-directory hygiene + run: | + python3 scripts/compiler_output_regression.py census-temp-hygiene \ + --perry target/debug/perry \ + --repeat 2 \ + --jobs 4 + - name: Promotion census run: | python3 scripts/compiler_output_regression.py census \ diff --git a/benchmarks/repsel_census/README.md b/benchmarks/repsel_census/README.md index 0a54f586b0..aa3a3b6f16 100644 --- a/benchmarks/repsel_census/README.md +++ b/benchmarks/repsel_census/README.md @@ -286,6 +286,66 @@ canonical-i32 draws from. A withdrawn proof cannot be selected. A knob that `--jobs` compiles arms in parallel; the whole corpus × 6 knobs × 4 arms is about 20 s on an M1. +## Temp-directory hygiene (#7144) + +The other consequence of content-addressing that `.ll`: two workers holding +identical IR now *share* the path, so a per-call unlink can race a sibling that +computed the path but has not yet handed it to clang. #7135 responded by not +deleting it — and then nothing did. The leftovers are bounded by **distinct IR +ever compiled** on the machine, which is fine in CI (runner temp dirs are +reclaimed) and not fine anywhere a compiler is being worked on, where the IR +changes on every rebuild: 1627 files / 951.8 MB after one day on one dev box, +~29 GB on another. + +#7144 removed the sharing rather than the deletion. Each `.ll` → `.o` compile +gets a private directory under the temp root and removes it on success; the +*basename* inside it is still a pure function of the IR, so the determinism +property above is untouched — the directory is not recorded in the object, only +the basename is. + +```bash +python3 scripts/compiler_output_regression.py census-temp-hygiene \ + --perry --repeat 2 --jobs 4 +``` + +It compiles the corpus with `TMPDIR` pointed at an empty directory of its own +and asserts the directory is still empty. Two things worth knowing before +changing it: + +* **"No growth run-over-run" would have been green on the broken compiler.** + Compiling the same corpus twice produces the same content-addressed names, so + a repeat-and-compare check sees a flat count while the machine fills up. The + property that goes red is the absolute one: *nothing* left behind. +* **The `TMPDIR` isolation is load-bearing**, not politeness. Counting entries + in the shared system temp dir measures every other process on the box. + +It fails on the clang driver's own temp names (`perry_llvm_*`, `perry_cgu_*`, +`perry_bc_*`) and merely *reports* anything else — today that is the compile +driver's `perry-objs--/` staging directory, which `--no-link` never +cleans up (#7167). Widen `OWNED_PREFIXES` to "everything" once that is closed. + +No exemption for `PERRY_DEBUG_SYMBOLS`, and that is a change of belief rather +than a change of policy. `-g` was documented as pulling the `.ll`'s **absolute** +path plus `DW_AT_comp_dir` into DWARF, which would have made the file part of +the shipped object and forced it to persist. Measured on a real Perry module +(Apple clang 21, `-target x86_64-unknown-linux-gnu` and +`aarch64-unknown-linux-gnu`): the `-g` object is **byte-identical** to the one +without it and has **no `.debug_*` sections at all**. Perry's codegen emits no +`DICompileUnit`/`DIFile`/`!dbg` metadata, and `clang -g` on a `.ll` lowers debug +info that is already in the IR rather than synthesising a compile unit for the +input file. So there is one layout, not two, and +`debug_symbols_do_not_change_what_the_object_records` in +`linker_temp_lifecycle_tests.rs` goes red the day that stops being true. (Not +measured on COFF/Windows.) + +That test has a sibling worth knowing about: +`the_ll_directory_is_not_recorded_in_the_object_but_the_basename_is` compiles +one `.ll` under the same basename from two different directories, for both +Linux ELF targets, and asserts the objects are identical — with a control that +a *different* basename does change them. That is the property this whole +directory rests on, and until #7144 it lived only in a comment and a hand +measurement taken once on a Raspberry Pi. + ## Editing the fixtures Don't tidy them. Every one is written against a specific collector's rules and diff --git a/changelog.d/7168-ll-temp-file-lifecycle.md b/changelog.d/7168-ll-temp-file-lifecycle.md new file mode 100644 index 0000000000..dba59d2633 --- /dev/null +++ b/changelog.d/7168-ll-temp-file-lifecycle.md @@ -0,0 +1,78 @@ +**Fixed:** the temp directory no longer accumulates one LLVM IR file per distinct +module ever compiled (#7144). + +`compile_ll_to_object` stopped unlinking its temp `.ll` in #7135, deliberately: +that change had just made the name a pure function of the IR (#7131 — clang +records a translation unit's basename into the ELF object), so two workers +holding identical IR **shared** the path and a per-call unlink could race a +sibling that had computed the path but not yet handed it to clang. Nothing else +deleted them, and because the name is content-addressed the leftovers are bounded +by *distinct IR ever compiled on the machine* — which grows without limit in +practice, since working on the compiler changes the IR on nearly every rebuild. +1627 files / 951.8 MB after a day on one box; 1069 files / 635 MB still sitting in +the temp dir of the machine this was fixed on. + +The fix removes the sharing rather than making the deletion more careful. Every +`.ll` → `.o` compile now gets a directory it owns — +`perry_llvm_scratch__/` — and the directory is removed once the +object bytes have been read: + +* no two calls, in one process or across processes, are handed the same `.ll` + path, so the unlink has nothing to race and there is no narrow window left to + lose. That matters because a narrow window is not testable: sabotaged to the + naive shape (one flat shared `.ll`, unlinked after use) the 8-way concurrent + test added here went red in one full-suite run and green in the next three; +* the *basename* inside the directory is still `perry_llvm_.ll`, and + the object records the basename and nothing else — so emission determinism + (#7131) is untouched, and the `.o` keeps the pid + counter that #7140 restored; +* failures keep their IR, and the error message names it. `PERRY_LLVM_KEEP_IR` + keeps everything, now collected in one directory (`.ll`, `.o`, `.clang-stderr`, + compile plan) instead of scattered across the temp root. + +**`PERRY_DEBUG_SYMBOLS` is not an exemption, and the reason it was believed to be +one was wrong.** `-g` was documented as pulling the `.ll`'s absolute path plus +`DW_AT_comp_dir` into DWARF, which would have made the file part of the shipped +object. Measured on a real Perry module (Apple clang 21, `-target +x86_64-unknown-linux-gnu` and `aarch64-unknown-linux-gnu`): the `-g` object is +**byte-identical** to the one without it and has **no `.debug_*` sections at +all**. Perry's codegen emits no `DICompileUnit`/`DIFile`/`!dbg` metadata, and +`clang -g` on a `.ll` lowers debug info already in the IR rather than synthesising +a compile unit for the input file. The second temp-file layout that premise +justified was implemented and then deleted — a mode justified by something that +does not happen is an unexercised configuration, not a feature. Not measured on +COFF/Windows. + +**New checks**, all sabotage-verified in both directions: + +* `census-temp-hygiene` (`scripts/compiler_output_regression.py`) compiles the + census corpus with `TMPDIR` pointed at an empty directory and asserts the + directory is still empty. Red on the old compiler (27 leftovers from 54 + compiles), green on the new one; wired into the `repsel-census` CI job with a + verdict self-test. Note the shape of that number — repeats share a content hash + and so share a filename, which means a *"no growth run-over-run"* check would + have been **green on the broken compiler**. That is why CI never saw this while + developer machines filled up, and why the gate asserts the absolute property. +* `the_ll_directory_is_not_recorded_in_the_object_but_the_basename_is` compiles + one `.ll` under the same basename from two different directories, for both Linux + ELF targets, and asserts the objects are identical — with a live control that a + *different* basename does change them. This is the property the whole design + rests on; until now it lived in a comment and one hand measurement taken on a + Raspberry Pi. It cross-compiles, because the embedding is a property of the ELF + writer rather than of the host, so it also runs on macOS where this defect class + is otherwise invisible. +* `debug_symbols_do_not_change_what_the_object_records` pins the measurement + above, including a direct assertion that the `-g` object carries no `.debug_` + section name. + +Verification: 27/27 census workloads emit **byte-identical objects** before and +after (this renames files and nothing else); `census-determinism --repeat 3 +--jobs 4` and `--repeat 4 --jobs 8` all green; 24 concurrent `perry` processes +compiling one identical source, three times, 0 failures and one object hash. + +Filed separately as **#7167**: the compile driver leaks its +`perry-objs--/` object staging directory on the `--no-link` path +(`run_pipeline.rs` removes it on both *link* exits and there is no third one). +That one is unbounded in the number of compiles rather than in distinct IR. The +new gate reports it without failing on it — a gate that goes red for another +module's defect gets muted rather than fixed — and widening it once #7167 lands is +a one-line change. diff --git a/crates/perry-codegen/src/linker.rs b/crates/perry-codegen/src/linker.rs index b460827dc8..ef12529cc2 100644 --- a/crates/perry-codegen/src/linker.rs +++ b/crates/perry-codegen/src/linker.rs @@ -44,11 +44,49 @@ static CLANG_PROBE: OnceLock> = OnceLock::new(); /// /// That is the whole reason only the `.ll` basename had to change: the counter /// may stay in every *output* name, where it costs nothing and still closes -/// #509. The one caveat is `PERRY_DEBUG_SYMBOLS`, which adds `-g` and pulls the -/// absolute `.ll` path plus `DW_AT_comp_dir` into DWARF — objects built with it -/// are reproducible only for a fixed `TMPDIR` and working directory. +/// #509. And because the *directory* is recorded nowhere, #7144 could put every +/// compile's `.ll` in a directory of its own and delete it again. +/// +/// **`PERRY_DEBUG_SYMBOLS` is not an exception**, contrary to what the comment +/// here used to say. `-g` was assumed to pull the `.ll`'s absolute path plus +/// `DW_AT_comp_dir` into DWARF, which would have made the file part of the +/// shipped object and forced it to persist at a stable path. Measured on a real +/// Perry module (Apple clang 21, `-target x86_64-unknown-linux-gnu` and +/// `aarch64-unknown-linux-gnu`): the `-g` object is **byte-identical** to the +/// one without it and carries **no `.debug_*` sections at all**. Perry's codegen +/// emits no `DICompileUnit`/`DIFile`/`!dbg` metadata, and `clang -g` on a `.ll` +/// lowers debug info that is in the IR rather than synthesising a compile unit +/// for the input file. So `-g` records nothing about where the `.ll` lived, and +/// the temp-file lifetime does not depend on it — see +/// `debug_symbols_do_not_change_what_the_object_records`. (Not measured on +/// COFF/Windows.) static TEMP_NONCE_COUNTER: AtomicU64 = AtomicU64::new(0); +/// The environment inputs that decide what happens to the temp files. +/// +/// Read once, in `compile_ll_to_object`, and threaded down rather than probed +/// where they are used: the lifecycle is then testable without a test mutating +/// process-wide environment underneath every other test in the binary. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +struct TempFilePolicy { + /// `PERRY_LLVM_KEEP_IR` — retain every intermediate and write the compile + /// plan alongside it. The only input that changes the files' lifetime. + keep: bool, + /// `PERRY_DEBUG_SYMBOLS` — clang gets `-g`. Carried here only so the flag + /// is a parameter rather than an env probe buried in plan construction; it + /// deliberately does **not** affect cleanup (see `TEMP_NONCE_COUNTER`). + debug_symbols: bool, +} + +impl TempFilePolicy { + fn from_env() -> Self { + Self { + keep: env::var_os("PERRY_LLVM_KEEP_IR").is_some(), + debug_symbols: env::var_os("PERRY_DEBUG_SYMBOLS").is_some(), + } + } +} + /// FNV-1a 64-bit over `ll_text`. Stable across platforms and rustc versions /// (unlike `DefaultHasher`), used only to content-address temp IR filenames so /// clang embeds a deterministic source path (#7131). Collision risk is @@ -81,28 +119,50 @@ fn ll_content_hash(ll_text: &str) -> u64 { /// same-source compiles failed with "Failed to read clang output … No such /// file or directory". This is #509 again, one scope out. /// +/// Both names additionally sit inside a `scratch_dir` that belongs to this call +/// alone (#7144). The directory name is *not* content-addressed — it must not +/// be, or two callers would share it again and the unlink would be racing a +/// sibling, which is the corner #7135 painted itself into. And it does not have +/// to be: only the basename reaches the object. +/// /// `pid` and `counter` are parameters rather than read in here so the property /// above is testable without spawning processes. -fn llvm_temp_paths_for( - tmp_dir: &Path, - ll_text: &str, - pid: u32, - counter: u64, -) -> (PathBuf, PathBuf) { +fn llvm_temp_paths_for(tmp_dir: &Path, ll_text: &str, pid: u32, counter: u64) -> LlvmTempPaths { let hash = ll_content_hash(ll_text); - let ll_path = tmp_dir.join(format!("perry_llvm_{hash:016x}.ll")); - let obj_path = tmp_dir.join(format!("perry_llvm_{hash:016x}_{pid:x}_{counter:x}.o")); - (ll_path, obj_path) + let ll_name = format!("perry_llvm_{hash:016x}.ll"); + // The `.o` keeps its uniquifiers even inside a private directory. They cost + // nothing, and the day someone flattens the layout again the object must + // not silently go back to colliding across processes (#7140). + let obj_name = format!("perry_llvm_{hash:016x}_{pid:x}_{counter:x}.o"); + let scratch = tmp_dir.join(format!("perry_llvm_scratch_{pid:x}_{counter:x}")); + LlvmTempPaths { + ll_path: scratch.join(&ll_name), + obj_path: scratch.join(&obj_name), + scratch_dir: scratch, + } +} + +/// Temp paths for one `compile_ll_to_object` call. +#[derive(Debug, Clone)] +struct LlvmTempPaths { + /// Directory owned exclusively by this call, removed once the object bytes + /// have been read. + scratch_dir: PathBuf, + ll_path: PathBuf, + obj_path: PathBuf, } /// `llvm_temp_paths_for` with this process's pid and the next counter value. -/// Returns `(ll_path, obj_path, pid, counter)` — the last two also name the +/// Returns the paths plus `(pid, counter)` — the last two also name the /// atomic-write staging file. -fn llvm_temp_paths(tmp_dir: &Path, ll_text: &str) -> (PathBuf, PathBuf, u32, u64) { +fn llvm_temp_paths(tmp_dir: &Path, ll_text: &str) -> (LlvmTempPaths, u32, u64) { let pid = std::process::id(); let counter = TEMP_NONCE_COUNTER.fetch_add(1, Ordering::Relaxed); - let (ll_path, obj_path) = llvm_temp_paths_for(tmp_dir, ll_text, pid, counter); - (ll_path, obj_path, pid, counter) + ( + llvm_temp_paths_for(tmp_dir, ll_text, pid, counter), + pid, + counter, + ) } /// Staging name for the atomic `.ll` write. Must be unique per process for the @@ -300,6 +360,7 @@ fn build_clang_compile_plan( target_triple: Option<&str>, ll_byte_size: usize, ll_fn_count: usize, + debug_symbols: bool, ) -> ClangCompilePlan { let effective_target = target_triple .map(|s| s.to_string()) @@ -335,7 +396,11 @@ fn build_clang_compile_plan( }; let mut clang_args = vec!["-c".to_string(), opt_flag.to_string()]; - if std::env::var("PERRY_DEBUG_SYMBOLS").is_ok() { + // A parameter rather than an env probe so a test can pin what `-g` does + // and does not reach — measured in #7144: on a Perry `.ll` it produces a + // byte-identical object with no `.debug_*` sections, because Perry's + // codegen emits no DI metadata for clang to lower. See `TEMP_NONCE_COUNTER`. + if debug_symbols { clang_args.push("-g".to_string()); } clang_args.push("-fno-math-errno".to_string()); @@ -389,6 +454,40 @@ fn build_clang_compile_plan( /// resulting `.o`, and clean up both on success. On failure the temp files /// are left behind for debugging — the caller can `grep /tmp/perry_llvm_*`. pub fn compile_ll_to_object(ll_text: &str, target_triple: Option<&str>) -> Result> { + compile_ll_to_object_in( + &env::temp_dir(), + ll_text, + target_triple, + TempFilePolicy::from_env(), + ) +} + +/// `compile_ll_to_object` with the temp root and the file-lifetime policy as +/// arguments instead of process state. +/// +/// Both are parameters so the temp-file *lifecycle* — the subject of #7144 — is +/// testable: a test can hand this an empty directory of its own and assert what +/// is left in it, without racing every other test in the binary over `TMPDIR` +/// or `PERRY_LLVM_KEEP_IR`. +/// +/// Cleanup policy, in one place: +/// +/// * **success**: the object is read into memory, and the per-call directory +/// goes with it — a compile leaves nothing behind (#7144). +/// * **failure** (clang non-zero, or the object cannot be read): everything is +/// left on disk. The error message names the `.ll`, and a failed compile is +/// exactly when someone wants to look at the IR that produced it. +/// * **`PERRY_LLVM_KEEP_IR`**: everything is kept and its location printed, +/// plus the compile plan as JSON. +/// * **`PERRY_DEBUG_SYMBOLS`**: no effect on any of the above. It was believed +/// to put the `.ll`'s absolute path into DWARF; measured, it does not put +/// anything there at all. See `TEMP_NONCE_COUNTER`. +fn compile_ll_to_object_in( + tmp_dir: &Path, + ll_text: &str, + target_triple: Option<&str>, + policy: TempFilePolicy, +) -> Result> { // Validate the toolchain before creating the potentially large `.ll` // scratch file. Unsupported clang releases should fail without leaving // an artifact that was never passed to the compiler. @@ -414,10 +513,18 @@ pub fn compile_ll_to_object(ll_text: &str, target_triple: Option<&str>) -> Resul })?; ensure_supported_clang(&clang)?; - let tmp_dir = env::temp_dir(); // #7131: content-address the `.ll` basename (clang embeds it into the // object on ELF). #509: keep the `.o` unique via the per-call counter. - let (ll_path, obj_path, write_pid, write_nonce) = llvm_temp_paths(&tmp_dir, ll_text); + // #7144: put both under a directory this call owns, so the `.ll` can be + // deleted again without racing a sibling that hashed the same IR. + let (paths, write_pid, write_nonce) = llvm_temp_paths(tmp_dir, ll_text); + let LlvmTempPaths { + scratch_dir, + ll_path, + obj_path, + } = paths; + fs::create_dir_all(&scratch_dir) + .with_context(|| format!("Failed to create temp dir at {}", scratch_dir.display()))?; write_ll_atomically(&ll_path, ll_text, write_pid, write_nonce)?; let plan = build_clang_compile_plan( @@ -427,6 +534,7 @@ pub fn compile_ll_to_object(ll_text: &str, target_triple: Option<&str>) -> Resul target_triple, ll_text.len(), count_ll_functions(ll_text), + policy.debug_symbols, ); // Pre-flight probe: capture clang's default Target: line once per process, @@ -499,15 +607,21 @@ pub fn compile_ll_to_object(ll_text: &str, target_triple: Option<&str>) -> Resul let bytes = fs::read(&obj_path) .with_context(|| format!("Failed to read clang output at {}", obj_path.display()))?; - // Clean up on success. The content-addressed `.ll` is **shared** across - // concurrent `compile_ll_to_object` calls with identical IR (#7131), so we - // must NOT delete it here — a sibling worker may still have clang open on - // that path (CodeRabbit). Only the unique `.o` is removed. The `.ll` is - // left in the process temp dir (same content is reused on cache misses; - // OS temp cleanup / `PERRY_LLVM_KEEP_IR` handle the rest). When KEEP_IR is - // set we also retain the object and write compile metadata. - let keep = env::var_os("PERRY_LLVM_KEEP_IR").is_some(); - if keep { + // Clean up on success. + // + // #7135 could not delete the `.ll`: it had just made the name a pure + // function of the IR, so two workers holding identical IR *shared* that + // path and either could unlink it in the window between the other computing + // the path and clang opening it. The consequence (#7144) was that nothing + // ever deleted them — one file per distinct IR ever compiled, measured at + // 29 GB on a dev box. + // + // The fix is not a more careful delete, it is removing the sharing: the + // `.ll` now sits in a directory that belongs to this call, so unlinking it + // is unobservable to anyone else and there is no window to lose. The name + // clang records — the basename — is untouched, so emission stays + // deterministic (#7131). + if policy.keep { let _ = fs::write(&plan.stderr_remarks_path, &output.stderr); let metadata_path = PathBuf::from(format!("{}.compile-plan.json", plan.obj_path.display())); write_compile_plan_metadata(&plan, &metadata_path)?; @@ -518,7 +632,13 @@ pub fn compile_ll_to_object(ll_text: &str, target_triple: Option<&str>) -> Resul metadata_path.display() ); } else { - let _ = fs::remove_file(&plan.obj_path); + // Ours alone: `remove_dir_all` rather than unlinking the two names we + // know about, so anything clang chose to drop beside them goes too and + // the directory cannot survive as an empty husk. + // + // Unconditional, including under `PERRY_DEBUG_SYMBOLS`: `-g` records + // nothing about this path, measured — see `TEMP_NONCE_COUNTER`. + let _ = fs::remove_dir_all(&scratch_dir); } Ok(bytes) @@ -1367,6 +1487,7 @@ mod tests { None, 0, 0, + false, ); assert!(plan.clang_args.contains(&"-fno-math-errno".to_string())); // Small module → optimized at -O3 (#4880). @@ -1394,6 +1515,7 @@ mod tests { None, huge, many_funcs, + false, ); assert!(plan.clang_args.contains(&"-Os".to_string())); assert!(!plan.clang_args.contains(&"-O3".to_string())); @@ -1413,6 +1535,7 @@ mod tests { None, huge, 2, // ~3 MB/fn — far above the density cap + false, ); assert!(plan.clang_args.contains(&"-O0".to_string())); assert!(!plan.clang_args.contains(&"-O3".to_string())); @@ -1428,6 +1551,7 @@ mod tests { Some("x86_64-unknown-linux-gnu"), 0, 0, + false, ); assert_eq!(plan.effective_target, "x86_64-unknown-linux-gnu"); assert_eq!(plan.native_tuning_arg, None); @@ -1520,6 +1644,7 @@ mod tests { Some("x86_64-unknown-linux-gnu"), 0, 0, + false, ); write_compile_plan_metadata(&plan, &temp).unwrap(); let text = fs::read_to_string(&temp).unwrap(); @@ -1570,8 +1695,10 @@ mod tests { // basename still differs via the counter. let tmp = env::temp_dir(); let ir = "define void @f() {\n ret void\n}\n"; - let (ll_a, obj_a, _, _) = llvm_temp_paths(&tmp, ir); - let (ll_b, obj_b, _, _) = llvm_temp_paths(&tmp, ir); + let (a, _, _) = llvm_temp_paths(&tmp, ir); + let (b, _, _) = llvm_temp_paths(&tmp, ir); + let (ll_a, obj_a) = (&a.ll_path, &a.obj_path); + let (ll_b, obj_b) = (&b.ll_path, &b.obj_path); assert_eq!( ll_a.file_name(), ll_b.file_name(), @@ -1583,8 +1710,8 @@ mod tests { ".o basenames must stay unique across calls (#509)" ); // Different IR → different .ll basename. - let (ll_c, _, _, _) = llvm_temp_paths(&tmp, "define void @g() {\n ret void\n}\n"); - assert_ne!(ll_a.file_name(), ll_c.file_name()); + let (c, _, _) = llvm_temp_paths(&tmp, "define void @g() {\n ret void\n}\n"); + assert_ne!(ll_a.file_name(), c.ll_path.file_name()); // No pid / wall-clock digits of variable width — only hex hash. let name = ll_a.file_name().unwrap().to_string_lossy(); assert!( @@ -1616,8 +1743,10 @@ mod tests { let ir = "define void @f() {\n ret void\n}\n"; // Same IR, same counter, DIFFERENT process. - let (ll_p1, obj_p1) = llvm_temp_paths_for(&tmp, ir, 1111, 0); - let (ll_p2, obj_p2) = llvm_temp_paths_for(&tmp, ir, 2222, 0); + let p1 = llvm_temp_paths_for(&tmp, ir, 1111, 0); + let p2 = llvm_temp_paths_for(&tmp, ir, 2222, 0); + let (ll_p1, obj_p1) = (&p1.ll_path, &p1.obj_path); + let (ll_p2, obj_p2) = (&p2.ll_path, &p2.obj_path); assert_eq!( ll_p1.file_name(), ll_p2.file_name(), @@ -1633,27 +1762,27 @@ mod tests { // Same process, different call: the counter still has to separate // in-process rayon workers. - let (_, obj_c0) = llvm_temp_paths_for(&tmp, ir, 1111, 0); - let (_, obj_c1) = llvm_temp_paths_for(&tmp, ir, 1111, 1); - assert_ne!(obj_c0.file_name(), obj_c1.file_name()); + let c0 = llvm_temp_paths_for(&tmp, ir, 1111, 0); + let c1 = llvm_temp_paths_for(&tmp, ir, 1111, 1); + assert_ne!(c0.obj_path.file_name(), c1.obj_path.file_name()); // The atomic-write staging name needs the same separation: both // processes reach it with the same hash and the same counter, and // `File::create` truncates. assert_ne!( - ll_staging_path(&ll_p1, 1111, 0).file_name(), - ll_staging_path(&ll_p1, 2222, 0).file_name(), + ll_staging_path(ll_p1, 1111, 0).file_name(), + ll_staging_path(ll_p1, 2222, 0).file_name(), "staging .tmp name must be per-process" ); assert_ne!( - ll_staging_path(&ll_p1, 1111, 0).file_name(), - ll_staging_path(&ll_p1, 1111, 1).file_name(), + ll_staging_path(ll_p1, 1111, 0).file_name(), + ll_staging_path(ll_p1, 1111, 1).file_name(), "staging .tmp name must be per-call" ); // …and the staging file must never be mistaken for the real `.ll`. assert_ne!( - ll_staging_path(&ll_p1, 1111, 0).file_name(), + ll_staging_path(ll_p1, 1111, 0).file_name(), ll_p1.file_name() ); } @@ -1665,3 +1794,10 @@ mod tests { assert_eq!(ll_content_hash("a"), 0xaf63_dc4c_8601_ec8c); } } + +/// Temp-file *lifecycle* — who owns the `.ll`, and when it is removed (#7144). +/// A sibling file only because of the 2,000-line cap; `use super::*` gives it +/// the same view of this module as the block above. +#[cfg(test)] +#[path = "linker_temp_lifecycle_tests.rs"] +mod linker_temp_lifecycle_tests; diff --git a/crates/perry-codegen/src/linker_temp_lifecycle_tests.rs b/crates/perry-codegen/src/linker_temp_lifecycle_tests.rs new file mode 100644 index 0000000000..4b372241b8 --- /dev/null +++ b/crates/perry-codegen/src/linker_temp_lifecycle_tests.rs @@ -0,0 +1,444 @@ +//! Temp-file lifecycle for the clang driver (#7144), and the path-shape +//! properties it has to hold constant (#7131, #7140, #509). +//! +//! Split out of `linker.rs` for the 2,000-line file cap, not because it is a +//! different subject: everything here is about the two temp files one +//! `.ll` → `.o` compile creates, who may see them, and when they are removed. +//! +//! The end-to-end tests drive the real `clang` into a temp root of their own +//! and assert on what is left in that root. That is the only way a leak is +//! observable — every path-shape test in this file passes just as happily +//! against a compiler that never deletes anything, which is exactly how #7144 +//! shipped inside a green #7135. + +use super::*; + +// ── Path shape ───────────────────────────────────────────────────────────── + +#[test] +fn scratch_dir_is_per_call_and_per_process_but_the_ll_basename_is_not() { + // #7144's shape, and the reason it does not undo #7131: the *directory* + // carries every uniquifier, the *basename* carries none. The object records + // the basename and nothing else, so the two properties do not compete — a + // call can own its `.ll` outright and still emit the same object bytes as + // any other call with the same IR. + // + // This is the structural guarantee the whole fix rests on: no two calls are + // ever handed the same `.ll` path, so the unlink has nothing to race. + let tmp = Path::new("/tmp"); + let ir = "define void @f() {\n ret void\n}\n"; + + let a = llvm_temp_paths_for(tmp, ir, 1111, 0); + let b = llvm_temp_paths_for(tmp, ir, 1111, 1); // same process, next call + let c = llvm_temp_paths_for(tmp, ir, 2222, 0); // other process, same counter + + assert_ne!( + a.scratch_dir, b.scratch_dir, + "two calls must not share a directory" + ); + assert_ne!( + a.scratch_dir, c.scratch_dir, + "two processes must not share a directory — every process starts the \ + counter at 0, so only the pid can separate them (#7140)" + ); + + for p in [&a, &b, &c] { + assert_eq!( + p.ll_path.parent(), + Some(p.scratch_dir.as_path()), + "the .ll must live inside the directory that gets removed" + ); + assert_eq!( + p.obj_path.parent(), + Some(p.scratch_dir.as_path()), + "the .o must go with it, so one remove_dir_all cleans up" + ); + assert_eq!( + p.ll_path.file_name(), + a.ll_path.file_name(), + "the recorded name — the basename — must stay content-only (#7131)" + ); + } + // …and the objects must still not collide, directory or no directory. + assert_ne!(a.obj_path.file_name(), c.obj_path.file_name()); +} + +// ── Temp-file lifecycle (#7144) ──────────────────────────────────────────── +// +// These drive the real `clang`, into a temp root of their own, and assert on +// what is left in that root. A leak is only observable end-to-end: every +// path-shape test above passes just as happily against a compiler that never +// deletes anything, which is exactly how #7144 shipped. + +/// A fresh, empty directory to use as the temp root, or `None` when this +/// host has no usable clang and the compile step cannot run at all. +fn temp_root_if_clang_available(tag: &str) -> Option { + let Some(clang) = find_clang() else { + eprintln!("[linker tests] skipping {tag}: no clang on this host"); + return None; + }; + if let Err(err) = ensure_supported_clang(&clang) { + eprintln!("[linker tests] skipping {tag}: unusable clang ({err:#})"); + return None; + } + let root = env::temp_dir().join(format!( + "perry_linker_test_{tag}_{}_{:x}", + std::process::id(), + TEMP_NONCE_COUNTER.fetch_add(1, Ordering::Relaxed) + )); + let _ = fs::remove_dir_all(&root); + fs::create_dir_all(&root).expect("failed to create test temp root"); + Some(root) +} + +fn entries(root: &Path) -> Vec { + let mut names: Vec = fs::read_dir(root) + .expect("temp root vanished") + .filter_map(|e| e.ok()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .collect(); + names.sort(); + names +} + +fn test_ir(nth: u32) -> String { + format!("\ndefine i32 @perry_temp_lifecycle_{nth}() {{\nentry:\n ret i32 {nth}\n}}\n") +} + +const CLEAN: TempFilePolicy = TempFilePolicy { + keep: false, + debug_symbols: false, +}; + +#[test] +fn successful_compile_leaves_nothing_behind() { + // THE #7144 regression test. Before the fix each compile left one + // `perry_llvm_.ll` in the temp dir forever — bounded by distinct + // IR ever compiled, which in practice means every rebuild of the + // compiler. 1627 files / 951.8 MB on one dev box after a day. + let Some(root) = temp_root_if_clang_available("clean") else { + return; + }; + + for nth in 0..3 { + let bytes = compile_ll_to_object_in(&root, &test_ir(nth), None, CLEAN) + .unwrap_or_else(|e| panic!("compile {nth} failed: {e:#}")); + assert!(!bytes.is_empty(), "compile {nth} produced no object bytes"); + assert_eq!( + entries(&root), + Vec::::new(), + "compile {nth} left temp files behind (#7144)" + ); + } + let _ = fs::remove_dir_all(&root); +} + +#[test] +fn concurrent_compiles_of_identical_ir_both_succeed_and_leave_nothing() { + // The race that made #7135 stop deleting: two workers holding the SAME + // IR agree on the content-addressed name, so one could unlink it in the + // window between the other computing the path and clang opening it. + // + // What this test does and does not prove, stated because a race test that + // is trusted for more than it shows is worse than none. It cannot *decide* + // the race: sabotaged to the naive shape (one shared flat `.ll`, unlinked + // after use) it went red in one full-suite run and green in the next three + // — which is the definition of the window being narrow, not absent. The + // guarantee comes from `scratch_dir_is_per_call_and_per_process_…`: no two + // calls are ever handed the same `.ll` path, so there is nothing to race + // over and no window to lose. This test is the end-to-end complement — + // under real concurrency, 8 identical-IR compiles must all succeed, emit + // the same bytes, and leave nothing — and it will occasionally catch a + // regression the structural test somehow passed. + use std::thread; + + let Some(root) = temp_root_if_clang_available("race") else { + return; + }; + let ir = test_ir(42); + + let results: Vec>> = thread::scope(|s| { + let handles: Vec<_> = (0..8) + .map(|_| { + let root = root.clone(); + let ir = ir.clone(); + s.spawn(move || compile_ll_to_object_in(&root, &ir, None, CLEAN)) + }) + .collect(); + handles.into_iter().map(|h| h.join().unwrap()).collect() + }); + + let mut first: Option> = None; + for (i, r) in results.into_iter().enumerate() { + let bytes = r.unwrap_or_else(|e| panic!("concurrent compile {i} failed: {e:#}")); + match &first { + // Identical IR must still give identical objects — the sharing + // this fix removed was never what made emission deterministic + // (#7131); the content-addressed basename is (#7140). + Some(expected) => assert_eq!(&bytes, expected, "compile {i} emitted other bytes"), + None => first = Some(bytes), + } + } + assert_eq!( + entries(&root), + Vec::::new(), + "8 concurrent identical-IR compiles left temp files behind" + ); + let _ = fs::remove_dir_all(&root); +} + +#[test] +fn failed_compile_keeps_the_ll_for_diagnosis() { + // Stated policy: failures retain their IR. The error message names the + // file, and a compile that just failed is precisely when someone wants + // to read the IR that produced it. + let Some(root) = temp_root_if_clang_available("failure") else { + return; + }; + + let err = compile_ll_to_object_in(&root, "this is not LLVM IR\n", None, CLEAN) + .expect_err("clang must reject non-IR input"); + let message = format!("{err:#}"); + assert!( + message.contains("LLVM IR left at:"), + "the failure must say where the IR is; got: {message}" + ); + + // Asserted on the whole tree rather than on the scratch directory: the + // claim is "the IR survives a failed compile", and it has to keep meaning + // that if the layout is ever rearranged again. + let surviving = ll_files_under(&root); + assert_eq!( + surviving.len(), + 1, + "exactly the failed compile's .ll must survive, found: {surviving:?}" + ); + assert!( + message.contains(&surviving[0].display().to_string()), + "the failure must name the file it left: {message}" + ); + let _ = fs::remove_dir_all(&root); +} + +/// Every `.ll` anywhere under `root`, so a lifetime assertion does not have to +/// know which layout produced the file. +fn ll_files_under(root: &Path) -> Vec { + let mut found = Vec::new(); + let mut stack = vec![root.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(read) = fs::read_dir(&dir) else { + continue; + }; + for entry in read.flatten() { + let path = entry.path(); + if path.is_dir() { + stack.push(path); + } else if path.extension().is_some_and(|e| e == "ll") { + found.push(path); + } + } + } + found.sort(); + found +} + +#[test] +fn keep_ir_retains_the_whole_scratch_dir() { + let Some(root) = temp_root_if_clang_available("keep") else { + return; + }; + let policy = TempFilePolicy { + keep: true, + debug_symbols: false, + }; + compile_ll_to_object_in(&root, &test_ir(7), None, policy).expect("compile failed"); + + let left = entries(&root); + assert_eq!(left.len(), 1, "expected one kept scratch dir: {left:?}"); + let kept = entries(&root.join(&left[0])); + for want in [".ll", ".o", ".compile-plan.json"] { + assert!( + kept.iter().any(|n| n.ends_with(want)), + "PERRY_LLVM_KEEP_IR must retain the {want}: {kept:?}" + ); + } + let _ = fs::remove_dir_all(&root); +} + +#[test] +fn debug_symbols_do_not_change_the_temp_file_lifetime() { + // #7144 question (b), answered by measurement rather than by inheriting the + // premise. `PERRY_DEBUG_SYMBOLS` was believed to make the `.ll` part of the + // shipped object — "`-g` pulls the absolute `.ll` path plus `DW_AT_comp_dir` + // into DWARF" — which would have required keeping the file at a stable path + // for as long as the object lived. It does not: see + // `debug_symbols_do_not_change_what_the_object_records` below and the note + // on `TEMP_NONCE_COUNTER`. So `-g` cleans up like everything else, and there + // is no second layout left sitting untested. + let Some(root) = temp_root_if_clang_available("debug") else { + return; + }; + let policy = TempFilePolicy { + keep: false, + debug_symbols: true, + }; + for nth in 0..2 { + compile_ll_to_object_in(&root, &test_ir(9 + nth), None, policy) + .unwrap_or_else(|e| panic!("-g compile {nth} failed: {e:#}")); + assert_eq!( + entries(&root), + Vec::::new(), + "a -g compile must leave nothing behind either" + ); + } + let _ = fs::remove_dir_all(&root); +} + +// ── What the object actually records about the `.ll` ─────────────────────── + +/// The property the whole design rests on, as a test rather than a comment. +/// +/// Everything above is safe *because* an emitted object records the `.ll`'s +/// **basename** and never its **directory** — that is what lets a per-call +/// directory coexist with byte-identical emission (#7131). Until now that was +/// measured by hand, once, on a Raspberry Pi (#7140), and written down. A +/// property this load-bearing that no test can restate is one that quietly +/// stops being true. +/// +/// ELF is the format that records the basename at all (Mach-O does not, which +/// is why this defect class is invisible on the machine most of this project's +/// work happens on), so this cross-compiles: the embedding is a property of the +/// ELF writer, not of the host or the arch. +#[test] +fn the_ll_directory_is_not_recorded_in_the_object_but_the_basename_is() { + let Some(root) = temp_root_if_clang_available("elf-record") else { + return; + }; + let clang = find_clang().unwrap(); + let ir = test_ir(1); + + for target in ["x86_64-unknown-linux-gnu", "aarch64-unknown-linux-gnu"] { + // Same basename, two different directories. + let mut objs = Vec::new(); + for dir_name in ["dirA", "dirB"] { + let dir = root.join(format!("{target}-{dir_name}")); + fs::create_dir_all(&dir).unwrap(); + let ll = dir.join("perry_llvm_00000000cafe0001.ll"); + fs::write(&ll, &ir).unwrap(); + let Some(obj) = elf_compile(&clang, &ll, target, &dir) else { + eprintln!("[linker tests] skipping {target}: clang cannot target it"); + return; + }; + objs.push(obj); + } + assert_eq!( + objs[0], objs[1], + "{target}: the .ll's DIRECTORY must not reach the object — a \ + per-call scratch dir would otherwise break emission determinism" + ); + + // Control: the instrument must be able to see a difference at all. If + // this ever stops differing, the assertion above proves nothing. + let dir = root.join(format!("{target}-control")); + fs::create_dir_all(&dir).unwrap(); + let other = dir.join("perry_llvm_00000000cafe0002.ll"); + fs::write(&other, &ir).unwrap(); + let control = elf_compile(&clang, &other, target, &dir).unwrap(); + assert_ne!( + objs[0], control, + "{target}: a different .ll BASENAME must change the object — if it \ + does not, this test cannot detect the directory leaking either" + ); + } + let _ = fs::remove_dir_all(&root); +} + +#[test] +fn debug_symbols_do_not_change_what_the_object_records() { + // The measurement that retired the `PERRY_DEBUG_SYMBOLS` exemption. Perry's + // codegen emits no `DICompileUnit`/`DIFile`/`!dbg` metadata, and `clang -g` + // on a `.ll` lowers debug info that is *in* the IR rather than synthesising + // a compile unit for the input file — so `-g` adds no DWARF, and in + // particular no record of where the `.ll` was. If Perry ever does emit debug + // metadata this goes red, and the temp-file lifetime has to be revisited. + let Some(root) = temp_root_if_clang_available("elf-debug") else { + return; + }; + let clang = find_clang().unwrap(); + let target = "x86_64-unknown-linux-gnu"; + + let dir = root.join("g"); + fs::create_dir_all(&dir).unwrap(); + let ll = dir.join("perry_llvm_00000000cafe0003.ll"); + fs::write(&ll, test_ir(2)).unwrap(); + + let Some(plain) = elf_compile(&clang, &ll, target, &dir) else { + eprintln!("[linker tests] skipping: clang cannot target {target}"); + return; + }; + let with_g = elf_compile_with(&clang, &ll, target, &dir, &["-g"]).unwrap(); + assert_eq!( + plain, with_g, + "-g changed the emitted object. Perry's IR has gained debug metadata, \ + or this clang synthesises a compile unit for .ll input. Either way the \ + `.ll` may now be referenced by the object and #7144's decision to \ + delete it unconditionally has to be re-taken." + ); + + // Said directly as well as comparatively: no DWARF is emitted at all, so + // there is nowhere for a path to have been recorded. An equality assertion + // can be defeated by an edit to itself; "this object contains no `.debug_` + // section name" is a claim about the artifact. + assert!( + !contains(&with_g, b".debug_"), + "the -g object has .debug_ sections — DWARF is being emitted now, and \ + `DW_AT_comp_dir` / `DW_AT_name` may point at the temp `.ll`" + ); + + // Control: this comparison must be capable of failing. `-O0` is a flag that + // definitely changes the bytes; if even that compares equal, the harness is + // not really building two objects. + let control = elf_compile_with(&clang, &ll, target, &dir, &["-O0"]).unwrap(); + assert_ne!( + plain, control, + "the object comparison cannot distinguish two different compiles, so \ + the -g assertion above proves nothing" + ); + let _ = fs::remove_dir_all(&root); +} + +fn elf_compile(clang: &Path, ll: &Path, target: &str, cwd: &Path) -> Option> { + elf_compile_with(clang, ll, target, cwd, &[]) +} + +/// Substring search over raw object bytes — enough to spot an ELF section name. +fn contains(haystack: &[u8], needle: &[u8]) -> bool { + haystack.windows(needle.len()).any(|w| w == needle) +} + +/// `clang -c` for an explicit target, returning the object bytes. `None` when +/// this clang has no backend for `target` (then the caller must skip, not pass). +fn elf_compile_with( + clang: &Path, + ll: &Path, + target: &str, + cwd: &Path, + extra: &[&str], +) -> Option> { + let obj = cwd.join(format!("out{}.o", extra.join(""))); + let run = Command::new(clang) + .current_dir(cwd) + .args(["-c", "-O3"]) + .args(extra) + .arg("-target") + .arg(target) + .arg(ll) + .arg("-o") + .arg(&obj) + .output() + .ok()?; + if !run.status.success() { + return None; + } + fs::read(&obj).ok() +} diff --git a/scripts/compiler_output_harness/cli.py b/scripts/compiler_output_harness/cli.py index c8148207e3..7b6d5a0e89 100644 --- a/scripts/compiler_output_harness/cli.py +++ b/scripts/compiler_output_harness/cli.py @@ -11,6 +11,9 @@ from .repsel_determinism import self_test as determinism_self_test from .repsel_knob_isolation import check_isolation from .repsel_knob_isolation import self_test as isolation_self_test +from .repsel_temp_hygiene import DEFAULT_REPEAT as DEFAULT_HYGIENE_REPEAT +from .repsel_temp_hygiene import check_temp_hygiene +from .repsel_temp_hygiene import self_test as temp_hygiene_self_test from .spec import WORKLOADS @@ -179,6 +182,33 @@ def build_parser() -> argparse.ArgumentParser: ) det_self_p.set_defaults(func=determinism_self_test) + # Temp-directory hygiene (#7144). The other half of the #7131 story: the + # `.ll` name became a function of the IR, which meant workers shared it, + # which meant nothing deleted it — one file per distinct IR ever compiled, + # forever, on every developer machine. + hyg_p = sub.add_parser( + "census-temp-hygiene", + help="assert compiling leaves no files behind in the temp directory", + ) + hyg_p.add_argument("--perry") + hyg_p.add_argument("--baseline") + hyg_p.add_argument("--workload", action="append", help="restrict to named workload(s)") + hyg_p.add_argument( + "--repeat", + type=int, + default=DEFAULT_HYGIENE_REPEAT, + help="compiles per workload; >1 puts identical IR in flight concurrently", + ) + hyg_p.add_argument("--compile-timeout", type=int, default=300) + hyg_p.add_argument("--jobs", type=int, default=4, help="parallel compiles") + hyg_p.set_defaults(func=check_temp_hygiene) + + hyg_self_p = sub.add_parser( + "census-temp-hygiene-self-test", + help="check the temp-hygiene verdict logic without compiling", + ) + hyg_self_p.set_defaults(func=temp_hygiene_self_test) + return parser diff --git a/scripts/compiler_output_harness/repsel_temp_hygiene.py b/scripts/compiler_output_harness/repsel_temp_hygiene.py new file mode 100644 index 0000000000..9d47b722e0 --- /dev/null +++ b/scripts/compiler_output_harness/repsel_temp_hygiene.py @@ -0,0 +1,307 @@ +"""Temp-directory hygiene check (#7144). + +`compile_ll_to_object` writes the module's LLVM IR to a temp `.ll`, hands the +path to `clang -c`, and reads back the object. #7131 made that name a pure +function of the IR — it had to, because clang records a translation unit's +source basename into the ELF object — and #7135, shipping that fix, stopped +deleting the file: workers holding identical IR now *shared* the path, so a +per-call unlink could race a sibling that had computed the path but not yet +opened it. + +Nothing else deleted them. The file count is bounded by the number of +**distinct IR contents ever compiled** on the machine, which sounds benign +until you notice that iterating on the compiler changes the IR on essentially +every rebuild: + + leftover perry_llvm_*.ll files: 1627, total 951.8 MB (one dev box, one day) + ~29,000 files, 29 GB (another, a month) + +#7144 removed the sharing instead of the deletion: the `.ll` now lives in a +directory that belongs to one call, so unlinking it is unobservable to anyone +else, and the *basename* clang records is untouched — `census-determinism` is +the check that the second half still holds. + +What this module checks is the first half, end to end, on the real compiler: +compile the census corpus with `TMPDIR` pointed at an empty directory of our +own, then look in that directory. It must be empty. + +Two design notes, because both alternatives were tried and are wrong: + +* **"No growth run-over-run" is not the property.** Compiling the same corpus + twice leaves the same content-addressed names, so a repeat-and-compare check + is *green on the broken compiler*. Growth needs new IR, which is why the leak + was invisible in CI and only ever showed up on developer machines. The + property that goes red immediately is the absolute one: nothing left at all. +* **`TMPDIR` isolation is not politeness, it is what makes the check sound.** + Counting entries in the shared system temp directory measures every other + process on the box — on a machine running several compiles at once, that is + noise large enough to swamp the signal in either direction. + +`PERRY_DEBUG_SYMBOLS` is *not* exempt, though it was going to be. `-g` was +documented as putting the `.ll`'s absolute path into DWARF; measured on a real +Perry module it emits no DWARF at all (Perry's codegen produces no DI metadata, +and `clang -g` on a `.ll` lowers what the IR already has rather than +synthesising a compile unit). One layout, no exemption — see +`debug_symbols_do_not_change_what_the_object_records`. +""" + +from __future__ import annotations + +import argparse +import platform +import shutil +import tempfile +from concurrent.futures import ThreadPoolExecutor +from pathlib import Path +from typing import Any, Callable + +from .capture import resolve_perry +from .common import HarnessError, REPO_ROOT +from .repsel_census import DEFAULT_BASELINE, compile_and_census, load_baseline + + +#: Compiles per workload. Two, not one, so the corpus contains pairs of +#: compiles with *identical* IR — the case that shares a content-addressed name +#: and the reason #7135 could not simply keep deleting the file. They run +#: concurrently for the same reason (#7140: a serial check never opens the +#: window). +DEFAULT_REPEAT = 2 + +#: Cap on how many leftover paths a failure prints. A leaking compiler leaks one +#: per compile, and 52 lines of the same shape teaches nothing the first 12 did +#: not. +MAX_REPORTED = 12 + +#: Every temp name `crates/perry-codegen/src/linker.rs` creates — the `.ll`/`.o` +#: pair and its scratch directory (`perry_llvm_*`), the multi-codegen-unit +#: staging objects (`perry_cgu_*`, #5391), and the bitcode-link intermediates +#: (`perry_bc_*`). This gate's subject is that module's file lifecycle, so it +#: fails on these and only these. +#: +#: Anything else found is REPORTED, loudly, and does not fail: as of #7144 the +#: compile driver leaks a `perry-objs--/` staging directory on the +#: `--no-link` path (#7167 — `run_pipeline.rs` removes it on both *link* exits and +#: there is no third one), which is a real defect but a different module's, and a gate +#: that goes red for someone else's bug gets muted rather than fixed. Widen this +#: to "nothing at all" once the driver's path is closed. +OWNED_PREFIXES = ("perry_llvm", "perry_cgu", "perry_bc") + + +def classify(leftovers: list[str]) -> tuple[list[str], list[str]]: + """Split leftovers into "this gate's subject" and "somebody else's". + + Classified on the FIRST path component: everything under a leaked scratch + directory is leaked by whoever leaked the directory. + """ + owned: list[str] = [] + other: list[str] = [] + for rel in leftovers: + top = rel.split("/", 1)[0] + (owned if top.startswith(OWNED_PREFIXES) else other).append(rel) + return owned, other + + +def leftovers_under(root: Path) -> list[str]: + """Every path under `root`, relative to it, deepest entries first. + + Directories are reported too: the pre-#7144 failure is a stray *file*, but + a half-finished cleanup that leaves empty scratch directories behind is the + same defect wearing a smaller coat, and it is just as unbounded. + """ + if not root.exists(): + raise HarnessError(f"the isolated temp root vanished during the run: {root}") + found = [p.relative_to(root).as_posix() for p in root.rglob("*")] + found.sort() + return found + + +def verdict( + leftovers: list[str], + *, + compiles: int, + printer: Callable[[str], None] = print, +) -> int: + """Turn "what was left in the temp dir" into an exit code. + + Split from the compile loop so the decision can be exercised without a + compiler (CLAUDE.md failure mode 4: a gate nobody can re-check is not a + gate), and so a run that compiled *nothing* is a harness error rather than + a green line — an empty directory is exactly what you get from doing no + work at all. + """ + if compiles <= 0: + raise HarnessError( + "no compiles ran, so an empty temp directory proves nothing; " + "this run checked nothing" + ) + owned, other = classify(leftovers) + + if other: + shown = other[:MAX_REPORTED] + printer( + f"Not this gate's subject — {len(other)} entr" + f"{'y' if len(other) == 1 else 'ies'} left by a module other than " + "perry-codegen's clang driver:" + ) + for name in shown: + printer(f" {name}") + if len(other) > len(shown): + printer(f" … and {len(other) - len(shown)} more") + printer( + " `perry-objs--/` is the compile driver's object " + "staging dir;\n" + " `run_pipeline.rs` removes it on both *link* exits and `--no-link` " + "returns\n" + " before either (#7167). Reported, not failed: a gate that goes " + "red for another\n" + " module's defect gets muted rather than fixed.\n" + ) + + if not owned: + printer( + f"Temp directory is clean: {compiles} compile(s) left 0 clang-driver " + "files behind. The #7144 leak is not present." + ) + return 0 + + shown = owned[:MAX_REPORTED] + printer( + f"TEMP FILES LEAKED: {compiles} compile(s) left {len(owned)} " + f"entr{'y' if len(owned) == 1 else 'ies'} in a temp directory that " + "started empty.\n" + ) + for name in shown: + printer(f" {name}") + if len(owned) > len(shown): + printer(f" … and {len(owned) - len(shown)} more") + printer( + "\n" + " This is #7144. The `.ll` handed to `clang -c` is content-addressed\n" + " (#7131 — clang records its basename into the ELF object), so the\n" + " leftovers are bounded by DISTINCT IR EVER COMPILED, not by compiles:\n" + " a repeat-and-compare check stays green while a developer machine\n" + " fills up. Measured before the fix: 1627 files / 951.8 MB after a day\n" + " of compiler work; 29 GB on a longer-lived box.\n" + "\n" + " The fix is not a more careful unlink — that races a sibling worker\n" + " holding the same IR, which is why #7135 stopped deleting at all. It\n" + " is to stop sharing: `crates/perry-codegen/src/linker.rs` gives each\n" + " compile a private scratch directory and removes it on success, while\n" + " the basename inside it stays a pure function of the IR so\n" + " `census-determinism` keeps passing.\n" + "\n" + " `PERRY_DEBUG_SYMBOLS` is not an exemption: measured, `-g` emits no\n" + " DWARF from a Perry `.ll` at all, so nothing records where the file\n" + " was and nothing needs to outlive the compile." + ) + return 1 + + +def check_temp_hygiene(args: argparse.Namespace) -> int: + """Compile the census corpus with an isolated `TMPDIR` and inspect it.""" + perry = resolve_perry(getattr(args, "perry", None)) + baseline = load_baseline(Path(args.baseline) if args.baseline else DEFAULT_BASELINE) + workloads: list[dict[str, Any]] = baseline["workloads"] + if getattr(args, "workload", None): + wanted = set(args.workload) + workloads = [w for w in workloads if w["name"] in wanted] + missing = wanted - {w["name"] for w in workloads} + if missing: + raise HarnessError(f"unknown workload(s): {', '.join(sorted(missing))}") + if not workloads: + raise HarnessError("no workloads selected") + + repeat = max(1, int(args.repeat)) + + print("Temp-directory hygiene (#7144)") + print("==============================\n") + print(f"compiler: {' '.join(perry)}") + print(f"host: {platform.system()} {platform.machine()}") + print(f"corpus: {len(workloads)} workload(s) x {repeat} compile(s)\n") + + outer = Path(tempfile.mkdtemp(prefix="repsel-temp-hygiene-")) + # The directory the *compiler* will treat as its temp dir. Separate from + # `outer` so the harness's own scratch never counts as a leftover. + isolated = outer / "compiler-tmp" + isolated.mkdir() + try: + jobs = [(w, i) for w in workloads for i in range(repeat)] + + def run(job: tuple[dict[str, Any], int]) -> None: + workload, _index = job + compile_and_census( + perry, + REPO_ROOT / workload["source"], + timeout=args.compile_timeout, + # TMP/TEMP alongside TMPDIR so the check means the same thing + # if this ever runs on Windows, where `env::temp_dir()` reads + # those instead. + extra_env={ + "TMPDIR": str(isolated), + "TMP": str(isolated), + "TEMP": str(isolated), + }, + ) + + with ThreadPoolExecutor(max_workers=max(1, args.jobs)) as pool: + list(pool.map(run, jobs)) + + left = leftovers_under(isolated) + return verdict(left, compiles=len(jobs)) + finally: + shutil.rmtree(outer, ignore_errors=True) + + +def self_test(_args: argparse.Namespace) -> int: + """Prove the verdict can go red, and that it refuses a vacuous run.""" + quiet: Callable[[str], None] = lambda _line: None + + assert verdict([], compiles=52, printer=quiet) == 0 + assert verdict(["perry_llvm_2791e842224ea99c.ll"], compiles=52, printer=quiet) == 1 + # An empty scratch directory left behind is the same defect, smaller. + assert verdict(["perry_llvm_scratch_1a2b_0"], compiles=1, printer=quiet) == 1 + # …and a file inside one is attributed to whoever leaked the directory. + assert verdict(["perry_llvm_scratch_1a2b_0/x.ll"], compiles=1, printer=quiet) == 1 + for owned in ("perry_cgu_1_2_0.o", "perry_bc_1_2_linked.bc"): + assert verdict([owned], compiles=1, printer=quiet) == 1, owned + + # Another module's leftovers are reported, not failed — see OWNED_PREFIXES. + assert verdict(["perry-objs-9-1/m.o"], compiles=1, printer=quiet) == 0 + lines: list[str] = [] + assert verdict(["perry-objs-9-1/m.o"], compiles=1, printer=lines.append) == 0 + joined = "\n".join(lines) + assert "perry-objs" in joined and "run_pipeline.rs" in joined, joined + # A mixture still fails, and the failure is about the owned half. + assert verdict(["perry-objs-9-1/m.o", "perry_llvm_a.ll"], compiles=1, printer=quiet) == 1 + + assert classify(["perry_llvm_a.ll", "perry-objs-9-1/m.o"]) == ( + ["perry_llvm_a.ll"], + ["perry-objs-9-1/m.o"], + ) + + # A run that compiled nothing finds an empty directory for the wrong + # reason. It must not be able to report success. + try: + verdict([], compiles=0, printer=quiet) + except HarnessError: + pass + else: # pragma: no cover - the raise below is the failure report + raise AssertionError("verdict() called a zero-compile run clean") + + lines = [] + verdict(["perry_llvm_a.ll", "perry_llvm_b.ll"], compiles=2, printer=lines.append) + report = "\n".join(lines) + for expected in ("#7144", "#7131", "PERRY_DEBUG_SYMBOLS", "linker.rs"): + assert expected in report, f"failure report must mention {expected}: {report}" + + # The truncation must announce itself rather than quietly dropping paths. + lines = [] + verdict( + [f"perry_llvm_{i}.ll" for i in range(MAX_REPORTED + 5)], + compiles=1, + printer=lines.append, + ) + assert "and 5 more" in "\n".join(lines) + + print("repsel temp-hygiene self-test OK") + return 0