fix(runtime): root repeat's receiver across the count coercion

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.

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::<StringHeader, _>(|| 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);
     }
 

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);