feat(codegen): make the in-process LLVM backend the default, statically linked - #7353
Merged
Conversation
…ly linked Perry now links LLVM 22 statically and ships self-contained. We own the assumption rather than pushing it onto the user, and there is no "install a compatible clang" step left to get wrong. It is load-bearing, not a preference. The explicit statepoint bridge is gone (#7348), so RS4GC is the only native-root backend, and RS4GC cannot round-trip its IR through an external `opt` plus a different clang (#7339). Keeping this opt-in meant the only working statepoint path was behind a flag nobody sets. Two defaults flip together, because either alone is half a feature: * `llvm-inprocess` becomes a default cargo feature. * `inprocess_requested()` defaults to ON -- but only iff the backend is actually compiled in. Defaulting to `true` unconditionally would route every compile in a `--no-default-features` build into the not-built-in stub and fail it outright. Verified both ways. `PERRY_LLVM_INPROCESS=0` reverts to the clang subprocess for bisection, and `--no-default-features` still builds the text path. CI: a new `.github/actions/setup-llvm22` composite action, referenced from all 44 toolchain steps across 18 workflows. One definition rather than 44 inline recipes, because the three platforms need three different sources and only one is obvious -- Ubuntu 24.04's own llvm-dev is 18, and chocolatey's `llvm` is the clang toolchain with no llvm-config.exe and none of the static libs. Every arm asserts the major version. Size: 98.9 MB, not the 185.9 MB this would have cost before #7350 -- `initialize_all()` was linking ~18 backends nothing can reach. Also fixed, surfaced by the flip: PERRY_LLVM_KEEP_IR promises the whole scratch dir including the .o. The clang path got that free because the object is a file; in-process returns bytes and silently dropped it, degrading a debugging aid exactly when someone is debugging. Verified on the 81-module zod corpus with no env set: compiles, output byte-identical to the clang path, and PERRY_RS4GC=1 now compiles a try-carrying probe with no further flags. 605 codegen tests pass.
📝 WalkthroughWalkthroughChangesLLVM 22 in-process compilation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant SetupLLVM22
participant Compiler
participant LLVM22
Workflow->>SetupLLVM22: install LLVM 22
SetupLLVM22->>LLVM22: validate version and export prefix
Workflow->>Compiler: build with LLVM 22
Compiler->>LLVM22: compile through in-process LLVM
LLVM22-->>Compiler: return object bytes
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 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: 7
🤖 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 @.github/actions/setup-llvm22/action.yml:
- Around line 47-48: Update the LLVM setup steps around the apt.llvm.org
signing-key command to use a repository-scoped keyring rather than
/etc/apt/trusted.gpg.d, validate the downloaded key against the committed
expected fingerprint before apt-get update, and configure the apt source with
deb [signed-by=...] pointing to that keyring. Remove reliance on globally
trusted APT metadata while preserving LLVM package installation.
In @.github/workflows/gc-moving-witnesses.yml:
- Line 200: Apply the relevance condition to both LLVM setup steps: add the
existing steps.relevance.outputs.run == 'true' job condition to
.github/workflows/gc-moving-witnesses.yml at lines 200-200 and
.github/workflows/gc-ratchet.yml at lines 118-118, so each setup runs only for
relevant pull requests.
In @.github/workflows/gc-native-roots.yml:
- Line 127: Remove the later macOS override that assigns the LLVM prefix from
brew, and retain the validated prefix exported by setup-llvm22. Ensure the
in-process build uses LLVM_SYS_221_PREFIX consistently.
In @.github/workflows/test.yml:
- Line 118: Remove the setup-llvm22 action from the formatting-only lint job in
.github/workflows/test.yml:118-118, and from the cargo audit job at
.github/workflows/security-audit.yml:26-26 and cargo-deny job at
.github/workflows/security-audit.yml:159-159; leave LLVM provisioning unchanged
for jobs that compile Rust.
- Line 808: Condition the setup-llvm22 step in .github/workflows/test.yml at
lines 808-808 on steps.scope.outputs.suites being non-empty. In
.github/workflows/container-tests.yml at lines 172-172, move the apple/container
availability probe before the Rust and LLVM setup steps so unavailable jobs skip
provisioning; update both workflow sites accordingly.
In `@crates/perry-codegen/src/linker.rs`:
- Around line 601-609: Align the self-contained LLVM documentation with the
actual backend behavior: in crates/perry-codegen/src/linker.rs (lines 601-609),
document the keep/failure paths that write .ll files and the statepoint path
that still requires system clang, or remove that external assembler dependency;
update the feature requirements and setup guidance in
crates/perry-codegen/Cargo.toml (lines 12-20) accordingly; and qualify the
“ships self-contained” claim in crates/perry/Cargo.toml (lines 143-146) unless
the backend is made fully self-contained.
- Around line 721-739: Update the policy.keep branch in the Ok(bytes) arm to
propagate failures from fs::create_dir_all and fs::write instead of discarding
or merely logging them, so the function does not return Ok(bytes) when requested
object retention fails. Preserve the existing success logging and returned bytes
on successful retention.
🪄 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: 8492b0a3-d5ee-40e0-9245-2cf10e771d96
📒 Files selected for processing (22)
.github/actions/setup-llvm22/action.yml.github/workflows/benchmark.yml.github/workflows/cache-warm.yml.github/workflows/container-tests.yml.github/workflows/coverage.yml.github/workflows/eh-transport.yml.github/workflows/feature-matrix.yml.github/workflows/gc-moving-witnesses.yml.github/workflows/gc-native-roots.yml.github/workflows/gc-ratchet.yml.github/workflows/gc-root-dominance.yml.github/workflows/node-compat-matrix.yml.github/workflows/node-core-subset.yml.github/workflows/node-suite-guard.yml.github/workflows/npm-package-sweep.yml.github/workflows/release-packages.yml.github/workflows/security-audit.yml.github/workflows/simctl-tests.yml.github/workflows/test.ymlcrates/perry-codegen/Cargo.tomlcrates/perry-codegen/src/linker.rscrates/perry/Cargo.toml
Comment on lines
+47
to
+48
| wget -qO- https://apt.llvm.org/llvm-snapshot.gpg.key \ | ||
| | sudo tee /etc/apt/trusted.gpg.d/apt.llvm.org.asc >/dev/null |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== action file =="
cat -n .github/actions/setup-llvm22/action.yml | sed -n '1,90p'
echo
echo "== repository references to setup-llvm22 and apt.key trust path =="
rg -n "setup-llvm22|apt\.llvm\.org|trusted\.gpg\.d|keyring:|signed-by:|LLVM_SYS_221_PREFIX|apt-get update|apt-get install" .github crates/perry-codegen/Cargo.toml || true
echo
echo "== local gpg/apt behavior probe (no repos/keyring changes) =="
python3 - <<'PY'
from subprocess import run, PIPE
cmds = [
("gpg --version", ["gpg", "--version"]),
("apt apt-secure man snippet", ["apt", "show", "-A", "apt", "Security"]),
]
for label, cmd in cmds:
print(f"\n-- {label} --")
r = run(cmd, text=True, stdout=PIPE, stderr=PIPE, start_new_session=False)
print(f"exit={r.returncode}")
out = r.stdout.splitlines()[:20] + r.stderr.splitlines()[:10]
print("\n".join(out))
PYRepository: PerryTS/perry
Length of output: 11702
🌐 Web query:
Debian apt trusted.gpg.d global keyring signed-by apt-key apt-secure official documentation
💡 Result:
In modern Debian systems, the use of apt-key and the global /etc/apt/trusted.gpg and /etc/apt/trusted.gpg.d/ keyrings is deprecated and discouraged [1][2][3]. The recommended practice is to use the signed-by option in your APT repository configuration to explicitly associate a repository with its specific public GPG key [1][4][5]. This approach prevents a key trusted for one repository from being automatically trusted for all repositories on the system, significantly enhancing security [6][5]. Recommended Workflow 1. Obtain the GPG key and ensure it is in the correct format (binary.gpg or ASCII-armored.asc) [4][7][2]. 2. Place the key file in a dedicated, secure directory: - Use /etc/apt/keyrings/ for administrator-managed keys [1][7][5]. - Use /usr/share/keyrings/ for keys managed by packages (e.g., provided by a.deb archive) [4][8][5]. - Ensure the file has appropriate read permissions (e.g., chmod 644) [5]. 3. Configure your repository by adding the signed-by option to your source entry [1][6]: - For a legacy.list file: deb [signed-by=/etc/apt/keyrings/example.gpg] https://example.com/debian stable main - For modern.sources files (deb822 format), you can either reference the file or embed the ASCII-armored key directly within the file [1][4][7]. Key Differences and Deprecation - apt-key: This utility is deprecated and has been removed in recent Debian releases (e.g., Debian 13) [9][2]. It should no longer be used for managing repository keys [1][3]. - Global Keyrings (/etc/apt/trusted.gpg and /etc/apt/trusted.gpg.d/): These locations are deprecated [3]. While some systems may still support them for backward compatibility, they are considered insecure because they grant global trust to any key placed within them [3][6][5]. - Signed-By: This option binds a specific repository to a specific key, providing a granular and secure mechanism for verifying package authenticity [4][6][5]. For more information, consult the apt-secure(8) manual page on your system, which provides the authoritative details on repository security and key configuration [4][3].
Citations:
- 1: https://manpages.debian.org/unstable/apt/apt-key.8.en.html
- 2: https://manpages.debian.org/bookworm/apt/apt-key.8.en.html
- 3: https://github.com/Debian/apt/blob/master/debian/NEWS
- 4: https://manpages.debian.org/testing/apt/apt-secure.8.en.html
- 5: https://pieterbakker.com/migrate-apt-repository-keys-debian-12-13/
- 6: https://agren.cc/p/trusted-gpg/
- 7: https://manpages.debian.org/testing/apt/apt-key.8
- 8: https://michael-prokop.at/blog/2021/02/16/how-to-properly-use-3rd-party-debian-repository-signing-keys-with-apt/
- 9: https://wiki.debian.org/SecureApt
Security Misconfiguration (CWE-345)
Reachability: External
Pin and scope the apt signing key.
The APT signing key is fetched at runtime and stored under global trusted APT metadata, so a compromised endpoint can make future packages from this source appear verified. Store the key in a repository-scoped keyring, verify a committed fingerprint before apt-get update, and configure deb [signed-by=...] instead of relying on /etc/apt/trusted.gpg.d.
🤖 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 @.github/actions/setup-llvm22/action.yml around lines 47 - 48, Update the
LLVM setup steps around the apt.llvm.org signing-key command to use a
repository-scoped keyring rather than /etc/apt/trusted.gpg.d, validate the
downloaded key against the committed expected fingerprint before apt-get update,
and configure the apt source with deb [signed-by=...] pointing to that keyring.
Remove reliance on globally trusted APT metadata while preserving LLVM package
installation.
| - name: Install Rust toolchain | ||
| if: steps.relevance.outputs.run == 'true' | ||
| uses: dtolnay/rust-toolchain@stable | ||
| - uses: ./.github/actions/setup-llvm22 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Apply the relevance condition to every LLVM setup step.
Both jobs gate their expensive work on steps.relevance.outputs.run, but the new LLVM setup step bypasses that gate. An irrelevant pull request can still perform external package installation and fail the CI job.
.github/workflows/gc-moving-witnesses.yml#L200-L200: addif: steps.relevance.outputs.run == 'true'..github/workflows/gc-ratchet.yml#L118-L118: addif: steps.relevance.outputs.run == 'true'.
Proposed fix
- uses: ./.github/actions/setup-llvm22
+ if: steps.relevance.outputs.run == 'true'📝 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
| - uses: ./.github/actions/setup-llvm22 | |
| - uses: ./.github/actions/setup-llvm22 | |
| if: steps.relevance.outputs.run == 'true' |
📍 Affects 2 files
.github/workflows/gc-moving-witnesses.yml#L200-L200(this comment).github/workflows/gc-ratchet.yml#L118-L118
🤖 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 @.github/workflows/gc-moving-witnesses.yml at line 200, Apply the relevance
condition to both LLVM setup steps: add the existing steps.relevance.outputs.run
== 'true' job condition to .github/workflows/gc-moving-witnesses.yml at lines
200-200 and .github/workflows/gc-ratchet.yml at lines 118-118, so each setup
runs only for relevant pull requests.
| with: | ||
| node-version-file: .node-version | ||
| - uses: dtolnay/rust-toolchain@stable | ||
| - uses: ./.github/actions/setup-llvm22 |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Preserve the validated LLVM 22 prefix.
Line 127 exports the prefix validated by setup-llvm22, but the later macOS build replaces it with brew --prefix llvm. That formula is unversioned and is not checked for major version. The in-process build can therefore use a different LLVM major. Remove the later override and use the action-provided LLVM_SYS_221_PREFIX.
🤖 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 @.github/workflows/gc-native-roots.yml at line 127, Remove the later macOS
override that assigns the LLVM prefix from brew, and retain the validated prefix
exported by setup-llvm22. Ensure the in-process build uses LLVM_SYS_221_PREFIX
consistently.
| uses: dtolnay/rust-toolchain@stable | ||
| with: | ||
| components: rustfmt, clippy | ||
| - uses: ./.github/actions/setup-llvm22 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Remove LLVM provisioning from jobs that do not compile Rust.
The action adds an unnecessary privileged external dependency to non-compiling jobs. Remove it at each site:
.github/workflows/test.yml#L118-L118: remove setup from the formatting-onlylintjob..github/workflows/security-audit.yml#L26-L26: remove setup fromcargo audit..github/workflows/security-audit.yml#L159-L159: remove setup fromcargo-deny.
📍 Affects 2 files
.github/workflows/test.yml#L118-L118(this comment).github/workflows/security-audit.yml#L26-L26.github/workflows/security-audit.yml#L159-L159
🤖 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 @.github/workflows/test.yml at line 118, Remove the setup-llvm22 action from
the formatting-only lint job in .github/workflows/test.yml:118-118, and from the
cargo audit job at .github/workflows/security-audit.yml:26-26 and cargo-deny job
at .github/workflows/security-audit.yml:159-159; leave LLVM provisioning
unchanged for jobs that compile Rust.
| - name: Install Rust toolchain | ||
| if: steps.scope.outputs.suites != '' | ||
| uses: dtolnay/rust-toolchain@stable | ||
| - uses: ./.github/actions/setup-llvm22 |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not provision LLVM before conditional jobs confirm execution.
Prevent the setup action from running when the job will skip its compile path:
.github/workflows/test.yml#L808-L808: addif: steps.scope.outputs.suites != ''..github/workflows/container-tests.yml#L172-L172: move the apple/container availability probe before Rust and LLVM setup.
📍 Affects 2 files
.github/workflows/test.yml#L808-L808(this comment).github/workflows/container-tests.yml#L172-L172
🤖 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 @.github/workflows/test.yml at line 808, Condition the setup-llvm22 step in
.github/workflows/test.yml at lines 808-808 on steps.scope.outputs.suites being
non-empty. In .github/workflows/container-tests.yml at lines 172-172, move the
apple/container availability probe before the Rust and LLVM setup steps so
unavailable jobs skip provisioning; update both workflow sites accordingly.
Comment on lines
+601
to
+609
| /// Route `.ll -> .o` through the LLVM C API inside this process (no clang | ||
| /// subprocess, no `.ll` on disk). | ||
| /// | ||
| /// **ON BY DEFAULT.** Perry links LLVM 22 statically and ships self-contained, | ||
| /// so there is no "find a compatible clang" step to get wrong. It is also | ||
| /// load-bearing rather than a preference: the explicit statepoint bridge is | ||
| /// gone (#7348), leaving RS4GC as the only native-root backend, and RS4GC | ||
| /// cannot round-trip its IR through an external `opt` + a different clang | ||
| /// (#7339). |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Align the self-contained documentation with the implementation.
The documentation describes an unconditional LLVM-only path. The implementation writes .ll files for keep and failure modes, and it still requires system clang for statepoint assembly. A documented LLVM-only setup can therefore fail during compilation.
- crates/perry-codegen/src/linker.rs#L601-L609: qualify the no-
.lland no-clang claims with the keep, failure, and statepoint exceptions, or remove the external assembler step. - crates/perry-codegen/Cargo.toml#L12-L20: update the feature requirements and setup instructions to match the actual exceptions.
- crates/perry/Cargo.toml#L143-L146: qualify the “ships self-contained” statement or make the backend fully self-contained.
📍 Affects 3 files
crates/perry-codegen/src/linker.rs#L601-L609(this comment)crates/perry-codegen/Cargo.toml#L12-L20crates/perry/Cargo.toml#L143-L146
🤖 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-codegen/src/linker.rs` around lines 601 - 609, Align the
self-contained LLVM documentation with the actual backend behavior: in
crates/perry-codegen/src/linker.rs (lines 601-609), document the keep/failure
paths that write .ll files and the statepoint path that still requires system
clang, or remove that external assembler dependency; update the feature
requirements and setup guidance in crates/perry-codegen/Cargo.toml (lines 12-20)
accordingly; and qualify the “ships self-contained” claim in
crates/perry/Cargo.toml (lines 143-146) unless the backend is made fully
self-contained.
Comment on lines
+721
to
+739
| Ok(bytes) => { | ||
| // `PERRY_LLVM_KEEP_IR` promises the whole scratch dir, `.o` | ||
| // included. The clang path gets that for free because the object | ||
| // IS a file; in-process returns bytes and would silently drop it — | ||
| // degrading a debugging aid at exactly the moment someone is | ||
| // debugging. Now that this backend is the default, write it. | ||
| if policy.keep { | ||
| let _ = fs::create_dir_all(&paths.scratch_dir); | ||
| if let Err(e) = fs::write(&plan.obj_path, &bytes) { | ||
| eprintln!( | ||
| "[perry-codegen] could not keep {}: {e}", | ||
| plan.obj_path.display() | ||
| ); | ||
| } else { | ||
| eprintln!("[perry-codegen] kept object: {}", plan.obj_path.display()); | ||
| } | ||
| } | ||
| Ok(bytes) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not silently lose a requested object.
When policy.keep is true, create_dir_all failures are discarded and object-write failures are only logged. The function still returns Ok(bytes). PERRY_LLVM_KEEP_IR can therefore succeed without retaining the .o file that its comment promises. Propagate retention errors, or explicitly define retention as best effort and test that contract.
Strict retention variant
if policy.keep {
- let _ = fs::create_dir_all(&paths.scratch_dir);
- if let Err(e) = fs::write(&plan.obj_path, &bytes) {
- eprintln!(
- "[perry-codegen] could not keep {}: {e}",
- plan.obj_path.display()
- );
- } else {
- eprintln!("[perry-codegen] kept object: {}", plan.obj_path.display());
- }
+ fs::create_dir_all(&paths.scratch_dir)
+ .with_context(|| format!("Failed to create {}", paths.scratch_dir.display()))?;
+ fs::write(&plan.obj_path, &bytes)
+ .with_context(|| format!("Failed to keep {}", plan.obj_path.display()))?;
+ eprintln!("[perry-codegen] kept object: {}", plan.obj_path.display());
}📝 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
| Ok(bytes) => { | |
| // `PERRY_LLVM_KEEP_IR` promises the whole scratch dir, `.o` | |
| // included. The clang path gets that for free because the object | |
| // IS a file; in-process returns bytes and would silently drop it — | |
| // degrading a debugging aid at exactly the moment someone is | |
| // debugging. Now that this backend is the default, write it. | |
| if policy.keep { | |
| let _ = fs::create_dir_all(&paths.scratch_dir); | |
| if let Err(e) = fs::write(&plan.obj_path, &bytes) { | |
| eprintln!( | |
| "[perry-codegen] could not keep {}: {e}", | |
| plan.obj_path.display() | |
| ); | |
| } else { | |
| eprintln!("[perry-codegen] kept object: {}", plan.obj_path.display()); | |
| } | |
| } | |
| Ok(bytes) | |
| } | |
| Ok(bytes) => { | |
| // `PERRY_LLVM_KEEP_IR` promises the whole scratch dir, `.o` | |
| // included. The clang path gets that for free because the object | |
| // IS a file; in-process returns bytes and would silently drop it — | |
| // degrading a debugging aid at exactly the moment someone is | |
| // debugging. Now that this backend is the default, write it. | |
| if policy.keep { | |
| fs::create_dir_all(&paths.scratch_dir) | |
| .with_context(|| format!("Failed to create {}", paths.scratch_dir.display()))?; | |
| fs::write(&plan.obj_path, &bytes) | |
| .with_context(|| format!("Failed to keep {}", plan.obj_path.display()))?; | |
| eprintln!("[perry-codegen] kept object: {}", plan.obj_path.display()); | |
| } | |
| Ok(bytes) | |
| } |
🤖 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-codegen/src/linker.rs` around lines 721 - 739, Update the
policy.keep branch in the Ok(bytes) arm to propagate failures from
fs::create_dir_all and fs::write instead of discarding or merely logging them,
so the function does not return Ok(bytes) when requested object retention fails.
Preserve the existing success logging and returned bytes on successful
retention.
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…7357) Measured on main, last ~10 runs each: security-audit 9 cancelled, 0 success <- REQUIRED context eh-transport 8 cancelled, 0 success llvm-inprocess 8 cancelled, 0 success gc-moving-witness 0 cancelled, 2 success <- already fixed Two of the three had a comment claiming they were already safe: "cancel superseded PR runs, never main runs -- a busy merge day would otherwise starve the gate to zero executions." A busy merge day starved them anyway, by a mechanism the comment did not anticipate. is not sufficient. GitHub allows at most one PENDING run per concurrency group and cancels the previously pending one when a new run enters, regardless of that setting. With the group keyed on , every main push shares one group, so a merge burst cancels the intermediate runs. gc-moving-witnesses already carries both the diagnosis and the fix (#7205); this applies it to the three that still had the old shape. security-audit additionally had , which cancels main runs outright. Keying push runs on the SHA gives every merged commit its own group. This is CLAUDE.md hazard 3, and it is worth noting how it was found: not by reading the config -- two of these LOOKED correct -- but by asking what each workflow's main runs actually concluded. llvm-inprocess is the gate for the backend #7353 just made the default, and it had never executed on main. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 4, 2026
…fork (#7371) Three corrections, one of which is a number the plan explicitly warns against quoting and was carrying anyway. 1. THE SIZE FIGURE. Only +18.95% appears on main -- a synthetic worst case with three heap values live across an allocation in EVERY one of 2000 functions. The dependency-scale measurement is +1.86% (zod, 81 native modules, 29 MB binary), an order of magnitude lower. The correction was written when the synthetic was retracted but never reached main: #7345 squash-merged as 24 insertions, the first commit only, so the follow-up correction commit was dropped. That is the same failure mode this document records for #7321 -- a wrong explanation outliving its own disproof -- so the real number now leads and the worst case is explicitly marked do-not-quote. 2. SEQUENCING STEP 2 said root density was a PREREQUISITE for adoption, reasoning from that retracted figure. Adoption shipped in #7370 without it. Still worth doing, and still the same lever #7296 proved worth 9.9x, but it gates nothing. 3. THE ADOPTION FORK IS CLOSED. Every gate shut: llvm-inprocess default (#7353), x86-64 (#7349), Windows (#7355), bridge deleted (#7348), and the 479-test suite with no env matching the shadow baseline exactly. The target-aware shape is recorded because it is the part that generalises: native roots where the runtime can walk, shadow stack where it cannot. Also: layer 2 now reads THE DEFAULT rather than landed opt-in, layer 3's count is 41 rather than 54 after #7363, and the 2026-08-03 status header no longer says 'not yet adopted'. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
…#7417) * fix(ci): stop interpolating the LLVM version into the pwsh setup step zizmor has been red on every `main` commit since #7353 created `.github/actions/setup-llvm22/action.yml` -- roughly 40 consecutive commits. #7388 and #7393 only shifted the reported line numbers, which is what made them look implicated; neither introduced a finding. Four high-severity findings, one of which is properly fixed here. The Windows arm interpolated a composite-action input straight into a PowerShell script body (`$ver = "${{ inputs.version }}"`), which `template-injection` flags at High confidence: the expansion is substituted as raw text before pwsh parses the line, so an input carrying a quote plus a statement separator would execute as code with the runner's privileges. The input now arrives through an `env:` block and is read as `$env:LLVM_VERSION`, a plain string load. The other three are `github-env` at Low confidence -- the single `LLVM_SYS_221_PREFIX=<prefix>` line the action exists to write, once per platform arm -- and are suppressed with reasoning in `.github/zizmor.yml`. Measured: the audit is satisfiable only by not writing the environment file at all, and the clean alternative ($GITHUB_OUTPUT plus composite outputs) costs 44 jobs and 140 downstream steps, recreating the duplication the action exists to remove. The carve-out is a dated ratchet with an explicit delete-condition. Verified with the repo's SRI-pinned zizmor 1.28.0: pristine config plus this fix reports 3 high and exits 14; with the carve-out it exits 0 and `ignored` rises 119 -> 122, matching the three suppressed findings exactly. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * docs: name the fragment for its real PR (#7417) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
…ds (#7418) * fix(ci): satisfy the phantom xml2s.lib the LLVM Windows tarball demands LLVM's official Windows release script builds a static libxml2 into a scratch directory and points cmake at it with -DLLVM_ENABLE_LIBXML2=FORCE_ON -DLIBXML2_LIBRARIES=%libxmldir%/lib/libxml2s.lib. %libxmldir% is never installed, so the published clang+llvm-*-pc-windows-msvc tarball carries the dependency but not the library. llvm-config --system-libs --link-static reports xml2s.lib, llvm-sys forwards every system lib verbatim with no knob to filter one out, and link.exe dies with LNK1181 before resolving a symbol. Synthesize an empty archive at the LLVM libdir when llvm-config reports xml2s.lib AND the libdir lacks it. It is a name dependency, not a symbol dependency: libxml2 is reachable only from LLVMWindowsManifest, which the LLVM-C surface inkwell drives never touches, and rustc bundles the component archives into libllvm_sys.rlib where link.exe pulls members lazily. If that stops being true the link fails loudly with LNK2019 rather than silently dropping manifest support. Checking both conditions makes the workaround self-deleting once a release ships or stops reporting the library. Latent since #7353 made the in-process LLVM backend the default and statically linked, not caused by #7388 (which touches only the Linux arm). It became visible when #7393's concurrency group let gc-native-roots.yml's windows-latest arm reach a runner for the first time. Fixing it in the composite action also unblocks test.yml's windows-build. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * docs: name the fragment for its real PR (#7418) --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.
Makes the in-process LLVM backend the default, statically linked. Perry ships self-contained: we own the LLVM assumption instead of pushing it onto the user, and there is no "install a compatible clang" step left to get wrong.
This is load-bearing, not a preference. The explicit statepoint bridge is gone (#7348), so RS4GC is the only native-root backend — and RS4GC cannot round-trip its IR through an external
optplus a different clang (#7339). Keeping this opt-in meant the only working statepoint path lived behind a flag nobody sets.Two defaults, flipped together
Either alone is half a feature — built-in but never used, or requested but not built.
llvm-inprocessbecomes a default cargo feature.inprocess_requested()defaults ON — but only iff the backend is compiled in.That second qualifier is not pedantry. Defaulting to
trueunconditionally routes every compile in a--no-default-featuresbuild into the not-built-in stub and fails the build outright. I hit exactly that and it is now covered both ways:PERRY_LLVM_INPROCESSunset=0=1--no-default-featuresCI
New
.github/actions/setup-llvm22composite action, referenced from all 44 toolchain steps across 18 workflows. One definition rather than 44 inline recipes, because the three platforms need three different sources and only one is obvious:llvm-devis 18, so the distro cannot supply this.clang+llvm-*-pc-windows-msvctarball. Not chocolatey: itsllvmpackage is the clang toolchain — nollvm-config.exe, none of the static libraries llvm-sys links — and it has no 22.x pin at all. (Learned the hard way; the feasibility spike's Windows cell failed on exactly this.)Every arm asserts the major version, because llvm-sys 221 needs 22 specifically and a runner image moving its formula must fail loudly rather than build something subtly different several steps later.
The insertion is mechanical and auditable: 44 insertions, zero deletions.
Size
98.9 MB, not the 185.9 MB this would have cost before #7350 —
initialize_all()was linking ~18 backends nothing can reach.A bug the flip surfaced
PERRY_LLVM_KEEP_IRpromises the whole scratch dir including the.o. The clang path got that for free because the object is a file; the in-process path returns bytes and silently dropped it — degrading a debugging aid at exactly the moment someone is debugging. Caught bykeep_ir_retains_the_whole_scratch_dir, fixed rather than relaxed.Verification
On the 81-module zod dependency corpus, with no env set: compiles, and output is byte-identical to the clang path.
PERRY_RS4GC=1now compiles a try-carrying probe with no further flags. 605 codegen tests pass.Known and accepted
Compile time is ~75% higher on that corpus (7.1s → 12.3s). Same parallelism (4.4× both) and same opt level, so it is LLVM 22's
default<O3>via PassBuilder versus Apple clang 21's driver-tuned-O3— a tuning gap, not a design flaw. Accepted deliberately; the reliability and the statepoint path are worth it, and it is tunable later.PERRY_LLVM_INPROCESS=native(function bodies built through the C API, no per-function text) stays opt-in: byte-identical objects on all 81 zod modules, but CI covers only two small programs so far.Summary by CodeRabbit
New Features
Documentation