fix(runtime): coerce lastIndex before borrowing the regex subject

changelog.d/8446-regexp-lastindex-rooting.md

@@ -0,0 +1,10 @@
+Fixed a moving-GC hazard in `RegExp.prototype.exec` and `String.prototype.matchAll`.
+RegExpBuiltinExec coerces `lastIndex` with `ToLength`, which runs user
+`valueOf`/`toString` when `lastIndex` holds an object — arbitrary JS that can
+run a copying minor. Both entry points captured the subject string (and, in
+`exec`, borrowed its inline payload) *before* that coercion, so a collection
+inside the callback left the match running over relocated-away bytes: the match
+silently evaporated or returned text from unrelated heap memory. The coercion
+now runs first with the regex header and subject rooted, and every later use
+reads the refreshed addresses — including the match array's `.input` property
+and the `lastIndex` write-back on the lookbehind/backreference path.

crates/perry-runtime/src/gc/tests/runtime_roots.rs

@@ -13,6 +13,7 @@ mod json_shape_template;
 mod native_module_name;
 mod old_defrag_contract;
 mod prototype_addr_cache;
+mod regexp_last_index;
 mod side_table_scanners;
 mod string_slice;
 mod symbol_description;

crates/perry-runtime/src/gc/tests/runtime_roots/regexp_last_index.rs

