diff --git a/changelog.d/7950-string-coerce-rooting-tail.md b/changelog.d/7950-string-coerce-rooting-tail.md new file mode 100644 index 0000000000..28fa8f73e8 --- /dev/null +++ b/changelog.d/7950-string-coerce-rooting-tail.md @@ -0,0 +1,17 @@ +### Fixed + +- **Finished the plain `ToString` operand-rooting audit from #6949.** The + remaining `String.prototype.split`, `RegExp.prototype.compile`, rebound + `RegExp` constructor, and patched typed-array `toLocaleString` paths now keep + their raw receivers and arguments in runtime handles across coercions, + callbacks, and result allocations. Each path re-reads the post-collection + address before using it, so an evacuating minor cannot leave the operation + reading or writing a forwarding stub. + +- **Closed adjacent allocation windows in string splitting and RegExp argument + coercion.** `js_string_split_n` now roots its source before allocating its + result array (rather than after), and a rebound RegExp's flags value remains + rooted while its pattern is coerced. + + The distinct raw-JSValues-in-Rust-containers family noted by #6949 is tracked + separately in #7949. diff --git a/crates/perry-runtime/src/object/class_registry/construct.rs b/crates/perry-runtime/src/object/class_registry/construct.rs index c0e0524c48..65434e5b14 100644 --- a/crates/perry-runtime/src/object/class_registry/construct.rs +++ b/crates/perry-runtime/src/object/class_registry/construct.rs @@ -632,16 +632,29 @@ pub unsafe extern "C" fn js_new_function_construct( // #2889: `new (rebound RegExp)(pattern, flags)`. #[cfg(feature = "regex-engine")] "RegExp" => { - let pattern = if args.is_empty() { - std::ptr::null_mut() - } else { - crate::builtins::js_string_coerce(args[0]) - }; - let flags = if args.len() < 2 || args[1].to_bits() == crate::value::TAG_UNDEFINED { - std::ptr::null_mut() + let flags_value = if args.len() < 2 { + f64::from_bits(crate::value::TAG_UNDEFINED) } else { - crate::builtins::js_string_coerce(args[1]) + args[1] }; + let scope = crate::gc::RuntimeHandleScope::new(); + let flags_value_handle = scope.root_nanbox_f64(flags_value); + let (pattern, flags_value) = flags_value_handle.across_nanbox(|| { + if args.is_empty() { + std::ptr::null_mut() + } else { + crate::builtins::js_string_coerce(args[0]) + } + }); + let pattern_handle = scope.root_string_ptr(pattern); + let (flags, pattern) = + pattern_handle.across_const::(|| { + if flags_value.to_bits() == crate::value::TAG_UNDEFINED { + std::ptr::null_mut() + } else { + crate::builtins::js_string_coerce(flags_value) + } + }); let re = crate::regex::js_regexp_new(pattern, flags); return crate::value::js_nanbox_pointer(re as i64); } diff --git a/crates/perry-runtime/src/object/native_call_method/typed_array.rs b/crates/perry-runtime/src/object/native_call_method/typed_array.rs index 9b11ad468f..b1337b663c 100644 --- a/crates/perry-runtime/src/object/native_call_method/typed_array.rs +++ b/crates/perry-runtime/src/object/native_call_method/typed_array.rs @@ -226,6 +226,8 @@ pub(crate) unsafe fn dispatch_typed_array_method( // through ordinary ToString, running `toString`/`valueOf` and // propagating abrupt completions). "toLocaleString" => { + let scope = crate::gc::RuntimeHandleScope::new(); + let ta_handle = scope.root_raw_mut_ptr(ta); let kind = crate::typedarray::lookup_typed_array_kind(ta as usize); let is_bigint = matches!( kind, @@ -234,11 +236,15 @@ pub(crate) unsafe fn dispatch_typed_array_method( let builtin: &[u8] = if is_bigint { b"BigInt" } else { b"Number" }; // #5901: no primitive receiver here — the element values are // formatted individually below, so pass `undefined`. - match builtin_proto_user_method( - builtin, - "toLocaleString", - f64::from_bits(crate::value::TAG_UNDEFINED), - ) { + let (patched, ta) = + ta_handle.across_mut::(|| { + builtin_proto_user_method( + builtin, + "toLocaleString", + f64::from_bits(crate::value::TAG_UNDEFINED), + ) + }); + match patched { None => { let s = crate::typedarray::js_typed_array_join_value( ta, @@ -247,16 +253,28 @@ pub(crate) unsafe fn dispatch_typed_array_method( f64::from_bits(JSValue::string_ptr(s).bits()) } Some(patched) => { + let patched_handle = scope.root_nanbox_u64(patched.bits()); let len = crate::typedarray::js_typed_array_length(ta); let mut out = String::new(); + let mut ta_now = ta; for k in 0..len { if k > 0 { out.push(','); } - let elem = crate::typedarray::js_typed_array_get(ta, k); - let r = call_primitive_closure_value(elem, patched, std::ptr::null(), 0) - .unwrap_or(f64::from_bits(crate::value::TAG_UNDEFINED)); - let s_hdr = crate::builtins::js_string_coerce(r); + let (elem, patched_now) = patched_handle + .across_nanbox(|| crate::typedarray::js_typed_array_get(ta_now, k)); + let patched_now = JSValue::from_bits(patched_now.to_bits()); + + let (r, _) = patched_handle.across_nanbox(|| { + call_primitive_closure_value(elem, patched_now, std::ptr::null(), 0) + .unwrap_or(f64::from_bits(crate::value::TAG_UNDEFINED)) + }); + + let (s_hdr, ta_after) = ta_handle + .across_mut::(|| { + crate::builtins::js_string_coerce(r) + }); + ta_now = ta_after; out.push_str( super::has_own_helpers::str_from_string_header(s_hdr).unwrap_or(""), ); diff --git a/crates/perry-runtime/src/regex/compile.rs b/crates/perry-runtime/src/regex/compile.rs index 443d6f9bae..548d2a01e8 100644 --- a/crates/perry-runtime/src/regex/compile.rs +++ b/crates/perry-runtime/src/regex/compile.rs @@ -31,6 +31,10 @@ pub extern "C" fn js_regexp_compile_value( if !is_valid_regex_ptr(re) { return f64::from_bits(crate::value::TAG_UNDEFINED); } + let scope = crate::gc::RuntimeHandleScope::new(); + let re_handle = scope.root_raw_mut_ptr(re); + let pattern_handle = scope.root_nanbox_f64(pattern_val); + let flags_value_handle = scope.root_nanbox_f64(flags_val); let pj = crate::value::JSValue::from_bits(pattern_val.to_bits()); let fj = crate::value::JSValue::from_bits(flags_val.to_bits()); @@ -43,13 +47,19 @@ pub extern "C" fn js_regexp_compile_value( ); } let src_re = pj.as_pointer::(); - let src = js_regexp_get_source(src_re); - let flg = js_regexp_get_flags(src_re); + let ((src, pattern_val), _) = re_handle.across_mut::(|| { + pattern_handle.across_nanbox(|| js_regexp_get_source(src_re)) + }); let src_s = if is_valid_ptr(src) { string_as_str(src).to_string() } else { String::new() }; + let src_re = + crate::value::JSValue::from_bits(pattern_val.to_bits()).as_pointer::(); + let ((flg, _), _) = re_handle.across_mut::(|| { + pattern_handle.across_nanbox(|| js_regexp_get_flags(src_re)) + }); let flg_s = if is_valid_ptr(flg) { string_as_str(flg).to_string() } else { @@ -61,22 +71,27 @@ pub extern "C" fn js_regexp_compile_value( // Abstract ToString (§7.1.17) rejects a Symbol with a `TypeError` — the // lenient `js_string_coerce` would otherwise stringify it to // "Symbol(desc)" (annexB `.../compile/{pattern,flags}-to-string-err`). - let pat = if pj.is_undefined() { - String::new() + let (pat, flags_val) = if pj.is_undefined() { + (String::new(), flags_val) } else { crate::builtins::reject_symbol_to_string(pattern_val); - let p = crate::builtins::js_string_coerce(pattern_val); - if is_valid_ptr(p) { + let ((p, flags_val), _) = re_handle.across_mut::(|| { + flags_value_handle.across_nanbox(|| crate::builtins::js_string_coerce(pattern_val)) + }); + let pat = if is_valid_ptr(p) { string_as_str(p).to_string() } else { String::new() - } + }; + (pat, flags_val) }; + let fj = crate::value::JSValue::from_bits(flags_val.to_bits()); let flg = if fj.is_undefined() { String::new() } else { crate::builtins::reject_symbol_to_string(flags_val); - let f = crate::builtins::js_string_coerce(flags_val); + let (f, _) = re_handle + .across_mut::(|| crate::builtins::js_string_coerce(flags_val)); if is_valid_ptr(f) { string_as_str(f).to_string() } else { @@ -146,8 +161,13 @@ pub extern "C" fn js_regexp_compile_value( None => std::ptr::null(), } }); - let canonical_flags_ptr = js_string_from_str(flags_str); - let pattern_ptr = js_string_from_str(pattern_str); + let (canonical_flags_ptr, _) = + re_handle.across_mut::(|| js_string_from_str(flags_str)); + let canonical_flags_handle = scope.root_string_ptr(canonical_flags_ptr); + let ((pattern_ptr, canonical_flags_ptr), re) = re_handle.across_mut::(|| { + canonical_flags_handle + .across_const::(|| js_string_from_str(pattern_str)) + }); unsafe { let old_regex_ptr = (*re).regex_ptr; let old_fancy_ptr = (*re).fancy_ptr; @@ -183,6 +203,7 @@ pub extern "C" fn js_regexp_compile_value( // (`Object.defineProperty(re, "lastIndex", { writable: false })`) makes this // a `TypeError` — but only *after* `.source`/`.flags` have already been // updated above (annexB `.../compile/pattern-regexp-immutable-lastindex`). - super::set_last_index_throwing(re, 0); + let ((), re) = + re_handle.across_mut::(|| super::set_last_index_throwing(re, 0)); f64::from_bits(crate::value::JSValue::pointer(re as *const u8).bits()) } diff --git a/crates/perry-runtime/src/string/split.rs b/crates/perry-runtime/src/string/split.rs index f9e920ad0e..d10c569e7a 100644 --- a/crates/perry-runtime/src/string/split.rs +++ b/crates/perry-runtime/src/string/split.rs @@ -360,6 +360,12 @@ pub extern "C" fn js_string_split_n( return crate::array::js_array_alloc(0); } + // The result-array and per-part string allocations below can evacuate the + // source. Root it before deriving offsets into its payload, then refresh + // its address after every allocation that precedes a read. + let scope = crate::gc::RuntimeHandleScope::new(); + let s_handle = scope.root_string_ptr(s); + let str_data = string_as_str(s); let delim = if !is_valid_string_ptr(delimiter) { "" @@ -424,9 +430,9 @@ pub extern "C" fn js_string_split_n( // and the array in a `RuntimeHandleScope`, re-read both after every // allocation, and store each part into the (rooted) array immediately — // from then on the array keeps it alive. - let arr = crate::array::js_array_alloc_pointer_elements(n as u32); - let scope = crate::gc::RuntimeHandleScope::new(); - let s_handle = scope.root_string_ptr(s); + let (arr, _) = s_handle.across_const::(|| { + crate::array::js_array_alloc_pointer_elements(n as u32) + }); let arr_handle = scope.root_raw_mut_ptr(arr); let mut i = 0usize; @@ -454,13 +460,15 @@ pub extern "C" fn js_string_split_n( // surrogate carved out of a WTF-8 source must keep its flag, or // `isWellFormed()` on the part wrongly reports true. let seq = &buf[..seq_len]; - let sh = if src_has_lone_surrogates && crate::string::bytes_have_lone_surrogate(seq) { - js_string_from_wtf8_bytes(seq.as_ptr(), seq_len as u32) - } else { - js_string_from_bytes(seq.as_ptr(), seq_len as u32) - }; + let (sh, arr_now) = arr_handle.across_mut::(|| { + if src_has_lone_surrogates && crate::string::bytes_have_lone_surrogate(seq) { + js_string_from_wtf8_bytes(seq.as_ptr(), seq_len as u32) + } else { + js_string_from_bytes(seq.as_ptr(), seq_len as u32) + } + }); unsafe { - store_split_string(arr_handle.get_raw_mut_ptr::(), idx, sh); + store_split_string(arr_now, idx, sh); } } return arr_handle.get_raw_mut_ptr::(); @@ -482,9 +490,9 @@ pub extern "C" fn js_string_split_n( } let n = part_ranges.len(); - let arr = crate::array::js_array_alloc_pointer_elements(n as u32); - let scope = crate::gc::RuntimeHandleScope::new(); - let s_handle = scope.root_string_ptr(s); + let (arr, _) = s_handle.across_const::(|| { + crate::array::js_array_alloc_pointer_elements(n as u32) + }); let arr_handle = scope.root_raw_mut_ptr(arr); unsafe { @@ -492,8 +500,10 @@ pub extern "C" fn js_string_split_n( let byte_len = byte_len_usize as u32; // Allocate the destination FIRST (it may move the source), then // re-read the source address before touching its bytes. - let (sh, data_ptr) = string_storage_alloc(byte_len); - let s_now = s_handle.get_raw_const_ptr::(); + let (((sh, data_ptr), s_now), arr_now) = + arr_handle.across_mut::(|| { + s_handle.across_const::(|| string_storage_alloc(byte_len)) + }); let part_ptr = string_data(s_now).add(offset); // Derive metadata from THIS PART's own bytes. The only shortcut // taken is the all-ASCII one, which was verified by scanning the @@ -515,7 +525,7 @@ pub extern "C" fn js_string_split_n( if byte_len > 0 { ptr::copy_nonoverlapping(part_ptr, data_ptr, byte_len as usize); } - store_split_string(arr_handle.get_raw_mut_ptr::(), i, sh); + store_split_string(arr_now, i, sh); } } @@ -538,7 +548,9 @@ fn split_limit_to_uint32(boxed: f64) -> u32 { fn split_single_element(s: *const StringHeader) -> *mut ArrayHeader { const STRING_TAG: u64 = 0x7FFF_0000_0000_0000; const POINTER_MASK: u64 = 0x0000_FFFF_FFFF_FFFF; - let arr = crate::array::js_array_alloc(1); + let scope = crate::gc::RuntimeHandleScope::new(); + let s_handle = scope.root_string_ptr(s); + let (arr, s) = s_handle.across_const::(|| crate::array::js_array_alloc(1)); unsafe { (*arr).length = 1; let elements_ptr = (arr as *mut u8).add(std::mem::size_of::()) as *mut f64; @@ -570,22 +582,30 @@ pub extern "C" fn js_string_split_value( use crate::value::JSValue; let sep_jv = JSValue::from_bits(separator.to_bits()); let lim_jv = JSValue::from_bits(limit.to_bits()); + let scope = crate::gc::RuntimeHandleScope::new(); + let s_handle = scope.root_string_ptr(s); + let separator_handle = scope.root_nanbox_f64(separator); // Step 2: a separator with a `[Symbol.split]` method (a RegExp) takes over. #[cfg(feature = "regex-engine")] if sep_jv.is_pointer() { let ptr = crate::value::js_nanbox_get_pointer(separator) as *const u8; if crate::regex::is_regex_pointer(ptr) { - let limit_i32 = if lim_jv.is_undefined() { - -1 - } else { - let u = split_limit_to_uint32(limit); - if u > i32::MAX as u32 { - i32::MAX - } else { - u as i32 - } - }; + let ((limit_i32, separator), s) = s_handle.across_const::(|| { + separator_handle.across_nanbox(|| { + if lim_jv.is_undefined() { + -1 + } else { + let u = split_limit_to_uint32(limit); + if u > i32::MAX as u32 { + i32::MAX + } else { + u as i32 + } + } + }) + }); + let ptr = crate::value::js_nanbox_get_pointer(separator) as *const u8; return crate::regex::js_string_split_regex_n( s, ptr as *const crate::regex::RegExpHeader, @@ -595,20 +615,24 @@ pub extern "C" fn js_string_split_value( } // Step 6: lim = limit===undefined ? 2^32-1 : ToUint32(limit) (may throw). - let lim: u32 = if lim_jv.is_undefined() { - u32::MAX - } else { - split_limit_to_uint32(limit) - }; + let ((lim, separator), s) = s_handle.across_const::(|| { + separator_handle.across_nanbox(|| { + if lim_jv.is_undefined() { + u32::MAX + } else { + split_limit_to_uint32(limit) + } + }) + }); // Step 7: R = ToString(separator) (may throw). For `undefined` the result // is unused (step 9) and `ToString(undefined)` is side-effect-free, so we // skip it. - let sep_is_undefined = sep_jv.is_undefined(); - let r_str: *mut StringHeader = if sep_is_undefined { - std::ptr::null_mut() + let sep_is_undefined = JSValue::from_bits(separator.to_bits()).is_undefined(); + let (r_str, s): (*mut StringHeader, *const StringHeader) = if sep_is_undefined { + (std::ptr::null_mut(), s) } else { - crate::builtins::js_string_coerce(separator) + s_handle.across_const::(|| crate::builtins::js_string_coerce(separator)) }; // Step 8: limit 0 → empty array.