From 4cdecb895b312ad3681e1a8f6a6a18e06c032aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:49:54 +0200 Subject: [PATCH 1/5] fix(compile): stop leaking the object staging dir on --no-link (#7167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `run_pipeline.rs` created a `perry-objs--/` staging directory on every compile and removed it on the two *link* exits only. `--no-link` returns before either, so every `--no-link` compile left the directory and its objects in the system temp dir forever — unbounded in compiles, not in distinct IR, because the name carries pid + wall-clock nanos. 3086 such directories (277 MB) had accumulated on the machine this was written on. The objects could not simply be deleted: on `--no-link` they are the product. The flag is documented as "produce object file only", it did not honour `-o` at all, and the census/knob-isolation/determinism gates hash the paths it prints. So the fix is about *where* they go, not whether they are removed: * `--no-link` no longer creates a staging directory. Its objects are delivered to `-o` — verbatim for the single-module case, into `-o`'s directory under module-derived names when a program has several modules (one `-o` cannot name N files). No `-o` means the current directory. * When linking, the staging directory is removed by `Drop`, so both link exits, the static-archive exit and every `?` in between clean up through one site. Three sites that must each remember is how the third came to be missing. `--keep-intermediates` is still the single opt-in for retaining staged objects, disarmed once where the directory is created. The codegen-failure paths now name the directory and say whether it survives. Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- crates/perry/src/commands/compile.rs | 1 + .../src/commands/compile/object_staging.rs | 379 ++++++++++++++++++ .../src/commands/compile/run_pipeline.rs | 105 +++-- 3 files changed, 460 insertions(+), 25 deletions(-) create mode 100644 crates/perry/src/commands/compile/object_staging.rs diff --git a/crates/perry/src/commands/compile.rs b/crates/perry/src/commands/compile.rs index 54c2a2e612..529875c6ca 100644 --- a/crates/perry/src/commands/compile.rs +++ b/crates/perry/src/commands/compile.rs @@ -36,6 +36,7 @@ mod link; mod lock_scan; mod lowering_report; mod object_cache; +mod object_staging; mod optimized_libs; mod output_path; mod parse_cache; diff --git a/crates/perry/src/commands/compile/object_staging.rs b/crates/perry/src/commands/compile/object_staging.rs new file mode 100644 index 0000000000..4d1af237db --- /dev/null +++ b/crates/perry/src/commands/compile/object_staging.rs @@ -0,0 +1,379 @@ +//! Where a compile puts the `.o` files it emits, and who deletes them again. +//! +//! A `perry compile` produces one object per native module. What happens to +//! those objects depends on whether this invocation is going to *link* them: +//! +//! * **Linking** — the objects are intermediates. Nothing outside this process +//! will ever look at them, so they go into a private staging directory that +//! this invocation owns and removes ([`StagingDir`]). +//! * **`--no-link`** — the objects *are* the product. There is nothing to clean +//! up, because they are delivered to the path the user named with `-o` +//! ([`NoLinkDestination`]). +//! +//! # Why this module exists (#7167) +//! +//! The staging directory used to be created unconditionally, and removed by +//! two hand-written `remove_dir` calls — one on the executable-link exit, one +//! on the shared-library exit. `--no-link` returns before either, so it had no +//! cleanup at all, and every `--no-link` compile left a +//! `perry-objs--/` directory and its objects in the system temp +//! directory forever. +//! +//! That leak is unbounded in **compiles**: the directory name carries the pid +//! and a wall-clock nanosecond component, so no two invocations ever reuse one. +//! The machine this was written on had accumulated 3086 of them. Every +//! `--no-link` user was affected, and the compiler-output census +//! (`scripts/compiler_output_harness/`) compiles the corpus with `--no-link` +//! constantly, so a measurement campaign bled gigabytes a day. +//! +//! Two structural changes, rather than a third `remove_dir`: +//! +//! 1. **`--no-link` no longer creates a staging directory at all.** It cannot +//! leak one. This is not a cleanup that has to fire — it is work that never +//! happens. Three call sites that must each remember is how the third one +//! came to be missing (#7167's own diagnosis). +//! 2. **The staging directory is removed by `Drop`**, so *every* exit from the +//! pipeline — both links, the static-archive path, and any `?` in between — +//! cleans up through one site. A future fourth exit inherits the cleanup +//! instead of having to remember it. +//! +//! # Why deleting is safe (and why it was not, for the `.ll`) +//! +//! #7144/#7168 could not simply unlink the `.ll` handed to `clang -c`: #7131 +//! had made that name a pure function of the IR, so two workers holding +//! identical IR *shared* the path, and a per-call unlink could race a sibling. +//! The fix there was to stop sharing. +//! +//! Nothing is shared here to begin with. The staging directory's name carries +//! the pid and a monotonic wall-clock component, so it belongs to exactly one +//! invocation; no other process can be looking at it, and removing it is +//! unobservable to a concurrent compile. That is a structural property of the +//! name, not a timing argument. +//! +//! # Failure policy +//! +//! * `--no-link`: a failed compile keeps every object it managed to write, at +//! the path the user named. Nothing deletes them, on any path. +//! * Linking: the staging directory is removed on failure too, and the error +//! names it. What diagnosing a codegen failure needs is the *IR*, which +//! `PERRY_LLVM_KEEP_IR` retains (and #7168 keeps for a failed +//! `compile_ll_to_object`) — a module that failed codegen emitted no object, +//! and the objects of the modules that succeeded are reproducible. +//! `--keep-intermediates` is the single, already-documented opt-in for +//! keeping them, and the failure message points at it. Deliberately *not* a +//! second retention mode: an escape hatch nobody exercises is a configuration +//! nobody has verified (CLAUDE.md's GC knob kill-policy, generalised). + +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; + +use anyhow::{Context, Result}; + +/// A per-invocation object staging directory, removed when this value drops. +/// +/// Created only when the compile is going to link. See the module docs for why +/// `--no-link` has none. +pub(super) struct StagingDir { + path: PathBuf, + /// Whether `Drop` should remove the directory. + /// + /// Starts armed. Every way of keeping the directory is an explicit, + /// user-visible decision that disarms it — never an exit that forgot. + /// The default direction matters: a new exit that does nothing cleans up, + /// where under the old scheme it leaked (#7167). + remove_on_drop: AtomicBool, +} + +impl StagingDir { + /// Create the staging directory under the system temp directory. + pub(super) fn create() -> Result { + Self::create_in(&std::env::temp_dir()) + } + + /// Create the staging directory under `parent`. + /// + /// Split out so the tests can exercise the real create/drop cycle without + /// writing into the shared system temp directory — which is exactly the + /// place this module exists to keep clean. + pub(super) fn create_in(parent: &Path) -> Result { + // #4266 (2026-07-02 audit fleet P0): objects used to land at + // CWD-relative name-only paths, so two concurrent compiles sharing a + // working directory overwrote each other's `.o` mid-link. + // pid + a strictly-monotonic wall component (the linker.rs #509 + // discipline) keeps simultaneous invocations disjoint. + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + let path = parent.join(format!("perry-objs-{}-{}", std::process::id(), nanos)); + std::fs::create_dir_all(&path) + .with_context(|| format!("failed to create object staging dir {}", path.display()))?; + Ok(Self { + path, + remove_on_drop: AtomicBool::new(true), + }) + } + + pub(super) fn path(&self) -> &Path { + &self.path + } + + /// Keep the directory instead of removing it on drop, and report the path + /// so the caller can name it. + /// + /// The one caller is `--keep-intermediates`; the failure paths use it to + /// name the directory in the error they are about to return. + pub(super) fn keep(&self) -> &Path { + self.remove_on_drop.store(false, Ordering::Relaxed); + &self.path + } +} + +impl Drop for StagingDir { + fn drop(&mut self) { + if !self.remove_on_drop.load(Ordering::Relaxed) { + return; + } + // `remove_dir_all`, not `remove_dir`: the directory belongs to this + // invocation, so anything inside it is ours and a stray file must not + // be able to turn cleanup into a silent no-op. The old code removed + // the directory only when it was already empty, which meant one + // unexpected file re-created the leak this module exists to close. + // + // Best-effort. A compile that produced the right answer must not fail + // because a temp directory could not be unlinked. + let _ = std::fs::remove_dir_all(&self.path); + } +} + +/// Where a `--no-link` compile writes the objects it emits. +/// +/// `--no-link` used to ignore `-o` entirely and leave its objects in the temp +/// staging directory — the flag's documented product ("produce object file +/// only") existed only as a path printed on stdout, in a directory nobody +/// deleted. Delivering to `-o` is what makes the objects the *user's* files: +/// they persist because they are wanted, not because cleanup was missing. +#[derive(Debug, PartialEq, Eq)] +pub(super) struct NoLinkDestination { + /// The directory every emitted object goes into. + dir: PathBuf, + /// The exact path for the single-module case, when `-o` was given. + single: Option, +} + +impl NoLinkDestination { + /// Resolve the destination for a `--no-link` compile. + /// + /// * `-o` given, one native module — the object is written to `-o` + /// verbatim. This is what `cc -c foo.c -o foo.o` does, and the case + /// essentially every caller means. + /// * `-o` given, several native modules — one `-o` cannot name N files + /// (`cc` rejects that combination outright). The objects go into `-o`'s + /// directory under their module-derived names, so they land where the + /// user pointed and a separate link step can find them together. + /// * no `-o` — the current directory, module-derived names. + /// + /// The rule keys on the *module count*, which is a property of the program, + /// rather than on how many objects codegen actually wrote — that varies + /// with object-cache warmth, and `-o` must not mean two different things + /// depending on whether a cache was hot. + pub(super) fn resolve(output: Option<&Path>, native_module_count: usize) -> Self { + let Some(out) = output else { + return Self { + dir: PathBuf::from("."), + single: None, + }; + }; + // `Path::new("x.o").parent()` is `Some("")`, not `None`. + let dir = match out.parent() { + Some(p) if !p.as_os_str().is_empty() => p.to_path_buf(), + _ => PathBuf::from("."), + }; + let single = (native_module_count == 1).then(|| out.to_path_buf()); + Self { dir, single } + } + + /// Create the destination directory, if it does not exist yet. + pub(super) fn prepare(&self) -> Result<()> { + std::fs::create_dir_all(&self.dir).with_context(|| { + format!( + "failed to create the --no-link object output directory {}", + self.dir.display() + ) + }) + } + + pub(super) fn dir(&self) -> &Path { + &self.dir + } + + /// The path for one emitted artifact. + /// + /// `ext` is `"o"` for an object and `"ll"` in bitcode-link mode. Only an + /// object is `--no-link`'s product, so only an object takes `-o` verbatim + /// — an `-o app.o` that produced LLVM IR would be a lie about the file's + /// contents. + pub(super) fn artifact_path(&self, stem: &str, ext: &str) -> PathBuf { + match &self.single { + Some(p) if ext == "o" => p.clone(), + _ => self.dir.join(format!("{}.{}", stem, ext)), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn tmp_root(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "perry-objstaging-test-{}-{}-{:?}", + std::process::id(), + tag, + std::thread::current().id() + )); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + dir + } + + /// The #7167 property, at the unit level: a staging directory that has been + /// used and dropped leaves *nothing* behind — not the objects, and not the + /// directory either. + /// + /// Asserted as "the parent is empty", not "the objects are gone". An empty + /// scratch directory left behind is the same unbounded leak wearing a + /// smaller coat: the name carries pid + nanos, so it is one fresh directory + /// per compile whether or not there is anything in it. + #[test] + fn dropping_a_staging_dir_leaves_nothing_behind() { + let root = tmp_root("drop"); + { + let staging = StagingDir::create_in(&root).unwrap(); + std::fs::write(staging.path().join("mod_a.o"), b"objectbytes").unwrap(); + std::fs::write(staging.path().join("mod_b.o"), b"objectbytes").unwrap(); + assert!(staging.path().is_dir()); + } + let left: Vec<_> = std::fs::read_dir(&root) + .unwrap() + .map(|e| e.unwrap().path()) + .collect(); + assert!(left.is_empty(), "staging dir leaked: {:?}", left); + let _ = std::fs::remove_dir_all(&root); + } + + /// A stray file must not be able to turn cleanup into a no-op. The old + /// `remove_dir` was non-recursive and silently did nothing when the + /// directory still held anything the cleanup loop had not listed. + #[test] + fn an_unexpected_file_does_not_defeat_cleanup() { + let root = tmp_root("stray"); + { + let staging = StagingDir::create_in(&root).unwrap(); + std::fs::create_dir_all(staging.path().join("nested")).unwrap(); + std::fs::write(staging.path().join("nested/surprise.txt"), b"x").unwrap(); + } + let left: Vec<_> = std::fs::read_dir(&root) + .unwrap() + .map(|e| e.unwrap().path()) + .collect(); + assert!( + left.is_empty(), + "stray content defeated cleanup: {:?}", + left + ); + let _ = std::fs::remove_dir_all(&root); + } + + /// `--keep-intermediates` (and the failure paths, which use the same call) + /// must actually keep the directory, and must report the path so the caller + /// can name it. A retention hatch that silently deletes is worse than none. + #[test] + fn keep_retains_the_directory_and_reports_its_path() { + let root = tmp_root("keep"); + let kept: PathBuf; + { + let staging = StagingDir::create_in(&root).unwrap(); + std::fs::write(staging.path().join("mod_a.o"), b"objectbytes").unwrap(); + kept = staging.keep().to_path_buf(); + } + assert!(kept.is_dir(), "keep() did not retain {}", kept.display()); + assert!(kept.join("mod_a.o").is_file()); + let _ = std::fs::remove_dir_all(&root); + } + + /// Two staging directories created back to back must not collide, because + /// "removal is unobservable to a concurrent compile" rests entirely on the + /// name being unique to one invocation. + #[test] + fn staging_dirs_are_unique_per_call() { + let root = tmp_root("unique"); + let a = StagingDir::create_in(&root).unwrap(); + let b = StagingDir::create_in(&root).unwrap(); + assert_ne!(a.path(), b.path()); + let _ = std::fs::remove_dir_all(&root); + } + + #[test] + fn single_module_no_link_honours_dash_o_verbatim() { + let d = NoLinkDestination::resolve(Some(Path::new("/build/app.o")), 1); + assert_eq!(d.dir(), Path::new("/build")); + assert_eq!( + d.artifact_path("app_ts", "o"), + PathBuf::from("/build/app.o") + ); + } + + /// One `-o` cannot name several files, so the module-derived names win and + /// `-o` contributes only the directory. + #[test] + fn multi_module_no_link_uses_the_dash_o_directory() { + let d = NoLinkDestination::resolve(Some(Path::new("/build/app.o")), 3); + assert_eq!(d.dir(), Path::new("/build")); + assert_eq!( + d.artifact_path("app_ts", "o"), + PathBuf::from("/build/app_ts.o") + ); + assert_eq!( + d.artifact_path("dep_ts", "o"), + PathBuf::from("/build/dep_ts.o") + ); + } + + /// `-o app.o` with no directory component must mean "here", not "" — a + /// `PathBuf::from("").join("app.o")` is a relative path that happens to + /// work, but `create_dir_all("")` fails. + #[test] + fn bare_dash_o_resolves_to_the_current_directory() { + let d = NoLinkDestination::resolve(Some(Path::new("app.o")), 1); + assert_eq!(d.dir(), Path::new(".")); + assert_eq!(d.artifact_path("app_ts", "o"), PathBuf::from("app.o")); + } + + #[test] + fn no_dash_o_writes_module_named_objects_into_the_current_directory() { + let d = NoLinkDestination::resolve(None, 1); + assert_eq!(d.dir(), Path::new(".")); + assert_eq!(d.artifact_path("app_ts", "o"), PathBuf::from("./app_ts.o")); + } + + /// Bitcode-link mode emits `.ll`, not an object. `-o app.o` must not be + /// handed LLVM IR under an object's name. + #[test] + fn bitcode_mode_never_takes_dash_o_verbatim() { + let d = NoLinkDestination::resolve(Some(Path::new("/build/app.o")), 1); + assert_eq!( + d.artifact_path("app_ts", "ll"), + PathBuf::from("/build/app_ts.ll") + ); + } + + /// The destination must not depend on object-cache warmth: the rule keys on + /// the module count, so a hot cache and a cold cache name the same file. + #[test] + fn the_rule_keys_on_module_count_not_on_what_codegen_wrote() { + let cold = NoLinkDestination::resolve(Some(Path::new("/build/app.o")), 1); + let hot = NoLinkDestination::resolve(Some(Path::new("/build/app.o")), 1); + assert_eq!(cold, hot); + } +} diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index 33c925b6a6..eb25e81485 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -2066,24 +2066,35 @@ pub fn run_with_parse_cache( let total_codegen_modules = ctx.native_modules.len(); let codegen_modules_started = AtomicUsize::new(0); - // Per-invocation object staging dir (2026-07-02 audit fleet P0). - // Objects used to land at CWD-relative name-only paths, so two - // concurrent perry compiles sharing a working directory overwrote each - // other's `.o` mid-link and each deleted the other's objects - // afterwards — deterministically wrong binaries whenever the object - // cache was bypassed (--no-cache / trace modes / store errors), and the - // fixed-name stub objects collided even with the cache ON. pid + a - // strictly-monotonic wall component (the linker.rs #509 discipline) - // keeps simultaneous invocations disjoint; the dir is removed with the - // intermediates below. - let object_output_dir = { - let nanos = std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_nanos()) - .unwrap_or(0); - let dir = std::env::temp_dir().join(format!("perry-objs-{}-{}", std::process::id(), nanos)); - std::fs::create_dir_all(&dir)?; - dir + // Where this compile's objects go — see `compile/object_staging.rs`. + // + // #7167: only a compile that is going to *link* gets a temp staging + // directory, and that directory is removed by `Drop` rather than by a + // cleanup call each exit has to remember. `--no-link` gets none at all: + // its objects are the product, so they are delivered to `-o` and stay + // there. The old code created the staging dir unconditionally and removed + // it on the two link exits only, so every `--no-link` compile leaked one + // — unbounded in compiles, because the name carries pid + nanos. + let object_staging: Option = if args.no_link { + None + } else { + let staging = object_staging::StagingDir::create()?; + if args.keep_intermediates { + // `--keep-intermediates` is the single opt-in for keeping the + // staged objects. Disarmed once, here, where the directory is + // created — not re-checked at each exit. + staging.keep(); + } + Some(staging) + }; + let no_link_destination = + object_staging::NoLinkDestination::resolve(args.output.as_deref(), total_codegen_modules); + let object_output_dir: PathBuf = match &object_staging { + Some(staging) => staging.path().to_path_buf(), + None => { + no_link_destination.prepare()?; + no_link_destination.dir().to_path_buf() + } }; let compile_results: Vec> = ctx .native_modules @@ -4229,7 +4240,16 @@ pub fn run_with_parse_cache( let obj_name = native_object_file_stem(&hir_module.name); // In bitcode mode the bytes are .ll text; use .ll extension. let ext = if bitcode_link { "ll" } else { "o" }; - let obj_path = object_output_dir.join(format!("{}.{}", obj_name, ext)); + // #7167: on `--no-link` the emitted object is the product, so it + // is written where `-o` points (verbatim, for the single-module + // case) rather than into a temp directory nobody deletes. When + // linking, `object_output_dir` is the staging dir and the two + // agree. + let obj_path = if args.no_link { + no_link_destination.artifact_path(&obj_name, ext) + } else { + object_output_dir.join(format!("{}.{}", obj_name, ext)) + }; if let Some((key, cached_path, ffi_symbols)) = cache_key .and_then(|k| object_cache.lookup_path_with_ffi(k).map(|(p, s)| (k, p, s))) @@ -4536,6 +4556,33 @@ pub fn run_with_parse_cache( eprintln!(" - {}{}{}{}", bold_on, m, marker, bold_off); } eprintln!(); + // #7167 failure policy: say where the objects of the modules that DID + // compile are, and how to keep them. On `--no-link` they are already + // the user's files at `-o` and nothing removes them; when linking, + // the staging directory is removed on the way out (by `Drop`) unless + // `--keep-intermediates` was given, so name both the path and the + // flag rather than deleting in silence. + if will_abort { + match &object_staging { + Some(staging) if args.keep_intermediates => eprintln!( + "Objects for the modules that compiled are kept in {} \ + (--keep-intermediates).\n", + staging.path().display() + ), + Some(staging) => eprintln!( + "Objects for the modules that compiled were staged in {} and are \ + removed on exit;\nre-run with --keep-intermediates to keep them. \ + Diagnosing a codegen failure normally\nwants the IR instead \ + (PERRY_LLVM_KEEP_IR=1).\n", + staging.path().display() + ), + None => eprintln!( + "Objects for the modules that compiled were written to {} and are \ + left in place.\n", + object_output_dir.display() + ), + } + } if entry_failed { eprintln!("Aborting: the entry module's `main` symbol is required by the linker."); eprintln!("Fix the codegen errors above (search for `Error compiling module`)"); @@ -5407,9 +5454,13 @@ pub fn run_with_parse_cache( for obj_path in &obj_cleanup_paths { let _ = fs::remove_file(obj_path); } - // Best-effort: drop the per-invocation staging dir (only when - // empty — keep_intermediates or stray files leave it in place). - let _ = fs::remove_dir(&object_output_dir); + // The staging directory itself is removed by `StagingDir`'s + // `Drop` (#7167), so every exit — including the `?`s between here + // and there — cleans up through one site rather than three that + // each have to remember. This loop stays because + // `obj_cleanup_paths` also holds objects from *outside* the + // staging dir (the bitcode-link merge output, the embedded-JS + // object under `perry-embed-/`). } let codegen_cache_stats = if object_cache.is_enabled() { @@ -5604,9 +5655,13 @@ pub fn run_with_parse_cache( for obj_path in &obj_cleanup_paths { let _ = fs::remove_file(obj_path); } - // Best-effort: drop the per-invocation staging dir (only when - // empty — keep_intermediates or stray files leave it in place). - let _ = fs::remove_dir(&object_output_dir); + // The staging directory itself is removed by `StagingDir`'s + // `Drop` (#7167), so every exit — including the `?`s between here + // and there — cleans up through one site rather than three that + // each have to remember. This loop stays because + // `obj_cleanup_paths` also holds objects from *outside* the + // staging dir (the bitcode-link merge output, the embedded-JS + // object under `perry-embed-/`). } let codegen_cache_stats = if object_cache.is_enabled() { From 8f8082d3bd9c7782d782a68c5f8d41f15adc1919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 09:57:57 +0200 Subject: [PATCH 2/5] test(repsel): widen census-temp-hygiene to every leftover, drop the allowlist (#7167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate shipped in #7144 with a carve-out: it failed on `perry_llvm_*`, `perry_cgu_*` and `perry_bc_*` and merely *reported* anything else, because the compile driver's `perry-objs--/` leak (#7167) was live and a gate that goes red for another module's defect gets muted rather than fixed. #7167 is closed, so the carve-out is gone and `classify()` with it. The gate now asserts the property it always wanted: an isolated TMPDIR is empty after the corpus compiles, full stop. No allowlist, deliberately. #7167 is the worked example of what one costs: it was known, printed on every run, and could not turn a run red. A gate that enumerates the leaks it may fail on cannot see the one nobody has written yet. The self-test asserts the flip itself — the `perry-objs-*` inputs that used to return 0 now return 1 — rather than letting it be implied by the absence of a list, and adds a name from neither family. Verified red under sabotage: re-introducing the filter fails the self-test at that assertion. Docs: `--no-link` now documents where it writes; the census README's A/B recipe no longer points both arms at one `-o` (which was safe only while `--no-link` ignored `-o`, and is a vacuous comparison now that it does not). Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- benchmarks/repsel_census/README.md | 39 ++-- .../7172-no-link-object-staging-dir.md | 97 ++++++++++ docs/src/cli/commands.md | 2 +- docs/src/cli/flags.md | 2 +- .../repsel_temp_hygiene.py | 180 +++++++++--------- 5 files changed, 217 insertions(+), 103 deletions(-) create mode 100644 changelog.d/7172-no-link-object-staging-dir.md diff --git a/benchmarks/repsel_census/README.md b/benchmarks/repsel_census/README.md index aa3a3b6f16..6791cde17c 100644 --- a/benchmarks/repsel_census/README.md +++ b/benchmarks/repsel_census/README.md @@ -74,20 +74,31 @@ of them is recorded at the site where the proof is dropped: reproducible without the report at all: compile the workload twice, once with `PERRY_PTR_SHAPE_LOCALS=0`, and compare the objects. -`--no-link` does **not** honour `-o`: the object goes to a per-run temp -directory and the path is printed. So capture the printed path in each arm and -compare those — comparing the `-o` arguments compares two files that were never -created. +`--no-link` writes its objects to `-o` (#7167): verbatim for a single-module +program, otherwise into `-o`'s directory under module-derived names. **Give each +arm its own `-o`.** Two arms pointed at one path is not a comparison — the +second compile overwrites the first and `cmp` then compares a file with itself, +which is "identical" for every arm forever. + +Read the paths back off stdout rather than assuming `-o` named them, because a +multi-module workload emits several and only one of them can be `-o`. An arm +that reported no object is a harness error, not a silent pass — the same reason +`_written_objects` in the census script raises. ```bash -obj() { # echo the object path this compile actually wrote +objs() { # echo every object path this compile actually wrote "$@" --no-link --no-cache 2>&1 | sed -n 's/^Wrote object file: //p' } -a=$(obj perry compile -o /tmp/ignored) -b=$(PERRY_PTR_SHAPE_LOCALS=0 obj perry compile -o /tmp/ignored) +a=$(objs perry compile -o "$PWD/ab/a.o") +b=$(PERRY_PTR_SHAPE_LOCALS=0 objs perry compile -o "$PWD/ab/b.o") cmp "$a" "$b" && echo "IDENTICAL — the promotion emitted nothing" ``` +Before #7167 the flag ignored `-o` entirely and left the objects in a +`perry-objs--/` directory under `TMPDIR` that nothing ever deleted, +which is why the older version of this recipe passed `-o /tmp/ignored` twice and +still worked. It does not any more, and the version above is the one to copy. + Byte-identical objects mean the promotions the report counted as wins changed nothing. `07_object_create` and `12_binary_trees` are byte-identical today. `09_method_calls` differs, but only by two `__pshape` clones with **zero call @@ -319,10 +330,16 @@ changing it: * **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. +It fails on **anything** left behind, with no allowlist. It shipped with one — +the clang driver's own names (`perry_llvm_*`, `perry_cgu_*`, `perry_bc_*`) +failed and everything else was merely reported — because the compile driver was +leaking a `perry-objs--/` staging directory on the `--no-link` path +at the time (#7167), and a gate that goes red for another module's defect gets +muted rather than fixed. #7167 closed that path and the carve-out went with it. + +The absence of an allowlist is the point. #7167 was *known*: this gate printed +it on every run and could not turn one red. A gate that enumerates the leaks it +is allowed to fail on cannot see the one nobody has written yet. 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** diff --git a/changelog.d/7172-no-link-object-staging-dir.md b/changelog.d/7172-no-link-object-staging-dir.md new file mode 100644 index 0000000000..9974536066 --- /dev/null +++ b/changelog.d/7172-no-link-object-staging-dir.md @@ -0,0 +1,97 @@ +Closes #7167: `perry compile --no-link` no longer leaks its object staging +directory into the system temp directory, and now writes its objects where +`-o` points. + +`run_pipeline.rs` created a per-invocation `perry-objs--/` +directory for every compile and removed it on the paths that *link*. +`--no-link` returns before those, so it removed nothing. Because the name +carries the pid **and** a wall-clock nanosecond component, no two invocations +ever reuse one: the leak is unbounded in **compiles**, not in distinct IR the +way #7144's `.ll` leak was, and the staged objects are far larger than the +`.ll`s. The machine this was written on had accumulated 3086 such directories +(277 MB). Every `--no-link` user was affected — the flag itself, the +separate-link workflow, and every harness in +`scripts/compiler_output_harness/` (the census, knob-isolation and determinism +gates all compile with `--no-link`), which is why running the representation +census bled gigabytes a day. + +**The objects could not simply be deleted.** On `--no-link` they are the +product: the flag is documented as "produce object file only", and the census +and knob-isolation gates hash the paths it prints (`_written_objects` raises +outright if a reported object does not exist on disk). What was wrong was +*where* they went, not that they survived. The flag also did not honour `-o` +at all — the census README carried a warning about it and a hand-written A/B +recipe built around the wart. + +Two structural changes rather than a third `remove_dir`: + +* **`--no-link` no longer creates a staging directory**, so it cannot leak + one. Its objects are delivered to `-o`: verbatim when the program has one + native module (`cc -c foo.c -o foo.o`), otherwise into `-o`'s directory + under the module-derived names, because one `-o` cannot name N files. With + no `-o` they land in the current directory. The rule keys on the module + count — a property of the program — rather than on how many objects codegen + actually wrote, so `-o` does not mean two different things depending on + whether the object cache was warm. Bitcode-link mode emits `.ll`, never + takes `-o` verbatim. +* **When linking, the staging directory is removed by `Drop`** (new + `crates/perry/src/commands/compile/object_staging.rs`), so both link exits, + the static-archive exit and every `?` in between clean up through one site. + Three call sites that must each remember is how the third came to be + missing. The default direction now matters: a future exit that does nothing + cleans up, where before it leaked. + +Removing the directory is unobservable to a concurrent compile, and that is a +property of the name rather than a timing argument: pid + monotonic nanos means +it belongs to exactly one invocation. This is the same conclusion #7144 reached +for the `.ll` by a different route — there the fix had to *create* per-call +ownership, because #7131 had made the `.ll` basename a pure function of the IR +and two workers holding identical IR shared it. Nothing is shared here. + +Two further leaks the same guard closed, both broader than #7167 described: + +* The **executable** link — the default path — never removed the directory at + all. `cleanup_intermediates` only unlinks files, so every successful + `perry compile` left an empty `perry-objs-*` directory behind. Empty, but one + per compile. +* The static-archive and shared-library exits used `remove_dir`, which is + non-recursive and silently no-ops when anything in the directory was not on + the cleanup list. `Drop` uses `remove_dir_all`. + +`--keep-intermediates` is still the single opt-in for retaining staged objects, +disarmed once where the directory is created rather than re-checked at each +exit. The codegen-failure paths now name the directory and say whether it +survives; on `--no-link` a failed compile keeps every object it managed to +write, at the path the user named. + +**Gate.** `census-temp-hygiene` (#7144) shipped with a carve-out: it failed on +the clang driver's own temp names and merely *reported* anything else, because +this leak was live at the time and a gate that goes red for another module's +defect gets muted rather than fixed. The carve-out is gone — the gate now +asserts the absolute property, that an isolated `TMPDIR` is empty after the +corpus compiles, with **no allowlist**. #7167 is the argument for that: it was +known, printed on every run, and could not turn a run red for a full release. +The self-test asserts the flip directly (`perry-objs-*` inputs that returned 0 +now return 1) and asserts that a name from neither family fails too. + +**Evidence.** Two arms built sequentially from one target dir, distinct binary +hashes, isolated `TMPDIR`, 27 census workloads × 2 compiles = 54: + +| arm | `perry-objs-*` entries left | harness exit | +|---|---|---| +| `main` | **108** (54 directories + 54 objects) | 1 | +| this branch | **0** | 0 | + +The absolute property, not "no growth" — #7144's lesson, and here growth would +have caught it, but on the next content-addressed leak it would not. + +No behavioural change: across all 27 census workloads the emitted objects are +**byte-identical** to `main`'s, and a linked two-module executable is +byte-identical and runs identically. That is structural rather than lucky — +`compile_ll_to_object` returns the object *bytes* and `run_pipeline` writes +them, so the staging path was never in the object. Verified directly: no +`perry-objs`/`perry_llvm` string and no `__debug_*` section appears in a Perry +Mach-O object, with or without `PERRY_DEBUG_SYMBOLS=1`. + +`census-determinism --repeat 3 --jobs 4` is 27/27 byte-identical on Darwin +arm64 on both arms. diff --git a/docs/src/cli/commands.md b/docs/src/cli/commands.md index ea3ff20b1c..3f719569b5 100644 --- a/docs/src/cli/commands.md +++ b/docs/src/cli/commands.md @@ -20,7 +20,7 @@ perry main.ts -o app | `--target ` | Platform target (see [Compiler Flags](flags.md)) | | `--output-type ` | `executable` (default) or `dylib` (plugin) | | `--print-hir` | Print HIR intermediate representation | -| `--no-link` | Produce object file only, skip linking | +| `--no-link` | Produce object file(s) only, skip linking; written to `-o` (see [Compiler Flags](flags.md)) | | `--keep-intermediates` | Keep `.o` and `.asm` files | | `--enable-js-runtime` | Enable V8 JavaScript runtime fallback | | `--enable-wasm-runtime` | Force-link the wasmi WebAssembly host runtime (auto-detected on `WebAssembly.*` use) | diff --git a/docs/src/cli/flags.md b/docs/src/cli/flags.md index b6b69441f7..537c7b72ca 100644 --- a/docs/src/cli/flags.md +++ b/docs/src/cli/flags.md @@ -99,7 +99,7 @@ accept either the `$perryfs/` virtual path or the embed-relative key. | `--print-hir` | Print HIR (intermediate representation) to stdout | | `--trace ` | Dump IR at one or more pipeline stages. Comma-separated: `hir` (post-transform HIR), `llvm` (per-module `.ll` into `.perry-trace/llvm/`), or `all` | | `--focus ` | Restrict `--trace hir` to functions/methods/classes whose name contains `NAME`, suppressing import/export/init noise. Implies `--trace hir` if no stage is given | -| `--no-link` | Produce `.o` object file only, skip linking | +| `--no-link` | Produce `.o` object file(s) only, skip linking. The objects are written to `-o` — verbatim for a single-module program, otherwise into `-o`'s directory under module-derived names, since one `-o` cannot name several files. With no `-o` they land in the current directory. Each path is printed as `Wrote object file: ` | | `--no-codegen` | Skip the `package.json` `perry.codegen` build-time steps (also `PERRY_SKIP_CODEGEN=1`). See [Project Configuration](../getting-started/project-config.md) | | `--keep-intermediates` | Keep `.o` and `.asm` intermediate files | | `--opt-report[=json]` | Report which values Perry could **not** statically type, why, and whether you can fix it. Text by default; `--opt-report=json` emits a stable schema for tooling. Also settable via `PERRY_OPT_REPORT=1` | diff --git a/scripts/compiler_output_harness/repsel_temp_hygiene.py b/scripts/compiler_output_harness/repsel_temp_hygiene.py index 9d47b722e0..555787bfda 100644 --- a/scripts/compiler_output_harness/repsel_temp_hygiene.py +++ b/scripts/compiler_output_harness/repsel_temp_hygiene.py @@ -1,5 +1,16 @@ -"""Temp-directory hygiene check (#7144). +"""Temp-directory hygiene check (#7144, #7167). +Compile the census corpus with `TMPDIR` pointed at an empty directory of our +own, then look in that directory. **It must be empty** — no allowlist, no +"known" leaks, nothing. + +It did not start that way. #7144 shipped this gate failing only on +`perry-codegen`'s clang-driver names and merely *reporting* everything else, +because a second leak was live at the time (#7167) and a gate that goes red for +another module's defect gets muted rather than fixed. #7167 closed that path +and the carve-out went with it. The two leaks it has caught: + +**#7144 — the clang driver's `.ll`.** `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 @@ -21,11 +32,22 @@ 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. +**#7167 — the compile driver's staged objects.** +`run_pipeline.rs` staged every emitted `.o` in a `perry-objs--/` +directory and removed it on the paths that *link*. `--no-link` returns before +those, so it removed nothing: one fresh directory plus its objects per compile, +unbounded in **compiles** rather than in distinct IR, and the objects are far +larger than the `.ll`s. 3086 such directories had accumulated on one dev box. +Every harness in this package compiles with `--no-link`, so running the census +was itself the heaviest source of the leak. -Two design notes, because both alternatives were tried and are wrong: +The fix was not a third `remove_dir`. On `--no-link` the objects are the +*product*, so they are delivered to `-o` and no staging directory is created at +all; when linking, the directory is removed by a `Drop` guard so every exit +cleans up through one site. See `crates/perry/src/commands/compile/ +object_staging.rs`. + +Design notes, because the 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 @@ -36,6 +58,10 @@ 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. +* **No allowlist.** Naming the leaks this gate is allowed to fail on means the + next leak — under a name nobody has written yet — passes silently. #7167 is + the worked example: it was known, printed on every run, and could not turn a + run red. `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 @@ -72,35 +98,6 @@ #: 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. @@ -134,65 +131,53 @@ def verdict( "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") + if not leftovers: 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." + f"Temp directory is clean: {compiles} compile(s) left 0 entries " + "behind. Neither the #7144 nor the #7167 leak is present." ) return 0 - shown = owned[:MAX_REPORTED] + shown = leftovers[: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 " + f"TEMP FILES LEAKED: {compiles} compile(s) left {len(leftovers)} " + f"entr{'y' if len(leftovers) == 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") + if len(leftovers) > len(shown): + printer(f" … and {len(leftovers) - 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" + " Nothing may survive a compile in the temp directory. Two known\n" + " leaks produced this failure before; the name above says which, and\n" + " a THIRD name means a new one.\n" + "\n" + " `perry_llvm_*` / `perry_cgu_*` / `perry_bc_*` — #7144, the clang\n" + " driver. The `.ll` handed to `clang -c` is content-addressed (#7131:\n" + " clang records its basename into the ELF object), so the leftovers\n" + " are bounded by DISTINCT IR EVER COMPILED, not by compiles — a\n" + " repeat-and-compare check stays green while a developer machine fills\n" + " up. 1627 files / 951.8 MB after a day; 29 GB on a longer-lived box.\n" + " The fix was not a more careful unlink (that races a sibling worker\n" + " holding the same IR, which is why #7135 stopped deleting at all) but\n" + " to stop sharing: `crates/perry-codegen/src/linker.rs` gives each\n" + " compile a private scratch directory. `PERRY_DEBUG_SYMBOLS` is not an\n" + " exemption — measured, `-g` emits no DWARF from a Perry `.ll` at all.\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" + " `perry-objs-*` — #7167, the compile driver. `run_pipeline.rs` staged\n" + " objects in a per-invocation temp directory and removed it on the link\n" + " exits only, so every `--no-link` compile leaked one, unbounded in\n" + " COMPILES. The fix was to stop creating it: on `--no-link` the objects\n" + " are the product and go to `-o`, and when linking the directory is\n" + " removed by `Drop` so no exit has to remember.\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." + " Anything else is a new leak. This gate has no allowlist on purpose:\n" + " #7167 was known, printed on every run, and could not turn the run\n" + " red for a full release. A gate that names its exceptions cannot see\n" + " the leak nobody has written yet." ) return 1 @@ -257,6 +242,8 @@ def self_test(_args: argparse.Namespace) -> int: quiet: Callable[[str], None] = lambda _line: None assert verdict([], compiles=52, printer=quiet) == 0 + + # #7144's family — the clang driver's own names. 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 @@ -265,19 +252,25 @@ def self_test(_args: argparse.Namespace) -> int: 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 + # #7167's family. These used to return 0 — reported, not failed — while the + # compile driver's `--no-link` path was still leaking them. The flip from 0 + # to 1 IS the widening, so it is asserted directly rather than implied by + # the absence of an allowlist. + assert verdict(["perry-objs-9-1"], compiles=1, printer=quiet) == 1 + assert verdict(["perry-objs-9-1/m.o"], compiles=1, printer=quiet) == 1 lines: list[str] = [] - assert verdict(["perry-objs-9-1/m.o"], compiles=1, printer=lines.append) == 0 + verdict(["perry-objs-9-1/m.o"], compiles=1, printer=lines.append) 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 "#7167" in joined and "run_pipeline.rs" in joined, joined - assert classify(["perry_llvm_a.ll", "perry-objs-9-1/m.o"]) == ( - ["perry_llvm_a.ll"], - ["perry-objs-9-1/m.o"], - ) + # A name from neither family must fail too. This is the case an allowlist + # cannot cover, and the reason there is no allowlist: the next leak has a + # name nobody has written yet. + assert verdict(["perry-embed-4242/bundle.o"], compiles=1, printer=quiet) == 1 + assert verdict(["something-nobody-has-written-yet"], compiles=1, printer=quiet) == 1 + lines = [] + verdict(["brand-new-leak-name"], compiles=1, printer=lines.append) + assert "new leak" in "\n".join(lines) # A run that compiled nothing finds an empty directory for the wrong # reason. It must not be able to report success. @@ -291,7 +284,14 @@ def self_test(_args: argparse.Namespace) -> int: 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"): + for expected in ( + "#7144", + "#7131", + "PERRY_DEBUG_SYMBOLS", + "linker.rs", + "#7167", + "run_pipeline.rs", + ): assert expected in report, f"failure report must mention {expected}: {report}" # The truncation must announce itself rather than quietly dropping paths. From c2ab6c7c99347858a83e17b7ff9d10c9378f6c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 10:12:41 +0200 Subject: [PATCH 3/5] fix(compile): honour -o on --no-link with the object cache enabled too (#7167) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The destination rule was keyed on the module count precisely so `-o` would not mean two different things depending on cache warmth — and then two cache paths bypassed it anyway, because both hand back the *cache entry's* path instead of the object they were asked to produce: cold (store) : "Stored cached object: /objects/host/.o" -o unwritten warm (hit) : "Reused cached object: /objects/host/.o" -o unwritten Harmless for a compile that links — the linker is the only reader, and the bytes are the bytes. Wrong for `--no-link`, which promised the user a file. A hit now copies the cached object out to the destination (copy, not hand back the path: the cache entry is shared with every other build and must not become an output the user may overwrite or delete). A store keeps storing — a later build still hits — but falls through to write the object it just produced. Both are labelled `Wrote object file`, so the census harness's `_written_objects`, which scrapes exactly those lines, sees a warm-cache compile at all instead of finding no objects. Verified: cold and warm both write `-o`, byte-identical, with zero perry-objs entries either way. On main neither wrote it and the leak grew per compile with the cache on. Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- .../src/commands/compile/run_pipeline.rs | 66 +++++++++++++++---- 1 file changed, 55 insertions(+), 11 deletions(-) diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index eb25e81485..d9f047e535 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -4264,11 +4264,35 @@ pub fn run_with_parse_cache( // `_js_ws_connect_start`. Replaying the manifest makes a hit // record exactly what the skipped codegen would have. perry_codegen::ext_registry::replay_ffi_symbols(&ffi_symbols); + // #7167: `-o` must name the same file whether the object cache + // was warm or cold. A linking compile can hand the linker the + // cache path directly — the bytes are the bytes and nothing + // else looks at them — but `--no-link` promised the user a file + // at `-o`, so a hit has to materialise one there too. Copy + // rather than hand back the cache path: the cache entry is + // shared with every other build and must not become an output + // the user may overwrite or delete. + let hit_path = if args.no_link { + fs::copy(&cached_path, &obj_path).map_err(|e| { + format!( + "failed to write cached object to {}: {}", + obj_path.display(), + e + ) + })?; + obj_path + } else { + cached_path + }; return Ok(NativeObjectArtifact { - path: cached_path, + path: hit_path, bytes: None, fingerprint: format!("cache:{:016x}", key), cleanup_after_link: false, + // The *cache* was reused either way — that is what the + // hit/miss stats and the `--keep-intermediates` exemption + // are about. The label printed below keys on whether the + // path we are reporting is the cache's or the user's. reused_cache_path: true, stored_cache_path: false, }); @@ -4342,20 +4366,34 @@ pub fn run_with_parse_cache( object_cache.store_ffi_manifest(k, &emitted_ffi_symbols); object_cache.store_and_get_path(k, &object_code) }) { - return Ok(NativeObjectArtifact { - path: cached_path, - bytes: None, - fingerprint: object_fingerprint, - cleanup_after_link: false, - reused_cache_path: false, - stored_cache_path: true, - }); + // #7167: handing back the cache path saves a copy for a + // compile that is going to *link* — the linker is the only + // reader and the bytes are the bytes. `--no-link` promised the + // user a file at `-o`, so it keeps the store (a later build + // still hits) but falls through to write the object it just + // produced to the destination. Otherwise `-o` would be honoured + // only with the cache disabled, which is the cache-warmth + // dependence this destination rule exists to avoid. + if !args.no_link { + return Ok(NativeObjectArtifact { + path: cached_path, + bytes: None, + fingerprint: object_fingerprint, + cleanup_after_link: false, + reused_cache_path: false, + stored_cache_path: true, + }); + } } Ok(NativeObjectArtifact { path: obj_path, bytes: Some(object_code), fingerprint: object_fingerprint, - cleanup_after_link: true, + // #7167: a staged object is an intermediate; a `--no-link` + // object is the product and must never be listed for cleanup. + // Nothing consults this before the `--no-link` return today — + // it is set correctly so that stays true if cleanup ever moves. + cleanup_after_link: !args.no_link, reused_cache_path: false, stored_cache_path: false, }) @@ -4434,7 +4472,13 @@ pub fn run_with_parse_cache( for artifact in artifacts { match format { OutputFormat::Text => { - let label = if artifact.reused_cache_path { + // #7167: on `--no-link` a cache hit is copied out to `-o`, so + // the path being reported is a file this compile wrote, not + // the cache entry. Say so — "Reused cached object" would name + // a path the caller cannot treat as its output, and the census + // harness's `_written_objects` (which scrapes exactly these + // lines) would see no objects at all from a warm cache. + let label = if artifact.reused_cache_path && !args.no_link { "Reused cached object" } else if artifact.stored_cache_path { "Stored cached object" From 2184fc269a1977ec9c7e56a3be25e7375e2cc2a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 10:15:46 +0200 Subject: [PATCH 4/5] docs(changelog): fold the object-cache paths into the #7167 fragment Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- changelog.d/7172-no-link-object-staging-dir.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/changelog.d/7172-no-link-object-staging-dir.md b/changelog.d/7172-no-link-object-staging-dir.md index 9974536066..35c7c8cadc 100644 --- a/changelog.d/7172-no-link-object-staging-dir.md +++ b/changelog.d/7172-no-link-object-staging-dir.md @@ -34,6 +34,20 @@ Two structural changes rather than a third `remove_dir`: actually wrote, so `-o` does not mean two different things depending on whether the object cache was warm. Bitcode-link mode emits `.ll`, never takes `-o` verbatim. + + Both object-cache paths had to be closed for that last property to be true. + A cold *store* and a warm *hit* each handed back the cache entry's path + (`Stored cached object:` / `Reused cached object:`) instead of the object the + user asked for, so `-o` went unwritten with the cache on. Harmless when + linking — the linker is the only reader — but not for `--no-link`. A hit now + copies the cached object out to the destination (copy, not hand back the + path: the cache entry is shared with every other build and must not become an + output the user may overwrite); a store keeps storing, so later builds still + hit, but falls through to write the object it just produced. Both report + `Wrote object file`, which also means the census harness's + `_written_objects` — which scrapes exactly those lines — sees a warm-cache + compile at all instead of finding no objects. Verified: cold and warm both + write `-o`, byte-identical. * **When linking, the staging directory is removed by `Drop`** (new `crates/perry/src/commands/compile/object_staging.rs`), so both link exits, the static-archive exit and every `?` in between clean up through one site. From 391eb554335642ac6a2bccb8722d74dae55886f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 1 Aug 2026 10:17:25 +0200 Subject: [PATCH 5/5] docs(changelog): key the fragment on the PR number (#7175) Claude-Session: https://claude.ai/code/session_018ZFER8EEg8K7ez2n6oDrT9 --- ...k-object-staging-dir.md => 7175-no-link-object-staging-dir.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename changelog.d/{7172-no-link-object-staging-dir.md => 7175-no-link-object-staging-dir.md} (100%) diff --git a/changelog.d/7172-no-link-object-staging-dir.md b/changelog.d/7175-no-link-object-staging-dir.md similarity index 100% rename from changelog.d/7172-no-link-object-staging-dir.md rename to changelog.d/7175-no-link-object-staging-dir.md