From 77db5f2a8eba60cce29c517700ce2c6c7321ecca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:08:36 +0200 Subject: [PATCH 01/13] fix(ci): track moved RS4GC unit gate (cherry picked from commit 3622fcdc3c6011cf0d9ad260fc7b5a95ace09d7c) --- .github/workflows/llvm-inprocess.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/llvm-inprocess.yml b/.github/workflows/llvm-inprocess.yml index e1ebd77026..ab89e6917c 100644 --- a/.github/workflows/llvm-inprocess.yml +++ b/.github/workflows/llvm-inprocess.yml @@ -126,7 +126,7 @@ jobs: cargo build --profile perry-dev -p perry -p perry-runtime-static \ -p perry-stdlib-static --features perry/llvm-inprocess - - name: Unit gates (528 incl. corpus construction + RS4GC pin) + - name: Unit gates (including corpus construction + RS4GC pin) run: | set -euo pipefail out=$(cargo test --profile perry-dev -p perry-codegen \ @@ -136,7 +136,9 @@ jobs: grep -q "dialect::tests::corpus_spike ... ok" <<<"$out" grep -q "dialect::tests::corpus_batch_kernel ... ok" <<<"$out" grep -q "dialect::tests::corpus_exception_handling ... ok" <<<"$out" - grep -q "inprocess::tests::rs4gc_schedules_in_process ... ok" <<<"$out" + # The implementation may move between inprocess submodules; the + # unique test name is the stable contract that proves this pin ran. + grep -Eq '^test .*::rs4gc_schedules_in_process \.\.\. ok$' <<<"$out" # The tracked `.ll` corpora above are a SNAPSHOT of what the compiler From c7e8a4e59dfd128231f75792aff3bc661e485a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 01:57:51 +0200 Subject: [PATCH 02/13] fix: restore GC root dominance scheduled gate (cherry picked from commit 29d52f4e3e06213cf44b91a2feeade7e6d65cbe7) --- .github/workflows/gc-root-dominance.yml | 15 +-- .github/workflows/test.yml | 11 ++ scripts/gc_root_dominance_check.py | 107 +++++++++++++----- scripts/gc_root_dominance_corpus.sh | 30 +---- scripts/gc_root_dominance_dep_corpus.sh | 6 +- .../gc_root_dominance_dep_native_corpus.sh | 17 +-- scripts/read_statepoint_rewrite_passes.py | 105 +++++++++++++++++ 7 files changed, 213 insertions(+), 78 deletions(-) create mode 100755 scripts/read_statepoint_rewrite_passes.py diff --git a/.github/workflows/gc-root-dominance.yml b/.github/workflows/gc-root-dominance.yml index 9f99738a70..b83116955e 100644 --- a/.github/workflows/gc-root-dominance.yml +++ b/.github/workflows/gc-root-dominance.yml @@ -345,13 +345,14 @@ jobs: - name: Check root-store dominance (dependency-scale) run: | set -euo pipefail - # Floors from the corpus as of this commit (81 modules, ~12900 - # functions, ~7700 root stores), set below that with room for the - # dependency's own churn. `zod` growing is fine; `zod` no longer - # compiling natively is the finding, and these are what make it one. + # The corpus retained 81 modules and ~14,000 root stores while + # compiler pruning reduced its function count from 12,909 to a + # stable 5,615-5,920. Keep the breadth floor below that measured + # population; module and root-store floors independently reject a + # compile that silently loses the dependency or the gate's subject. python3 scripts/gc_root_dominance_check.py ir-corpus-dep \ --moving-only \ - --min-files 60 --min-binds 4000 --min-funcs 6000 \ + --min-files 60 --min-binds 4000 --min-funcs 5000 \ --allowlist scripts/gc_root_dominance_allowlist.json \ --seeded-violations 40 \ -v @@ -362,7 +363,7 @@ jobs: python3 scripts/gc_root_dominance_check.py ir-corpus-dep \ --unrooted-allocas \ --moving-only \ - --min-files 60 --min-binds 4000 --min-funcs 6000 \ + --min-files 60 --min-binds 4000 --min-funcs 5000 \ --allowlist scripts/gc_root_dominance_allowlist.json \ -v @@ -401,7 +402,7 @@ jobs: set -euo pipefail python3 scripts/gc_root_dominance_check.py ir-corpus-dep \ --stale-registers --moving-only \ - --min-files 60 --min-binds 4000 --min-funcs 6000 \ + --min-files 60 --min-binds 4000 --min-funcs 5000 \ --max-stale 118 - name: Upload the IR corpus on failure diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index fc7a369410..e884953480 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -502,6 +502,17 @@ jobs: python3 scripts/check_llvm_corpus_currency.py --self-test python3 scripts/check_llvm_corpus_currency.py + # #9925. Both native root-dominance corpus generators scraped the + # production statepoint pass constant from inprocess.rs, so splitting + # that module left the scheduled gate unable to emit either corpus. The + # shared reader follows the unique declaration across codegen modules; + # keep its parser and the live repository lookup in required lint. + - name: Statepoint pass source reader + if: ${{ !cancelled() }} + run: | + python3 scripts/read_statepoint_rewrite_passes.py --self-test + python3 scripts/read_statepoint_rewrite_passes.py --check + # #7877, second round. The knob audit above covers env-var names; it says # nothing about the paths and numbers the same pages state. Both drifted: # the memory-model source map pointed at a `gc.rs` deleted in the module diff --git a/scripts/gc_root_dominance_check.py b/scripts/gc_root_dominance_check.py index cf5d7f881b..d982dc1d45 100755 --- a/scripts/gc_root_dominance_check.py +++ b/scripts/gc_root_dominance_check.py @@ -122,16 +122,15 @@ # function into one and producing exactly the line-order false positives the # docstring above says real dominance avoids. # -# `-` is in the identifier class because LLVM's own identifiers allow it and -# LLVM's own passes USE it. `rewrite-statepoints-for-gc` splits critical edges -# into landing pads and names the halves `eh.lpad.8.split-lp`, -# `…split-lp.split-lp`, and so on: 677 such labels in a 21-module native -# corpus. Under the old class those lines matched LABEL_SHAPED_RE but not -# LABEL_RE, so every native module raised MalformedIR and the mode could not -# read its own corpus. (Loudly, at least — the parser's refusal-not-skip rule -# working as designed.) Widening is safe for the shadow corpus: perry's writer -# emits no hyphens, so no line changes classification there. -LABEL_RE = re.compile(r"^([-\w.$][-\w.$]*):\s*(?:;.*)?$") +# `-` is in the bare identifier class because LLVM's own identifiers allow it +# and LLVM's own passes USE it. LLVM quotes labels that contain other bytes, +# including the `$`-bearing names produced when a repsel-specialised function +# is inlined. A declaration prints as `"name$part":` and its references as +# `%"name$part"`; both must normalize to the same CFG key. Keep escape spelling +# intact rather than decoding it: LLVM prints the same spelling at declaration +# and use sites, and equality is the only operation this parser needs. +LLVM_LABEL_TOKEN = r'(?:"(?:[^"\\]|\\.)*"|[-\w.$]+)' +LABEL_RE = re.compile(rf"^({LLVM_LABEL_TOKEN}):\s*(?:;.*)?$") # Anything that ends in `:` and is not an instruction is label-SHAPED. If the # strict form above declines it, that is a parser gap and must be loud. LABEL_SHAPED_RE = re.compile(r"^[^\s=]+:\s*(?:;.*)?$") @@ -157,17 +156,28 @@ BIND_RE = re.compile(r"call void @js_shadow_slot_bind\(i32 (\d+), ptr %([\w.$]+)\)") CLEAR_RE = re.compile(r"call void @js_shadow_slot_set\(i32 (\d+), i64 0\)") STORE_RE = re.compile(r"^\s*store\s+([\w\[\]x* ]+?)\s+([^,]+),\s*ptr %([\w.$]+)") -# `[-\w.$]` throughout, for the reason spelled out on LABEL_RE: LLVM's own -# `split-lp` landing-pad labels carry hyphens, and a branch regex that cannot -# name them drops the edge rather than failing. -BR_UNCOND_RE = re.compile(r"^\s*br label %([-\w.$]+)") -BR_COND_RE = re.compile(r"^\s*br i1 [^,]+, label %([-\w.$]+), label %([-\w.$]+)") -SWITCH_LABEL_RE = re.compile(r"label %([-\w.$]+)") +# All CFG-edge readers use the same token grammar as LABEL_RE. A narrower +# branch regex drops the edge rather than failing, which makes reachable blocks +# look dead and can turn a real dominance violation into a clean result. +BR_UNCOND_RE = re.compile(rf"^\s*br label %({LLVM_LABEL_TOKEN})") +BR_COND_RE = re.compile( + rf"^\s*br i1 [^,]+, label %({LLVM_LABEL_TOKEN}), " + rf"label %({LLVM_LABEL_TOKEN})" +) +SWITCH_LABEL_RE = re.compile(rf"label %({LLVM_LABEL_TOKEN})") # Invoke edges (#7302): normal destination + unwind destination. The invoke # terminates its block; the continuation label follows immediately in the # emitted text and both successors must appear in the CFG or the landing pad # (and everything reached through it) would be dropped as unreachable. -INVOKE_EDGE_RE = re.compile(r"\binvoke\b.*\bto label %([-\w.$]+) unwind label %([-\w.$]+)") +INVOKE_EDGE_RE = re.compile( + rf"\binvoke\b.*\bto label %({LLVM_LABEL_TOKEN}) unwind label " + rf"%({LLVM_LABEL_TOKEN})" +) + + +def llvm_label_name(token): + """Normalize a bare or quoted LLVM block token for CFG comparisons.""" + return token[1:-1] if token.startswith('"') else token # ------------------------------------------------- statepoint IR vocabulary @@ -373,7 +383,7 @@ def parse_file(path): continue lm = LABEL_RE.match(line) if lm: - curblk = lm.group(1) + curblk = llvm_label_name(lm.group(1)) if curblk not in cur.insns: cur.blocks.append(curblk) cur.insns[curblk] = [] @@ -422,21 +432,21 @@ def build_cfg(f): t = ins.text m = BR_COND_RE.match(t) if m: - f.succs[b].add(m.group(1)) - f.succs[b].add(m.group(2)) + f.succs[b].add(llvm_label_name(m.group(1))) + f.succs[b].add(llvm_label_name(m.group(2))) continue m = BR_UNCOND_RE.match(t) if m: - f.succs[b].add(m.group(1)) + f.succs[b].add(llvm_label_name(m.group(1))) continue if t.strip().startswith("switch"): for lbl in SWITCH_LABEL_RE.findall(t): - f.succs[b].add(lbl) + f.succs[b].add(llvm_label_name(lbl)) continue m = INVOKE_EDGE_RE.search(t) if m: - f.succs[b].add(m.group(1)) - f.succs[b].add(m.group(2)) + f.succs[b].add(llvm_label_name(m.group(1))) + f.succs[b].add(llvm_label_name(m.group(2))) for b, ss in list(f.succs.items()): for s in ss: f.preds[s].add(b) @@ -3517,7 +3527,9 @@ def _is_phi(ins): return body.strip().startswith("phi ") -_PHI_EDGE_RE = re.compile(r"\[\s*([^,\[\]]+?)\s*,\s*%([-\w.$]+)\s*\]") +_PHI_EDGE_RE = re.compile( + rf"\[\s*([^,\[\]]+?)\s*,\s*%({LLVM_LABEL_TOKEN})\s*\]" +) def phi_incoming(ins): @@ -3525,7 +3537,10 @@ def phi_incoming(ins): edges, in LLVM's printed order. `operand_text` is `%reg` for a register operand or the constant's own spelling (`0.000000e+00`, `null`, ...) — never itself tainted, which is why callers filter on the leading `%`.""" - return _PHI_EDGE_RE.findall(ins.text) + return [ + (value, llvm_label_name(predecessor)) + for value, predecessor in _PHI_EDGE_RE.findall(ins.text) + ] def transparent_use_graph(f): @@ -4251,16 +4266,28 @@ def _sp(tok="tok", callee="js_gc_loop_safepoint", live=()): } """ -# A repsel specialisation, whose name LLVM must quote. Skipped silently for as -# long as this checker has existed; 175 of 2452 defines in the native corpus. +# A repsel specialisation whose function and inlined block names LLVM must +# quote. The phi's tainted edge is the quoted block, so this fixture also goes +# false-clean if declarations, branch targets, or phi predecessors disagree on +# how `%"name$part"` is normalized. _SELFTEST_SP_QUOTED_NAME = """\ define internal double @"perry_fn_selftest__probe$typed_f64"(double %a) gc "statepoint-example" { entry.0: %rs4gc.b1 = bitcast double %a to i64 %rs4gc.s1 = inttoptr i64 %rs4gc.b1 to ptr addrspace(1) %raw = ptrtoint ptr addrspace(1) %rs4gc.s1 to i64 + br i1 true, label %"inlined$pshape.exit", label %safe.1 + +"inlined$pshape.exit": ; preds = %entry.0 __SAFEPOINT__ - %r = call double @js_object_get_field_by_name_f64(i64 %raw, i64 0) + br label %join.2 + +safe.1: ; preds = %entry.0 + br label %join.2 + +join.2: ; preds = %"inlined$pshape.exit", %safe.1 + %merged = phi i64 [ %raw, %"inlined$pshape.exit" ], [ 0, %safe.1 ] + %r = call double @js_object_get_field_by_name_f64(i64 %merged, i64 0) ret double %r } """.replace("__SAFEPOINT__", _sp()) @@ -4562,9 +4589,27 @@ def statepoint_self_test(): "names bare, LLVM's printer quotes them).", file=sys.stderr) ok = False + elif funcs[0].blocks != [ + "entry.0", "inlined$pshape.exit", "safe.1", "join.2"]: + print("self-test FAIL: a quoted basic-block declaration must " + "normalize to the same name used by its CFG references, got " + f"{funcs[0].blocks!r}", file=sys.stderr) + ok = False + elif funcs[0].succs["entry.0"] != {"inlined$pshape.exit", "safe.1"}: + print("self-test FAIL: a conditional branch to a quoted block " + f"must retain both CFG edges, got {funcs[0].succs['entry.0']!r}", + file=sys.stderr) + ok = False + elif phi_incoming(funcs[0].insns["join.2"][0]) != [ + ("%raw", "inlined$pshape.exit"), ("0", "safe.1")]: + print("self-test FAIL: a quoted phi predecessor must normalize to " + "the declared block name, got " + f"{phi_incoming(funcs[0].insns['join.2'][0])!r}", + file=sys.stderr) + ok = False if len(_scan_statepoints([paths["quoted"]], moving_only=True)) != 1: - print("self-test FAIL: the quoted-name fixture carries the same " - "planted hazard as the unrooted one and must report it", + print("self-test FAIL: the quoted-name fixture carries a planted " + "phi-edge hazard through the quoted block and must report it", file=sys.stderr) ok = False diff --git a/scripts/gc_root_dominance_corpus.sh b/scripts/gc_root_dominance_corpus.sh index 3df473b14c..1102a74ca6 100755 --- a/scripts/gc_root_dominance_corpus.sh +++ b/scripts/gc_root_dominance_corpus.sh @@ -47,8 +47,9 @@ # # That pass string is single-sourced from the Rust # (`STATEPOINT_REWRITE_PASSES`) and checked, not copied: a reproduction of a -# pipeline that has silently drifted is a corpus about nothing. See -# `rs4gc_pass_string`. +# pipeline that has silently drifted is a corpus about nothing. The shared +# reader searches perry-codegen's Rust sources for the unique declaration, so +# moving the constant between modules cannot darken both corpus generators. # # THE CORPUS DOES NOT LINK, and that is load-bearing (#8810) # --------------------------------------------------------- @@ -206,30 +207,7 @@ fi # corpus generated by a pass pipeline production stopped using is a corpus # about nothing, and it would report a serene zero. rs4gc_pass_string() { - local src="crates/perry-codegen/src/inprocess.rs" - if [ ! -f "$src" ]; then - echo "::error::$src not found; run from the repository root" >&2 - return 1 - fi - local value - # rustfmt may keep this declaration on one line or wrap the string onto the - # following line. Read the declaration as a record so formatting cannot - # silently disconnect this corpus from production's pass pipeline. - value="$(perl -0777 -ne ' - if (/^pub\(crate\) const STATEPOINT_REWRITE_PASSES: &str\s*=\s*"([^"]+)";/m) { - print "$1\n"; - } - ' "$src")" - if [ -z "$value" ]; then - echo "::error::could not read STATEPOINT_REWRITE_PASSES out of $src." >&2 - echo "The native corpus reproduces production's statepoint rewrite, and it" >&2 - echo "single-sources the pass string from that const so the two cannot" >&2 - echo "drift. If the const was renamed or reformatted, update this reader --" >&2 - echo "do NOT hardcode the string here, which is the drift this exists to" >&2 - echo "prevent." >&2 - return 1 - fi - printf '%s\n' "$value" + python3 scripts/read_statepoint_rewrite_passes.py } find_opt() { diff --git a/scripts/gc_root_dominance_dep_corpus.sh b/scripts/gc_root_dominance_dep_corpus.sh index 0c719872e4..43e4bf79fb 100755 --- a/scripts/gc_root_dominance_dep_corpus.sh +++ b/scripts/gc_root_dominance_dep_corpus.sh @@ -128,11 +128,13 @@ fi # # So the registry is the import graph and this is the check on it: the module # name perry derives from a path is the path with every non-alphanumeric -# character replaced by `_`, so each source names exactly one expected `.ll`. +# character replaced by `_`. Release worktrees can add their directory name as +# a prefix, so require exactly one emitted module with the source-path suffix. dark=() for src in test-files/gc-dep-corpus/*.ts; do sanitized="$(printf '%s' "$src" | tr -c 'A-Za-z0-9' '_')" - if [ ! -f "$OUTDIR/dep__${sanitized}.ll" ]; then + matches=("$OUTDIR"/dep__*"${sanitized}.ll") + if [ "${#matches[@]}" -ne 1 ] || [ ! -f "${matches[0]}" ]; then dark+=("$src") fi done diff --git a/scripts/gc_root_dominance_dep_native_corpus.sh b/scripts/gc_root_dominance_dep_native_corpus.sh index 8d988358fd..60692c47f9 100755 --- a/scripts/gc_root_dominance_dep_native_corpus.sh +++ b/scripts/gc_root_dominance_dep_native_corpus.sh @@ -29,18 +29,11 @@ case "$PERRY_BIN" in /*) ;; *) PERRY_BIN="$PWD/$PERRY_BIN" ;; esac [ -x "$PERRY_BIN" ] || { echo "::error::$PERRY_BIN not found or not executable" >&2; exit 2; } ENTRY="${ENTRY:-test-files/gc-dep-corpus/main.ts}" -# Single-sourced from the Rust const, never retyped (same reason the curated -# script gives: a fourth copy is how the pass string drifts from production). -# Join continuation lines up to the `;` first: rustfmt wraps the initializer -# when the line grows (#8068 did), and a single-line match then reads nothing. -PASSES="$(awk '/const STATEPOINT_REWRITE_PASSES: &str/ { - buf = $0 - while (buf !~ /;[[:space:]]*$/ && (getline line) > 0) buf = buf " " line - if (match(buf, /"[^"]*"/)) print substr(buf, RSTART + 1, RLENGTH - 2) - exit - }' crates/perry-codegen/src/inprocess.rs)" -[ -n "$PASSES" ] || { echo "could not read STATEPOINT_REWRITE_PASSES" >&2; exit 2; } -OPT_BIN="${PERRY_LLVM_OPT:-/opt/homebrew/opt/llvm/bin/opt}" +# Single-sourced from the Rust const through the same reader as the curated +# native corpus. The reader requires one literal declaration anywhere under +# perry-codegen/src, so a module split cannot leave this arm on a stale path. +PASSES="$(python3 scripts/read_statepoint_rewrite_passes.py)" || exit 2 +OPT_BIN="${PERRY_LLVM_OPT:-}" if [ ! -x "$OPT_BIN" ]; then for c in "${LLVM_SYS_221_PREFIX:-}/bin/opt" /opt/homebrew/opt/llvm/bin/opt /usr/local/opt/llvm/bin/opt; do [ -n "$c" ] && [ -x "$c" ] && OPT_BIN="$c" && break diff --git a/scripts/read_statepoint_rewrite_passes.py b/scripts/read_statepoint_rewrite_passes.py new file mode 100755 index 0000000000..a0dd1fa13b --- /dev/null +++ b/scripts/read_statepoint_rewrite_passes.py @@ -0,0 +1,105 @@ +#!/usr/bin/env python3 +"""Read production's statepoint pass pipeline from its unique Rust constant.""" + +from __future__ import annotations + +import argparse +import re +import sys +from pathlib import Path + + +REPO = Path(__file__).resolve().parent.parent +CODEGEN_SRC = REPO / "crates" / "perry-codegen" / "src" +DECLARATION = re.compile( + r"\bpub\(crate\)\s+const\s+STATEPOINT_REWRITE_PASSES\s*:\s*&str\s*=\s*" + r'"([^"\\]*)"\s*;', + re.MULTILINE | re.DOTALL, +) + + +def extract(text: str) -> str | None: + match = DECLARATION.search(text) + return match.group(1) if match else None + + +def find_declaration(root: Path = CODEGEN_SRC) -> tuple[Path, str]: + matches = [] + for path in sorted(root.rglob("*.rs")): + value = extract(path.read_text(encoding="utf-8")) + if value is not None: + matches.append((path, value)) + + if len(matches) != 1: + locations = ", ".join(str(path) for path, _value in matches) or "none" + raise ValueError( + "expected exactly one literal STATEPOINT_REWRITE_PASSES declaration " + f"under {root}, found {len(matches)} ({locations})" + ) + path, value = matches[0] + if not value or "rewrite-statepoints-for-gc" not in value: + raise ValueError( + f"{path} does not contain a usable statepoint rewrite pipeline" + ) + return path, value + + +def self_test() -> int: + failures = [] + expected = "always-inline,function(mem2reg),rewrite-statepoints-for-gc" + one_line = ( + 'pub(crate) const STATEPOINT_REWRITE_PASSES: &str = "' + expected + '";' + ) + wrapped = ( + "pub(crate) const STATEPOINT_REWRITE_PASSES: &str =\n" + f' "{expected}";\n' + ) + for label, source in (("one-line", one_line), ("rustfmt-wrapped", wrapped)): + if extract(source) != expected: + failures.append(f"{label} declaration was not read") + if extract(one_line.replace("STATEPOINT_REWRITE_PASSES", "OTHER_PASSES")) is not None: + failures.append("a differently named constant was accepted") + + try: + path, value = find_declaration() + if not path.is_relative_to(CODEGEN_SRC): + failures.append("the repository declaration escaped perry-codegen/src") + if value != extract(path.read_text(encoding="utf-8")): + failures.append("the repository scan disagrees with the source parser") + except (OSError, ValueError) as error: + failures.append(str(error)) + + for failure in failures: + print(f"statepoint-pass reader self-test FAILED: {failure}", file=sys.stderr) + if failures: + return 1 + print("statepoint-pass reader self-test: OK") + return 0 + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--check", action="store_true") + parser.add_argument("--self-test", action="store_true") + args = parser.parse_args() + if args.self_test: + return self_test() + + try: + path, value = find_declaration() + except (OSError, ValueError) as error: + print(f"statepoint-pass reader: {error}", file=sys.stderr) + return 2 + + if args.check: + print( + "statepoint-pass source OK: " + f"{path.relative_to(REPO)} -> {value}" + ) + else: + print(value) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) From 244f1554fa82c96047beba4bc82ec1912e33a4c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 23:51:39 +0200 Subject: [PATCH 03/13] fix(gc): preserve young logs across promotion rollback (cherry picked from commit f1e048b188360bbf638d4a3918a8bd0d15215960) --- .../src/gc/tests/promote_in_place.rs | 50 +++++++++++++++++++ crates/perry-runtime/src/gc/young_log.rs | 30 +++++++---- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/crates/perry-runtime/src/gc/tests/promote_in_place.rs b/crates/perry-runtime/src/gc/tests/promote_in_place.rs index 8b4dc5b516..be38b8f59e 100644 --- a/crates/perry-runtime/src/gc/tests/promote_in_place.rs +++ b/crates/perry-runtime/src/gc/tests/promote_in_place.rs @@ -788,6 +788,56 @@ fn a_first_cycle_attempt_that_its_own_trace_refutes_rolls_back_and_evacuates() { ); } +/// A speculative promotion temporarily gives every nursery block old-gen +/// semantics. Young-entry root logs must nevertheless survive an attempt that +/// is rolled back: the evacuation retry still depends on them to find values +/// reachable only through a side table. +#[test] +fn first_cycle_rollback_preserves_young_side_table_roots_for_the_retry() { + let _guard = CopyingNurseryTestGuard::new(0); + let _trigger_guard = GcTriggerThresholdTestGuard::suppress_automatic_triggers(); + let _promote = InPlacePromotionTestGuard::enabled(1000); + clear_young_survival_for_tests(); + // Exhaust the blind-promotion budget so the speculative attempt performs + // the mutable-root mark/rewrite walks that consume young logs. + seed_untraced_promoted_bytes_for_tests(usize::MAX); + gc_register_mutable_root_scanner(crate::closure::scan_closure_dynamic_props_roots_mut); + + let owner = crate::arena::arena_alloc_gc_old( + std::mem::size_of::(), + std::mem::align_of::(), + GC_TYPE_CLOSURE, + ) as usize; + unsafe { init_test_closure(owner as *mut u8) }; + // Arm the remembered-set reconstruction before publishing the value, then + // remove the ordinary old-object edge so the young log is the only root. + let _ = remembered_dirty_snapshot(); + let value = young_leaf(); + crate::closure::closure_set_dynamic_prop(owner, "memo", f64::from_bits(string_bits(value))); + remembered_set_clear(); + for _ in 0..64 { + let _garbage = young_leaf(); + } + + let attempts_before = first_cycle_promotion_attempts(); + let rollbacks_before = first_cycle_promotion_rollbacks(); + let trace = collect_minor_trace(GcTriggerKind::Direct); + + assert_copied_minor_trace(&trace, true, CopiedMinorFallbackReason::None, false); + assert_eq!(first_cycle_promotion_attempts() - attempts_before, 1); + assert_eq!(first_cycle_promotion_rollbacks() - rollbacks_before, 1); + let bits = crate::closure::closure_get_own_dynamic_prop(owner, "memo") + .expect("the old owner must retain its side-table value") + .to_bits(); + let value_after = (bits & POINTER_MASK) as usize; + assert_eq!(bits & TAG_MASK, STRING_TAG); + assert_ne!( + value_after, value, + "the retry must evacuate a value reachable only through the young log" + ); + assert!(crate::arena::pointer_in_nursery(value_after)); +} + // --------------------------------------------------------------------------- // #7913 interaction: old-page relocation vs a still-DESCRIBED promoted run // --------------------------------------------------------------------------- diff --git a/crates/perry-runtime/src/gc/young_log.rs b/crates/perry-runtime/src/gc/young_log.rs index 368d146149..9f6d43b22f 100644 --- a/crates/perry-runtime/src/gc/young_log.rs +++ b/crates/perry-runtime/src/gc/young_log.rs @@ -70,7 +70,7 @@ use std::cell::RefCell; use super::GC_HEADER_SIZE; -use crate::arena::HeapGeneration; +use crate::arena::HeapSpace; use crate::value::{BIGINT_TAG, POINTER_MASK, POINTER_TAG, STRING_TAG, TAG_MASK}; /// Keys of side-table entries that may hold a pointer a minor can act on. @@ -179,21 +179,29 @@ impl YoungLog { /// Can a minor-scoped pass act on the object at `addr`? /// -/// `false` is authoritative for the old generation only: an old object is -/// neither moved nor swept by any minor, and it never becomes young again. -/// Everything a minor moves, marks-through or sweeps answers `true` — -/// nursery (eden + both survivor halves), longlived, and malloc-GC objects. -/// A non-heap word (handle id, foreign-thread address, integer) answers -/// `false` through the exact malloc-registry probe, never a header sniff. +/// `false` is authoritative for ordinary old-generation space only: an old +/// object is neither moved nor swept by any minor, and it never becomes young +/// again. `PromotedYoung` remains relevant while an in-place promotion is in +/// flight because a speculative first-cycle attempt can still roll its retag +/// back and evacuate those objects. Dropping their keys during the attempt +/// would make the retry skip live side-table roots. Everything else a minor +/// moves, marks-through or sweeps answers `true` — nursery (eden + both +/// survivor halves), longlived, and malloc-GC objects. A non-heap word (handle +/// id, foreign-thread address, integer) answers `false` through the exact +/// malloc-registry probe, never a header sniff. #[inline] pub(crate) fn addr_is_minor_relevant(addr: usize) -> bool { if addr == 0 { return false; } - match crate::arena::classify_heap_generation(addr) { - HeapGeneration::Old => false, - HeapGeneration::Nursery | HeapGeneration::Longlived => true, - HeapGeneration::Unknown => { + match crate::arena::classify_heap_space(addr) { + HeapSpace::Old => false, + HeapSpace::NurseryEden + | HeapSpace::Survivor0 + | HeapSpace::Survivor1 + | HeapSpace::Longlived + | HeapSpace::PromotedYoung => true, + HeapSpace::Unknown => { addr > GC_HEADER_SIZE && super::malloc::gc_malloc_header_is_tracked( (addr - GC_HEADER_SIZE) as *const super::GcHeader, From 08a290f485652a2c70e19bcd1111c18373f46588 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sun, 6 Sep 2026 23:53:37 +0200 Subject: [PATCH 04/13] docs: add changelog for promotion rollback fix (cherry picked from commit ab2e6510c6d0f06e7f8d88a46e2663e0e5c01ee6) --- changelog.d/9924-gc-promotion-rollback-young-logs.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/9924-gc-promotion-rollback-young-logs.md diff --git a/changelog.d/9924-gc-promotion-rollback-young-logs.md b/changelog.d/9924-gc-promotion-rollback-young-logs.md new file mode 100644 index 0000000000..ed0ef206c8 --- /dev/null +++ b/changelog.d/9924-gc-promotion-rollback-young-logs.md @@ -0,0 +1,5 @@ +### Fixed + +- Preserve side-table young-root logs when the first copying collection + abandons speculative in-place promotion, so the evacuation retry does not + lose live objects or corrupt full applications during startup. From ba334946fabc4a5caf6c9ad80134fbcc75fe1937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:32:32 +0200 Subject: [PATCH 05/13] fix(hir): resolve typeof class expression self names (cherry picked from commit 475498f7c8a5bfade0e0fc2e0f971dc67f51fa56) --- .../src/lower/lower_expr/arm_unary.rs | 9 +++++++ crates/perry-hir/src/lower/tests.rs | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/crates/perry-hir/src/lower/lower_expr/arm_unary.rs b/crates/perry-hir/src/lower/lower_expr/arm_unary.rs index 848592b200..49d98308eb 100644 --- a/crates/perry-hir/src/lower/lower_expr/arm_unary.rs +++ b/crates/perry-hir/src/lower/lower_expr/arm_unary.rs @@ -124,6 +124,15 @@ pub(crate) fn lower_unary_expr(ctx: &mut LoweringContext, unary: &ast::UnaryExpr && ctx.lookup_native_module(n).is_none() && ctx.lookup_imported_func(n).is_none() && ctx.lookup_class(n).is_none() + // A named class expression can use a synthetic registry key + // while its source-level inner binding keeps the written + // name. Let ordinary identifier lowering resolve that binding + // to the current class instead of treating it as an optional + // global. For example, `var B = class l { static f() { + // return typeof l } }` registers the class under a generated + // `l__class_expr_*` key, but `l` is still lexically bound in + // the class body. + && ctx.current_class_inner_name.as_deref() != Some(n) && !is_builtin_function(n) && !is_known_global_identifier_name(n) && !matches!(n, "undefined" | "null" | "NaN" | "Infinity") diff --git a/crates/perry-hir/src/lower/tests.rs b/crates/perry-hir/src/lower/tests.rs index fcf1c2943d..946f1264e9 100644 --- a/crates/perry-hir/src/lower/tests.rs +++ b/crates/perry-hir/src/lower/tests.rs @@ -1182,6 +1182,33 @@ fn named_class_expr_static_private_update_in_arrow_keeps_lexical_brand_owner() { ); } +/// A named class expression whose outer binding has a different name uses a +/// synthetic registry key. `typeof` must still resolve the source-level inner +/// name through the class body's lexical binding rather than an optional +/// global lookup. +#[test] +fn typeof_named_class_expr_inner_binding_uses_the_current_class() { + let source = r#" + var B = class l { + static selfType(): string { return typeof l; } + }; + "#; + let module = perry_parser::parse_typescript(source, "t.ts").expect("source parses"); + let hir = super::lower_module(&module, "t", "t.ts").expect("source lowers"); + let method = hir + .classes + .iter() + .flat_map(|class| &class.static_methods) + .find(|method| method.name == "selfType") + .expect("static selfType method is lowered"); + let body = format!("{:#?}", method.body); + + assert!( + body.contains("ClassRef") && !body.contains("js_global_get_optional"), + "the class's inner name must resolve to its synthetic ClassRef: {body}" + ); +} + /// A sibling class declaration is already a known lexical binding while an /// earlier class method is lowered, even though its registry entry is emitted /// later. The unresolved-constructor guard must preserve that forward binding. From 73dac5fc2944a732f3fb656401a11fb80f33195e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:33:09 +0200 Subject: [PATCH 06/13] docs(changelog): note class expression typeof fix (cherry picked from commit fd35c335df75bb63be9e87087ee1838c741ab320) --- changelog.d/9931-class-expression-typeof-self.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/9931-class-expression-typeof-self.md diff --git a/changelog.d/9931-class-expression-typeof-self.md b/changelog.d/9931-class-expression-typeof-self.md new file mode 100644 index 0000000000..b08f8f753a --- /dev/null +++ b/changelog.d/9931-class-expression-typeof-self.md @@ -0,0 +1,5 @@ +### Fixed + +- Resolve a named class expression's inner name inside `typeof`, including + when its outer binding has a different name and the compiler registers the + class under a generated key. From 4e7391eef6917eaad15e42439e7bc2b5a6080e59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:51:55 +0200 Subject: [PATCH 07/13] docs(changelog): clarify class self binding fix (cherry picked from commit de9887c18d5fb34ca9acf49e1cf490c95ebd327b) --- changelog.d/9931-class-expression-typeof-self.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/changelog.d/9931-class-expression-typeof-self.md b/changelog.d/9931-class-expression-typeof-self.md index b08f8f753a..1a805508fa 100644 --- a/changelog.d/9931-class-expression-typeof-self.md +++ b/changelog.d/9931-class-expression-typeof-self.md @@ -2,4 +2,6 @@ - Resolve a named class expression's inner name inside `typeof`, including when its outer binding has a different name and the compiler registers the - class under a generated key. + class under a generated key. The unresolved-global shortcut now excludes the + active class inner name so lexical lowering can resolve the class binding; + an HIR regression covers that path. From fe13883d2772ed7fc2a7aae1cdf23bda382063b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:52:13 +0200 Subject: [PATCH 08/13] fix(runtime): preserve typed array own property order (cherry picked from commit 16db28ecda55cbe8f700b880c097e354f6d95984) --- crates/perry-runtime/src/buffer/own_props.rs | 74 +++++++++++++++---- .../src/object/field_get_set/enumeration.rs | 7 +- 2 files changed, 61 insertions(+), 20 deletions(-) diff --git a/crates/perry-runtime/src/buffer/own_props.rs b/crates/perry-runtime/src/buffer/own_props.rs index f022ab0f9b..64a114ee76 100644 --- a/crates/perry-runtime/src/buffer/own_props.rs +++ b/crates/perry-runtime/src/buffer/own_props.rs @@ -30,7 +30,13 @@ use std::collections::HashMap; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Mutex, OnceLock}; -type BufferProps = HashMap>; +#[derive(Default)] +struct BufferOwnProps { + values: HashMap, + order: Vec, +} + +type BufferProps = HashMap; fn buffer_props() -> &'static Mutex { static PROPS: OnceLock> = OnceLock::new(); @@ -93,10 +99,11 @@ pub fn buffer_define_own_data_prop(addr: usize, prop: &str, value: f64) { } BUFFER_OWN_PROPS_EVER.store(true, Ordering::Release); if let Ok(mut props) = buffer_props().lock() { - props - .entry(addr) - .or_default() - .insert(prop.to_string(), value.to_bits()); + let own = props.entry(addr).or_default(); + if !own.values.contains_key(prop) { + own.order.push(prop.to_string()); + } + own.values.insert(prop.to_string(), value.to_bits()); } } @@ -108,7 +115,12 @@ pub fn buffer_get_own_prop(addr: usize, prop: &str) -> Option { buffer_props() .lock() .ok() - .and_then(|props| props.get(&addr).and_then(|m| m.get(prop)).copied()) + .and_then(|props| { + props + .get(&addr) + .and_then(|own| own.values.get(prop)) + .copied() + }) .map(f64::from_bits) } @@ -140,8 +152,7 @@ pub fn buffer_read_own_prop(addr: usize, prop: &str) -> Option { buffer_get_own_prop(addr, prop) } -/// Every own dynamic prop key recorded for `addr`, in insertion-independent -/// (sorted) order. +/// Every own dynamic prop key recorded for `addr`, in property-creation order. /// /// #8149: `Object.keys` / `getOwnPropertyNames` / `for…in` need these. Before, /// the enumeration paths had no registered-buffer arm at all and walked a @@ -156,13 +167,11 @@ pub fn buffer_own_prop_names(addr: usize) -> Vec { if addr == 0 || !buffer_own_props_possible() { return Vec::new(); } - let mut names: Vec = buffer_props() + buffer_props() .lock() .ok() - .and_then(|props| props.get(&addr).map(|m| m.keys().cloned().collect())) - .unwrap_or_default(); - names.sort(); - names + .and_then(|props| props.get(&addr).map(|own| own.order.clone())) + .unwrap_or_default() } /// Whether the buffer carries any own dynamic prop under `prop`. @@ -183,10 +192,13 @@ pub fn buffer_delete_own_prop(addr: usize, prop: &str) -> bool { let Some(entries) = props.get_mut(&addr) else { return false; }; - let removed = entries.remove(prop).is_some(); + let removed = entries.values.remove(prop).is_some(); + if removed { + entries.order.retain(|key| key != prop); + } crate::object::clear_accessor_descriptor(addr, prop); crate::object::clear_property_attrs(addr, prop); - if entries.is_empty() { + if entries.values.is_empty() { props.remove(&addr); } removed @@ -210,7 +222,7 @@ pub fn scan_buffer_own_props_roots_mut(visitor: &mut crate::gc::RuntimeRootVisit }; let mut new_owner = owner; visitor.visit_metadata_usize_slot(&mut new_owner); - for bits in entries.values_mut() { + for bits in entries.values.values_mut() { let mut v = f64::from_bits(*bits); visitor.visit_nanbox_f64_slot(&mut v); *bits = v.to_bits(); @@ -253,3 +265,33 @@ pub fn clear_buffer_own_props(addr: usize) { props.remove(&addr); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn own_property_names_preserve_creation_order() { + let owner_marker = Box::new(0_u8); + let owner = (&*owner_marker as *const u8) as usize; + + clear_buffer_own_props(owner); + buffer_define_own_data_prop(owner, "second", 2.0); + buffer_define_own_data_prop(owner, "first", 1.0); + buffer_define_own_data_prop(owner, "second", 22.0); + assert_eq!( + buffer_own_prop_names(owner), + ["second", "first"], + "updating a property must keep its original position" + ); + + assert!(buffer_delete_own_prop(owner, "second")); + buffer_define_own_data_prop(owner, "second", 222.0); + assert_eq!( + buffer_own_prop_names(owner), + ["first", "second"], + "deleting and recreating a property must append it" + ); + clear_buffer_own_props(owner); + } +} diff --git a/crates/perry-runtime/src/object/field_get_set/enumeration.rs b/crates/perry-runtime/src/object/field_get_set/enumeration.rs index 1d84623080..beb29a4cf6 100644 --- a/crates/perry-runtime/src/object/field_get_set/enumeration.rs +++ b/crates/perry-runtime/src/object/field_get_set/enumeration.rs @@ -1154,10 +1154,9 @@ pub(super) fn strip_nanbox_addr(obj: *const ObjectHeader) -> usize { /// `Object.keys(new DataView(new ArrayBuffer(8)))` in any program that had also /// allocated a `Buffer`. /// -/// Expando ordering among the non-index keys is alphabetical, not insertion -/// order: `buffer::own_props` is a `HashMap`, so insertion order was never -/// recorded. Node uses insertion order. Deterministic-but-different beats the -/// previous nondeterministic-and-crashing. +/// Expando ordering among the non-index keys follows property creation order, +/// recorded by `buffer::own_props`. Canonical indices are still separated and +/// sorted below, as required by `OrdinaryOwnPropertyKeys`. pub(crate) fn registered_buffer_own_keys(addr: usize) -> Option> { if addr == 0 || !crate::buffer::is_registered_buffer(addr) { return None; From fd51f348618d05b33dae785576722595c4ee4945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 02:52:46 +0200 Subject: [PATCH 09/13] docs(changelog): note typed array property order fix (cherry picked from commit 644de51f721e605637221ee71de2be21366bef90) --- changelog.d/9932-typed-array-own-property-order.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/9932-typed-array-own-property-order.md diff --git a/changelog.d/9932-typed-array-own-property-order.md b/changelog.d/9932-typed-array-own-property-order.md new file mode 100644 index 0000000000..9dcdf7aea3 --- /dev/null +++ b/changelog.d/9932-typed-array-own-property-order.md @@ -0,0 +1,5 @@ +### Fixed + +- Preserve property creation order when enumerating ordinary properties on + buffers, data views, and buffer-backed typed arrays. Updating a property now + keeps its position, while deleting and recreating it appends the key. From 3b0fb7a541f0adcd39ffe74a8a49c6dd559dbdc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 03:26:12 +0200 Subject: [PATCH 10/13] fix(runtime): materialize error stacks during inspect (cherry picked from commit dd4a1a15bc4bc6f58a5a3cb64ec570a2770d409f) --- changelog.d/9933-util-inspect-lazy-error-stack.md | 5 +++++ crates/perry-runtime/src/builtins/formatting/errors.rs | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 changelog.d/9933-util-inspect-lazy-error-stack.md diff --git a/changelog.d/9933-util-inspect-lazy-error-stack.md b/changelog.d/9933-util-inspect-lazy-error-stack.md new file mode 100644 index 0000000000..71a4dbb821 --- /dev/null +++ b/changelog.d/9933-util-inspect-lazy-error-stack.md @@ -0,0 +1,5 @@ +### Fixed + +- Materialize lazy Error stacks when `util.inspect()` formats an Error with a + `cause` or an `AggregateError` with `errors`, preserving Node-compatible + stack/body layout. diff --git a/crates/perry-runtime/src/builtins/formatting/errors.rs b/crates/perry-runtime/src/builtins/formatting/errors.rs index 76eaa7543c..d940f895d9 100644 --- a/crates/perry-runtime/src/builtins/formatting/errors.rs +++ b/crates/perry-runtime/src/builtins/formatting/errors.rs @@ -49,7 +49,13 @@ unsafe fn format_error_headline(error_ptr: *const crate::error::ErrorHeader) -> } unsafe fn format_error_stack_frame(error_ptr: *const crate::error::ErrorHeader) -> Option { - let stack = string_header_to_string((*error_ptr).stack, ""); + // #9486 made Error stacks lazy: `ErrorHeader.stack` stays null until the + // first observable read materializes the captured frame payload. Inspect + // is one of those reads. Looking at the slot directly made Errors with a + // cause or AggregateError.errors print a standalone `{` where Node keeps + // it attached to the final stack line. + let stack_ptr = crate::error::materialize_error_stack(error_ptr.cast_mut()); + let stack = string_header_to_string(stack_ptr, ""); stack .lines() .skip(1) From a898adeaeee396460540b2ec7a89f4713cd0fb6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 03:44:11 +0200 Subject: [PATCH 11/13] fix(module): keep extensions out of builtin inventory (cherry picked from commit 85b9fea5193e56cb8f11adcbd6307e3f17bf2081) --- changelog.d/9934-module-builtin-inventory.md | 4 ++ crates/perry-runtime/src/module_require.rs | 6 +-- crates/perry-runtime/src/process.rs | 55 +++++++++++++------- 3 files changed, 44 insertions(+), 21 deletions(-) create mode 100644 changelog.d/9934-module-builtin-inventory.md diff --git a/changelog.d/9934-module-builtin-inventory.md b/changelog.d/9934-module-builtin-inventory.md new file mode 100644 index 0000000000..16cc937547 --- /dev/null +++ b/changelog.d/9934-module-builtin-inventory.md @@ -0,0 +1,4 @@ +### Fixed + +- Keep Perry's `node:ffi` extension resolvable without exposing it through + Node's `module.builtinModules` and `Module.isBuiltin()` inventory. diff --git a/crates/perry-runtime/src/module_require.rs b/crates/perry-runtime/src/module_require.rs index 59ba05ac50..f7424c02c7 100644 --- a/crates/perry-runtime/src/module_require.rs +++ b/crates/perry-runtime/src/module_require.rs @@ -126,9 +126,9 @@ fn validate_create_require_base(filename_or_url: f64) { /// from the static-import tables — `v8` (and `sea`, `fs/promises`, /// `stream/consumers`, `stream/web`, `trace_events`, `test/reporters`) were /// implemented and statically importable but rejected here as "package/file". -/// Both resolvers now share one source of truth (`MODULE_BUILTIN_MODULES`, -/// i.e. `module.builtinModules`), including the `node:` normalization and the -/// scheme-only / `_`-internal carve-outs. +/// Both resolvers now share the Node inventory plus Perry's explicit builtin +/// extensions, including the `node:` normalization and the scheme-only / +/// `_`-internal carve-outs. fn supported_require_builtin(specifier: &str) -> Option<&str> { crate::process::supported_builtin_module_name(specifier) } diff --git a/crates/perry-runtime/src/process.rs b/crates/perry-runtime/src/process.rs index 0c77d5c427..7c7ac675a3 100644 --- a/crates/perry-runtime/src/process.rs +++ b/crates/perry-runtime/src/process.rs @@ -114,21 +114,24 @@ pub(crate) fn is_function_value(value: f64) -> bool { /// #6651: single source of truth for the RUNTIME dynamic builtin resolvers. /// `process.getBuiltinModule(id)` and the `require` returned by -/// `module.createRequire(...)` accept exactly the module set of -/// `module.builtinModules` (`MODULE_BUILTIN_MODULES`), so the three surfaces -/// can never drift apart again — pi walls #3 (#6644, `diagnostics_channel`) -/// and #5 (#6651, `v8`) were both a module implemented and statically -/// importable but missing from one hand-copied allowlist. Two carve-outs: +/// `module.createRequire(...)` derive their Node surface from +/// `module.builtinModules` (`MODULE_BUILTIN_MODULES`). Pi walls #3 (#6644, +/// `diagnostics_channel`) and #5 (#6651, `v8`) were both a module implemented +/// and statically importable but missing from one hand-copied allowlist. Perry +/// extensions are listed separately so resolving one cannot add it to Node's +/// public inventory. Two carve-outs apply: /// /// - `_`-prefixed legacy internals (`_http_agent`, …): Node still serves /// them, Perry has no implementation — they must keep failing with an /// error that names the module, not resolve to a method-dead namespace. -/// - Scheme-only builtins (`node:ffi`, `node:sea`, `node:sqlite`, `node:test`, +/// - Scheme-only builtins (`node:sea`, `node:sqlite`, `node:test`, and /// `node:test/reporters` — stored WITH the prefix, exactly as Node spells -/// them in `module.builtinModules`): resolve only when the caller wrote -/// the `node:` prefix. The bare spelling is an ordinary npm package name -/// in Node (`require('sqlite')` is `MODULE_NOT_FOUND`, -/// `getBuiltinModule('sqlite')` is `undefined`). +/// them in `module.builtinModules`): resolve only when the caller wrote the +/// `node:` prefix. The bare spelling is an ordinary npm package name in +/// Node (`require('sqlite')` is `MODULE_NOT_FOUND`, +/// `getBuiltinModule('sqlite')` is `undefined`). Perry's scheme-only +/// extensions use the same resolver rule but live outside the public Node +/// inventory. /// /// Takes the RAW specifier (either spelling); returns the prefixless name. pub(crate) fn supported_builtin_module_name(specifier: &str) -> Option<&str> { @@ -148,7 +151,9 @@ pub(crate) fn supported_builtin_module_name(specifier: &str) -> Option<&str> { return None; } if MODULE_BUILTIN_MODULES.contains(&name) - || (had_node_prefix && MODULE_BUILTIN_MODULES.contains(&specifier)) + || (had_node_prefix + && (MODULE_BUILTIN_MODULES.contains(&specifier) + || PERRY_BUILTIN_MODULE_EXTENSIONS.contains(&specifier))) { return Some(name); } @@ -189,6 +194,11 @@ pub(crate) fn builtin_module_value(module_name: &str) -> f64 { pub(crate) const MODULE_CJS_CLASS_ID: u32 = 0xC0_00_4D; +/// Perry-provided scheme-only modules which resolve like builtins but are not +/// part of Node 26's public `module.builtinModules` / `Module.isBuiltin()` +/// inventory. +pub(crate) const PERRY_BUILTIN_MODULE_EXTENSIONS: &[&str] = &["node:ffi"]; + pub(crate) const MODULE_BUILTIN_MODULES: &[&str] = &[ "_http_agent", "_http_client", @@ -252,7 +262,6 @@ pub(crate) const MODULE_BUILTIN_MODULES: &[&str] = &[ "wasi", "worker_threads", "zlib", - "node:ffi", "node:sea", "node:sqlite", "node:test", @@ -818,10 +827,10 @@ thread_local! { } /// #6651 family regression guard: the dynamic builtin resolvers -/// (`createRequire(...)`'s `require` + `process.getBuiltinModule`) derive from -/// `MODULE_BUILTIN_MODULES`, so every module Perry lists in +/// (`createRequire(...)`'s `require` + `process.getBuiltinModule`) derive their +/// Node surface from `MODULE_BUILTIN_MODULES`, so every module Perry lists in /// `module.builtinModules` must resolve through them — and only through the -/// spellings Node itself accepts. +/// spellings Node itself accepts. Perry extensions are checked separately. #[cfg(test)] mod builtin_module_list_tests { use super::*; @@ -836,9 +845,9 @@ mod builtin_module_list_tests { let prefixed = format!("node:{entry}"); assert_eq!(supported_builtin_module_name(&prefixed), None, "{prefixed}"); } else if let Some(bare) = entry.strip_prefix("node:") { - // Scheme-only builtins (node:ffi, node:sea, node:sqlite, - // node:test, node:test/reporters): the prefixed spelling resolves, the - // bare spelling is an ordinary npm name (Node parity). + // Scheme-only builtins (node:sea, node:sqlite, node:test, + // node:test/reporters): the prefixed spelling resolves, the bare + // spelling is an ordinary npm name (Node parity). assert_eq!(supported_builtin_module_name(entry), Some(bare), "{entry}"); assert_eq!(supported_builtin_module_name(bare), None, "{bare}"); } else { @@ -854,6 +863,16 @@ mod builtin_module_list_tests { } } + #[test] + fn perry_extensions_resolve_without_polluting_the_node_inventory() { + for &entry in PERRY_BUILTIN_MODULE_EXTENSIONS { + let bare = entry.strip_prefix("node:").expect("scheme-only extension"); + assert!(!MODULE_BUILTIN_MODULES.contains(&entry), "{entry}"); + assert_eq!(supported_builtin_module_name(entry), Some(bare), "{entry}"); + assert_eq!(supported_builtin_module_name(bare), None, "{bare}"); + } + } + #[test] fn non_builtins_are_rejected() { for specifier in [ From 14d8da55574aafe9363bb5b5a1a622ef0a08ca22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 00:44:51 +0200 Subject: [PATCH 12/13] fix(child_process): align exec metadata and sync limits (cherry picked from commit f5010a97caf999e3a13aa8ef196cd5de61c7d48d) --- .../perry-runtime/src/child_process/exec.rs | 28 ++-- .../src/child_process/reactor.rs | 12 +- .../src/child_process/sync_run.rs | 129 ++++++++++++------ 3 files changed, 121 insertions(+), 48 deletions(-) diff --git a/crates/perry-runtime/src/child_process/exec.rs b/crates/perry-runtime/src/child_process/exec.rs index adfc1ac7bc..7ed352e59e 100644 --- a/crates/perry-runtime/src/child_process/exec.rs +++ b/crates/perry-runtime/src/child_process/exec.rs @@ -62,8 +62,7 @@ pub extern "C" fn js_child_process_exec_sync( }; cp_apply_options(&mut command, opts_val); - let mut run_options = cp_read_sync_stdio_run_options(opts_val); - run_options.mark_shell_command(); + let run_options = cp_read_sync_stdio_run_options(opts_val); let run = cp_run_to_completion(command, &run_options); let stdout_box = cp_box_run_output(&run.stdout, run.stdout_piped, &mode); if run.success() { @@ -298,8 +297,7 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64, c }; cp_apply_options(&mut command, arg1); - let mut run_options = cp_read_async_run_options(arg1); - run_options.mark_shell_command(); + let run_options = cp_read_async_run_options(arg1); if cb.is_null() { // Legacy no-callback shape — run synchronously and return stdout @@ -312,7 +310,7 @@ pub extern "C" fn js_child_process_exec(cmd_ptr: *const StringHeader, arg1: f64, // With a callback, run asynchronously: off the main thread, with the // callback fired on a later event-loop tick (#4912). - reactor::cp_exec_async(command, cmd_str, cb_val, run_options, mode) + reactor::cp_exec_async(command, cmd_str, None, cb_val, run_options, mode) } /// `child_process.execFile(file[, args][, options][, callback])` — like `exec` @@ -383,6 +381,7 @@ pub extern "C" fn js_child_process_exec_file( reactor::cp_exec_async( command, cp_file_cmd_display(&file_str, &arg_strs), + Some(file_str), cb_nanbox, run_options, mode, @@ -471,7 +470,12 @@ extern "C" fn cp_promise_settle_cb( /// `command` through the async exec reactor (#4912). Returns the NaN-boxed /// pending promise. The settle closure (and through it the promise) is kept /// alive by the reactor's exec-callback GC root. -fn cp_promisified_run(command: Command, cmd_str: String, opts: f64) -> f64 { +fn cp_promisified_run( + command: Command, + cmd_str: String, + public_spawnfile: Option, + opts: f64, +) -> f64 { let run_options = cp_read_async_run_options(opts); // promisify(exec)/promisify(execFile) yield string stdout/stderr (utf8). let mode = cp_read_output_mode(opts, true); @@ -480,7 +484,14 @@ fn cp_promisified_run(command: Command, cmd_str: String, opts: f64) -> f64 { let cb = js_closure_alloc(cp_promise_settle_cb as *const u8, 1); js_closure_set_capture_ptr(cb, 0, cp_box_ptr(promise as *const u8).to_bits() as i64); let cb_val = crate::value::js_nanbox_pointer(cb as i64); - let child = reactor::cp_exec_async(command, cmd_str, cb_val, run_options, mode); + let child = reactor::cp_exec_async( + command, + cmd_str, + public_spawnfile, + cb_val, + run_options, + mode, + ); crate::object::exotic_expando::value_store( crate::object::exotic_expando::ExoticKind::Promise, promise as usize, @@ -509,7 +520,7 @@ extern "C" fn cp_promisified_exec(_closure: *const ClosureHeader, cmd_val: f64, c }; cp_apply_options(&mut command, opts); - cp_promisified_run(command, cmd, opts) + cp_promisified_run(command, cmd, None, opts) } extern "C" fn cp_promisified_exec_file( @@ -529,6 +540,7 @@ extern "C" fn cp_promisified_exec_file( cp_promisified_run( command, cp_file_cmd_display(&file, &arg_strs), + Some(file), f64::from_bits(TAG_UNDEFINED_BITS), ) } diff --git a/crates/perry-runtime/src/child_process/reactor.rs b/crates/perry-runtime/src/child_process/reactor.rs index be104b70c7..0b7653c2d6 100644 --- a/crates/perry-runtime/src/child_process/reactor.rs +++ b/crates/perry-runtime/src/child_process/reactor.rs @@ -1223,6 +1223,7 @@ pub(super) fn cp_register_reactor_arities() { pub(super) fn cp_exec_async( mut command: Command, cmd_str: String, + public_spawnfile: Option, cb_val: f64, run_options: CpRunOptions, mode: CpOutput, @@ -1283,7 +1284,16 @@ pub(super) fn cp_exec_async( cp_set_field(cp, b"signalCode", TAG_NULL_F64); cp_set_field(cp, b"killed", TAG_FALSE_F64); cp_set_field(cp, b"connected", TAG_FALSE_F64); - cp_set_field(cp, b"spawnfile", cp_box_string(&file)); + // `cp_command_for_program` may resolve a bare executable against PATH to + // keep macOS on `posix_spawn`, but Node exposes the caller's original + // `execFile` spelling through `ChildProcess.spawnfile`. Keep the resolved + // path above for launch-error metadata and publish the explicit spelling + // when that API supplied one. `exec` continues to expose its real shell. + cp_set_field( + cp, + b"spawnfile", + cp_box_string(public_spawnfile.as_deref().unwrap_or(&file)), + ); match command.spawn() { Ok(mut child) => { diff --git a/crates/perry-runtime/src/child_process/sync_run.rs b/crates/perry-runtime/src/child_process/sync_run.rs index 403721df20..a790b78bce 100644 --- a/crates/perry-runtime/src/child_process/sync_run.rs +++ b/crates/perry-runtime/src/child_process/sync_run.rs @@ -1,5 +1,7 @@ -use std::io::Write; -use std::process::{Command, Stdio}; +use std::io::{Read, Write}; +use std::process::{Child, Command, ExitStatus, Stdio}; +use std::sync::mpsc::{self, Sender}; +use std::thread::JoinHandle; use std::time::{Duration, Instant}; use crate::value::JSValue; @@ -16,7 +18,6 @@ pub(crate) struct CpRunOptions { input: Option>, timeout: Option, kill_signal: i32, - shell_command: bool, pub(super) max_buffer: usize, stdio: [CpStdio; 3], } @@ -33,10 +34,6 @@ impl CpRunOptions { pub(super) fn timeout(&self) -> Option { self.timeout } - - pub(super) fn mark_shell_command(&mut self) { - self.shell_command = true; - } } impl Default for CpRunOptions { @@ -45,7 +42,6 @@ impl Default for CpRunOptions { input: None, timeout: None, kill_signal: CP_SIGTERM, - shell_command: false, max_buffer: CP_DEFAULT_MAX_BUFFER, stdio: [CpStdio::Pipe; 3], } @@ -129,7 +125,6 @@ pub(super) fn cp_read_spawn_sync_run_options(opts_val: f64) -> CpRunOptions { stdio.get(1).copied().unwrap_or(CpStdio::Pipe), stdio.get(2).copied().unwrap_or(CpStdio::Pipe), ]; - options.shell_command = crate::value::js_is_truthy(cp_get_field(opts_val, b"shell")) != 0; options } @@ -218,10 +213,6 @@ impl CpRun { /// Piped stdin without input is closed so children that read stdin see EOF /// instead of blocking. Used by synchronous + buffered-callback entry points. pub(super) fn cp_run_to_completion(mut command: Command, options: &CpRunOptions) -> CpRun { - // A shell that has already completed its short command reports its real - // exit status with ENOBUFS; a direct child is still terminable at the - // buffer threshold and reports the configured signal. - let shell_command = options.shell_command; let stdin_piped = matches!(options.stdio[0], CpStdio::Pipe) && options.input.is_some(); let stdout_piped = matches!(options.stdio[1], CpStdio::Pipe); let stderr_piped = matches!(options.stdio[2], CpStdio::Pipe); @@ -246,33 +237,37 @@ pub(super) fn cp_run_to_completion(mut command: Command, options: &CpRunOptions) match command.spawn() { Ok(mut child) => { let pid = child.id(); + let (limit_tx, limit_rx) = mpsc::channel(); + let stdout_reader = + cp_read_piped_output(child.stdout.take(), options, pid, limit_tx.clone()); + let stderr_reader = cp_read_piped_output(child.stderr.take(), options, pid, limit_tx); if stdin_piped { if let (Some(input), Some(mut stdin)) = (&options.input, child.stdin.take()) { let _ = stdin.write_all(input); } } - let mut run_error = - cp_wait_for_timeout(&mut child, options.timeout, options.kill_signal); - match child.wait_with_output() { - Ok(o) => { - let CpExit { code, signal } = cp_decode_status(&o.status); + drop(child.stdin.take()); + + let mut run_error = None; + match cp_wait_for_buffered_child(&mut child, options, &limit_rx, &mut run_error) { + Ok(status) => { + let stdout = cp_join_piped_output(stdout_reader); + let stderr = cp_join_piped_output(stderr_reader); + let CpExit { code, signal } = cp_decode_status(&status); if run_error.is_none() && options.max_buffer > 0 - && ((stdout_piped && o.stdout.len() > options.max_buffer) - || (stderr_piped && o.stderr.len() > options.max_buffer)) + && ((stdout_piped && stdout.len() > options.max_buffer) + || (stderr_piped && stderr.len() > options.max_buffer)) { run_error = Some(CpRunError::MaxBuffer); } let (code, signal) = match run_error { Some(CpRunError::Timeout) => (None, Some(options.kill_signal)), - Some(CpRunError::MaxBuffer) if !shell_command => { - (None, Some(options.kill_signal)) - } _ => (code, signal), }; CpRun { - stdout: o.stdout, - stderr: o.stderr, + stdout, + stderr, stdout_piped, stderr_piped, code, @@ -309,25 +304,81 @@ pub(super) fn cp_run_to_completion(mut command: Command, options: &CpRunOptions) } } -fn cp_wait_for_timeout( - child: &mut std::process::Child, - timeout: Option, - kill_signal: i32, -) -> Option { - let timeout = timeout?; - let deadline = Instant::now() + timeout; +fn cp_read_piped_output( + reader: Option, + options: &CpRunOptions, + _pid: u32, + limit_tx: Sender<()>, +) -> Option>> { + let max_buffer = options.max_buffer; + #[cfg(unix)] + let kill_signal = options.kill_signal; + reader.map(|mut reader| { + std::thread::spawn(move || { + let mut output = Vec::new(); + let mut chunk = [0_u8; 8192]; + let mut reported_limit = false; + loop { + match reader.read(&mut chunk) { + Ok(0) | Err(_) => break, + Ok(read) => { + output.extend_from_slice(&chunk[..read]); + if !reported_limit && max_buffer > 0 && output.len() > max_buffer { + // Signal at the read that crosses the limit. Going + // through the parent's polling loop here leaves a + // fast child enough time to exit before the signal. + #[cfg(unix)] + unsafe { + let _ = libc::kill(_pid as i32, kill_signal); + } + let _ = limit_tx.send(()); + reported_limit = true; + } + } + } + } + output + }) + }) +} + +fn cp_join_piped_output(reader: Option>>) -> Vec { + reader + .and_then(|reader| reader.join().ok()) + .unwrap_or_default() +} + +fn cp_wait_for_buffered_child( + child: &mut Child, + options: &CpRunOptions, + limit_rx: &mpsc::Receiver<()>, + run_error: &mut Option, +) -> std::io::Result { + let deadline = options.timeout.map(|timeout| Instant::now() + timeout); loop { + if run_error.is_none() && limit_rx.try_recv().is_ok() { + *run_error = Some(CpRunError::MaxBuffer); + // A short child can exit between filling the pipe and this + // notification. Preserve that real exit status, as Node does, + // and terminate only a child that is still running. + if let Some(status) = child.try_wait()? { + return Ok(status); + } + cp_terminate_child(child, options.kill_signal); + } + match child.try_wait() { - Ok(Some(_)) => return None, + Ok(Some(status)) => return Ok(status), Ok(None) => { - if Instant::now() >= deadline { - cp_terminate_child(child, kill_signal); - return Some(CpRunError::Timeout); + if run_error.is_none() + && deadline.is_some_and(|deadline| Instant::now() >= deadline) + { + *run_error = Some(CpRunError::Timeout); + cp_terminate_child(child, options.kill_signal); } - let remaining = deadline.saturating_duration_since(Instant::now()); - std::thread::sleep(remaining.min(Duration::from_millis(5))); + std::thread::sleep(Duration::from_millis(1)); } - Err(_) => return None, + Err(error) => return Err(error), } } } From 3d1fe638c2a71273c58c82c35fe26f3fce1ad45c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 7 Sep 2026 00:45:38 +0200 Subject: [PATCH 13/13] docs(changelog): note child process parity fixes (cherry picked from commit a1b66b46423456301fdd2c8a1f2cd10a27f32c9e) --- changelog.d/9926-child-process-parity.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/9926-child-process-parity.md diff --git a/changelog.d/9926-child-process-parity.md b/changelog.d/9926-child-process-parity.md new file mode 100644 index 0000000000..1396899def --- /dev/null +++ b/changelog.d/9926-child-process-parity.md @@ -0,0 +1,5 @@ +### Fixed + +- Match Node's `child_process` metadata and synchronous buffer-limit behavior by + preserving the caller's `execFile` spelling and signaling a process when its + output crosses `maxBuffer` only while it is still running.