@@ -0,0 +1,128 @@
+//! Moving-GC regression for `RegExp.prototype.exec` (#8428).
+//!
+//! RegExpBuiltinExec step 4 is `ToLength(Get(R, "lastIndex"))`. When
+//! `lastIndex` holds an object the ToNumber half runs OrdinaryToPrimitive —
+//! user `valueOf`/`toString`, i.e. arbitrary JS, which reaches safepoints and
+//! can run a copying minor. `js_regexp_exec` borrowed the subject's inline
+//! WTF-8 payload (`string_as_str`) BEFORE that coercion, and rooting rewrites
+//! slots, never an already materialized `&str` — so the whole match ran over
+//! from-space bytes.
+//!
+//! The planted `valueOf` runs a copying minor and then refills the just
+//! retired Eden with a distinctive pattern, so a pre-fix run matches against
+//! the stomped bytes instead of the relocated subject and the match evaporates.
+//! Liveness is asserted both ways (a copying minor ran; the subject actually
+//! moved), per CLAUDE.md's "a gate must assert its subject was live".
+
+use super::super::super::*;
+use super::super::support::*;
+
+use crate::array::ArrayHeader;
+use crate::object::ObjectHeader;
+use crate::regex::RegExpHeader;
+use crate::string::StringHeader;
+
+/// Stand-in for `{ valueOf() { …allocating user JS…; return 0 } }`.
+extern "C" fn collect_then_zero(_closure: *const crate::closure::ClosureHeader) -> f64 {
+    crate::gc::gc_collect_minor();
+    // Recycle the just-retired from-space blocks back out as fresh strings, so
+    // a stale borrow reads THESE bytes rather than the subject's old ones.
+    let filler = [b'#'; 64];
+    for _ in 0..64 {
+        let _ = crate::string::js_string_from_bytes(filler.as_ptr(), filler.len() as u32);
+    }
+    0.0
+}
+
+fn heap_string(bytes: &[u8]) -> *mut StringHeader {
+    crate::string::js_string_from_bytes(bytes.as_ptr(), bytes.len() as u32)
+}
+
+fn capture_text(element: f64, index: usize) -> String {
+    let mut scratch = [0u8; crate::value::SHORT_STRING_MAX_LEN];
+    let (data, len) = crate::string::str_bytes_from_jsvalue(element, &mut scratch)
+        .unwrap_or_else(|| panic!("capture {index} must be a string"));
+    let bytes = unsafe { std::slice::from_raw_parts(data, len as usize) };
+    String::from_utf8_lossy(bytes).into_owned()
+}
+
+#[test]
+fn regexp_exec_survives_a_moving_minor_inside_the_lastindex_coercion() {
+    let _guard = CopyingNurseryTestGuard::new(0);
+    let _scan = ConservativeScanDisabledGuard::new();
+    let _triggers = GcTriggerThresholdTestGuard::suppress_automatic_triggers();
+    let _force = ForcedEvacuationTestGuard::on();
+    register_runtime_handle_root_scanner_for_tests();
+
+    let scope = crate::gc::RuntimeHandleScope::new();
+
+    // The subject must live in the movable nursery — a large string goes
+    // straight to the non-moving old generation and the window never opens.
+    let subject = heap_string(b"prefix-young-42-suffix");
+    assert!(crate::arena::pointer_in_nursery(subject as usize));
+    let s_handle = scope.root_string_ptr(subject);
+    let subject_before = subject as usize;
+
+    let re = crate::regex::js_regexp_new(heap_string(br"(young)-(\d+)"), heap_string(b"g"));
+    let re_handle = scope.root_raw_mut_ptr(re);
+
+    // `re.lastIndex = { valueOf() { … } }`.
+    let coercer = crate::object::js_object_alloc(0, 1);
+    let coercer_handle = scope.root_raw_mut_ptr(coercer);
+    let fp = collect_then_zero as *const u8;
+    crate::closure::js_register_closure_arity(fp, 0);
+    let value_of = crate::closure::js_closure_alloc_singleton(fp);
+    let value_of_value = crate::value::js_nanbox_pointer(value_of as i64);
+    let value_of_key = heap_string(b"valueOf");
+    coercer_handle.with_mut_ptr::<ObjectHeader, _>(|obj| {
+        crate::object::js_object_set_field_by_name(obj, value_of_key, value_of_value)
+    });
+    let coercer_value = coercer_handle
+        .with_mut_ptr::<ObjectHeader, _>(|obj| crate::value::js_nanbox_pointer(obj as i64));
+    re_handle.with_mut_ptr::<RegExpHeader, _>(|hdr| unsafe {
+        (*hdr).last_index = coercer_value.to_bits();
+    });
+
+    let cycles_before = copying_minor_cycles();
+    let matched = s_handle.with_const_ptr::<StringHeader, _>(|subject_now| {
+        re_handle.with_mut_ptr::<RegExpHeader, _>(|re_now| {
+            crate::regex::js_regexp_exec(re_now, subject_now)
+        })
+    });
+    let cycles_after = copying_minor_cycles();
+
+    assert!(
+        cycles_after > cycles_before,
+        "subject not live: the lastIndex coercion must run a copying minor \
+         (before={cycles_before}, after={cycles_after})"
+    );
+    let subject_after = s_handle.with_const_ptr::<StringHeader, _>(|p| p as usize);
+    assert_ne!(
+        subject_before, subject_after,
+        "subject not live: the collection must actually move the subject string"
+    );
+
+    // The discriminating check: pre-fix the match ran over the retired
+    // from-space copy of the subject, so `exec` returned null.
+    assert!(
+        !matched.is_null(),
+        r"/(young)-(\d+)/g must match the relocated subject"
+    );
+    let matched_handle = scope.root_raw_mut_ptr(matched);
+    for (index, expected) in ["young-42", "young", "42"].into_iter().enumerate() {
+        let element = matched_handle.with_mut_ptr::<ArrayHeader, _>(|arr| {
+            crate::array::js_array_get_f64(arr, index as u32)
+        });
+        assert_eq!(
+            capture_text(element, index),
+            expected,
+            "capture {index} read from-space bytes"
+        );
+    }
+    let last_index =
+        re_handle.with_const_ptr::<RegExpHeader, _>(crate::regex::regex_last_index_offset);
+    assert_eq!(
+        last_index, 15,
+        "lastIndex must advance past the relocated match"
+    );
+}

crates/perry-runtime/src/regex/exec.rs

@@ -33,22 +33,47 @@ pub extern "C" fn js_regexp_exec(
         return ptr::null_mut();
     }
 
