diff --git a/.github/workflows/auto-opt-app-patterns.yml b/.github/workflows/auto-opt-app-patterns.yml index f8e6d3aae0..5e89f4777b 100644 --- a/.github/workflows/auto-opt-app-patterns.yml +++ b/.github/workflows/auto-opt-app-patterns.yml @@ -40,8 +40,10 @@ name: Auto-Optimize App Patterns # every output comparison while testing the exact configuration this job # does not care about. # -# The one skip (`promise_all_chains`) is named with a reason inside the script, -# and a skip entry that matches no kernel FAILS, so it cannot outlive its fix. +# The skip list inside the script is EMPTY as of #7497 — every kernel in +# `benchmarks/app-patterns/kernels` is gated. A skip entry that matches no kernel +# FAILS, so an exemption cannot outlive its fix; that rule is what took +# `promise_all_chains` out of the list in the same PR that fixed it. on: pull_request: diff --git a/CLAUDE.md b/CLAUDE.md index 017709f0db..6f0f55235b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co 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. -**Current Version:** 0.5.1296 +**Current Version:** 0.5.1297 ## TypeScript Parity Status diff --git a/Cargo.lock b/Cargo.lock index 027ab2ba1f..3999f1027c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5547,7 +5547,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" [[package]] name = "perry" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "base64", @@ -5607,14 +5607,14 @@ dependencies = [ [[package]] name = "perry-api-manifest" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "serde", ] [[package]] name = "perry-audio-miniaudio" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "cc", "libc", @@ -5622,7 +5622,7 @@ dependencies = [ [[package]] name = "perry-codegen" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "inkwell", @@ -5639,7 +5639,7 @@ dependencies = [ [[package]] name = "perry-codegen-arkts" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-hir", @@ -5647,7 +5647,7 @@ dependencies = [ [[package]] name = "perry-codegen-glance" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-hir", @@ -5655,7 +5655,7 @@ dependencies = [ [[package]] name = "perry-codegen-js" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-dispatch", @@ -5664,7 +5664,7 @@ dependencies = [ [[package]] name = "perry-codegen-swiftui" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-hir", @@ -5672,7 +5672,7 @@ dependencies = [ [[package]] name = "perry-codegen-wasm" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "base64", @@ -5684,7 +5684,7 @@ dependencies = [ [[package]] name = "perry-codegen-wear-tiles" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-hir", @@ -5692,7 +5692,7 @@ dependencies = [ [[package]] name = "perry-container-compose" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "async-trait", @@ -5721,14 +5721,14 @@ dependencies = [ [[package]] name = "perry-container-e2e" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", ] [[package]] name = "perry-diagnostics" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "serde", "serde_json", @@ -5736,7 +5736,7 @@ dependencies = [ [[package]] name = "perry-dispatch" -version = "0.5.1296" +version = "0.5.1297" [[package]] name = "perry-doc-fixture-my-bindings" @@ -5747,7 +5747,7 @@ dependencies = [ [[package]] name = "perry-doc-tests" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "clap", @@ -5762,7 +5762,7 @@ dependencies = [ [[package]] name = "perry-ext-ads" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "block2", "objc2", @@ -5772,7 +5772,7 @@ dependencies = [ [[package]] name = "perry-ext-argon2" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "argon2", "perry-ffi", @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "perry-ext-axios" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "reqwest", @@ -5789,7 +5789,7 @@ dependencies = [ [[package]] name = "perry-ext-bcrypt" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "bcrypt", "perry-ffi", @@ -5797,7 +5797,7 @@ dependencies = [ [[package]] name = "perry-ext-better-sqlite3" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "rusqlite", @@ -5805,7 +5805,7 @@ dependencies = [ [[package]] name = "perry-ext-cheerio" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "scraper", @@ -5813,7 +5813,7 @@ dependencies = [ [[package]] name = "perry-ext-commander" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "perry-runtime", @@ -5821,7 +5821,7 @@ dependencies = [ [[package]] name = "perry-ext-cron" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "chrono", "cron", @@ -5831,7 +5831,7 @@ dependencies = [ [[package]] name = "perry-ext-dayjs" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "chrono", "perry-ffi", @@ -5839,7 +5839,7 @@ dependencies = [ [[package]] name = "perry-ext-decimal" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "rust_decimal", @@ -5847,7 +5847,7 @@ dependencies = [ [[package]] name = "perry-ext-dotenv" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "serde_json", @@ -5855,7 +5855,7 @@ dependencies = [ [[package]] name = "perry-ext-ethers" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "rand 0.10.1", @@ -5863,7 +5863,7 @@ dependencies = [ [[package]] name = "perry-ext-events" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "perry-runtime", @@ -5871,14 +5871,14 @@ dependencies = [ [[package]] name = "perry-ext-exponential-backoff" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-fastify" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "bytes", "http-body-util", @@ -5896,7 +5896,7 @@ dependencies = [ [[package]] name = "perry-ext-fetch" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "bytes", "lazy_static", @@ -5909,7 +5909,7 @@ dependencies = [ [[package]] name = "perry-ext-http" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "bytes", "h2", @@ -5933,7 +5933,7 @@ dependencies = [ [[package]] name = "perry-ext-ioredis" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "lazy_static", "perry-ffi", @@ -5943,7 +5943,7 @@ dependencies = [ [[package]] name = "perry-ext-jsonwebtoken" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "jsonwebtoken", @@ -5954,7 +5954,7 @@ dependencies = [ [[package]] name = "perry-ext-lru-cache" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "lru", "perry-ffi", @@ -5963,7 +5963,7 @@ dependencies = [ [[package]] name = "perry-ext-moment" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "chrono", "perry-ffi", @@ -5971,7 +5971,7 @@ dependencies = [ [[package]] name = "perry-ext-mongodb" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "bson", "futures-util", @@ -5983,7 +5983,7 @@ dependencies = [ [[package]] name = "perry-ext-mysql2" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "chrono", "perry-ffi", @@ -5993,7 +5993,7 @@ dependencies = [ [[package]] name = "perry-ext-nanoid" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "nanoid", "perry-ffi", @@ -6002,7 +6002,7 @@ dependencies = [ [[package]] name = "perry-ext-net" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "bytes", "perry-ffi", @@ -6015,7 +6015,7 @@ dependencies = [ [[package]] name = "perry-ext-node-forge" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "const-oid 0.9.6", "der 0.7.10", @@ -6034,7 +6034,7 @@ dependencies = [ [[package]] name = "perry-ext-nodemailer" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "lettre", "perry-ffi", @@ -6044,7 +6044,7 @@ dependencies = [ [[package]] name = "perry-ext-pdf" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "printpdf", @@ -6052,7 +6052,7 @@ dependencies = [ [[package]] name = "perry-ext-pg" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "sqlx", @@ -6061,7 +6061,7 @@ dependencies = [ [[package]] name = "perry-ext-ratelimit" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "governor", "perry-ffi", @@ -6069,7 +6069,7 @@ dependencies = [ [[package]] name = "perry-ext-sharp" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "fast_image_resize", "image", @@ -6079,14 +6079,14 @@ dependencies = [ [[package]] name = "perry-ext-slugify" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", ] [[package]] name = "perry-ext-streams" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "lazy_static", "perry-ffi", @@ -6095,7 +6095,7 @@ dependencies = [ [[package]] name = "perry-ext-undici" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "perry-runtime", @@ -6104,7 +6104,7 @@ dependencies = [ [[package]] name = "perry-ext-uuid" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "uuid", @@ -6112,7 +6112,7 @@ dependencies = [ [[package]] name = "perry-ext-validator" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ffi", "regex", @@ -6122,7 +6122,7 @@ dependencies = [ [[package]] name = "perry-ext-ws" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "futures-util", "lazy_static", @@ -6135,7 +6135,7 @@ dependencies = [ [[package]] name = "perry-ext-zlib" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "brotli", "flate2", @@ -6145,7 +6145,7 @@ dependencies = [ [[package]] name = "perry-ffi" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "dashmap", "once_cell", @@ -6154,7 +6154,7 @@ dependencies = [ [[package]] name = "perry-hir" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-api-manifest", @@ -6172,7 +6172,7 @@ dependencies = [ [[package]] name = "perry-parser" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-diagnostics", @@ -6184,7 +6184,7 @@ dependencies = [ [[package]] name = "perry-runtime" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "base64", @@ -6226,14 +6226,14 @@ dependencies = [ [[package]] name = "perry-runtime-static" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-runtime", ] [[package]] name = "perry-stdlib" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "aes 0.8.4", "aes 0.9.1", @@ -6328,14 +6328,14 @@ dependencies = [ [[package]] name = "perry-stdlib-static" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-stdlib", ] [[package]] name = "perry-transform" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "perry-hir", @@ -6344,14 +6344,14 @@ dependencies = [ [[package]] name = "perry-ui" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ui-model", ] [[package]] name = "perry-ui-android" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "itoa", @@ -6368,7 +6368,7 @@ dependencies = [ [[package]] name = "perry-ui-geisterhand" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "rand 0.10.1", "serde", @@ -6378,7 +6378,7 @@ dependencies = [ [[package]] name = "perry-ui-gtk4" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "cairo-rs 0.22.0", @@ -6401,7 +6401,7 @@ dependencies = [ [[package]] name = "perry-ui-ios" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "block2", @@ -6417,7 +6417,7 @@ dependencies = [ [[package]] name = "perry-ui-macos" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "block2", @@ -6432,7 +6432,7 @@ dependencies = [ [[package]] name = "perry-ui-model" -version = "0.5.1296" +version = "0.5.1297" [[package]] name = "perry-ui-test" @@ -6443,11 +6443,11 @@ dependencies = [ [[package]] name = "perry-ui-testkit" -version = "0.5.1296" +version = "0.5.1297" [[package]] name = "perry-ui-tvos" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "block2", @@ -6463,7 +6463,7 @@ dependencies = [ [[package]] name = "perry-ui-visionos" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "block2", @@ -6479,7 +6479,7 @@ dependencies = [ [[package]] name = "perry-ui-watchos" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "block2", "libc", @@ -6492,7 +6492,7 @@ dependencies = [ [[package]] name = "perry-ui-windows" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "base64", "libc", @@ -6509,14 +6509,14 @@ dependencies = [ [[package]] name = "perry-ui-windows-winui" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "perry-ui-windows", ] [[package]] name = "perry-updater" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "anyhow", "base64", @@ -6532,7 +6532,7 @@ dependencies = [ [[package]] name = "perry-wasm-host" -version = "0.5.1296" +version = "0.5.1297" dependencies = [ "wasmi", ] diff --git a/Cargo.toml b/Cargo.toml index 1f1466ccfc..ed5acc1f5c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -315,7 +315,7 @@ codegen-units = 16 codegen-units = 16 [workspace.package] -version = "0.5.1296" +version = "0.5.1297" edition = "2021" license = "MIT" repository = "https://github.com/PerryTS/perry" diff --git a/changelog.d/7516-promise-all-chains.md b/changelog.d/7516-promise-all-chains.md new file mode 100644 index 0000000000..284deeb987 --- /dev/null +++ b/changelog.d/7516-promise-all-chains.md @@ -0,0 +1,160 @@ +### Fixed + +**`Promise.all` at scale rejected with a resolution value, or with +`TypeError: value is not a function` — eight stale from-space reads, none of them +in the promise machinery's *logic* (#7497).** + +The app-pattern kernel `promise_all_chains` printed `Uncaught (in promise) 0` +under `PERRY_NO_AUTO_OPTIMIZE=1` and a rooting-shaped `TypeError` under the +default link, and was the last blocker on the public benchmark artifact. No +settle/reject/microtask *decision* changed. Every defect is the #7341 family — +*a value read out of a root and held in a register across a call that allocates +is not rooted* — and the fix is the same shape each time: `RuntimeHandleScope` +plus a re-read at the point of use, so the pre-collection address is never +nameable. + +They were found one at a time. Each `PERRY_GC_PROTECT_FROMSPACE=1` fault named a +site; fixing it moved the fault and exposed the next. + +1. **`js_get_global_this_builtin_value`** — the canonical `globalThis.` + read behind `instance.constructor`, bare `Date`/`Array`/`Object` identifier + resolution, and `is_default_promise_constructor`: + + ```rust + let global_obj = js_nanbox_get_pointer(js_get_global_this()); + let key = js_string_from_bytes(name); // ALLOCATES -- may collect + js_object_get_field_by_name(global_obj, key); // from-space deref + ``` + + The root is fine — `THREAD_GLOBAL_THIS` is registered and evacuation rewrites + it. The ORDER is not: this lookup interns nothing, so every call mints a fresh + string, and any of those allocations can be the copying minor that moves + `globalThis`. + +2. **`js_promise_resolve_spec`** — `Promise.all` calls it once per element, and + it asks `is_default_promise_constructor` (i.e. 1) before touching its own + argument. For `Promise.all([...promises])` that argument IS a promise, so + `js_promise_resolved` dereferenced a from-space `GC_TYPE_PROMISE`. + `js_promise_reject_spec`, `js_promise_try_spec`, + `js_promise_with_resolvers_spec` and `promise_prototype_then_thunk` share the + shape. + +3. **`js_array_clone`** — the widest. It read the source array pointer, called + `js_array_alloc(len)` for the destination (which can collect and MOVE the + source), then `copy_nonoverlapping`'d from the pre-collection address. That is + every `[...arr]`, every `Array.from(arr)` and every combinator's iterable + snapshot. + +4. **The spec combinators' own locals.** `perform` ran its per-element loop — + `Call(promiseResolve, C, «next»)` and `Invoke(nextPromise, "then", …)`, both + of which run USER JS — while holding the `elements` snapshot, the shared + `values` and remaining-count arrays, the capability's `resolve`/`reject` and + the constructor in bare Rust locals. + +5. **Two publishers, worse than a stale read.** `build_element_closure` and + `make_resolving_functions` take their GC arguments in registers, allocate, and + then *store the pre-collection addresses into capture slots* — putting + from-space into an object the collector goes on maintaining. + `new_promise_capability`, the two `Promise.allSettled` element functions, + `build_settled_{fulfilled,rejected}` and `combinator_iterable_to_array`'s + array fast path had shape (4) or (5). + +6. **The microtask runner's dispatch arms.** Every arm read its callback pointer + (and the value it passes) out of the popped `Task` into a bare local, ran + `async_hooks::before` / `v8::promise_hook_before` — both allocate — and only + then loaded `func_ptr` out of that pointer. #1663 had already rooted `promise` + and `next` here; the callback was missed. + +7. **…and rooting them inside the arm was still too late.** A `Task` stops being + a scanned root the instant it is popped, and `enter_microtask_context` runs + before any of the arm's own bookkeeping. The first attempt seeded the handle + with an address the collection had already invalidated. Every arm now roots as + its first statement and re-reads after the context switch. Disassembling the + faulting instruction — `bl get_nanbox_u64; ldr x8,[x21]` — is what showed the + re-read was present and *still* wrong, which is what made the ordering the + suspect rather than the rooting. + +8. **The producer side.** `js_async_step_chain` carried the step closure through + `adapt_foreign_promise_value`, `js_promise_new`, `build_async_step_thunks`, + `js_promise_resolved` and `capture_context` and then STORED it into a + `Task::AsyncStep`. The queue is a scanned root and the runner now re-reads + what it pops — neither helps when the pointer was already dead at the push. + `js_async_step_done` had the mirror image: it settled `trap_next` (which + allocates) and returned the *pre-call* copy as the async function's own result + promise. + +Review (CodeRabbit) found six more of the same shape, all fixed here: the +SEARCHED closure value in `class_meta.rs`'s lookup loop (which *misses* rather +than crashing, so the caller silently falls through to the generic construct +tail); `perform_promise_then_with_cap`, which filled one wrapper's captures and +then allocated the other; `js_array_values`, carrying (3)'s shape; +`combinator_iterable_to_array`'s `GC_TYPE_OBJECT` arm, whose receiver crossed a +user `[Symbol.iterator]` getter; `js_async_step_chain`'s three early-return +suspend paths together with `then_backpatch_result` (which allocates the result +promise and then STORES it into both thunks); and one encoding mismatch between +`boxed_closure` and `rooted_closure` in the `AsyncStep` arm. + +All handles are NaN-boxed rather than `root_raw_*_ptr`, so +`scripts/raw_handle_debt.py` is unchanged at 999. The per-element handles in +`perform` live in a scope INSIDE the loop, so a 50 000-element combinator does not +push 50 000 entries onto the handle stack. + +**Three more callers of `js_get_global_this()` had (1)'s shape** and are fixed +the same way. These come from auditing the callers, not from a reproducer, and +are called out as such: `class_meta.rs`'s builtin-constructor name walk (worse +than the proven site — a fresh key allocation inside a ~50-iteration loop against +one address read before the loop), `js_globalthis_seed_async_local_storage` +(`globalThis` is the RECEIVER of a store that follows two allocations), and the +`Temporal..prototype` walk. The four sites of this shape in `error.rs` / +`with_env.rs` were already fixed by #6943; these are the ones that sweep missed. + +**Why it read as "a separate promise-rejection defect".** A stale read returns +whatever from-space happens to hold, so `globalThis.Promise` came back as a +non-callable — or, when the garbage was zero, as the resolution value `0` +arriving on the rejection path. The two link modes printed different messages for +the same defect. It was untouched by #7495 only because #7495 fixed a different +function. + +**Localisation, for the next person** (each knob against the unfixed binary): +`PERRY_GEN_GC=0` and `PERRY_WRITE_BARRIERS=0` both make it pass while +`PERRY_GC_MOVING_SAFEPOINT=0` does not — the first two make the copying minor +ineligible, the third only disables the *safepoint* collection, and the one that +matters is the alloc-point direct minor (`trigger=ArenaBytes +declared_safepoint=false`). `PERRY_GC_FROMSPACE_SCAN=1` reported `clean` every +time: no HEAP slot was ever stale, which is the signature of a holder in a native +frame rather than in a table. + +**What is still open, stated rather than papered over.** A protected run of the +*auto-optimize* binary is not silent: it prints the correct checksum and then +faults inside `js_async_step_done` on another 72-byte `GC_TYPE_PROMISE`. That is +a ninth site of the same family, after the program's observable output, and the +kernel matches the oracle byte for byte with and without the instrument. The +`PERRY_NO_AUTO_OPTIMIZE=1` binary and the new gap test are both silent under the +instrument. Separately, `test_gap_gc_iterator_drain_rooting` and +`test_gap_iterator_helpers_2874` fail on `origin/main` too — verified by building +`origin/main`'s runtime from a clean `git archive` export and running both +against it — and belong to #7498's `array_from_spread_value` prototype walk. + +### Added + +**`test-files/test_gap_gc_global_builtin_lookup_rooting.ts`**, registered in +`test-parity/gc_repsel_corpus.txt` so `gc-moving-witnesses` runs it. One wide +`Promise.all` (50 000 elements) rather than the kernel's 1000 × 50: the single +wide call packs enough lookups between two collections to fail on the shipped +default in a fraction of a second, where the kernel needs ~20× the work for the +same window. Deterministic — 6/6 runs failing before, 5/5 passing after — and +byte-diffed against node 26.5.1. + +Verified with the instrument asserted live rather than merely quiet: a protected +run prints `[gc-fromspace-protect] retired_set=#0` and +`[gc-copy-minor] ran copied_objects=175416`, so objects really did move, and no +fault follows. + +### Changed + +`scripts/auto_opt_app_patterns.sh` no longer skips `promise_all_chains`; the skip +list is empty and the gate is 12/12. Its rot check means the line had to come out +with the fix. Every array expansion is now guarded on `${#…[@]}`: macOS ships +bash 3.2, where `set -u` turns `"${EMPTY[@]}"` into an "unbound variable" abort, +so an empty skip list would have stopped the gate before its first kernel — +CLAUDE.md hazard 4 wearing a different hat. `--self-test` still passes. diff --git a/crates/perry-runtime/src/array/flat_clone.rs b/crates/perry-runtime/src/array/flat_clone.rs index b072b21f0a..48990bd25a 100644 --- a/crates/perry-runtime/src/array/flat_clone.rs +++ b/crates/perry-runtime/src/array/flat_clone.rs @@ -407,9 +407,22 @@ pub extern "C" fn js_array_clone(src: *const ArrayHeader) -> *mut ArrayHeader { if src.is_null() { return js_array_alloc(0); } + // #7497: `js_array_alloc` below can trigger the copying minor, which MOVES + // `src`. Pre-fix, `src_elements` was derived from the pre-collection address + // and `copy_nonoverlapping` read retired from-space — so `[...arr]`, + // `Array.from(arr)` and every combinator's iterable snapshot could copy + // whatever the recycled bytes now hold. `PERRY_GC_PROTECT_FROMSPACE=1` faults + // here on the `Promise.all` snapshot at minor #0. Root the source across the + // allocation and re-read both addresses from their handles afterwards. + let scope = crate::gc::RuntimeHandleScope::new(); + let src_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(src as i64)); unsafe { let len = (*src).length; - let result = js_array_alloc(len); + let result_h = + scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_array_alloc(len) as i64)); + let src = crate::value::js_nanbox_get_pointer(src_h.get_nanbox_f64()) as *const ArrayHeader; + let result = + crate::value::js_nanbox_get_pointer(result_h.get_nanbox_f64()) as *mut ArrayHeader; if len > 0 { let src_elements = (src as *const u8).add(std::mem::size_of::()) as *const f64; @@ -545,8 +558,16 @@ pub extern "C" fn js_array_values(arr: *const ArrayHeader) -> *mut ArrayHeader { } _ => {} } + // #7497 (CodeRabbit): the same shape `js_array_clone` had — `arr` is the + // memcpy SOURCE and `js_array_alloc` below can move it. Root and re-read. + let scope = crate::gc::RuntimeHandleScope::new(); + let src_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(arr as i64)); let len = (*arr).length; - let result = js_array_alloc(len); + let result_h = + scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_array_alloc(len) as i64)); + let arr = crate::value::js_nanbox_get_pointer(src_h.get_nanbox_f64()) as *const ArrayHeader; + let result = + crate::value::js_nanbox_get_pointer(result_h.get_nanbox_f64()) as *mut ArrayHeader; if len > 0 { let src_elements = (arr as *const u8).add(std::mem::size_of::()) as *const f64; diff --git a/crates/perry-runtime/src/object/class_registry/class_meta.rs b/crates/perry-runtime/src/object/class_registry/class_meta.rs index 7a2d5abd6f..7d5ba8b387 100644 --- a/crates/perry-runtime/src/object/class_registry/class_meta.rs +++ b/crates/perry-runtime/src/object/class_registry/class_meta.rs @@ -311,15 +311,31 @@ pub(crate) fn identify_global_builtin_constructor(func_value: f64) -> Option<&'s // singleton. Walk via the existing // `js_get_global_this_builtin_value` helper — short loop (≤ ~50 // entries), only fires on the constructFrom hot path. - let global_this_f64 = js_get_global_this(); - let global_obj = crate::value::js_nanbox_get_pointer(global_this_f64) as *const ObjectHeader; - if global_obj.is_null() { - return None; - } + // + // #7497: the same ordering defect `js_get_global_this_builtin_value` had, + // and worse here — the loop below allocates a fresh key string on EVERY + // iteration, so a `globalThis` address read once before the loop is exposed + // to ~50 collection points instead of one. Root it and re-read the address + // after each allocation. + let scope = crate::gc::RuntimeHandleScope::new(); + let global_handle = scope.root_nanbox_f64(js_get_global_this()); + // #7497 (CodeRabbit): the SEARCHED value needs the same treatment. `jv` was + // computed at entry and never refreshed; if a key allocation evacuates the + // ClosureHeader, the `globalThis` field slot is rewritten to the new address + // while `jv.bits()` still names from-space, the equality below never matches, + // and the caller silently falls through to the generic construct tail. + let func_handle = scope.root_nanbox_f64(func_value); for name in GLOBAL_THIS_BUILTIN_CONSTRUCTORS.iter().copied() { - let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + let (key, global_this_f64) = global_handle.across_nanbox(|| { + crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32) + }); + let global_obj = + crate::value::js_nanbox_get_pointer(global_this_f64) as *const ObjectHeader; + if global_obj.is_null() { + return None; + } let v = js_object_get_field_by_name(global_obj, key); - if v.bits() == jv.bits() { + if v.bits() == func_handle.get_nanbox_f64().to_bits() { return Some(name); } } diff --git a/crates/perry-runtime/src/object/global_this/math_temporal.rs b/crates/perry-runtime/src/object/global_this/math_temporal.rs index 615044c4fe..d334f46b88 100644 --- a/crates/perry-runtime/src/object/global_this/math_temporal.rs +++ b/crates/perry-runtime/src/object/global_this/math_temporal.rs @@ -833,24 +833,42 @@ pub(crate) fn temporal_kind_prototype(kind: crate::temporal::TemporalKind) -> f6 PlainMonthDay => b"PlainMonthDay", ZonedDateTime => b"ZonedDateTime", }; - let g = super::js_get_global_this(); + // #7497 (same shape as the proven site in `js_get_global_this_builtin_value`): + // this walk allocates a fresh key string before EACH of its three hops, and + // each receiver — `globalThis`, the `Temporal` namespace, the constructor — + // used to be a raw Rust local read BEFORE the allocation that precedes its + // own dereference. Root each hop and re-read its address across the key + // allocation. + let scope = crate::gc::RuntimeHandleScope::new(); + let g_handle = scope.root_nanbox_f64(super::js_get_global_this()); + let (tkey, g) = + g_handle.across_nanbox(|| crate::string::js_string_from_bytes(b"Temporal".as_ptr(), 8)); let gp = (g.to_bits() & crate::value::POINTER_MASK) as *const ObjectHeader; if gp.is_null() { return undef; } - let tkey = crate::string::js_string_from_bytes(b"Temporal".as_ptr(), 8); let temporal = js_object_get_field_by_name(gp, tkey); if !temporal.is_pointer() { return undef; } - let tp = (temporal.bits() & crate::value::POINTER_MASK) as *const ObjectHeader; - let ckey = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + let t_handle = scope.root_nanbox_f64(f64::from_bits(temporal.bits())); + let (ckey, temporal) = t_handle + .across_nanbox(|| crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32)); + let tp = (temporal.to_bits() & crate::value::POINTER_MASK) as *const ObjectHeader; + if tp.is_null() { + return undef; + } let ctor = js_object_get_field_by_name(tp, ckey); if !ctor.is_pointer() { return undef; } - let cp = (ctor.bits() & crate::value::POINTER_MASK) as *const ObjectHeader; - let pkey = crate::string::js_string_from_bytes(b"prototype".as_ptr(), 9); + let c_handle = scope.root_nanbox_f64(f64::from_bits(ctor.bits())); + let (pkey, ctor) = + c_handle.across_nanbox(|| crate::string::js_string_from_bytes(b"prototype".as_ptr(), 9)); + let cp = (ctor.to_bits() & crate::value::POINTER_MASK) as *const ObjectHeader; + if cp.is_null() { + return undef; + } let proto = js_object_get_field_by_name(cp, pkey); f64::from_bits(proto.bits()) } diff --git a/crates/perry-runtime/src/object/native_module_registry.rs b/crates/perry-runtime/src/object/native_module_registry.rs index ab4e98eadc..0552fe0107 100644 --- a/crates/perry-runtime/src/object/native_module_registry.rs +++ b/crates/perry-runtime/src/object/native_module_registry.rs @@ -201,16 +201,24 @@ pub extern "C" fn js_nm_install_assert() { /// its absence will see it present under Perry.) #[no_mangle] pub extern "C" fn js_globalthis_seed_async_local_storage() { - let global = crate::object::js_get_global_this(); + // #7497 (same shape as the proven site in `js_get_global_this_builtin_value`): + // both the export-value lookup and the key allocation below can collect, and + // `global` is the RECEIVER of the store that follows. Root it and re-read the + // address after the last allocation; root the constructor value too, since it + // is live across the key allocation. + let scope = crate::gc::RuntimeHandleScope::new(); + let global_handle = scope.root_nanbox_f64(crate::object::js_get_global_this()); let ctor = crate::object::native_module::bound_native_callable_export_value( "async_hooks", "AsyncLocalStorage", ); + let ctor_handle = scope.root_nanbox_f64(ctor); let key = crate::string::js_string_from_bytes(b"AsyncLocalStorage".as_ptr(), 17); crate::object::js_object_set_field_by_name( - crate::value::js_nanbox_get_pointer(global) as *mut crate::object::ObjectHeader, + crate::value::js_nanbox_get_pointer(global_handle.get_nanbox_f64()) + as *mut crate::object::ObjectHeader, key, - ctor, + ctor_handle.get_nanbox_f64(), ); } diff --git a/crates/perry-runtime/src/object/object_ops/prototype.rs b/crates/perry-runtime/src/object/object_ops/prototype.rs index d5782fd8c6..46970617d4 100644 --- a/crates/perry-runtime/src/object/object_ops/prototype.rs +++ b/crates/perry-runtime/src/object/object_ops/prototype.rs @@ -24,12 +24,28 @@ pub extern "C" fn js_get_global_this_builtin_value(name_ptr: *const u8, name_len }; // Force the singleton init the first time so the lookup below has // a populated field bag. - let global_this_f64 = js_get_global_this(); + // + // #7497: `globalThis` must be ROOTED across the key allocation, and its + // address re-read AFTER it. `js_string_from_bytes` allocates a fresh + // string on EVERY call (this lookup interns nothing), so it can trigger a + // copying minor that evacuates the `globalThis` object. `THREAD_GLOBAL_THIS` + // is a registered root and gets rewritten — but a raw `*const ObjectHeader` + // already read out of it does not, and `js_object_get_field_by_name` then + // dereferences retired from-space. That is the whole of #7497: every + // element of a `Promise.all` runs `Promise.resolve`, which asks + // `is_default_promise_constructor` for `globalThis.Promise` through here, + // so one 50 000-element `Promise.all` performs 50 000 of these lookups and + // one of them straddles the collection. + let scope = crate::gc::RuntimeHandleScope::new(); + let global_handle = scope.root_nanbox_f64(js_get_global_this()); + let (key, global_this_f64) = global_handle + .across_nanbox(|| crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32)); let global_obj = crate::value::js_nanbox_get_pointer(global_this_f64) as *const ObjectHeader; if global_obj.is_null() { return f64::from_bits(crate::value::TAG_UNDEFINED); } - let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + // Nothing between the re-read above and this call allocates, so + // `global_obj` is still the post-collection address here. let value = js_object_get_field_by_name(global_obj, key); let bits = value.bits(); f64::from_bits(bits) diff --git a/crates/perry-runtime/src/promise/async_step.rs b/crates/perry-runtime/src/promise/async_step.rs index 30dd532905..7110b79112 100644 --- a/crates/perry-runtime/src/promise/async_step.rs +++ b/crates/perry-runtime/src/promise/async_step.rs @@ -69,6 +69,83 @@ pub(crate) fn trace_async_settle(promise: *const Promise, how: &str) { /// forwarded to `js_promise_then_checked`. Null (the "no handler" sentinel) /// becomes `undefined`, matching what `IsCallable(x) is false` resolves to /// on the `_checked` entry's own tag check. +/// #7497: NaN-box a possibly-null promise / closure pointer so it can be parked +/// in a `RuntimeHandleScope`, and read it back. NaN-boxed rather than +/// `root_raw_*_ptr` because reading a raw handle back needs `get_raw_*_ptr`, +/// which `scripts/raw_handle_debt.py` counts. +#[inline] +fn boxed_promise_or_undef(p: *mut Promise) -> f64 { + if p.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + crate::value::js_nanbox_pointer(p as i64) + } +} + +#[inline] +fn boxed_closure_or_undef(c: ClosurePtr) -> f64 { + if c.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + crate::value::js_nanbox_pointer(c as i64) + } +} + +#[inline] +fn unboxed_promise(h: &crate::gc::RuntimeHandle<'_>) -> *mut Promise { + let v = h.get_nanbox_f64(); + if v.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null_mut() + } else { + crate::value::js_nanbox_get_pointer(v) as *mut Promise + } +} + +#[inline] +fn unboxed_closure(h: &crate::gc::RuntimeHandle<'_>) -> ClosurePtr { + let v = h.get_nanbox_f64(); + if v.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null() + } else { + crate::value::js_nanbox_get_pointer(v) as ClosurePtr + } +} + +/// #7497 (CodeRabbit): shared, rooted tail for `js_async_step_chain`'s three +/// suspend paths. Each of them crosses `build_async_step_thunks` (two closure +/// allocations) and, on two of them, `js_promise_resolved` as well, while +/// holding the awaited promise, the two fresh thunks and `trap_next` — and +/// `then_backpatch_result` both STORES into the thunks and RETURNS `trap_next`. +/// `awaited_in` is the already-known awaited promise (the pending-inner path); +/// pass null to have `make_awaited` produce it after the thunks exist. +fn suspend_on_awaited( + step: ClosurePtr, + trap_next: *mut Promise, + awaited_in: *mut Promise, + make_awaited: impl FnOnce() -> *mut Promise, +) -> *mut Promise { + let scope = crate::gc::RuntimeHandleScope::new(); + let step_h = scope.root_nanbox_f64(boxed_closure_or_undef(step)); + let trap_h = scope.root_nanbox_f64(boxed_promise_or_undef(trap_next)); + let awaited_in_h = scope.root_nanbox_f64(boxed_promise_or_undef(awaited_in)); + let (fulfill, reject) = + build_async_step_thunks(unboxed_closure(&step_h), unboxed_promise(&trap_h)); + let fulfill_h = scope.root_nanbox_f64(boxed_closure_or_undef(fulfill)); + let reject_h = scope.root_nanbox_f64(boxed_closure_or_undef(reject)); + let awaited = if unboxed_promise(&awaited_in_h).is_null() { + make_awaited() + } else { + unboxed_promise(&awaited_in_h) + }; + let awaited_h = scope.root_nanbox_f64(boxed_promise_or_undef(awaited)); + then_backpatch_result( + unboxed_promise(&awaited_h), + unboxed_closure(&fulfill_h), + unboxed_closure(&reject_h), + unboxed_promise(&trap_h), + ) +} + #[inline] fn closure_ptr_to_arg(ptr: ClosurePtr) -> f64 { if ptr.is_null() { @@ -273,19 +350,35 @@ fn then_backpatch_result( trap_next: *mut Promise, ) -> *mut Promise { if trap_next.is_null() { - let result = super::then::js_promise_new_with_parent(awaited); + // #7497 (CodeRabbit): `js_promise_new_with_parent` allocates, and the two + // thunks it is about to be STORED into — plus the promise the handlers + // are attached to — are live across it. This is the publishing shape: + // without the re-read, a copying minor here writes a from-space `result` + // into two closures the collector goes on maintaining. + let scope = crate::gc::RuntimeHandleScope::new(); + let awaited_h = scope.root_nanbox_f64(boxed_promise_or_undef(awaited)); + let fulfill_h = scope.root_nanbox_f64(boxed_closure_or_undef(fulfill)); + let reject_h = scope.root_nanbox_f64(boxed_closure_or_undef(reject)); + let result_h = scope.root_nanbox_f64(boxed_promise_or_undef( + super::then::js_promise_new_with_parent(unboxed_promise(&awaited_h)), + )); + let result = unboxed_promise(&result_h); crate::closure::js_closure_set_capture_ptr( - fulfill as *mut crate::closure::ClosureHeader, + unboxed_closure(&fulfill_h) as *mut crate::closure::ClosureHeader, 1, result as i64, ); crate::closure::js_closure_set_capture_ptr( - reject as *mut crate::closure::ClosureHeader, + unboxed_closure(&reject_h) as *mut crate::closure::ClosureHeader, 1, result as i64, ); - super::then::js_promise_attach_handlers(awaited, fulfill, reject); - result + super::then::js_promise_attach_handlers( + unboxed_promise(&awaited_h), + unboxed_closure(&fulfill_h), + unboxed_closure(&reject_h), + ); + unboxed_promise(&result_h) } else { // #6728: `trap_next` non-null means this is a SUBSEQUENT `await` (the // 2nd or later) in the activation — the result promise already exists @@ -327,7 +420,30 @@ pub extern "C" fn js_async_step_chain(value: f64, step_closure: ClosurePtr) -> * // string) observes `[object Promise]` instead. The other arms of // this function already handle native pending Promises correctly // via the `js_value_is_promise` + thunk path. + // #7497: `step_closure` is a GC-heap closure that this function stores into + // a `Task::AsyncStep` at the end, after `adapt_foreign_promise_value`, + // `js_promise_new`, `build_async_step_thunks` and `js_promise_resolved` have + // all had a chance to collect. Pre-fix it rode through all of them in a + // register, so the ENQUEUED task could carry a pre-collection address — and + // the microtask runner, which correctly re-reads everything it pops, then + // faithfully dispatched that dead pointer. `PERRY_GC_PROTECT_FROMSPACE=1` + // reports it at `call_async_step_direct`, one frame away from the producer. + let step_scope = crate::gc::RuntimeHandleScope::new(); + let step_handle = step_scope.root_nanbox_f64(if step_closure.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + crate::value::js_nanbox_pointer(step_closure as i64) + }); + let rooted_step = || -> ClosurePtr { + let v = step_handle.get_nanbox_f64(); + if v.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null() + } else { + crate::value::js_nanbox_get_pointer(v) as ClosurePtr + } + }; let value = adapt_foreign_promise_value(value); + let step_closure = rooted_step(); // Reuse predicate. `next` reuse is sound only when AsyncStepChain // is being called from the body of the SAME step closure that the @@ -413,32 +529,51 @@ pub extern "C" fn js_async_step_chain(value: f64, step_closure: ClosurePtr) -> * // that will queue the right Task when called. bump(&MT_STEP_CHAIN_REUSE_MISS); trace_async_suspend(inner); - let (fulfill, reject) = build_async_step_thunks(step_closure, trap_next); - return then_backpatch_result(inner, fulfill, reject, trap_next); + return suspend_on_awaited(rooted_step(), trap_next, inner, || { + std::ptr::null_mut() + }); } } } else { bump(&MT_STEP_CHAIN_REUSE_MISS); - let (fulfill, reject) = build_async_step_thunks(step_closure, trap_next); - let p = js_promise_resolved(value); - return then_backpatch_result(p, fulfill, reject, trap_next); + return suspend_on_awaited(rooted_step(), trap_next, std::ptr::null_mut(), || { + js_promise_resolved(value) + }); } } else { // Pointer-tagged but not a Promise (thenable etc.). Take the // fully-general path so assimilation runs. bump(&MT_STEP_CHAIN_REUSE_MISS); - let (fulfill, reject) = build_async_step_thunks(step_closure, trap_next); - let p = js_promise_resolved(value); - return then_backpatch_result(p, fulfill, reject, trap_next); + return suspend_on_awaited(rooted_step(), trap_next, std::ptr::null_mut(), || { + js_promise_resolved(value) + }); }; + // #7497: `capture_context()` allocates, and `next` / `queued_value` are the + // OTHER two GC values this task carries, so root them across it too and take + // every address from a handle at the push. + let next_handle = step_scope.root_nanbox_f64(if next.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + crate::value::js_nanbox_pointer(next as i64) + }); + let queued_value_handle = step_scope.root_nanbox_f64(queued_value); + let context = capture_context(); + let next = { + let v = next_handle.get_nanbox_f64(); + if v.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null_mut() + } else { + crate::value::js_nanbox_get_pointer(v) as *mut Promise + } + }; TASK_QUEUE.with(|q| { q.borrow_mut().push_back(Task::AsyncStep( - step_closure, - queued_value, + rooted_step(), + queued_value_handle.get_nanbox_f64(), next, is_error, - capture_context(), + context, )); }); crate::event_pump::js_notify_main_thread(); @@ -507,8 +642,18 @@ pub extern "C" fn js_async_step_done(value: f64, step_closure: ClosurePtr) -> *m // Mirror `js_promise_resolved`'s adoption probes here, but settle // the existing `trap_next` instead of allocating a fresh promise // so the runner's self-chain fast path still fires. - resolve_trap_next_with_adoption(trap.trap_next, value); - trap.trap_next + // #7497: `resolve_trap_next_with_adoption` settles the promise, which + // can enqueue jobs and allocate — and the RETURN VALUE of this function + // is that same promise. Returning the pre-call copy hands the async + // state machine a from-space `GC_TYPE_PROMISE`; + // `PERRY_GC_PROTECT_FROMSPACE=1` faults on it here under the + // auto-optimize link. + let scope = crate::gc::RuntimeHandleScope::new(); + let target_h = + scope.root_nanbox_f64(crate::value::js_nanbox_pointer(trap.trap_next as i64)); + let value_h = scope.root_nanbox_f64(value); + resolve_trap_next_with_adoption(trap.trap_next, value_h.get_nanbox_f64()); + crate::value::js_nanbox_get_pointer(target_h.get_nanbox_f64()) as *mut Promise } else { bump(&MT_STEP_DONE_REUSE_MISS); // An async fn always returns a FRESH promise — `js_promise_resolved`'s @@ -519,9 +664,19 @@ pub extern "C" fn js_async_step_done(value: f64, step_closure: ClosurePtr) -> *m if js_value_is_promise(value) != 0 { let inner = crate::value::js_nanbox_get_pointer(value) as *mut Promise; if !inner.is_null() { - let fresh = js_promise_new(); - super::assimilate::enqueue_native_adoption_job(fresh, inner); - return fresh; + // #7497: `js_promise_new` allocates and `inner` is its adoption + // source, so root `inner` across it and re-read; likewise the + // fresh promise across `enqueue_native_adoption_job`. + let scope = crate::gc::RuntimeHandleScope::new(); + let inner_h = scope.root_nanbox_f64(value); + let fresh_h = + scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_promise_new() as i64)); + super::assimilate::enqueue_native_adoption_job( + crate::value::js_nanbox_get_pointer(fresh_h.get_nanbox_f64()) as *mut Promise, + crate::value::js_nanbox_get_pointer(inner_h.get_nanbox_f64()) as *mut Promise, + ); + return crate::value::js_nanbox_get_pointer(fresh_h.get_nanbox_f64()) + as *mut Promise; } } js_promise_resolved(value) diff --git a/crates/perry-runtime/src/promise/combinators.rs b/crates/perry-runtime/src/promise/combinators.rs index 66263f6f58..f9efc3972e 100644 --- a/crates/perry-runtime/src/promise/combinators.rs +++ b/crates/perry-runtime/src/promise/combinators.rs @@ -357,14 +357,21 @@ pub(crate) fn combinator_iterable_to_array( // (the overwhelming common case) pay one extra side-table probe and // keep the existing raw clone. if crate::array::js_array_is_array(value).to_bits() == crate::value::TAG_TRUE { + // #7497: `well_known_symbol` and `own_symbol_property` both allocate + // (and the latter can run a user getter), so the array being cloned is + // re-read from a root rather than carried across them in a register. + let scope = crate::gc::RuntimeHandleScope::new(); + let value_h = scope.root_nanbox_f64(value); let iter_sym = crate::symbol::well_known_symbol("iterator"); if !iter_sym.is_null() { let sym_f64 = f64::from_bits(crate::value::JSValue::pointer(iter_sym as *const u8).bits()); - let _ = unsafe { crate::symbol::own_symbol_property(value, sym_f64) }; + let _ = + unsafe { crate::symbol::own_symbol_property(value_h.get_nanbox_f64(), sym_f64) }; } return Ok(crate::array::js_array_clone( - crate::value::js_nanbox_get_pointer(value) as *const crate::array::ArrayHeader, + crate::value::js_nanbox_get_pointer(value_h.get_nanbox_f64()) + as *const crate::array::ArrayHeader, )); } let jsval = JSValue::from_bits(value.to_bits()); @@ -409,6 +416,12 @@ pub(crate) fn combinator_iterable_to_array( (*hdr).obj_type }; if obj_type == crate::gc::GC_TYPE_OBJECT { + // #7497 (CodeRabbit): `raw` was computed above and then carried across a + // user `[Symbol.iterator]` getter and a `"next"` key allocation before + // being dereferenced. Root the receiver and re-derive its address after + // each of those. + let scope = crate::gc::RuntimeHandleScope::new(); + let value_h = scope.root_nanbox_f64(value); let has_iterator = { let iter_sym = crate::symbol::well_known_symbol("iterator"); if iter_sym.is_null() { @@ -416,15 +429,17 @@ pub(crate) fn combinator_iterable_to_array( } else { let sym_f64 = f64::from_bits(crate::value::JSValue::pointer(iter_sym as *const u8).bits()); - let iter_fn = - unsafe { crate::symbol::js_object_get_symbol_property(value, sym_f64) }; + let iter_fn = unsafe { + crate::symbol::js_object_get_symbol_property(value_h.get_nanbox_f64(), sym_f64) + }; iter_fn.to_bits() != crate::value::TAG_UNDEFINED } }; let has_next_field = { let next_key = crate::string::js_string_from_bytes(b"next".as_ptr(), 4); let next_val = crate::object::js_object_get_field_by_name( - raw as *const crate::object::ObjectHeader, + crate::value::js_nanbox_get_pointer(value_h.get_nanbox_f64()) + as *const crate::object::ObjectHeader, next_key, ); let next_ptr = crate::value::js_nanbox_get_pointer(f64::from_bits(next_val.bits())); @@ -432,7 +447,8 @@ pub(crate) fn combinator_iterable_to_array( }; if has_iterator || has_next_field { return Ok(crate::array::js_array_clone( - raw as *const crate::array::ArrayHeader, + crate::value::js_nanbox_get_pointer(value_h.get_nanbox_f64()) + as *const crate::array::ArrayHeader, )); } } @@ -671,23 +687,47 @@ pub(super) fn make_resolving_functions( ) { use crate::closure::{js_closure_alloc, js_closure_set_capture_ptr}; ensure_native_resolving_arity_registered(); - let guard = alloc_already_resolved_guard(); - let resolve = js_closure_alloc(promise_resolve_fn as *const u8, 2); - js_closure_set_capture_ptr(resolve, 0, promise as i64); - js_closure_set_capture_ptr(resolve, 1, guard as i64); - let reject = js_closure_alloc(promise_reject_fn as *const u8, 2); - js_closure_set_capture_ptr(reject, 0, promise as i64); - js_closure_set_capture_ptr(reject, 1, guard as i64); + // #7497: four allocations follow, and `promise` / `guard` are STORED into + // capture slots after them. Pre-fix all three lived in bare Rust locals, so + // a copying minor here wrote pre-collection addresses into the two closures + // — the from-space-publishing shape, not merely a stale read. + let scope = crate::gc::RuntimeHandleScope::new(); + let promise_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(promise as i64)); + let guard_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer( + alloc_already_resolved_guard() as i64, + )); + let resolve_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_closure_alloc( + promise_resolve_fn as *const u8, + 2, + ) as i64)); + let reject_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_closure_alloc( + promise_reject_fn as *const u8, + 2, + ) as i64)); + let ptr_of = |h: &crate::gc::RuntimeHandle<'_>| -> i64 { + crate::value::js_nanbox_get_pointer(h.get_nanbox_f64()) + }; + for handle in [&resolve_h, &reject_h] { + let f = ptr_of(handle) as *mut crate::closure::ClosureHeader; + js_closure_set_capture_ptr(f, 0, ptr_of(&promise_h)); + js_closure_set_capture_ptr(f, 1, ptr_of(&guard_h)); + } // Spec 27.2.1.3: the resolving functions are anonymous built-in functions // with own `length` = 1, `name` = "" (both non-writable, non-enumerable, // configurable), and NO `[[Construct]]` (`new resolve()` throws). test262 // `resolve-function-*` / `reject-function-*` assert all four. - for f in [resolve, reject] { - crate::object::set_builtin_closure_length(f as usize, 1); - crate::object::set_bound_native_closure_name(f, ""); - crate::object::set_builtin_closure_non_constructable(f as usize); - } - (resolve, reject) + for handle in [&resolve_h, &reject_h] { + crate::object::set_builtin_closure_length(ptr_of(handle) as usize, 1); + crate::object::set_bound_native_closure_name( + ptr_of(handle) as *mut crate::closure::ClosureHeader, + "", + ); + crate::object::set_builtin_closure_non_constructable(ptr_of(handle) as usize); + } + ( + ptr_of(&resolve_h) as *mut crate::closure::ClosureHeader, + ptr_of(&reject_h) as *mut crate::closure::ClosureHeader, + ) } /// Internal resolve function for Promise executor callbacks. diff --git a/crates/perry-runtime/src/promise/microtasks.rs b/crates/perry-runtime/src/promise/microtasks.rs index be1bb7893a..b5a2e83c2d 100644 --- a/crates/perry-runtime/src/promise/microtasks.rs +++ b/crates/perry-runtime/src/promise/microtasks.rs @@ -127,6 +127,49 @@ enum MicrotaskDrainMode { AwaitLoop, } +/// NaN-box a possibly-null promise / closure pointer so it can be parked in a +/// `RuntimeHandleScope` (#7497). NaN-boxed rather than `root_raw_*_ptr` because +/// reading a raw handle back needs `get_raw_*_ptr`, which +/// `scripts/raw_handle_debt.py` counts; the NaN-boxed round trip is free and is +/// the RE-READ these sites exist for. +#[inline] +fn boxed_promise(p: *mut Promise) -> f64 { + if p.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + crate::value::js_nanbox_pointer(p as i64) + } +} + +#[inline] +fn boxed_closure(c: ClosurePtr) -> f64 { + if c.is_null() { + f64::from_bits(crate::value::TAG_UNDEFINED) + } else { + crate::value::js_nanbox_pointer(c as i64) + } +} + +#[inline] +fn rooted_promise(h: &crate::gc::RuntimeHandle<'_>) -> *mut Promise { + let v = h.get_nanbox_f64(); + if v.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null_mut() + } else { + crate::value::js_nanbox_get_pointer(v) as *mut Promise + } +} + +#[inline] +fn rooted_closure(h: &crate::gc::RuntimeHandle<'_>) -> ClosurePtr { + let v = h.get_nanbox_f64(); + if v.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null() + } else { + crate::value::js_nanbox_get_pointer(v) as ClosurePtr + } +} + fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { mt_profile_register(); let reentrant = MICROTASK_RUN_DEPTH.with(|depth| { @@ -293,7 +336,14 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { None => break, Some(Task::Promise(promise, value, is_fulfilled, task_context)) => { bump(&MT_RUN_COUNT); + // #7497: root BEFORE `enter_microtask_context` — a popped + // Task is no longer a scanned root and that call allocates. + let task_scope = crate::gc::RuntimeHandleScope::new(); + let task_promise_handle = task_scope.root_nanbox_f64(boxed_promise(promise)); + let task_value_handle = task_scope.root_nanbox_f64(value); enter_microtask_context(&task_context); + let promise = rooted_promise(&task_promise_handle); + let value = task_value_handle.get_nanbox_f64(); unsafe { let callback = if is_fulfilled { (*promise).on_fulfilled @@ -341,9 +391,23 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { // nested drain leaves the enclosing arm — and its // exception-trap routing — intact. This mirrors the // INLINE_TRAP save/restore in the Inline/AsyncStep arms. + // + // #7497: `callback` and `value` need the same treatment. + // `callback` was read out of `(*promise).on_fulfilled` + // above and then carried in a register across + // `async_hooks::before` and `v8::promise_hook_before`, + // both of which can allocate — and the very next + // instruction after them loads `func_ptr` out of it. + // The `CURRENT_MICROTASK_CALLBACK` cell IS a scanned + // root, so the collector rewrites the CELL and leaves + // this copy naming from-space. + // `PERRY_GC_PROTECT_FROMSPACE=1` faults exactly there, + // on a 112-byte `GC_TYPE_CLOSURE`. let scope = crate::gc::RuntimeHandleScope::new(); let promise_handle = scope.root_raw_mut_ptr(promise); let next_handle = scope.root_raw_mut_ptr((*promise).next); + let callback_handle = scope.root_nanbox_f64(boxed_closure(callback)); + let value_handle = scope.root_nanbox_f64(value); let prev_promise = CURRENT_MICROTASK_PROMISE.with(|c| c.get()); let prev_callback = CURRENT_MICROTASK_CALLBACK.with(|c| c.get()); let prev_value = CURRENT_MICROTASK_VALUE.with(|c| c.get()); @@ -372,7 +436,12 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { let trigger_async_id = (*promise).trigger_async_id; crate::async_hooks::before(async_id, trigger_async_id); crate::v8::promise_hook_before(promise); - let result = crate::closure::js_closure_call1(callback, value); + // #7497: re-read BOTH from their handles — the two calls + // above allocate. + let result = crate::closure::js_closure_call1( + rooted_closure(&callback_handle), + value_handle.get_nanbox_f64(), + ); // Keep the callback result rooted across `after()` (which // can run JS when async_hooks are active) via the value // cell, then reload promise/next from our handles — never @@ -419,16 +488,43 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { restore_microtask_context(); ran += 1; } - Some(Task::PromiseAll(state, value, is_fulfilled, task_context)) => { + Some(Task::PromiseAll(mut state, value, is_fulfilled, task_context)) => { bump(&MT_RUN_COUNT); + // #7497: root BEFORE `enter_microtask_context` — see the + // Task::AsyncStep arm. The combinator state carries three + // heap pointers plus the settlement value. + let task_scope = crate::gc::RuntimeHandleScope::new(); + let result_h = task_scope.root_nanbox_f64(boxed_promise(state.result_promise)); + let results_h = task_scope + .root_nanbox_f64(crate::value::js_nanbox_pointer(state.results_arr as i64)); + let state_arr_h = task_scope + .root_nanbox_f64(crate::value::js_nanbox_pointer(state.state_arr as i64)); + let value_h = task_scope.root_nanbox_f64(value); enter_microtask_context(&task_context); + state.result_promise = rooted_promise(&result_h); + state.results_arr = + crate::value::js_nanbox_get_pointer(results_h.get_nanbox_f64()) + as *mut crate::array::ArrayHeader; + state.state_arr = + crate::value::js_nanbox_get_pointer(state_arr_h.get_nanbox_f64()) + as *mut crate::array::ArrayHeader; + let value = value_h.get_nanbox_f64(); combinators::promise_all_settle(state, value, is_fulfilled); restore_microtask_context(); ran += 1; } Some(Task::Inline(callback, value, next, is_fulfilled, task_context)) => { bump(&MT_RUN_COUNT); + // #7497: root BEFORE `enter_microtask_context` — see the + // Task::AsyncStep arm. + let trap_scope = crate::gc::RuntimeHandleScope::new(); + let callback_handle = trap_scope.root_nanbox_f64(boxed_closure(callback)); + let value_handle = trap_scope.root_nanbox_f64(value); + let next_handle = trap_scope.root_nanbox_f64(boxed_promise(next)); enter_microtask_context(&task_context); + let callback = rooted_closure(&callback_handle); + let value = value_handle.get_nanbox_f64(); + let next = rooted_promise(&next_handle); // Inline tasks are produced by `js_promise_resolved_then` // (the `Promise.resolve().then(cb_f, cb_e)` // fast path). We've already skipped allocating the @@ -461,7 +557,6 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { // when the runner is invoked re-entrantly from inside // a non-transformed async closure's busy-wait. let prev_trap = INLINE_TRAP.with(|c| c.get()); - let trap_scope = crate::gc::RuntimeHandleScope::new(); let prev_trap_next_handle = trap_scope.root_raw_mut_ptr(prev_trap.trap_next); let prev_trap_step_handle = trap_scope.root_raw_const_ptr( prev_trap.current_step as *const crate::closure::ClosureHeader, @@ -482,7 +577,9 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { None }; crate::v8::promise_hook_before(next); - let result = crate::closure::js_closure_call1(callback, value); + let callback = rooted_closure(&callback_handle); + let result = + crate::closure::js_closure_call1(callback, value_handle.get_nanbox_f64()); CURRENT_MICROTASK_VALUE.with(|c| c.set(result)); let next_for_after = CURRENT_MICROTASK_NEXT.with(|c| c.get()); crate::v8::promise_hook_after(next_for_after); @@ -527,8 +624,12 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { trigger_async_id, }) => { bump(&MT_RUN_COUNT); - enter_microtask_context(&context); + // #7497: root BEFORE `enter_microtask_context` — see the + // Task::AsyncStep arm. let scope = crate::gc::RuntimeHandleScope::new(); + let callback_handle = scope.root_nanbox_f64(boxed_closure(callback)); + enter_microtask_context(&context); + let callback = rooted_closure(&callback_handle); let prev_promise = CURRENT_MICROTASK_PROMISE.with(|c| c.get()); let prev_callback = CURRENT_MICROTASK_CALLBACK.with(|c| c.get()); let prev_value = CURRENT_MICROTASK_VALUE.with(|c| c.get()); @@ -540,7 +641,7 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { CURRENT_MICROTASK_VALUE.with(|c| c.set(0.0)); CURRENT_MICROTASK_NEXT.with(|c| c.set(std::ptr::null_mut())); crate::async_hooks::before(async_id, trigger_async_id); - crate::closure::js_closure_call0(callback); + crate::closure::js_closure_call0(rooted_closure(&callback_handle)); crate::async_hooks::after(async_id); crate::async_hooks::destroy(async_id); CURRENT_MICROTASK_PROMISE @@ -554,7 +655,24 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { } Some(Task::AsyncStep(step_closure, value, next, is_error, task_context)) => { bump(&MT_RUN_COUNT); + // #7497: ROOT BEFORE `enter_microtask_context`. A Task stops + // being a scanned root the instant it is popped off + // TASK_QUEUE, and everything between the pop and the + // dispatch — the context switch, the async-hook bookkeeping, + // the promise hooks — can allocate. Rooting inside the arm + // but AFTER those calls preserves an address that is already + // stale, which is what the first attempt at this fix did: + // the instrument still faulted at `call_async_step_direct`'s + // `(*step_closure).func_ptr`, on a value re-read from a + // handle that had been seeded too late. + let trap_scope = crate::gc::RuntimeHandleScope::new(); + let step_handle = trap_scope.root_nanbox_f64(boxed_closure(step_closure)); + let value_handle = trap_scope.root_nanbox_f64(value); + let next_handle = trap_scope.root_nanbox_f64(boxed_promise(next)); enter_microtask_context(&task_context); + let step_closure = rooted_closure(&step_handle); + let value = value_handle.get_nanbox_f64(); + let next = rooted_promise(&next_handle); // Direct dispatch of the async-step closure. Skips the // then_v_arrow / then_e_arrow wrapper that would // otherwise be invoked as the on_fulfilled / on_rejected @@ -678,7 +796,6 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { // settles with the awaited value rather than the // explicit return expression. let prev_trap = INLINE_TRAP.with(|c| c.get()); - let trap_scope = crate::gc::RuntimeHandleScope::new(); let prev_trap_next_handle = trap_scope.root_raw_mut_ptr(prev_trap.trap_next); let prev_trap_step_handle = trap_scope.root_raw_const_ptr( prev_trap.current_step as *const crate::closure::ClosureHeader, @@ -721,7 +838,12 @@ fn run_microtasks(mode: MicrotaskDrainMode) -> i32 { }; crate::async_hooks::before(step_async_id, step_trigger_id); crate::v8::promise_hook_before(next); - let result = call_async_step_direct(step_closure, value, is_error_bits); + // #7497: re-read both across the two calls above. + let result = call_async_step_direct( + rooted_closure(&step_handle), + value_handle.get_nanbox_f64(), + is_error_bits, + ); CURRENT_MICROTASK_VALUE.with(|c| c.set(result)); if let Some(t) = t1 { MT_TIME_NS_CALLBACK diff --git a/crates/perry-runtime/src/promise/spec_combinators.rs b/crates/perry-runtime/src/promise/spec_combinators.rs index 2d6aa11363..dd2f95f08f 100644 --- a/crates/perry-runtime/src/promise/spec_combinators.rs +++ b/crates/perry-runtime/src/promise/spec_combinators.rs @@ -39,6 +39,30 @@ fn is_undef(v: f64) -> bool { v.to_bits() == TAG_UNDEFINED } +/// NaN-box a possibly-null raw heap pointer so it can be parked in a +/// `RuntimeHandleScope` (#7497). NaN-boxed rather than `root_raw_*_ptr` so +/// `scripts/raw_handle_debt.py` stays where it is — the round trip through +/// `get_nanbox_f64` is the RE-READ, which is the whole point: no call site below +/// may keep a pre-collection address nameable. +#[inline] +fn boxed_ptr(p: *mut T) -> f64 { + if p.is_null() { + undef() + } else { + js_nanbox_pointer(p as i64) + } +} + +/// Inverse of [`boxed_ptr`]. `undefined` decodes back to null. +#[inline] +fn unboxed_ptr(v: f64) -> *mut T { + if is_undef(v) { + std::ptr::null_mut() + } else { + crate::value::js_nanbox_get_pointer(v) as *mut T + } +} + #[derive(Clone, Copy, PartialEq, Eq)] pub enum CombinatorKind { All, @@ -157,21 +181,38 @@ extern "C" fn capability_executor_fn( /// failure, the executor "already called" failure (propagated from the /// constructor), and the post-construct "resolve/reject not callable" failure. pub(super) fn new_promise_capability(c: f64) -> Capability { - if !crate::object::js_value_is_constructor(c) { + // #7497: `is_default_promise_constructor` performs a `globalThis.Promise` + // lookup that allocates its key, and both paths below allocate repeatedly + // while holding the values they are about to return. Root `c` for the whole + // helper and re-read every address at its point of use. + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor_h = scope.root_nanbox_f64(c); + + if !crate::object::js_value_is_constructor(ctor_h.get_nanbox_f64()) { throw_type_error("Promise.all called on non-constructor"); } // Fast path: the intrinsic `Promise` constructor. Build a native capability // directly (the generic construct path does not model `new Promise`). - if is_default_promise_constructor(c) { - let promise = js_promise_new(); - let (resolve, reject) = super::combinators::make_resolving_functions(promise); - crate::object::set_builtin_closure_length(resolve as usize, 1); - crate::object::set_builtin_closure_length(reject as usize, 1); + if is_default_promise_constructor(ctor_h.get_nanbox_f64()) { + let promise_h = scope.root_nanbox_f64(boxed_ptr(js_promise_new())); + // `make_resolving_functions` allocates a guard array and two closures. + let (resolve, reject) = + super::combinators::make_resolving_functions(unboxed_ptr(promise_h.get_nanbox_f64())); + let resolve_h = scope.root_nanbox_f64(boxed_ptr(resolve)); + let reject_h = scope.root_nanbox_f64(boxed_ptr(reject)); + crate::object::set_builtin_closure_length( + unboxed_ptr::(resolve_h.get_nanbox_f64()) as usize, + 1, + ); + crate::object::set_builtin_closure_length( + unboxed_ptr::(reject_h.get_nanbox_f64()) as usize, + 1, + ); return Capability { - promise: js_nanbox_pointer(promise as i64), - resolve: js_nanbox_pointer(resolve as i64), - reject: js_nanbox_pointer(reject as i64), + promise: promise_h.get_nanbox_f64(), + resolve: resolve_h.get_nanbox_f64(), + reject: reject_h.get_nanbox_f64(), }; } @@ -182,18 +223,33 @@ pub(super) fn new_promise_capability(c: f64) -> Capability { } js_array_set_f64(storage, 0, undef()); js_array_set_f64(storage, 1, undef()); + let storage_h = scope.root_nanbox_f64(boxed_ptr(storage)); + + let executor_h = scope.root_nanbox_f64(boxed_ptr(js_closure_alloc( + capability_executor_fn as *const u8, + 1, + ))); + js_closure_set_capture_ptr( + unboxed_ptr(executor_h.get_nanbox_f64()), + 0, + unboxed_ptr::(storage_h.get_nanbox_f64()) as i64, + ); + crate::object::set_builtin_closure_length( + unboxed_ptr::(executor_h.get_nanbox_f64()) as usize, + 2, + ); - let executor = js_closure_alloc(capability_executor_fn as *const u8, 1); - js_closure_set_capture_ptr(executor, 0, storage as i64); - crate::object::set_builtin_closure_length(executor as usize, 2); - - let executor_val = js_nanbox_pointer(executor as i64); - let args = [executor_val]; + let args = [executor_h.get_nanbox_f64()]; // Any exception thrown by the constructor (including the executor's // "called twice" TypeError) propagates as a real throw — correct for the // `? NewPromiseCapability(C)` step in Promise.all. - let promise = unsafe { crate::object::js_new_function_construct(c, args.as_ptr(), args.len()) }; + let promise = unsafe { + crate::object::js_new_function_construct(ctor_h.get_nanbox_f64(), args.as_ptr(), args.len()) + }; + let promise_h = scope.root_nanbox_f64(promise); + // `storage` is re-read here: the user constructor above ran arbitrary JS. + let storage = unboxed_ptr(storage_h.get_nanbox_f64()); let resolve = js_array_get_f64(storage, 0); let reject = js_array_get_f64(storage, 1); if !is_callable(resolve) || !is_callable(reject) { @@ -201,7 +257,7 @@ pub(super) fn new_promise_capability(c: f64) -> Capability { } Capability { - promise, + promise: promise_h.get_nanbox_f64(), resolve, reject, } @@ -294,18 +350,33 @@ fn build_element_closure( cap_resolve: f64, cap_reject: f64, ) -> *mut crate::closure::ClosureHeader { - let c = js_closure_alloc(func, 6); - js_closure_set_capture_ptr(c, 0, guard as i64); + // #7497: `js_closure_alloc` allocates, and every one of the five GC values + // handed in has to be STORED at its post-collection address afterwards. + // Pre-fix, all five were read at the call site, held in argument registers + // across the allocation, and then written into the capture slots — so a + // copying minor here published from-space addresses into a closure the + // collector will happily keep rewriting from that point on. + let scope = crate::gc::RuntimeHandleScope::new(); + let guard_h = scope.root_nanbox_f64(boxed_ptr(guard)); + let values_h = scope.root_nanbox_f64(boxed_ptr(values)); + let state_h = scope.root_nanbox_f64(boxed_ptr(state)); + let cap_resolve_h = scope.root_nanbox_f64(cap_resolve); + let cap_reject_h = scope.root_nanbox_f64(cap_reject); + + let closure_h = scope.root_nanbox_f64(boxed_ptr(js_closure_alloc(func, 6))); + let c: *mut crate::closure::ClosureHeader = unboxed_ptr(closure_h.get_nanbox_f64()); + js_closure_set_capture_ptr(c, 0, unboxed_ptr::(guard_h.get_nanbox_f64()) as i64); js_closure_set_capture_f64(c, 1, index as f64); - js_closure_set_capture_ptr(c, 2, values as i64); - js_closure_set_capture_ptr(c, 3, state as i64); - js_closure_set_capture_f64(c, 4, cap_resolve); - js_closure_set_capture_f64(c, 5, cap_reject); + js_closure_set_capture_ptr(c, 2, unboxed_ptr::(values_h.get_nanbox_f64()) as i64); + js_closure_set_capture_ptr(c, 3, unboxed_ptr::(state_h.get_nanbox_f64()) as i64); + js_closure_set_capture_f64(c, 4, cap_resolve_h.get_nanbox_f64()); + js_closure_set_capture_f64(c, 5, cap_reject_h.get_nanbox_f64()); crate::object::set_builtin_closure_length(c as usize, 1); // Spec: the resolve/reject element functions are anonymous built-in // functions and are NOT constructors — `new resolveElement()` throws. + let c: *mut crate::closure::ClosureHeader = unboxed_ptr(closure_h.get_nanbox_f64()); crate::object::set_builtin_closure_non_constructable(c as usize); - c + unboxed_ptr(closure_h.get_nanbox_f64()) } /// Decrement the shared remaining-count; return true if it just hit zero. @@ -343,41 +414,53 @@ extern "C" fn settled_fulfill_element_fn( closure: *const crate::closure::ClosureHeader, value: f64, ) -> f64 { - let guard = js_closure_get_capture_ptr(closure, 0) as *mut crate::array::ArrayHeader; - if !take_already_called(guard) { - return undef(); - } - let index = js_closure_get_capture_f64(closure, 1) as u32; - let values = js_closure_get_capture_ptr(closure, 2) as *mut crate::array::ArrayHeader; - let state = js_closure_get_capture_ptr(closure, 3) as *mut crate::array::ArrayHeader; - let cap_resolve = js_closure_get_capture_f64(closure, 4); - - js_array_set_f64(values, index, build_settled_fulfilled(value)); - if dec_remaining(state) { - let arr = js_nanbox_pointer(values as i64); - let _ = call_with_this(cap_resolve, undef(), &[arr]); - } - undef() + settled_element(closure, value, true) } /// Promise.allSettled Reject Element Function → `{status:"rejected", reason}`. extern "C" fn settled_reject_element_fn( closure: *const crate::closure::ClosureHeader, reason: f64, +) -> f64 { + settled_element(closure, reason, false) +} + +/// Shared body of the two `Promise.allSettled` element functions. +/// +/// #7497: `build_settled_{fulfilled,rejected}` allocates an object AND a string, +/// so the shared arrays and the capability function cannot be carried across it +/// in registers — they are re-read from the closure's capture slots afterwards +/// (the collector rewrites those; a register copy it cannot see). +fn settled_element( + closure: *const crate::closure::ClosureHeader, + value: f64, + fulfilled: bool, ) -> f64 { let guard = js_closure_get_capture_ptr(closure, 0) as *mut crate::array::ArrayHeader; if !take_already_called(guard) { return undef(); } + let scope = crate::gc::RuntimeHandleScope::new(); + let closure_h = scope.root_nanbox_f64(boxed_ptr(closure as *mut crate::closure::ClosureHeader)); + let value_h = scope.root_nanbox_f64(value); let index = js_closure_get_capture_f64(closure, 1) as u32; + + let record = if fulfilled { + build_settled_fulfilled(value_h.get_nanbox_f64()) + } else { + build_settled_rejected(value_h.get_nanbox_f64()) + }; + let record_h = scope.root_nanbox_f64(record); + + let closure: *const crate::closure::ClosureHeader = + unboxed_ptr(closure_h.get_nanbox_f64()) as *const _; let values = js_closure_get_capture_ptr(closure, 2) as *mut crate::array::ArrayHeader; let state = js_closure_get_capture_ptr(closure, 3) as *mut crate::array::ArrayHeader; let cap_resolve = js_closure_get_capture_f64(closure, 4); - js_array_set_f64(values, index, build_settled_rejected(reason)); + js_array_set_f64(values, index, record_h.get_nanbox_f64()); if dec_remaining(state) { - let arr = js_nanbox_pointer(values as i64); - let _ = call_with_this(cap_resolve, undef(), &[arr]); + let _ = call_with_this(cap_resolve, undef(), &[boxed_ptr(values)]); } undef() } @@ -395,13 +478,21 @@ extern "C" fn any_reject_element_fn( let index = js_closure_get_capture_f64(closure, 1) as u32; let errors = js_closure_get_capture_ptr(closure, 2) as *mut crate::array::ArrayHeader; let state = js_closure_get_capture_ptr(closure, 3) as *mut crate::array::ArrayHeader; - let cap_reject = js_closure_get_capture_f64(closure, 5); js_array_set_f64(errors, index, reason); if dec_remaining(state) { + // #7497: the message allocates; `errors` is the AggregateError's own + // payload and `cap_reject` is what we are about to call, so both are + // read from the capture slots AFTER it. + let scope = crate::gc::RuntimeHandleScope::new(); + let closure_h = + scope.root_nanbox_f64(boxed_ptr(closure as *mut crate::closure::ClosureHeader)); let msg = crate::string::js_string_from_bytes(b"All promises were rejected".as_ptr(), 26); - let agg = crate::error::js_aggregateerror_new(errors, msg); - let agg_v = js_nanbox_pointer(agg as i64); + let closure: *const crate::closure::ClosureHeader = + unboxed_ptr(closure_h.get_nanbox_f64()) as *const _; + let errors = js_closure_get_capture_ptr(closure, 2) as *mut crate::array::ArrayHeader; + let cap_reject = js_closure_get_capture_f64(closure, 5); + let agg_v = boxed_ptr(crate::error::js_aggregateerror_new(errors, msg)); let _ = call_with_this(cap_reject, undef(), &[agg_v]); } undef() @@ -410,29 +501,56 @@ extern "C" fn any_reject_element_fn( fn build_settled_fulfilled(value: f64) -> f64 { use crate::object::{js_object_alloc_with_shape, js_object_set_field}; let packed = b"status\0value\0"; - let obj = js_object_alloc_with_shape(0x7FFF_FF10, 2, packed.as_ptr(), packed.len() as u32); + // #7497: `js_string_from_bytes` below allocates, so neither the freshly + // allocated record nor the caller's value may be held across it raw. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_nanbox_f64(boxed_ptr(js_object_alloc_with_shape( + 0x7FFF_FF10, + 2, + packed.as_ptr(), + packed.len() as u32, + ))); + let value_h = scope.root_nanbox_f64(value); let status = crate::string::js_string_from_bytes(b"fulfilled".as_ptr(), 9); + let obj = unboxed_ptr(obj_h.get_nanbox_f64()); js_object_set_field( obj, 0, JSValue::from_bits(crate::value::js_nanbox_string(status as i64).to_bits()), ); - js_object_set_field(obj, 1, JSValue::from_bits(value.to_bits())); - js_nanbox_pointer(obj as i64) + js_object_set_field( + obj, + 1, + JSValue::from_bits(value_h.get_nanbox_f64().to_bits()), + ); + obj_h.get_nanbox_f64() } fn build_settled_rejected(reason: f64) -> f64 { use crate::object::{js_object_alloc_with_shape, js_object_set_field}; let packed = b"status\0reason\0"; - let obj = js_object_alloc_with_shape(0x7FFF_FF11, 2, packed.as_ptr(), packed.len() as u32); + // #7497: see `build_settled_fulfilled`. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_h = scope.root_nanbox_f64(boxed_ptr(js_object_alloc_with_shape( + 0x7FFF_FF11, + 2, + packed.as_ptr(), + packed.len() as u32, + ))); + let reason_h = scope.root_nanbox_f64(reason); let status = crate::string::js_string_from_bytes(b"rejected".as_ptr(), 8); + let obj = unboxed_ptr(obj_h.get_nanbox_f64()); js_object_set_field( obj, 0, JSValue::from_bits(crate::value::js_nanbox_string(status as i64).to_bits()), ); - js_object_set_field(obj, 1, JSValue::from_bits(reason.to_bits())); - js_nanbox_pointer(obj as i64) + js_object_set_field( + obj, + 1, + JSValue::from_bits(reason_h.get_nanbox_f64().to_bits()), + ); + obj_h.get_nanbox_f64() } // --------------------------------------------------------------------------- @@ -449,6 +567,21 @@ fn perform( promise_resolve: f64, elements: *mut crate::array::ArrayHeader, ) -> Result<(), f64> { + // #7497: this loop runs USER JS twice per element — `Call(promiseResolve, C, + // «next»)` and `Invoke(nextPromise, "then", …)` — and allocates a guard array + // plus one or two closures in between. Pre-fix, the shared `state` and + // `values` arrays, the `elements` snapshot, the two capability functions and + // the constructor were all bare Rust locals held across every one of those + // calls. Root them; re-read every address at its point of use. The + // per-element handles live in a scope INSIDE the loop so a 50 000-element + // combinator does not push 50 000 entries onto the handle stack. + let scope = crate::gc::RuntimeHandleScope::new(); + let elements_h = scope.root_nanbox_f64(boxed_ptr(elements)); + let ctor_h = scope.root_nanbox_f64(c); + let resolve_fn_h = scope.root_nanbox_f64(promise_resolve); + let cap_resolve_h = scope.root_nanbox_f64(cap.resolve); + let cap_reject_h = scope.root_nanbox_f64(cap.reject); + let count = unsafe { (*elements).length }; // Shared state: remaining-count (init 1, spec's remainingElementsCount). @@ -457,111 +590,139 @@ fn perform( (*state).length = 1; } js_array_set_f64(state, 0, 1.0); + // Rooted BEFORE the `values` allocation below, which can collect. + let state_h = scope.root_nanbox_f64(boxed_ptr(state)); // Shared values/errors array (not used by Race). - let values = if kind == CombinatorKind::Race { - std::ptr::null_mut() + let values_h = if kind == CombinatorKind::Race { + scope.root_nanbox_f64(undef()) } else { let v = js_array_alloc(count.max(1)); unsafe { (*v).length = count; } + let v_h = scope.root_nanbox_f64(boxed_ptr(v)); for i in 0..count { - js_array_set_f64(v, i, undef()); + js_array_set_f64(unboxed_ptr(v_h.get_nanbox_f64()), i, undef()); } - v + v_h }; + let state_ptr = || -> *mut crate::array::ArrayHeader { unboxed_ptr(state_h.get_nanbox_f64()) }; + let values_ptr = + || -> *mut crate::array::ArrayHeader { unboxed_ptr(values_h.get_nanbox_f64()) }; for i in 0..count { - let next = js_array_get_f64(elements, i); + let iter = crate::gc::RuntimeHandleScope::new(); + let next = js_array_get_f64(unboxed_ptr(elements_h.get_nanbox_f64()), i); // nextPromise = ? Call(promiseResolve, C, «next») - let next_promise = call_with_this(promise_resolve, c, &[next])?; + let next_promise = call_with_this( + resolve_fn_h.get_nanbox_f64(), + ctor_h.get_nanbox_f64(), + &[next], + )?; + let next_promise_h = iter.root_nanbox_f64(next_promise); match kind { CombinatorKind::All => { - let guard = new_guard(); - let elem = build_element_closure( + let guard_h = iter.root_nanbox_f64(boxed_ptr(new_guard())); + let elem_h = iter.root_nanbox_f64(boxed_ptr(build_element_closure( all_resolve_element_fn as *const u8, - guard, + unboxed_ptr(guard_h.get_nanbox_f64()), i, - values, - state, - cap.resolve, - cap.reject, - ); + values_ptr(), + state_ptr(), + cap_resolve_h.get_nanbox_f64(), + cap_reject_h.get_nanbox_f64(), + ))); + let state = state_ptr(); js_array_set_f64(state, 0, js_array_get_f64(state, 0) + 1.0); - invoke_then(next_promise, &[js_nanbox_pointer(elem as i64), cap.reject])?; + invoke_then( + next_promise_h.get_nanbox_f64(), + &[elem_h.get_nanbox_f64(), cap_reject_h.get_nanbox_f64()], + )?; } CombinatorKind::AllSettled => { - let guard = new_guard(); - let on_ful = build_element_closure( + let guard_h = iter.root_nanbox_f64(boxed_ptr(new_guard())); + let on_ful_h = iter.root_nanbox_f64(boxed_ptr(build_element_closure( settled_fulfill_element_fn as *const u8, - guard, + unboxed_ptr(guard_h.get_nanbox_f64()), i, - values, - state, - cap.resolve, - cap.reject, - ); - let on_rej = build_element_closure( + values_ptr(), + state_ptr(), + cap_resolve_h.get_nanbox_f64(), + cap_reject_h.get_nanbox_f64(), + ))); + let on_rej_h = iter.root_nanbox_f64(boxed_ptr(build_element_closure( settled_reject_element_fn as *const u8, - guard, + unboxed_ptr(guard_h.get_nanbox_f64()), i, - values, - state, - cap.resolve, - cap.reject, - ); + values_ptr(), + state_ptr(), + cap_resolve_h.get_nanbox_f64(), + cap_reject_h.get_nanbox_f64(), + ))); + let state = state_ptr(); js_array_set_f64(state, 0, js_array_get_f64(state, 0) + 1.0); invoke_then( - next_promise, - &[ - js_nanbox_pointer(on_ful as i64), - js_nanbox_pointer(on_rej as i64), - ], + next_promise_h.get_nanbox_f64(), + &[on_ful_h.get_nanbox_f64(), on_rej_h.get_nanbox_f64()], )?; } CombinatorKind::Any => { - let guard = new_guard(); - let on_rej = build_element_closure( + let guard_h = iter.root_nanbox_f64(boxed_ptr(new_guard())); + let on_rej_h = iter.root_nanbox_f64(boxed_ptr(build_element_closure( any_reject_element_fn as *const u8, - guard, + unboxed_ptr(guard_h.get_nanbox_f64()), i, - values, - state, - cap.resolve, - cap.reject, - ); + values_ptr(), + state_ptr(), + cap_resolve_h.get_nanbox_f64(), + cap_reject_h.get_nanbox_f64(), + ))); + let state = state_ptr(); js_array_set_f64(state, 0, js_array_get_f64(state, 0) + 1.0); invoke_then( - next_promise, - &[cap.resolve, js_nanbox_pointer(on_rej as i64)], + next_promise_h.get_nanbox_f64(), + &[cap_resolve_h.get_nanbox_f64(), on_rej_h.get_nanbox_f64()], )?; } CombinatorKind::Race => { - invoke_then(next_promise, &[cap.resolve, cap.reject])?; + invoke_then( + next_promise_h.get_nanbox_f64(), + &[ + cap_resolve_h.get_nanbox_f64(), + cap_reject_h.get_nanbox_f64(), + ], + )?; } } } // Iterator exhausted: remainingElementsCount -= 1. if kind != CombinatorKind::Race { + let state = state_ptr(); let remaining = js_array_get_f64(state, 0) - 1.0; js_array_set_f64(state, 0, remaining); if remaining == 0.0 { match kind { CombinatorKind::All | CombinatorKind::AllSettled => { - let arr = js_nanbox_pointer(values as i64); - call_with_this(cap.resolve, undef(), &[arr])?; + call_with_this( + cap_resolve_h.get_nanbox_f64(), + undef(), + &[values_h.get_nanbox_f64()], + )?; } CombinatorKind::Any => { + // The message allocates; `values` (the errors array) is the + // AggregateError's own payload, so re-read it afterwards. let msg = crate::string::js_string_from_bytes( b"All promises were rejected".as_ptr(), 26, ); - let agg = crate::error::js_aggregateerror_new(values, msg); - let agg_v = js_nanbox_pointer(agg as i64); - call_with_this(cap.reject, undef(), &[agg_v])?; + let agg_v = boxed_ptr(crate::error::js_aggregateerror_new(values_ptr(), msg)); + // Nothing allocates between the line above and the call, so + // `agg_v` needs no handle of its own. + call_with_this(cap_reject_h.get_nanbox_f64(), undef(), &[agg_v])?; } CombinatorKind::Race => unreachable!(), } @@ -590,20 +751,47 @@ fn new_guard() -> *mut crate::array::ArrayHeader { pub fn run_combinator(kind: CombinatorKind, c: f64, iterable: f64) -> f64 { ensure_arity_registered(); + // #7497: `c`, `iterable` and all three capability slots stay live across + // `get_promise_resolve` (a property read that can run a getter), + // `combinator_iterable_to_array_caught` (a full iterator drain) and + // `perform` (the per-element user-JS loop). `cap.promise` in particular is + // the value this function RETURNS, so a stale copy hands the caller a + // from-space promise. + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor_h = scope.root_nanbox_f64(c); + let iterable_h = scope.root_nanbox_f64(iterable); + // 2. Let promiseCapability be ? NewPromiseCapability(C). (may throw) - let cap = new_promise_capability(c); + let cap = new_promise_capability(ctor_h.get_nanbox_f64()); + let cap_promise_h = scope.root_nanbox_f64(cap.promise); + let cap_resolve_h = scope.root_nanbox_f64(cap.resolve); + let cap_reject_h = scope.root_nanbox_f64(cap.reject); // 3-8. The remaining steps are IfAbruptRejectPromise-guarded. let result: Result<(), f64> = (|| { - let promise_resolve = get_promise_resolve(c)?; - let elements = combinator_iterable_to_array_caught(iterable)?; - perform(kind, c, &cap, promise_resolve, elements) + let promise_resolve = get_promise_resolve(ctor_h.get_nanbox_f64())?; + let resolve_fn_h = scope.root_nanbox_f64(promise_resolve); + let elements = combinator_iterable_to_array_caught(iterable_h.get_nanbox_f64())?; + // Rebuild the capability from the handles AFTER the drain, so `perform` + // roots post-collection addresses. + let cap = Capability { + promise: cap_promise_h.get_nanbox_f64(), + resolve: cap_resolve_h.get_nanbox_f64(), + reject: cap_reject_h.get_nanbox_f64(), + }; + perform( + kind, + ctor_h.get_nanbox_f64(), + &cap, + resolve_fn_h.get_nanbox_f64(), + elements, + ) })(); if let Err(reason) = result { - let _ = call_with_this(cap.reject, undef(), &[reason]); + let _ = call_with_this(cap_reject_h.get_nanbox_f64(), undef(), &[reason]); } - cap.promise + cap_promise_h.get_nanbox_f64() } // --------------------------------------------------------------------------- @@ -672,16 +860,23 @@ fn is_object_value(value: f64) -> bool { #[no_mangle] pub extern "C" fn js_promise_reject_spec(this_ctor: f64, reason: f64) -> f64 { ensure_arity_registered(); + // #7497: `is_default_promise_constructor` reaches `globalThis.Promise`, which + // allocates a key string on every call — so neither argument may be carried + // across it in a register. See `js_promise_resolve_spec`. + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor_h = scope.root_nanbox_f64(this_ctor); + let reason_h = scope.root_nanbox_f64(reason); // Default `Promise`: the optimized native rejected-promise path. - if is_default_promise_constructor(this_ctor) { - let p = crate::promise::js_promise_rejected(reason); + if is_default_promise_constructor(ctor_h.get_nanbox_f64()) { + let p = crate::promise::js_promise_rejected(reason_h.get_nanbox_f64()); return js_nanbox_pointer(p as i64); } - let cap = new_promise_capability(this_ctor); - if let Err(thrown) = call_with_this(cap.reject, undef(), &[reason]) { + let cap = new_promise_capability(ctor_h.get_nanbox_f64()); + let cap_promise_h = scope.root_nanbox_f64(cap.promise); + if let Err(thrown) = call_with_this(cap.reject, undef(), &[reason_h.get_nanbox_f64()]) { crate::exception::js_throw(thrown); } - cap.promise + cap_promise_h.get_nanbox_f64() } /// `Promise.resolve(x)` (ECMA-262 27.2.4.7 → PromiseResolve) with `this` = C: @@ -691,34 +886,45 @@ pub extern "C" fn js_promise_reject_spec(this_ctor: f64, reason: f64) -> f64 { #[no_mangle] pub extern "C" fn js_promise_resolve_spec(this_ctor: f64, value: f64) -> f64 { ensure_arity_registered(); - if !is_object_value(this_ctor) { + // #7497, and this is THE hot one: `Promise.all` calls this once per element, + // and `is_default_promise_constructor` below reaches `globalThis.Promise` + // through a lookup that allocates a fresh key string EVERY time. `value` — + // the element, which for `Promise.all([...promises])` is itself a promise — + // used to sit in a register across that allocation and was then dereferenced + // by `js_promise_resolved`. `PERRY_GC_PROTECT_FROMSPACE=1` faults here on a + // 72-byte `GC_TYPE_PROMISE` at minor #0. + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor_h = scope.root_nanbox_f64(this_ctor); + let value_h = scope.root_nanbox_f64(value); + if !is_object_value(ctor_h.get_nanbox_f64()) { throw_type_error("Promise.resolve called on non-object"); } // Default `Promise`: keep the optimized native path — it preserves promise // identity AND assimilates thenables (object-literal `then`), which the // generic capability path below would not (its resolve just stores the // value). The per-element resolve of the combinators routes through here. - if is_default_promise_constructor(this_ctor) { - let p = crate::promise::js_promise_resolved(value); + if is_default_promise_constructor(ctor_h.get_nanbox_f64()) { + let p = crate::promise::js_promise_resolved(value_h.get_nanbox_f64()); return js_nanbox_pointer(p as i64); } - if crate::promise::js_value_is_promise(value) != 0 { + if crate::promise::js_value_is_promise(value_h.get_nanbox_f64()) != 0 { let xctor = unsafe { crate::value::js_dynamic_object_get_property( - value, + value_h.get_nanbox_f64(), b"constructor".as_ptr() as *const i8, 11, ) }; - if xctor.to_bits() == this_ctor.to_bits() { - return value; + if xctor.to_bits() == ctor_h.get_nanbox_f64().to_bits() { + return value_h.get_nanbox_f64(); } } - let cap = new_promise_capability(this_ctor); - if let Err(thrown) = call_with_this(cap.resolve, undef(), &[value]) { + let cap = new_promise_capability(ctor_h.get_nanbox_f64()); + let cap_promise_h = scope.root_nanbox_f64(cap.promise); + if let Err(thrown) = call_with_this(cap.resolve, undef(), &[value_h.get_nanbox_f64()]) { crate::exception::js_throw(thrown); } - cap.promise + cap_promise_h.get_nanbox_f64() } /// `Promise.withResolvers()` (ECMA-262 27.2.4.x) with `this` = C: @@ -728,22 +934,43 @@ pub extern "C" fn js_promise_resolve_spec(this_ctor: f64, value: f64) -> f64 { #[no_mangle] pub extern "C" fn js_promise_with_resolvers_spec(this_ctor: f64) -> f64 { ensure_arity_registered(); - if is_default_promise_constructor(this_ctor) { + // #7497: see `js_promise_resolve_spec` — `is_default_promise_constructor` + // allocates, and the three capability slots below outlive the record + // allocation they are stored into. + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor_h = scope.root_nanbox_f64(this_ctor); + if is_default_promise_constructor(ctor_h.get_nanbox_f64()) { let obj = crate::promise::js_promise_with_resolvers(); return js_nanbox_pointer(obj as i64); } - let cap = new_promise_capability(this_ctor); + let cap = new_promise_capability(ctor_h.get_nanbox_f64()); + let cap_promise_h = scope.root_nanbox_f64(cap.promise); + let cap_resolve_h = scope.root_nanbox_f64(cap.resolve); + let cap_reject_h = scope.root_nanbox_f64(cap.reject); let packed = b"promise\0resolve\0reject\0"; - let obj = crate::object::js_object_alloc_with_shape( + let obj_h = scope.root_nanbox_f64(boxed_ptr(crate::object::js_object_alloc_with_shape( 0xFFF0_0001, 3, packed.as_ptr(), packed.len() as u32, + ))); + let obj = unboxed_ptr(obj_h.get_nanbox_f64()); + crate::object::js_object_set_field( + obj, + 0, + JSValue::from_bits(cap_promise_h.get_nanbox_f64().to_bits()), + ); + crate::object::js_object_set_field( + obj, + 1, + JSValue::from_bits(cap_resolve_h.get_nanbox_f64().to_bits()), + ); + crate::object::js_object_set_field( + obj, + 2, + JSValue::from_bits(cap_reject_h.get_nanbox_f64().to_bits()), ); - crate::object::js_object_set_field(obj, 0, JSValue::from_bits(cap.promise.to_bits())); - crate::object::js_object_set_field(obj, 1, JSValue::from_bits(cap.resolve.to_bits())); - crate::object::js_object_set_field(obj, 2, JSValue::from_bits(cap.reject.to_bits())); - js_nanbox_pointer(obj as i64) + obj_h.get_nanbox_f64() } /// `Promise.try(callbackfn, ...args)` (ECMA-262 27.2.4.x) with `this` = C: @@ -753,23 +980,30 @@ pub extern "C" fn js_promise_with_resolvers_spec(this_ctor: f64) -> f64 { #[no_mangle] pub extern "C" fn js_promise_try_spec(this_ctor: f64, callback: f64, rest: f64) -> f64 { ensure_arity_registered(); - if !is_object_value(this_ctor) { + // #7497: see `js_promise_resolve_spec` — `is_default_promise_constructor` + // allocates, and `callback` / `rest` are dereferenced after it. + let scope = crate::gc::RuntimeHandleScope::new(); + let ctor_h = scope.root_nanbox_f64(this_ctor); + let callback_h = scope.root_nanbox_f64(callback); + let rest_h = scope.root_nanbox_f64(rest); + if !is_object_value(ctor_h.get_nanbox_f64()) { throw_type_error("Promise.try called on non-object"); } // Default `Promise`: keep the optimized native path (synchronous call + // resolve/reject), which also assimilates a thenable return value. - if is_default_promise_constructor(this_ctor) { - let rest_v = JSValue::from_bits(rest.to_bits()); + if is_default_promise_constructor(ctor_h.get_nanbox_f64()) { + let rest_v = JSValue::from_bits(rest_h.get_nanbox_f64().to_bits()); let rest_ptr = if rest_v.is_pointer() { rest_v.as_pointer::() } else { std::ptr::null() }; - let p = crate::promise::js_promise_try(callback, rest_ptr); + let p = crate::promise::js_promise_try(callback_h.get_nanbox_f64(), rest_ptr); return js_nanbox_pointer(p as i64); } - let cap = new_promise_capability(this_ctor); - let rest_v = JSValue::from_bits(rest.to_bits()); + let cap = new_promise_capability(ctor_h.get_nanbox_f64()); + let cap_promise_h = scope.root_nanbox_f64(cap.promise); + let rest_v = JSValue::from_bits(rest_h.get_nanbox_f64().to_bits()); let args: Vec = if rest_v.is_pointer() { let arr = rest_v.as_pointer::(); let n = unsafe { (*arr).length }; @@ -777,13 +1011,15 @@ pub extern "C" fn js_promise_try_spec(this_ctor: f64, callback: f64, rest: f64) } else { Vec::new() }; - match call_with_this(callback, undef(), &args) { + let cap_resolve_h = scope.root_nanbox_f64(cap.resolve); + let cap_reject_h = scope.root_nanbox_f64(cap.reject); + match call_with_this(callback_h.get_nanbox_f64(), undef(), &args) { Ok(value) => { - let _ = call_with_this(cap.resolve, undef(), &[value]); + let _ = call_with_this(cap_resolve_h.get_nanbox_f64(), undef(), &[value]); } Err(reason) => { - let _ = call_with_this(cap.reject, undef(), &[reason]); + let _ = call_with_this(cap_reject_h.get_nanbox_f64(), undef(), &[reason]); } } - cap.promise + cap_promise_h.get_nanbox_f64() } diff --git a/crates/perry-runtime/src/promise/then.rs b/crates/perry-runtime/src/promise/then.rs index a2659a408b..0367e35d21 100644 --- a/crates/perry-runtime/src/promise/then.rs +++ b/crates/perry-runtime/src/promise/then.rs @@ -1218,19 +1218,51 @@ fn perform_promise_then_with_cap( cap_promise: f64, ) -> f64 { use crate::closure::{js_closure_alloc, js_closure_set_capture_f64}; - let ful_wrap = js_closure_alloc(then_cap_fulfill_fn as *const u8, 3); - js_closure_set_capture_f64(ful_wrap, 0, on_fulfilled); - js_closure_set_capture_f64(ful_wrap, 1, cap_resolve); - js_closure_set_capture_f64(ful_wrap, 2, cap_reject); - - let rej_wrap = js_closure_alloc(then_cap_reject_fn as *const u8, 3); - js_closure_set_capture_f64(rej_wrap, 0, on_rejected); - js_closure_set_capture_f64(rej_wrap, 1, cap_resolve); - js_closure_set_capture_f64(rej_wrap, 2, cap_reject); + // #7497 (CodeRabbit): the SAME publishing shape as `make_resolving_functions` + // and `build_element_closure`. Pre-fix this filled `ful_wrap`'s captures, then + // allocated `rej_wrap` — so a copying minor at that second allocation left + // `promise`, `ful_wrap` and the four capability/handler values naming + // from-space, and the addresses written into `rej_wrap` afterwards were + // pre-collection. Allocate BOTH wrappers first, root everything, then write + // every capture at a post-collection address. + let scope = crate::gc::RuntimeHandleScope::new(); + let promise_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(promise as i64)); + let on_fulfilled_h = scope.root_nanbox_f64(on_fulfilled); + let on_rejected_h = scope.root_nanbox_f64(on_rejected); + let cap_resolve_h = scope.root_nanbox_f64(cap_resolve); + let cap_reject_h = scope.root_nanbox_f64(cap_reject); + let cap_promise_h = scope.root_nanbox_f64(cap_promise); + + let ful_wrap_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_closure_alloc( + then_cap_fulfill_fn as *const u8, + 3, + ) as i64)); + let rej_wrap_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(js_closure_alloc( + then_cap_reject_fn as *const u8, + 3, + ) as i64)); + + let ful_wrap = crate::value::js_nanbox_get_pointer(ful_wrap_h.get_nanbox_f64()) + as *mut crate::closure::ClosureHeader; + js_closure_set_capture_f64(ful_wrap, 0, on_fulfilled_h.get_nanbox_f64()); + js_closure_set_capture_f64(ful_wrap, 1, cap_resolve_h.get_nanbox_f64()); + js_closure_set_capture_f64(ful_wrap, 2, cap_reject_h.get_nanbox_f64()); + + let rej_wrap = crate::value::js_nanbox_get_pointer(rej_wrap_h.get_nanbox_f64()) + as *mut crate::closure::ClosureHeader; + js_closure_set_capture_f64(rej_wrap, 0, on_rejected_h.get_nanbox_f64()); + js_closure_set_capture_f64(rej_wrap, 1, cap_resolve_h.get_nanbox_f64()); + js_closure_set_capture_f64(rej_wrap, 2, cap_reject_h.get_nanbox_f64()); // Attach handlers; discard the returned native next promise. - let _ = js_promise_then(promise, ful_wrap, rej_wrap); - cap_promise + let _ = js_promise_then( + crate::value::js_nanbox_get_pointer(promise_h.get_nanbox_f64()) as *mut Promise, + crate::value::js_nanbox_get_pointer(ful_wrap_h.get_nanbox_f64()) + as *mut crate::closure::ClosureHeader, + crate::value::js_nanbox_get_pointer(rej_wrap_h.get_nanbox_f64()) + as *mut crate::closure::ClosureHeader, + ); + cap_promise_h.get_nanbox_f64() } // --------------------------------------------------------------------------- @@ -1369,24 +1401,38 @@ pub(crate) extern "C" fn promise_prototype_then_thunk( throw_promise_prototype_incompatible_receiver("then", receiver); }; + // #7497: `promise_species_constructor` reads `receiver.constructor` and + // `is_default_promise_constructor` performs a `globalThis.Promise` lookup + // that allocates a fresh key string — the receiver promise and both handler + // arguments are live across both, and `promise` is the receiver of the + // `then` that follows. + let scope = crate::gc::RuntimeHandleScope::new(); + let promise_h = scope.root_nanbox_f64(crate::value::js_nanbox_pointer(promise as i64)); + let on_fulfilled_h = scope.root_nanbox_f64(on_fulfilled); + let on_rejected_h = scope.root_nanbox_f64(on_rejected); + // SpeciesConstructor(promise, %Promise%) — reads this.constructor once. let c = promise_species_constructor(receiver); + let c_h = scope.root_nanbox_f64(c); - if super::spec_combinators::is_default_promise_constructor(c) { + if super::spec_combinators::is_default_promise_constructor(c_h.get_nanbox_f64()) { // Fast path: native then. + let promise = + crate::value::js_nanbox_get_pointer(promise_h.get_nanbox_f64()) as *mut Promise; return box_promise_ptr(js_promise_then( promise, - arg_to_closure(on_fulfilled), - arg_to_closure(on_rejected), + arg_to_closure(on_fulfilled_h.get_nanbox_f64()), + arg_to_closure(on_rejected_h.get_nanbox_f64()), )); } // Slow path: NewPromiseCapability(C) + PerformPromiseThen. - let cap = super::spec_combinators::new_promise_capability(c); + let cap = super::spec_combinators::new_promise_capability(c_h.get_nanbox_f64()); + let promise = crate::value::js_nanbox_get_pointer(promise_h.get_nanbox_f64()) as *mut Promise; perform_promise_then_with_cap( promise, - on_fulfilled, - on_rejected, + on_fulfilled_h.get_nanbox_f64(), + on_rejected_h.get_nanbox_f64(), cap.resolve, cap.reject, cap.promise, diff --git a/scripts/auto_opt_app_patterns.sh b/scripts/auto_opt_app_patterns.sh index b2fff48969..15918ff2c6 100755 --- a/scripts/auto_opt_app_patterns.sh +++ b/scripts/auto_opt_app_patterns.sh @@ -59,14 +59,15 @@ fi # names a kernel which no longer exists FAILS below — the same rule # `scripts/gc_root_dominance_allowlist.json` uses, so a fixed kernel cannot keep # its exemption by inertia. -SKIPS=( - # #7497: fails under BOTH link modes, differently ("Uncaught (in promise) 0" - # with PERRY_NO_AUTO_OPTIMIZE=1, a rooting-shaped TypeError without), and was - # unchanged by #7495's iterator-drain rooting fix in both — which is the - # evidence that it is a separate promise-rejection defect rather than the - # rooting family this gate was created for. - "promise_all_chains:promise rejects with a resolution value at scale, #7497" -) +# +# EMPTY IS THE INTENDED STEADY STATE. `promise_all_chains` was the only entry; +# #7497 fixed it (the `globalThis` builtin lookup read its receiver out of a root +# and only then allocated the key, so every `Promise.resolve` inside a wide +# combinator was a chance to deref retired from-space). Every array expansion +# below is guarded on `${#…[@]}` because macOS ships bash 3.2, where `set -u` +# makes `"${EMPTY[@]}"` an "unbound variable" error — an empty skip list must +# leave the gate RUNNING, not abort it before the first kernel. +SKIPS=() # LIVENESS MATCHER. Reads the archive the linker was actually handed out of a # `perry -v` compile log. Deliberately reads the `[link] invoking:` command @@ -158,22 +159,26 @@ if [[ ${#all_kernels[@]} -eq 0 ]]; then fi skip_names=() -for entry in "${SKIPS[@]}"; do - skip_names+=("${entry%%:*}") -done +if [[ ${#SKIPS[@]} -gt 0 ]]; then + for entry in "${SKIPS[@]}"; do + skip_names+=("${entry%%:*}") + done +fi # A skip entry that matches nothing is a failure, not a no-op. rotted=0 -for name in "${skip_names[@]}"; do - found=0 - for k in "${all_kernels[@]}"; do - [[ "$k" == "$name" ]] && found=1 +if [[ ${#skip_names[@]} -gt 0 ]]; then + for name in "${skip_names[@]}"; do + found=0 + for k in "${all_kernels[@]}"; do + [[ "$k" == "$name" ]] && found=1 + done + if [[ $found -eq 0 ]]; then + echo "error: skip entry '$name' matches no kernel in $KERNEL_DIR — delete its line." >&2 + rotted=1 + fi done - if [[ $found -eq 0 ]]; then - echo "error: skip entry '$name' matches no kernel in $KERNEL_DIR — delete its line." >&2 - rotted=1 - fi -done +fi [[ $rotted -eq 0 ]] || exit 1 requested=("$@") @@ -183,9 +188,11 @@ if [[ ${#requested[@]} -gt 0 ]]; then else for k in "${all_kernels[@]}"; do skip=0 - for name in "${skip_names[@]}"; do - [[ "$k" == "$name" ]] && skip=1 - done + if [[ ${#skip_names[@]} -gt 0 ]]; then + for name in "${skip_names[@]}"; do + [[ "$k" == "$name" ]] && skip=1 + done + fi [[ $skip -eq 1 ]] || selected+=("$k") done fi @@ -262,9 +269,13 @@ for name in "${selected[@]}"; do done echo -for entry in "${SKIPS[@]}"; do - echo "SKIP ${entry%%:*} — ${entry#*:}" -done +if [[ ${#SKIPS[@]} -gt 0 ]]; then + for entry in "${SKIPS[@]}"; do + echo "SKIP ${entry%%:*} — ${entry#*:}" + done +else + echo "SKIP (none — every kernel in $KERNEL_DIR is gated)" +fi if [[ $failures -gt 0 ]]; then echo diff --git a/test-files/test_gap_gc_global_builtin_lookup_rooting.ts b/test-files/test_gap_gc_global_builtin_lookup_rooting.ts new file mode 100644 index 0000000000..882722843c --- /dev/null +++ b/test-files/test_gap_gc_global_builtin_lookup_rooting.ts @@ -0,0 +1,85 @@ +// #7497: `js_get_global_this_builtin_value` read the `globalThis` object out of +// its (correctly registered, correctly rewritten) root into a bare +// `*const ObjectHeader`, and only THEN allocated the lookup key: +// +// let global_obj = js_nanbox_get_pointer(js_get_global_this()); +// let key = js_string_from_bytes(name); // ALLOCATES — may collect +// js_object_get_field_by_name(global_obj, key); // from-space deref +// +// The root is not the problem; the ORDER is. This lookup interns nothing, so +// every call mints a fresh key string, and any of those allocations can trigger +// the copying minor that evacuates `globalThis`. `THREAD_GLOBAL_THIS` is +// rewritten to the forwarding address — the raw copy already read out of it is +// not. +// +// WHY `Promise.all` IS THE SHAPE THAT FINDS IT. Each element of a combinator +// runs `Call(promiseResolve, C, «next»)`, i.e. `Promise.resolve`, and +// `js_promise_resolve_spec` asks `is_default_promise_constructor` whether `this` +// is the intrinsic `Promise` — which is a `globalThis.Promise` read through the +// function above. One `Promise.all` over 50 000 already-resolved promises +// therefore performs 50 000 of these lookups inside a single native call, so one +// of them straddles the collection. Nothing else in the program has to be large. +// +// STRUCTURE IS LOAD-BEARING. The single big `Promise.all` is what packs enough +// lookups between two collections; the same total work split into many small +// `Promise.all` calls (`benchmarks/app-patterns/kernels/promise_all_chains.ts` +// is 1000 × 50) needs ~20× the work to hit the same window. Do not "simplify" +// this to a loop of small combinator calls. +// +// SYMPTOM BEFORE THE FIX (release, default link, default env): the stale read +// returns whatever from-space holds, so `globalThis.Promise` comes back as a +// non-callable, and the run dies with `Uncaught (in promise) TypeError: value is +// not a function` or, when the garbage happens to be zero, with the resolution +// value arriving on the rejection path: `Uncaught (in promise) 0`. +// `PERRY_GC_PROTECT_FROMSPACE=1` turns it into a precise SIGBUS inside +// `js_object_get_field_by_name`, reached from `js_get_global_this_builtin_value` +// ← `js_promise_resolve_spec` ← `spec_combinators::call_with_this`. + +async function main() { + // One combinator call, wide enough that its per-element + // `Promise.resolve` lookups span a copying minor. + const wide: Promise[] = []; + for (let i = 0; i < 50_000; i++) { + wide.push(Promise.resolve(i)); + } + const settled = await Promise.all(wide); + + let checksum = 0; + let wrong = 0; + for (let i = 0; i < settled.length; i++) { + if (settled[i] !== i) wrong++; + checksum += settled[i]; + } + + // The `globalThis.Promise` identity the stale read corrupts is observable + // directly too: `Promise.resolve` must keep returning real promises, and a + // native promise passed to `Promise.resolve` must come back unchanged + // (`PromiseResolve` short-circuits when `x.constructor === C`). + const one = Promise.resolve(7); + const same = Promise.resolve(one) === one; + const isPromise = one instanceof Promise; + + // The app-pattern shape from the issue: many small combinator calls over + // three-deep await chains. Smaller here (the wide case above is the + // sensitive one) but it is the workload #7497 was filed against. + async function unitOfWork(i: number): Promise { + const a = await Promise.resolve(i); + const b = await Promise.resolve(a + 1); + return b * 2; + } + let chained = 0; + for (let batch = 0; batch < 40; batch++) { + const promises: Promise[] = []; + for (let i = 0; i < 50; i++) { + promises.push(unitOfWork(batch * 50 + i)); + } + const results = await Promise.all(promises); + for (let i = 0; i < results.length; i++) chained += results[i]; + } + + console.log("checksum:", checksum, "wrong:", wrong); + console.log("same:", same, "isPromise:", isPromise); + console.log("chained:", chained); +} + +main(); diff --git a/test-parity/gc_repsel_corpus.txt b/test-parity/gc_repsel_corpus.txt index 47ce330124..836e74768e 100644 --- a/test-parity/gc_repsel_corpus.txt +++ b/test-parity/gc_repsel_corpus.txt @@ -602,3 +602,34 @@ test_gap_gc_optional_param_receiver_rooting # #7498 lands, a protected run of this file should go silent — if it does not, # there is a third site. test_gap_gc_iterator_drain_rooting + +# --- #7497: the globalThis builtin-value lookup ----------------------------- +# +# `js_get_global_this_builtin_value` read the `globalThis` object out of its +# root into a raw `*const ObjectHeader` and only THEN allocated the lookup key. +# The root is correct and IS rewritten by evacuation; the raw copy already read +# out of it is not. That lookup interns nothing, so every call mints a fresh +# string, and any of those allocations can be the copying minor that moves +# `globalThis` — after which `js_object_get_field_by_name` dereferences retired +# from-space. +# +# `Promise.all` is what finds it: each element runs `Promise.resolve`, and +# `js_promise_resolve_spec` asks `is_default_promise_constructor` for +# `globalThis.Promise` through that function. One 50 000-element combinator +# call therefore performs 50 000 of these lookups, so one straddles the +# collection. That is why the app-pattern kernel `promise_all_chains` +# (1000 x 50) needed its full scale while a single wide call needs 0.2 s. +# +# Measured on this branch, release, PERRY_NO_AUTO_OPTIMIZE=1 and default env: +# before Uncaught (in promise) TypeError: value is not a function +# before, PROTECT_FROMSPACE=1 DEPTH=200 +# FAULT in js_object_get_field_by_name, reached from +# js_get_global_this_builtin_value <- js_promise_resolve_spec <- +# spec_combinators::call_with_this <- run_combinator <- +# js_promise_all_iterable, on a GC_TYPE_OBJECT at minor #0 +# after byte-exact with the oracle, and that fault is gone +# +# The failure is NOT arm-conditional — it fails on the shipped default — but it +# is registered here because the moving arms are what turn it into a fault with +# an address instead of a wrong answer. +test_gap_gc_global_builtin_lookup_rooting