From 93824be1935d6cd280bb76f591c24f1ea6722aec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 20 Aug 2026 06:54:19 +0200 Subject: [PATCH 1/2] fix(runtime): root repeat's receiver across the count coercion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `js_string_repeat` borrowed the receiver's inline WTF-8 payload and only then coerced `count`. ToNumber on an object count runs user JS (`valueOf` / `Symbol.toPrimitive`), whose loop back-edge polls are moving-GC safepoints, so an evacuating minor inside the callback left the borrow naming retired from-space and the result was copied out of garbage. Coerce first, then borrow. Deferring the borrow alone is not enough: `s` is a raw pointer in a native Rust frame, which the collector does not scan by default, so it is parked in a `RuntimeHandleScope` and the post-collection address is taken back out via `across_const`. padStart/padEnd were audited for the same shape and are unaffected — codegen emits both of their coercions (`js_number_coerce` for maxLength, `js_string_pad_fill` for the fill) before the receiver handle is re-read, so no user code runs inside those helpers while the payload is borrowed. The new fixture covers them too, so a future move of either coercion into the helper lands on a test that is already watching. --- crates/perry-runtime/src/string/pad.rs | 32 ++++- ...st_gap_gc_string_repeat_reentrant_count.ts | 133 ++++++++++++++++++ 2 files changed, 162 insertions(+), 3 deletions(-) create mode 100644 test-files/test_gap_gc_string_repeat_reentrant_count.ts diff --git a/crates/perry-runtime/src/string/pad.rs b/crates/perry-runtime/src/string/pad.rs index 4cd20a6325..89126ea3a1 100644 --- a/crates/perry-runtime/src/string/pad.rs +++ b/crates/perry-runtime/src/string/pad.rs @@ -299,14 +299,40 @@ pub extern "C" fn js_string_repeat(s: *const StringHeader, count_value: f64) -> return js_string_from_bytes("".as_ptr(), 0); } - let str_data = string_as_str(s); - let count_number = crate::builtins::js_number_coerce(count_value); + // `count` may be an object, so ToNumber runs its + // `valueOf`/`Symbol.toPrimitive` — arbitrary user JS, whose loop back-edge + // polls are moving-GC safepoints (default-on since #7721). Two distinct + // hazards follow, and the ordering below is what closes both (#8427): + // + // * The receiver's WTF-8 payload must NOT be borrowed across the + // coercion. `string_as_str` materializes a `&str` at the *pre-move* + // address; rooting rewrites slots, never an already-live borrow, so no + // root can repair one (`HeapKeyBytes`, `object/field_get_set.rs`). + // Pre-fix, an evacuating minor inside `valueOf` left `str_data` + // pointing at retired from-space and `repeat` copied garbage. + // * `s` itself is a raw pointer in a native Rust frame, which the + // collector does not scan by default — so deferring the borrow is not + // enough on its own. Park it in a transient root and take the + // *post-collection* address back out of the handle. + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver_root = scope.root_string_ptr(s); + let (count_number, s) = receiver_root + .across_const::(|| crate::builtins::js_number_coerce(count_value)); + let count_integer = to_integer_or_infinity(count_number); if count_integer < 0.0 || count_integer.is_infinite() { throw_repeat_range_error(count_number); } - if count_integer == 0.0 || str_data.is_empty() { + // ECMA-262 §22.1.3.17 step 4 returns "" for n = 0 before the receiver is + // consulted at all; an empty receiver repeats to "" for every remaining n. + // Both are checked here so the payload is borrowed only on the path that + // actually reads it — and only once no user code is left to run. + if count_integer == 0.0 { + return js_string_from_bytes("".as_ptr(), 0); + } + let str_data = string_as_str(s); + if str_data.is_empty() { return js_string_from_bytes("".as_ptr(), 0); } diff --git a/test-files/test_gap_gc_string_repeat_reentrant_count.ts b/test-files/test_gap_gc_string_repeat_reentrant_count.ts new file mode 100644 index 0000000000..d2b751c7c0 --- /dev/null +++ b/test-files/test_gap_gc_string_repeat_reentrant_count.ts @@ -0,0 +1,133 @@ +// #8427: `String.prototype.repeat` used to borrow the receiver's inline WTF-8 +// payload BEFORE coercing `count`, and ToNumber on an object count runs user +// JS. A moving collection inside that `valueOf` relocated the receiver, so the +// borrow named retired from-space and the result was copied out of garbage +// ("repeat [invalid utf8]" under the instruments below). Keep the reentrant +// count shapes under moving-GC pressure so the ordering contract stays gated. +// parity-env: PERRY_GC_MOVING_LOOP_POLLS=1 PERRY_GC_SCHEDULE_SEED=8427 PERRY_GC_SCHEDULE_RATE=1 PERRY_GC_SCHEDULE_ALLOC_KB=0 PERRY_GC_FORCE_EVACUATE=1 PERRY_GC_VERIFY_EVACUATION=1 PERRY_GC_PROTECT_FROMSPACE=1 + +// Allocate hard enough inside the callback to reach loop back-edge safepoints +// while the receiver is still young. +function churn(): number { + let junk = ""; + for (let i = 0; i < 25; i++) { + junk = junk + "x"; + } + return junk.length; +} + +let badRepeat = 0; +let callbackRuns = 0; +for (let i = 0; i < 3; i++) { + // A fresh, joined receiver is a young heap string, not SSO and not interned. + const subject = ["abc", "def", String(i % 10)].join(""); + const repeated = subject.repeat({ + valueOf() { + callbackRuns++; + churn(); + return 3; + }, + } as unknown as number); + if (repeated !== subject + subject + subject) badRepeat++; +} +console.log("repeat under reentrant count:", badRepeat === 0, callbackRuns); + +// `Symbol.toPrimitive` takes precedence over `valueOf` and is the same window. +let badPrimitive = 0; +for (let i = 0; i < 3; i++) { + const subject = ["uvw", "xyz", String(i % 10)].join(""); + const repeated = subject.repeat({ + [Symbol.toPrimitive]() { + churn(); + return 2; + }, + } as unknown as number); + if (repeated !== subject + subject) badPrimitive++; +} +console.log("repeat under toPrimitive count:", badPrimitive === 0); + +// A count of 0 returns "" without ever reading the receiver — the callback +// still runs, and still allocates, exactly once per call. +let zeroRuns = 0; +const zeroSubject = ["mno", "pqr"].join(""); +const zeroResult = zeroSubject.repeat({ + valueOf() { + zeroRuns++; + churn(); + return 0; + }, +} as unknown as number); +console.log("zero count:", JSON.stringify(zeroResult), zeroRuns); + +// ToIntegerOrInfinity is observable even when the receiver is empty, and a +// negative count must throw before repeat's empty-string return. +let emptyCoercions = 0; +try { + "".repeat({ + valueOf() { + emptyCoercions++; + churn(); + throw new Error("empty-count"); + }, + } as unknown as number); +} catch (error) { + console.log("empty throw:", emptyCoercions, (error as Error).message); +} + +try { + "".repeat({ + valueOf() { + emptyCoercions++; + churn(); + return -1; + }, + } as unknown as number); +} catch (error) { + console.log("empty negative:", emptyCoercions, error instanceof RangeError); +} + +// padStart/padEnd take the same shape of reentrant arguments, but their +// coercions (`js_number_coerce` for maxLength, `js_string_pad_fill` for the +// fill) are emitted by codegen BEFORE the receiver handle is re-read, so no +// user code runs inside the runtime helper while its payload is borrowed. +// Pin that ordering here so a future move of either coercion into the helper +// reintroduces #8427's window with a test already watching. +let badPadStart = 0; +let badPadEnd = 0; +for (let i = 0; i < 3; i++) { + const subject = ["pad", "me", String(i % 10)].join(""); + const started = subject.padStart( + { + valueOf() { + churn(); + return 12; + }, + } as unknown as number, + { + toString() { + churn(); + return "-"; + }, + } as unknown as string, + ); + const ended = subject.padEnd( + { + valueOf() { + churn(); + return 12; + }, + } as unknown as number, + { + toString() { + churn(); + return "+"; + }, + } as unknown as string, + ); + // Literal expectations: the receiver is 6 units, so 6 fill units are added. + // (Spelling these out keeps the pad assertions independent of `repeat`.) + if (started !== "------" + subject) badPadStart++; + if (ended !== subject + "++++++") badPadEnd++; +} +console.log("padStart under reentrant args:", badPadStart === 0); +console.log("padEnd under reentrant args:", badPadEnd === 0); From fad400ad83f01c17c30e8de847a02c7cc3a3208e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 20 Aug 2026 06:55:28 +0200 Subject: [PATCH 2/2] chore: key changelog fragment to PR 8443 --- changelog.d/8443-string-repeat-reentrant-count.md | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changelog.d/8443-string-repeat-reentrant-count.md diff --git a/changelog.d/8443-string-repeat-reentrant-count.md b/changelog.d/8443-string-repeat-reentrant-count.md new file mode 100644 index 0000000000..1e0d7328bd --- /dev/null +++ b/changelog.d/8443-string-repeat-reentrant-count.md @@ -0,0 +1,3 @@ +### Fixed + +- `String.prototype.repeat()` no longer reads a stale string payload when the `count` argument's `valueOf`/`Symbol.toPrimitive` triggers a moving garbage collection. The receiver is now rooted across the coercion and its payload borrowed only afterwards, so a relocated receiver repeats correct bytes instead of from-space garbage. `padStart`/`padEnd` were audited for the same window and are unaffected — codegen runs both of their coercions before the receiver handle is re-read.