fix(compile): stop leaking the object staging dir on --no-link, deliver its objects to -o (#7167) - #7175
Merged
Merged
Conversation
added 5 commits
August 1, 2026 09:53
`run_pipeline.rs` created a `perry-objs-<pid>-<nanos>/` 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
…llowlist (#7167) 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-<pid>-<nanos>/` 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
#7167) 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: <cache>/objects/host/<key>.o" -o unwritten warm (hit) : "Reused cached object: <cache>/objects/host/<key>.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
📝 WalkthroughWalkthroughThe compiler now writes ChangesNo-link object staging
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CompileCommand
participant run_pipeline
participant NoLinkDestination
participant StagingDir
participant Filesystem
CompileCommand->>run_pipeline: compile with link or no-link mode
run_pipeline->>NoLinkDestination: resolve and prepare no-link output
run_pipeline->>StagingDir: create staging directory for linking
run_pipeline->>Filesystem: write or copy object artifact
StagingDir->>Filesystem: recursively remove staged objects on drop
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
benchmarks/repsel_census/README.md (1)
92-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCreate the output directory before writing to it.
The recipe writes to
"$PWD/ab/a.o"and"$PWD/ab/b.o"without creatingabfirst. Addmkdir -pso the recipe works standalone when copied.📝 Proposed fix
+mkdir -p ab a=$(objs perry compile <src> -o "$PWD/ab/a.o") b=$(PERRY_PTR_SHAPE_LOCALS=0 objs perry compile <src> -o "$PWD/ab/b.o")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@benchmarks/repsel_census/README.md` around lines 92 - 93, Update the benchmark recipe containing the `a` and `b` compile commands to create the `ab` output directory with `mkdir -p` before either compilation writes its object file, so the recipe works standalone.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmarks/repsel_census/README.md`:
- Around line 83-95: Update the README’s objs comparison example so multi-object
outputs are handled element by element rather than passing multiline captures as
single cmp arguments. Preserve the existing Wrote object file parsing and
no-output harness-error behavior, then iterate over corresponding paths from a
and b and compare each pair directly.
In `@changelog.d/7175-no-link-object-staging-dir.md`:
- Around line 58-60: Update the changelog text describing staging-directory
uniqueness to say it uses wall-clock nanoseconds rather than monotonic nanos,
matching the timestamp source in object staging and preserving the existing
pid-plus-timestamp explanation.
In `@crates/perry/src/commands/compile/run_pipeline.rs`:
- Around line 2069-2098: Update the stub-object emission flow associated with
`_perry_stubs.o` and `_perry_failed_stubs.o` so `--no-link` does not
unconditionally write these link-only artifacts into `no_link_destination`.
Guard their generation using the existing `args.no_link` state, while preserving
normal stub emission for linking compilations; alternatively, explicitly
document and justify them as intentional `--no-link` product files if that is
the required behavior.
In `@docs/src/cli/commands.md`:
- Line 23: Update the --no-link entry in the command overview to state that
object files are written to the path specified by -o, or to the current
directory when -o is omitted; preserve the existing compiler-flags reference.
---
Nitpick comments:
In `@benchmarks/repsel_census/README.md`:
- Around line 92-93: Update the benchmark recipe containing the `a` and `b`
compile commands to create the `ab` output directory with `mkdir -p` before
either compilation writes its object file, so the recipe works standalone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 79c8badb-22c8-4e24-897d-888b21dbd6af
📒 Files selected for processing (8)
benchmarks/repsel_census/README.mdchangelog.d/7175-no-link-object-staging-dir.mdcrates/perry/src/commands/compile.rscrates/perry/src/commands/compile/object_staging.rscrates/perry/src/commands/compile/run_pipeline.rsdocs/src/cli/commands.mddocs/src/cli/flags.mdscripts/compiler_output_harness/repsel_temp_hygiene.py
Comment on lines
+83
to
95
| 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 <src> -o /tmp/ignored) | ||
| b=$(PERRY_PTR_SHAPE_LOCALS=0 obj perry compile <src> -o /tmp/ignored) | ||
| a=$(objs perry compile <src> -o "$PWD/ab/a.o") | ||
| b=$(PERRY_PTR_SHAPE_LOCALS=0 objs perry compile <src> -o "$PWD/ab/b.o") | ||
| cmp "$a" "$b" && echo "IDENTICAL — the promotion emitted nothing" | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm the literal stdout format the compile driver uses when
# it writes an object file for --no-link.
set -euo pipefail
rg -n 'Wrote object file' crates/perry/src/commands/compile/Repository: PerryTS/perry
Length of output: 246
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== run_pipeline.rs relevant block =="
sed -n '4460,4505p' crates/perry/src/commands/compile/run_pipeline.rs
echo
echo "== README excerpt =="
sed -n '75,100p' benchmarks/repsel_census/README.md
echo
echo "== shell behavior probe for multi-line captured output =="
python3 - <<'PY'
from subprocess import run, PIPE
objs = "Wrote object file: /tmp/ab/a.o\nWrote object file: /tmp/ab/b.o\n"
a = objs
b = objs.rstrip("\n")
r = run(["cmp", "-s"], input=f"{a}\n{b}\n".encode(), text=False, stderr=PIPE, stdin=PIPE)
print("shell_cmp_eq_rc=", r.returncode, r.stderr.decode(errors="replace").splitlines(True))
a_file = "/tmp/this-file-contains-a-newline-obj\npath"
b_file = "/tmp/this-file-contains-a-newline-obj\npath"
try:
r2 = run(["cmp", a_file, b_file], stderr=PIPE)
print("shell_cmp_with_newlines_rc=", r2.returncode, r2.stderr.decode(errors="replace").splitlines(True))
except Exception as e:
print("exception_with_newlines:", type(e).__name__, e)
PYRepository: PerryTS/perry
Length of output: 3923
🏁 Script executed:
#!/bin/bash
set -euo pipefail
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
echo "== compiled perry binary =="
find . -maxdepth 4 -type f -name perry* -not -path './target/*' | head -20
echo
echo "== shell probe: sed output from real driver text section =="
perry="$(find . -maxdepth 4 -type f \( -name perry -o -name perry.exe \) | head -1)"
if [ -n "$perry" ] && [ -x "$perry" ]; then
"$perry" compile --help >/tmp/help.txt 2>&1 || true
sed -n '/This tool is a/,/Usage:/p' /tmp/help.txt >/tmp/driver_text.txt || true
sed -n '1,80p' /tmp/driver_text.txt
echo
echo "$perry compile foo.perry --no-link --no-cache -o "$tmpdir/a.o" 2>&1 | sed -n 's/^Wrote object file: //p'"
"$perry" compile foo.perry --no-link --no-cache -o "$tmpdir/a.o" 2>&1 | sed -n 's/^Wrote object file: //p'
echo
echo "cmp with newlines in variable =="
a="$(printf 'Wrote object file: %s\nWrote object file: %s\n' "$tmpdir/a.o" "$tmpdir/b.o")"
b="$(printf 'Wrote object file: %s\nWrote object file: %s\n' "$tmpdir/a.o" "$tmpdir/b.o")"
cmp "$a" "$b"; rc=$?
echo "cmp_rc=$rc"
else
echo "No executable perry binary found in repository tree; cannot probe runtime shell behavior."
fi
echo
echo "== exact stdout label text in Rust source =="
rg -n 'println!\("\{:20\}\{:20\}"|"Perry Version"|"perry compile"|"Wrote object file"|Wrote object file' crates/perry/src/commands/compile/run_pipeline.rsRepository: PerryTS/perry
Length of output: 868
Avoid comparing multi-line captured stdout as file paths.
The Wrote object file: label matches the compile driver text output, but objs returns every emitted object path. If a workload emits multiple objects, cmd.exe captures all lines in $a and $b, then cmp "$a" "$b" gets one argument instead of comparing paths at the same position or files directly. Iterate through the corresponding paths from each arm before comparing them.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@benchmarks/repsel_census/README.md` around lines 83 - 95, Update the README’s
objs comparison example so multi-object outputs are handled element by element
rather than passing multiline captures as single cmp arguments. Preserve the
existing Wrote object file parsing and no-output harness-error behavior, then
iterate over corresponding paths from a and b and compare each pair directly.
Comment on lines
+58
to
+60
| 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 |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'perry-objs|SystemTime|Instant|UNIX_EPOCH' crates/perry/src/commands/compileRepository: PerryTS/perry
Length of output: 19453
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '8,14p;55,62p' changelog.d/7175-no-link-object-staging-dir.mdRepository: PerryTS/perry
Length of output: 1162
Use wall-clock nanoseconds consistently.
crates/perry/src/commands/compile/object_staging.rs builds the staging directory name with SystemTime::now().duration_since(UNIX_EPOCH).as_nanos(), not Instant::now(). Update lines 58-60 to keep the changelog consistent with the lines above.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@changelog.d/7175-no-link-object-staging-dir.md` around lines 58 - 60, Update
the changelog text describing staging-directory uniqueness to say it uses
wall-clock nanoseconds rather than monotonic nanos, matching the timestamp
source in object staging and preserving the existing pid-plus-timestamp
explanation.
Comment on lines
+2069
to
2098
| // 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<object_staging::StagingDir> = 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() | ||
| } | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether stub-object generation is guarded by --no-link,
# and whether CLI docs mention stub files as part of --no-link output.
set -euo pipefail
echo "=== stub-generation block context ==="
rg -n -B5 -A5 '_perry_stubs\.o|_perry_failed_stubs\.o' crates/perry/src/commands/compile/run_pipeline.rs
echo "=== check for any no_link guard near stub generation ==="
rg -n -B30 '_perry_stubs\.o' crates/perry/src/commands/compile/run_pipeline.rs | rg -n 'no_link'
echo "=== docs mentioning --no-link output contents ==="
fd -e md . docs/src/cli | xargs rg -n -i 'no-link|no_link' || trueRepository: PerryTS/perry
Length of output: 1239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== run_pipeline outlines around relevant symbols ==="
ast-grep outline crates/perry/src/commands/ compile/run_pipeline.rs --match 'run_pipeline' --view expanded || true
echo "=== object_output_dir and early return context ==="
sed -n '2060,2105p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '5280,5300p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '5700,5715p' crates/perry/src/commands/compile/run_pipeline.rs
echo "=== stub-generation block context ==="
sed -n '4880,4945p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '5240,5285p' crates/perry/src/commands/compile/run_pipeline.rs
echo "=== object_staging docs/types ==="
fd -a 'object_staging\.rs$' . | xargs -r sed -n '1,240p'
echo "=== search for cleanup_after_link and obj_cleanup_paths handling ==="
rg -n 'cleanup_after_link|obj_cleanup_paths|_perry_stubs\.o|_perry_failed_stubs\.o' crates/perry/src/commands/compile/run_pipeline.rs crates/perry/src/commands/compile/object_staging.rsRepository: PerryTS/perry
Length of output: 22515
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== run pipeline file top and compilation phases around obj paths ==="
sed -n '440,485p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '5284,5330p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '5484,5515p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '5688,5712p' crates/perry/src/commands/compile/run_pipeline.rs
sed -n '6204,6220p' crates/perry/src/commands/compile/run_pipeline.rs
echo "=== no-link behavior tests and relevant CLI docs ==="
fd -e rs . crates/perry | xargs rg -n 'no_link|no-link|single_module_no_link|_perry_stubs|_perry_failed_stubs|stub' || true
sed -n '1,220p' crates/perry/src/commands/compile/object_staging.rs
# Read-only model of the relevant invariant: stubs append to object_output_dir unconditionally;
# no-link destination points at user output when args.no_link.
python3 - <<'PY'
from pathlib import Path
p = Path('crates/perry/src/commands/compile/run_pipeline.rs')
s = p.read_text()
checks = {
'stub_missing_unconditionally': "let stub_path = object_output_dir.join(\"_perry_stubs.o\");" in s and 'obj_cleanup_paths.push(stub_path.clone());' in s,
'stub_failed_unconditionally': "let stub_path = object_output_dir.join(\"_perry_failed_stubs.o\");" in s and 'obj_cleanup_paths.push(stub_path.clone());' in s,
'no_link_early_return': 'if args.no_link {' in s and 'return Ok(CompileResult { output_path: exe_path,' in s,
'no_link_destination_used': 'let object_output_dir: PathBuf = match &object_staging { Some(staging) => staging.path().to_path_buf(), None => {\n no_link_destination.prepare()?;\n no_link_destination.dir().to_path_buf()\n }' in s,
}
for k, v in checks.items():
print(f'{k}: {v}')
PYRepository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== generate_stub_object stub generation contents ==="
rg -n 'generate_stub_object|generate_stub_object_full' crates/perry-codegen/src crates/perry-runtime/src | head -100
echo "=== focused stub generation implementations ==="
rg -n -B10 -A30 'pub.*fn generate_stub_object|generate_stub_object_full' crates/perry-codegen/src crates/perry-runtime/src | head -220
echo "=== references to generated stub filenames in compiler ==="
rg -n '_perry_stubs\.o|_perry_failed_stubs\.o' crates/perry-codegen crates/perry-runtime crates/perry/src/commands/compile crates/perry/tests crates/perry/src/main.rsRepository: PerryTS/perry
Length of output: 7041
Check whether --no-link should generate _perry_stubs.o and _perry_failed_stubs.o.
For --no-link, object_output_dir is the user’s output directory, and these stub objects are written unconditionally into that path before the --no-link return. The generated stubs exist because missing imports or failed modules will not be linkable unless the rest of the link injects runtime definitions. Skip stub-object emission for --no-link, or document and justify these extra user-visible artifacts as intentional product files.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry/src/commands/compile/run_pipeline.rs` around lines 2069 - 2098,
Update the stub-object emission flow associated with `_perry_stubs.o` and
`_perry_failed_stubs.o` so `--no-link` does not unconditionally write these
link-only artifacts into `no_link_destination`. Guard their generation using the
existing `args.no_link` state, while preserving normal stub emission for linking
compilations; alternatively, explicitly document and justify them as intentional
`--no-link` product files if that is the required behavior.
| | `--output-type <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)) | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Document the no--o fallback in the command overview.
Line 23 says that objects are written to -o. This can imply that -o is required. The implementation writes objects to the current directory when -o is omitted.
Clarify the fallback in this entry.
Proposed wording
-| `--no-link` | Produce object file(s) only, skip linking; written to `-o` (see [Compiler Flags](flags.md)) |
+| `--no-link` | Produce object file(s) only, skip linking; write to `-o` when supplied, otherwise the current directory (see [Compiler Flags](flags.md)) |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `--no-link` | Produce object file(s) only, skip linking; written to `-o` (see [Compiler Flags](flags.md)) | | |
| | `--no-link` | Produce object file(s) only, skip linking; write to `-o` when supplied, otherwise the current directory (see [Compiler Flags](flags.md)) | |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/src/cli/commands.md` at line 23, Update the --no-link entry in the
command overview to state that object files are written to the path specified by
-o, or to the current directory when -o is omitted; preserve the existing
compiler-flags reference.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #7167.
run_pipeline.rscreated aperry-objs-<pid>-<nanos>/staging directory onevery compile and removed it on the paths that link.
--no-linkreturnsbefore those, so it removed nothing.
The name carries the pid and a wall-clock nanosecond component, so no two
invocations ever reuse one: unlike #7144's
.ll, this is unbounded incompiles, not in distinct IR, and the staged objects are much larger. The
machine this was written on held 3086 of them, 277 MB. Every harness in
scripts/compiler_output_harness/compiles with--no-link, so running therepresentation census was itself the heaviest source of the leak.
The objects could not just be deleted
--no-linkis documented as "produce object file only". Its objects are theproduct: the census, knob-isolation and determinism gates hash the paths it
prints, and
_written_objectsraises outright if a reported object is not ondisk. Deleting them would have satisfied the hygiene gate and broken three
others.
What was wrong was where they went. The flag did not honour
-oat all — thecensus README carried a warning about it and a hand-written A/B recipe built
around the wart.
Two structural changes, not a third
remove_dir--no-linkno longer creates a staging directory, so it cannot leak one.Its objects are delivered to
-o:-o-o(cc -c foo.c -o foo.o)-o's directory, module-derived names — one-ocannot name N filesThe rule keys on the module count — a property of the program — not on how
many objects codegen actually wrote, so
-odoes not mean two different thingsdepending on cache warmth. Both object-cache paths had to be closed for that to
be true: a cold store and a warm hit each handed back the cache entry's path
instead of the object the user asked for, so
-owent unwritten with the cacheon. A hit now copies 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 delete); a store keeps storing but falls through to write what it just
produced. Both report
Wrote object file, so_written_objectssees awarm-cache compile at all instead of finding no objects. Bitcode-link mode emits
.lland never takes-overbatim.When linking, the staging directory is removed by
Drop(newcrates/perry/src/commands/compile/object_staging.rs), so both link exits, thestatic-archive exit and every
?in between clean up through one site. Thedefault direction is what matters: a future exit that does nothing now cleans
up, where before it leaked.
Removal is unobservable to a concurrent compile as a property of the name —
pid + monotonic nanos means one invocation owns it — not as a timing argument.
Same conclusion #7144 reached for the
.llby the opposite route: there the fixhad to create per-call ownership, because #7131 had made the
.llbasename apure function of the IR and identical-IR workers shared it. Nothing is shared
here.
The issue understates the blast radius — two more leaks the guard closed
#7167 says the directory "is removed on both link exits". Measured, neither
claim fully holds:
cleanup_intermediatesonly unlinks files, so every successfulperry compileleft an emptyperry-objs-*directory behind. Empty, but oneper compile, and unbounded the same way. Reproduced on
main: 24 concurrentsuccessful executable links leave 24 directories; on this branch, 0.
remove_dir, which isnon-recursive and silently no-ops if anything in the directory was not on the
cleanup list.
Dropusesremove_dir_all, andan_unexpected_file_does_not_defeat_cleanuppins that.Relationship to #7168 (merged as 3251d32)
Rebased onto it. No conflict. We share
benchmarks/repsel_census/README.mdin far-apart hunks (its new temp-hygiene section at the end, my
-oparagraphnear the top) and
repsel_temp_hygiene.py, where the widening below is the flipits own comment asked for.
The temp paths are disjoint by construction — both direct children of
TMPDIR,neither nested in the other:
remove_dir_allon one cannot reach the other, and #7168's scratch is createdand destroyed inside the rayon codegen closure, long before this
Dropruns.Confirmed rather than assumed: after the corpus compiles under an isolated
TMPDIR, 0 entries of either kind remain.Gate widened, allowlist deleted
census-temp-hygieneshipped with a carve-out — it failed onperry_llvm_*/perry_cgu_*/perry_bc_*and merely reported everything else,because this leak was live and a gate that goes red for another module's defect
gets muted rather than fixed. The carve-out is gone, and
classify()with it.That carve-out was not theoretical. Run main's own gate against main's own
compiler, today, with #7168 already merged:
It prints "clean" and exits 0 with 108 entries sitting in the directory —
CLAUDE.md failure mode 4, in the gate's own output. No allowlist now,
deliberately: a gate that enumerates the leaks it may fail on cannot see the one
nobody has written yet.
The self-test asserts the flip directly — the
perry-objs-*inputs thatreturned 0 now return 1 — rather than letting it be implied by the absence of a
list, and adds names from neither family (
perry-embed-*,brand-new-leak-name). Verified red under sabotage: re-introducing the filterfails the self-test at exactly that assertion.
Red then green
Two arms built sequentially from one
CARGO_TARGET_DIR(-p perry -p perry-runtime-static -p perry-stdlib-staticon both hops), distinct binaryhashes
f3d63800…/44a4b85d…, isolatedTMPDIR, 27 workloads x 2 = 54compiles.
census-temp-hygiene(this PR's gate)4cede62f6(main)The absolute property, not "no growth". Growth would have caught this one
(names are unique per compile), but #7144's was content-addressed and
repeat-and-compare was green on the broken compiler — so the gate asserts what
must be true rather than what happened to move.
No behavioural change
census-determinism --repeat 3 --jobs 4census-knob-isolationcensus(ratcheted floor)42perryprocesses,--no-link, x3perryprocesses, full link, x3object_stagingunit testsByte-identity is structural, not luck:
compile_ll_to_objectreturns theobject bytes and
run_pipelinewrites them, so the staging path was never inthe object. Verified directly on a Perry Mach-O object — no
perry-objsorperry_llvmstring anywhere in it, and no__debug_*section, with or withoutPERRY_DEBUG_SYMBOLS=1.On the macOS
--no-linkpath drift reported via #7171I was asked to check whether the
--no-linkoutput path lands in Mach-O debugrecords, making a double compile differ by ~2 KB. It does not reproduce here,
at this HEAD or on main.
census-determinism --repeat 3 --jobs 4is 27/27byte-identical on Darwin arm64 on both arms; a hand double-compile of
fixture_ptr_shapeis byte-identical with and withoutPERRY_DEBUG_SYMBOLS=1; and the object contains no temp-path string and nodebug section at all (
otool -llists__text __cstring __const __literal8 __bss __compact_unwind __eh_frameand nothing else). Consistent with #7168'sfinding that Perry IR carries no DI metadata. So object-hash A/Bs are valid on
macOS and the IR-comparison workaround #7171's agent adopted is not needed for
this reason. No issue filed: I have no reproduction to attach, and filing one
from a second-hand symptom I measured as absent would be noise. If #7171 still
has the failing pair, the delta is somewhere other than the
--no-linkdestination path — which this PR now proves is not in the bytes.
Failure policy
--no-link: a failed compile keeps every object it managed to write, at thepath the user named. Nothing deletes them on any path (
cleanup_after_linkisnow
!no_linkso that stays true if cleanup ever moves earlier).message now names it and says whether it survived.
--keep-intermediatesremains the single, already-documented opt-in,disarmed once where the directory is created rather than re-checked at each
exit. Deliberately not a second retention mode — what diagnosing a codegen
failure needs is the IR, which fix(codegen): give each .ll compile a private temp dir so it can be deleted again (#7144) #7168 and
PERRY_LLVM_KEEP_IRretain, and anescape hatch nobody exercises is a configuration nobody has verified.
Not measured / caveats
--no-link -o <existing directory>now fails with "failed to write objectfile" instead of silently succeeding and writing nothing.
-onames a filehere exactly as on the link path; not special-cased, because a rule that reads
the filesystem makes
-omean different things on different machines.perry-embed-<pid>/(the embedded-JS object directory, same file) is asmaller leak of the same family — bounded by pid rather than per invocation,
and only reachable with JS modules, which no census workload has. Left alone
rather than folded in; the widened gate fails on it the day a JS workload
joins the corpus, which is the right time to fix it.
PathBufthroughout and the unit testsare host-agnostic, but no Windows run was made.
--profile perry-dev, not--release. The property undertest is where the driver puts files, which its own optimization level does not
reach; the byte-identical-objects result is the direct evidence that nothing
else moved.
scripts/check_file_size.shandscripts/addr_class_inventory.pyare red onorigin/mainindependently of this PR (16 over-cap files and a band literal inperry-runtime/.../constants.rs, none touched here). Verified by running bothat
origin/main. Judge this PR'slintagainst main's.Summary by CodeRabbit
New Features
--no-linknow supports multiple object-file outputs, placing them at the specified-opath or the current directory by default.Bug Fixes
Documentation
--no-linkdocumentation to explain multi-file output behavior and destination handling.