Skip to content

Commit 40214c5

Browse files
proggeramlugRalph Küpper
andauthored
fix(gc): root the iterator drain's live values across .next() (#7475) (#7495)
* fix(gc): root the iterator drain's live values across `.next()` (#7475) `js_iterator_to_array` — the `[...iterable]` / `Array.from(iterable)` drain — held the iterator object, the accumulator array, the `next` closure and the two property keys in bare Rust locals across a `.next()` call that allocates the `{ value, done }` result. Any of those allocations can trigger the copying minor, which moves the values and rewrites only the slots it can see. A moved iterator leaves its pre-move copy in retired from-space; the next dispatch reads that copy's STALE field 0 and `dispatch_array_iterator_method` calls `js_array_length` on a from-space address. `make_iter_result` / `make_sqlite_iter_result` had the same shape one level down: the caller-supplied `value` (usually a heap element) and the freshly allocated result object were live across four more allocations before being stored. And `dispatch_array_iterator_method` re-used a backing-array pointer read BEFORE its cursor store, which can allocate. Root them all in a `RuntimeHandleScope` and re-read every address at its point of use. All handles are NaN-boxed rather than `root_raw_*_ptr`, so `scripts/raw_handle_debt.py` stays at 999. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * test(gc): witness + auto-optimize gate for the iterator-drain rooting bug (#7475) `test_gap_gc_iterator_drain_rooting.ts` mirrors the app-pattern kernel the bug was found in. Measured: `TypeError: next is not a function` before the fix under BOTH the default and the auto-optimize link, a from-space FAULT under `PERRY_GC_PROTECT_FROMSPACE=1`, byte-exact with the oracle after. Registered in `test-parity/gc_repsel_corpus.txt`, so `gc-moving-witnesses` runs it and rejects a cell where nothing moved. `scripts/auto_opt_app_patterns.sh` + `auto-opt-app-patterns.yml` close the blind spot that let this ship: every other gate sets `PERRY_NO_AUTO_OPTIMIZE=1` for a deterministic link, so the default path — which rebuilds the runtime with a per-app feature set and links it over PERRY_RUNTIME_DIR — was tested by nothing. The gate asserts its subject was live: it reads the linker command line out of `perry -v` and requires a `perry-auto-*/libperry_runtime.a` that exists on disk, because the auto-optimizer falls back to the prebuilt archives by design and a fallback run would pass every output comparison while exercising the wrong binary. Also fixes a handle-kind mismatch in the iterator drain: `across_const` panics on a NaN-boxed handle, so the `.done` read uses `across_nanbox`. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * docs(changelog): fragment for #7495 Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * test(ci): prove the auto-optimize gate's liveness matcher can fail (#7475) `--self-test` feeds `archive_from_log` three canned compile logs and asserts it accepts a real auto-optimize link line, rejects a run that printed `auto-optimize: built …` and then linked the PREBUILT archive (the driver's documented fallback when its cargo rebuild fails), and rejects an empty log. The middle case is not hypothetical: the first matcher grepped the whole log and accepted it, so the gate would have passed a run that exercised the wrong binary — the exact hazard the liveness assertion exists for. The matcher now reads only the `[link] invoking:` command line, and CI runs the self-test before the kernels. Also validated all twelve app-pattern kernels through the auto-optimize link on this branch: eleven PASS (each linking a freshly built `perry-auto-*` archive and matching the node oracle byte for byte), `promise_all_chains` is the one documented skip. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * docs: point the two residual #7475 defects at their own issues `promise_all_chains` (#7497) and the `array_from_spread_value` symbol-lookup stale deref (#7498) are separate defects from the iterator-drain rooting bug, and both are unchanged by it. Naming them individually keeps them out of a vague remainder on #7475. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * fix(gc): root the iterator receiver before the first allocation (#7475) Four review findings, all accepted. `js_iterator_to_array` rooted `iter_f64` AFTER `js_array_alloc(8)` — an allocation, so a copying minor could move the iterator while it existed only in the raw argument and the handle would then root a pre-move address. `iter_h` is now the first thing created in the scope; the null check and the `next` lookup read back through it. `dispatch_array_iterator_method` re-derived `arr_ptr` from field 0 after its cursor store but kept using the raw `iter_obj` PARAMETER to do so, which the same store could have invalidated. It now roots the receiver at entry and reads the current address at every use through a shadowing `iter_obj()` closure, so the pre-collection address is not nameable after that line. `scripts/auto_opt_app_patterns.sh` refuses to run when the node oracle disagrees with `.node-version`. The oracle version is a correctness input — every kernel is diffed byte for byte against it and node patch releases change observable output — and `gc_repsel_matrix.sh` refuses on the same grounds. The workflow's relevance filter now also matches `.github/actions/setup-llvm22/`: it configures the LLVM the gate's compiler is built against, so a change there can move generated code without touching a line under `crates/`. Claude-Session: https://claude.ai/code/session_019EHcmXKArA7m42SihYCcgH * chore: bump version to 0.5.1287 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent eb8a9ac commit 40214c5

10 files changed

Lines changed: 886 additions & 133 deletions

File tree

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
name: Auto-Optimize App Patterns
2+
3+
# Runs the `benchmarks/app-patterns` kernels through the AUTO-OPTIMIZE link —
4+
# the default path, and the one no other gate in this repo covers.
5+
#
6+
# WHY THIS EXISTS (#7475)
7+
#
8+
# `perry file.ts -o out` rebuilds perry-runtime + perry-stdlib with a per-app
9+
# Cargo feature set into `target/perry-auto-<hash>/` and links those archives
10+
# OVER whatever `PERRY_RUNTIME_DIR` points at. Almost every other gate sets
11+
# `PERRY_NO_AUTO_OPTIMIZE=1` for a deterministic link — gc-ratchet says so
12+
# inline, and so do a dozen `crates/perry/tests` cases — so the binary users
13+
# actually get was, until this job, tested by nothing.
14+
#
15+
# #7475 is what that cost. `object_deep_clone` threw `TypeError: next is not a
16+
# function` under auto-optimize and printed the correct checksum under
17+
# `PERRY_NO_AUTO_OPTIMIZE=1`. The bug was in neither the kernel nor the feature
18+
# set: the iterator drain held live values in bare Rust locals across an
19+
# allocating `.next()`. BOTH links had the defect —
20+
# `PERRY_GC_PROTECT_FROMSPACE=1` faults on both — but only the feature-stripped
21+
# one allocated in the order that made the stale read observable. That is the
22+
# general shape: a latent stale-root read is invisible until something perturbs
23+
# allocation timing, and the auto-optimize link perturbs it per app.
24+
#
25+
# CHECKED AGAINST CLAUDE.md's FOUR WAYS A GATE CAN BE UNABLE TO FAIL
26+
#
27+
# 1. no `continue-on-error`, no `|| true`, no pipe swallowing the script's
28+
# exit status;
29+
# 2. NOT in branch protection's required contexts yet, deliberately — a new
30+
# gate has never been green, so promoting it on day one blocks every open
31+
# PR. Promote after the first green run on `main`; leaving that undone is
32+
# itself hazard 2 (see `gc-root-dominance`);
33+
# 3. `concurrency` cancels pull-request runs only; push runs are keyed on the
34+
# commit so queued `main` runs cannot cancel each other (#7205);
35+
# 4. the subject is ASSERTED live. `scripts/auto_opt_app_patterns.sh` reads
36+
# the linker command line out of `perry -v` and requires it to name a
37+
# `perry-auto-*/…/libperry_runtime.a` that exists on disk. A run in which
38+
# the auto-optimizer quietly fell back to the prebuilt archives — which it
39+
# does, by design, whenever the cargo rebuild fails — would otherwise pass
40+
# every output comparison while testing the exact configuration this job
41+
# does not care about.
42+
#
43+
# The one skip (`promise_all_chains`) is named with a reason inside the script,
44+
# and a skip entry that matches no kernel FAILS, so it cannot outlive its fix.
45+
46+
on:
47+
pull_request:
48+
push:
49+
branches: [main]
50+
workflow_dispatch:
51+
52+
permissions:
53+
contents: read
54+
55+
concurrency:
56+
group: auto-opt-app-patterns-${{ github.event_name }}-${{ github.event_name == 'push' && github.sha || github.ref }}
57+
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
58+
59+
env:
60+
CARGO_TERM_COLOR: always
61+
62+
jobs:
63+
auto-opt-app-patterns:
64+
runs-on: ubuntu-latest
65+
# The auto-optimize rebuild is a second full release build of
66+
# perry-runtime + perry-stdlib on top of the workspace build, so this is a
67+
# long job even with a warm cargo cache.
68+
timeout-minutes: 90
69+
permissions:
70+
contents: read
71+
pull-requests: read
72+
steps:
73+
- uses: actions/checkout@v7
74+
with:
75+
persist-credentials: false
76+
77+
- name: Decide whether this change can affect a compiled app
78+
id: relevance
79+
env:
80+
GH_TOKEN: ${{ github.token }}
81+
EVENT_NAME: ${{ github.event_name }}
82+
REPOSITORY: ${{ github.repository }}
83+
PR_NUMBER: ${{ github.event.pull_request.number }}
84+
run: |
85+
set -euo pipefail
86+
if [[ "$EVENT_NAME" != "pull_request" ]]; then
87+
echo "run=true" >> "$GITHUB_OUTPUT"
88+
echo "Not a pull request; running the kernels."
89+
exit 0
90+
fi
91+
gh api "repos/$REPOSITORY/pulls/$PR_NUMBER/files" --paginate --jq '.[].filename' > changed.txt
92+
# Deliberately broad: anything under crates/ changes the compiled
93+
# binary, and the auto-optimize feature selection reads the manifests.
94+
# The filter only spares docs-only PRs a compiler build; `set -e`
95+
# already aborted if the listing failed, so this cannot silently fall
96+
# through to "not relevant".
97+
# `.github/actions/setup-llvm22/` is in the list because it configures
98+
# the LLVM the gate's compiler is built against — a change there can
99+
# move the generated code without touching a single line under crates/.
100+
if grep -qE '^(crates/|benchmarks/app-patterns/|scripts/auto_opt_app_patterns\.sh$|Cargo\.(toml|lock)$|\.node-version$|\.github/actions/setup-llvm22/|\.github/workflows/auto-opt-app-patterns\.yml$)' changed.txt; then
101+
echo "run=true" >> "$GITHUB_OUTPUT"
102+
echo "Change can affect a compiled app; running the kernels."
103+
else
104+
echo "run=false" >> "$GITHUB_OUTPUT"
105+
echo "No app-affecting paths changed."
106+
fi
107+
108+
- name: Install Rust toolchain
109+
if: steps.relevance.outputs.run == 'true'
110+
uses: dtolnay/rust-toolchain@stable
111+
- uses: ./.github/actions/setup-llvm22
112+
113+
- uses: Swatinem/rust-cache@v2
114+
if: steps.relevance.outputs.run == 'true'
115+
with:
116+
shared-key: "${{ runner.os }}-perry"
117+
save-if: ${{ github.ref == 'refs/heads/main' }}
118+
119+
- name: Install clang
120+
if: steps.relevance.outputs.run == 'true'
121+
run: |
122+
sudo apt-get update
123+
sudo apt-get install -y clang
124+
125+
- name: Setup Node oracle
126+
if: steps.relevance.outputs.run == 'true'
127+
uses: actions/setup-node@v7
128+
with:
129+
# Single source of truth: .node-version. Every kernel's stdout is
130+
# diffed against this node, so the pin is a correctness input.
131+
node-version-file: .node-version
132+
133+
- name: Build perry and the prebuilt runtime archives
134+
if: steps.relevance.outputs.run == 'true'
135+
env:
136+
CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUSTFLAGS: "-C linker-features=-lld"
137+
run: |
138+
set -euo pipefail
139+
# The auto-optimize path builds its OWN archives, but the driver still
140+
# needs the prebuilt ones on disk for its fallback probe — and they are
141+
# what the failure mode under test silently substitutes, so a run
142+
# without them could not distinguish the two. perry-runtime and
143+
# perry-stdlib are rlib-only; the `.a`s come from the -static wrappers.
144+
cargo build --release \
145+
-p perry -p perry-runtime -p perry-stdlib \
146+
-p perry-runtime-static -p perry-stdlib-static
147+
for artifact in perry libperry_runtime.a libperry_stdlib.a; do
148+
test -s "target/release/$artifact" \
149+
|| { echo "::error::target/release/$artifact was not produced"; exit 1; }
150+
done
151+
152+
# GATING, and the reason hazard 4 is actually closed rather than asserted.
153+
# The liveness check is a text matcher over the linker command line; a
154+
# matcher that stops matching reports "no archive" (loud), but one that
155+
# matches too much reports a PASS for a fallback run (silent). `--self-test`
156+
# feeds it a canned log of exactly that shape — the `auto-optimize: built …`
157+
# message followed by a link line naming the PREBUILT archive — and fails
158+
# if it is accepted. It caught a real over-match while this gate was being
159+
# written.
160+
- name: Prove the liveness matcher can still fail
161+
if: steps.relevance.outputs.run == 'true'
162+
run: ./scripts/auto_opt_app_patterns.sh --self-test
163+
164+
# GATING. No pipe, no `|| true`: this step's exit status IS the gate. The
165+
# liveness assertion (the link line must name a perry-auto archive) lives
166+
# inside the script so a local run gets it too.
167+
- name: Run the app-pattern kernels through the auto-optimize link
168+
if: steps.relevance.outputs.run == 'true'
169+
env:
170+
PERRY_RUNTIME_DIR: ${{ github.workspace }}/target/release
171+
run: ./scripts/auto_opt_app_patterns.sh

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
88

99
Perry is a native TypeScript compiler written in Rust that compiles TypeScript source code directly to native executables. It uses SWC for TypeScript parsing and LLVM for code generation.
1010

11-
**Current Version:** 0.5.1286
11+
**Current Version:** 0.5.1287
1212

1313

1414
## TypeScript Parity Status

0 commit comments

Comments
 (0)