+    // Spec RegExpBuiltinExec step 4 is `ToLength(Get(R, "lastIndex"))`, and it
+    // runs before anything else. The ToNumber half of that coercion executes
+    // USER JS whenever `lastIndex` is coercible (`re.lastIndex = { valueOf() {
+    // … } }` — test262 prototype/exec/{success,failure}-lastindex-access covers
+    // exactly this), and user JS reaches back-edge safepoint polls, so a moving
+    // minor can relocate BOTH arguments inside the window.
+    //
+    // Do the coercion FIRST, with `re` and `s` rooted, and take the subject's
+    // payload borrow only afterwards. `string_as_str` hands out a `&str` into
+    // the inline WTF-8 bytes; rooting rewrites *slots*, never an already
+    // materialized borrow, so a borrow taken ahead of this call reads from-space
+    // for the entire match (#8428, the `HeapKeyBytes` doc states the rule).
+    //
+    // Audited in the same pass: `set_last_index_throwing` does NOT reopen the
+    // window. It reads the property attributes and stores a number; its only
+    // allocation is on the non-writable arm, which throws and therefore never
+    // returns to the borrow.
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let re_handle = scope.root_raw_mut_ptr(re);
+    let s_handle = scope.root_string_ptr(s);
+    let ((last_index_read, re), s) = s_handle.across_const::<StringHeader, _>(|| {
+        re_handle
+            .across_mut::<RegExpHeader, _>(|| re_handle.with_const_ptr(regex_last_index_offset))
+    });
     let str_data = string_as_str(s);
 
     unsafe {
         let regex = &*(*re).regex_ptr;
         let global = (*re).global;
         let sticky = (*re).sticky;
+        let has_indices = (*re).has_indices;
         // Per spec RegExpBuiltinExec, `lastIndex` drives the search start for
         // BOTH global and sticky regexes (and lastIndex is reset/updated for
         // either). A sticky match must additionally *anchor* at lastIndex.
         let use_last_index = global || sticky;
-        // Spec RegExpBuiltinExec step 4 reads `lastIndex` (Get → ToLength) once,
-        // up front and *before* the global/sticky branch (step 8). So the read
-        // — and any `valueOf`/`toString` side effect of a coercible lastIndex —
-        // is observed exactly once even for a non-global/non-sticky regex
-        // (test262 prototype/exec/{success,failure}-lastindex-access).
-        let last_index_read = regex_last_index_offset(re);
+        // `last_index_read` was taken at the top of the function, before the
+        // borrow: spec step 4 reads `lastIndex` (Get → ToLength) once, up front
+        // and *before* the global/sticky branch (step 8), so the read — and any
+        // `valueOf`/`toString` side effect of a coercible lastIndex — is observed
+        // exactly once even for a non-global/non-sticky regex (test262
+        // prototype/exec/{success,failure}-lastindex-access).
         // Step 8: a non-global/non-sticky search always starts at 0; the value
         // read above only drives the search start for a stateful regex.
         let last_index = if use_last_index { last_index_read } else { 0 };
@@ -92,6 +117,16 @@ pub extern "C" fn js_regexp_exec(
                     let match_byte_offset = full.start() + search_start_byte;
                     let match_char_offset =
                         super::exec_array::byte_index_to_utf16_index(str_data, match_byte_offset);
+                    // Spec order (step 15 precedes the ArrayCreate of step 16),
+                    // and it keeps `re` out of the window opened by the capture
+                    // allocations below — the standard arm already does this.
+                    if use_last_index {
+                        let match_end_byte = full.end() + search_start_byte;
+                        set_last_index_throwing(
+                            re,
+                            super::exec_array::byte_index_to_utf16_index(str_data, match_end_byte),
+                        );
+                    }
                     let arr = crate::array::js_array_alloc(caps.len() as u32);
                     let scope = crate::gc::RuntimeHandleScope::new();
                     let arr_handle = scope.root_raw_mut_ptr(arr);
@@ -109,13 +144,6 @@ pub extern "C" fn js_regexp_exec(
                             crate::array::store_array_slot(arr, i, undefined.to_bits());
                         }
                     }
-                    if use_last_index {
-                        let match_end_byte = full.end() + search_start_byte;
-                        set_last_index_throwing(
-                            re,
-                            super::exec_array::byte_index_to_utf16_index(str_data, match_end_byte),
-                        );
-                    }
                     set_exec_array_metadata(
                         arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
                         str_data,
@@ -129,7 +157,7 @@ pub extern "C" fn js_regexp_exec(
                     LAST_EXEC_GROUPS.with(|g| *g.borrow_mut() = groups_obj);
                     set_exec_array_groups(arr_handle.get_raw_mut_ptr::<ArrayHeader>(), groups_obj);
                     // Build indices array if `d` flag (hasIndices) is set
-                    if (*re).has_indices {
+                    if has_indices {
                         set_exec_array_indices_fancy(
                             arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
                             str_data,
@@ -243,24 +271,34 @@ pub extern "C" fn js_regexp_exec(
                         *g.borrow_mut() =
                             groups_handle.get_raw_mut_ptr::<crate::object::ObjectHeader>()
                     });
-                    super::exec_array::set_exec_array_metadata_groups_fresh(
-                        arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
-                        s,
-                        match_char_offset as f64,
-                        groups_handle.get_raw_mut_ptr::<crate::object::ObjectHeader>(),
-                    );
+                    // `.input` re-boxes the subject, so it must be the CURRENT
+                    // address: the capture/groups allocations above can have
+                    // moved it since `s` was refreshed. The helper itself
+                    // performs no GC allocation, which is what makes the scoped
+                    // `with_const_ptr` argument shape sound here.
+                    s_handle.with_const_ptr::<StringHeader, _>(|s_now| {
+                        super::exec_array::set_exec_array_metadata_groups_fresh(
+                            arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
+                            s_now,
+                            match_char_offset as f64,
+                            groups_handle.get_raw_mut_ptr::<crate::object::ObjectHeader>(),
+                        )
+                    });
                 } else {
                     LAST_EXEC_GROUPS.with(|g| *g.borrow_mut() = ptr::null_mut());
-                    super::exec_array::set_exec_array_metadata_groups_fresh(
-                        arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
-                        s,
-                        match_char_offset as f64,
-                        ptr::null_mut(),
-                    );
+                    // Current subject address — see the named-groups arm above.
+                    s_handle.with_const_ptr::<StringHeader, _>(|s_now| {
+                        super::exec_array::set_exec_array_metadata_groups_fresh(
+                            arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
+                            s_now,
+                            match_char_offset as f64,
+                            ptr::null_mut(),
+                        )
+                    });
                 }
 
                 // Build indices array if `d` flag (hasIndices) is set
-                if (*re).has_indices {
+                if has_indices {
                     set_exec_array_indices(
                         arr_handle.get_raw_mut_ptr::<ArrayHeader>(),
                         str_data,

crates/perry-runtime/src/regex/match_all.rs

@@ -236,18 +236,24 @@ pub extern "C" fn js_string_match_all_value(
     } else {
         0
     };
-    let (re, start_index) = if raw != 0 && is_valid_regex_ptr(raw as *const RegExpHeader) {
+    let (start_index, re) = if raw != 0 && is_valid_regex_ptr(raw as *const RegExpHeader) {
         let re = raw as *const RegExpHeader;
         unsafe {
             if !(*re).global {
                 throw_match_all_non_global_regex();
             }
-            (re, crate::regex::regex_last_index_offset(re))
+            // The ToLength coercion inside `regex_last_index_offset` runs user
+            // `valueOf`/`toString`, so it can move the regex header too — not
+            // just the subject `s_handle` already covers (#8428).
+            let re_handle = scope.root_raw_const_ptr(re);
+            re_handle.across_const::<RegExpHeader, _>(|| {
+                re_handle.with_const_ptr(crate::regex::regex_last_index_offset)
+            })
         }
     } else {
         (
-            match_all_pattern_to_regex(pattern_value) as *const RegExpHeader,
             0,
+            match_all_pattern_to_regex(pattern_value) as *const RegExpHeader,
         )
     };
 
@@ -275,8 +281,21 @@ pub extern "C" fn js_string_match_all(
         if !(*re).global {
             throw_match_all_non_global_regex();
         }
-        let matches =
-            materialize_match_all_results(s, re, crate::regex::regex_last_index_offset(re));
+        // `regex_last_index_offset` ToLength-coerces `lastIndex`, which runs
+        // user `valueOf`/`toString` and can therefore move both arguments
+        // (#8428). Rust evaluates call arguments left to right, so the previous
+        // shape passed the PRE-coercion `s` — a from-space subject that
+        // `materialize_match_all_results` then rooted and snapshotted. Root
+        // first, coerce, then hand over the refreshed addresses.
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let s_handle = scope.root_string_ptr(s);
+        let re_handle = scope.root_raw_const_ptr(re);
+        let ((start_index, re), s) = s_handle.across_const::<StringHeader, _>(|| {
+            re_handle.across_const::<RegExpHeader, _>(|| {
+                re_handle.with_const_ptr(crate::regex::regex_last_index_offset)
+            })
+        });
+        let matches = materialize_match_all_results(s, re, start_index);
         alloc_regexp_string_iterator(matches)
     }
 }