diff --git a/changelog.d/7451-string-reflective-arg-coercion.md b/changelog.d/7451-string-reflective-arg-coercion.md new file mode 100644 index 0000000000..1dcb67be98 --- /dev/null +++ b/changelog.d/7451-string-reflective-arg-coercion.md @@ -0,0 +1 @@ +fix(runtime,hir): reflective `String.prototype` methods now apply the observable spec coercions to their arguments, in spec order (#5902). `dispatch_string`'s `replace`/`replaceAll` arm extracted only already-string arguments — a number pattern was a silent no-op, a `{valueOf(){throw}}` pattern never threw — and now runs `ToString(searchValue)` before `ToString(replaceValue)` (§22.1.3.19/20) with RegExp patterns and callable replacements classified first, un-coerced. `padStart`/`padEnd` read the raw NaN-boxed maxLength (any object → NaN → target 0, its `valueOf` never invoked) and coerced the fill first; now `ToNumber(maxLength)` runs first with the spec early-return before `ToString(fillString)`. `lastIndexOf` gets a real `ToNumber(position)`, and every `indexOf`-family sub-arm re-extracts its needle/receiver pointers from their GC roots only after the observable coercions (a rooted-pointer-goes-stale hazard). `call_replace_callback` ToString-coerces the replacer-function result instead of dropping every non-string return to `""` (`undefined` → `"undefined"`). On the HIR side, `is_primitive_wrapper_brand_method` gains the `String` arm so `String.prototype.toString`/`valueOf` `.call(x)` (direct or via an extracted local) stays reflective and reaches the `thisStringValue` brand thunks instead of folding to `x.toString()` — Number/Boolean have been guarded since #4100. test262 `built-ins/String`: pass 903→916, runtime-fail 29→16 (13 flips, zero regressions), parity 96.9%→98.3%. New per-PR coverage: 6 behavioral unit tests on `js_native_call_method` plus a `perry-hir` fold-predicate test. diff --git a/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs b/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs index 9ab40b4bd3..d6ec74993f 100644 --- a/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs +++ b/crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs @@ -319,6 +319,13 @@ fn is_primitive_wrapper_brand_method(recv: &ast::Expr, method: &str) -> bool { match base.sym.as_ref() { "Number" => matches!(method, "valueOf" | "toString" | "toLocaleString"), "Boolean" => matches!(method, "valueOf" | "toString"), + // `String.prototype.toString`/`valueOf` are `thisStringValue` brand + // checks (ECMA-262 §22.1.3), NOT ToString-coercing like the rest of + // `String.prototype` — the installed thunks throw a `TypeError` for a + // non-String receiver, so `String.prototype.toString.call(42)` (and + // the extracted-local form) must stay reflective (test262 + // built-ins/String/prototype/{toString,valueOf}/non-generic.js). + "String" => matches!(method, "valueOf" | "toString"), _ => false, } } @@ -785,3 +792,46 @@ fn is_builtin_prototype_receiver(ctx: &LoweringContext, recv: &ast::Expr) -> boo _ => false, } } + +#[cfg(test)] +mod tests { + use super::*; + use swc_common::DUMMY_SP; + + fn prototype_member(base: &str) -> ast::Expr { + ast::Expr::Member(ast::MemberExpr { + span: DUMMY_SP, + obj: Box::new(ast::Expr::Ident(ast::Ident::new( + base.into(), + DUMMY_SP, + Default::default(), + ))), + prop: ast::MemberProp::Ident(ast::IdentName { + span: DUMMY_SP, + sym: "prototype".into(), + }), + }) + } + + /// #5902: `String.prototype.toString`/`valueOf` are `thisStringValue` + /// brand checks — the `.call`/`.apply` fold (and the extracted-local + /// tracking in `as_builtin_proto_method_ref`, which consults the same + /// predicate) must leave them reflective so the runtime thunk can throw + /// a TypeError on a non-String receiver (test262 non-generic.js). + /// Deleting the `"String"` arm from `is_primitive_wrapper_brand_method` + /// turns this red. + #[test] + fn string_to_string_and_value_of_are_brand_methods() { + let recv = prototype_member("String"); + assert!(is_primitive_wrapper_brand_method(&recv, "toString")); + assert!(is_primitive_wrapper_brand_method(&recv, "valueOf")); + // The ToString-coercing generics are guarded by the sibling + // `is_string_prototype_generic_method` predicate, not the brand one — + // and the brand pair must stay OUT of the generic list (a generic + // thunk would coerce instead of throwing). + assert!(!is_primitive_wrapper_brand_method(&recv, "charAt")); + assert!(is_string_prototype_generic_method(&recv, "charAt")); + assert!(!is_string_prototype_generic_method(&recv, "toString")); + assert!(!is_string_prototype_generic_method(&recv, "valueOf")); + } +} diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 2f6b9b50ef..4862ecf315 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -16,6 +16,9 @@ mod object_proto; mod primitive_methods; mod proto_dispatch; mod string_methods; + +#[cfg(test)] +mod dispatch_arg_coercion_tests; mod typed_array; use disposal::{ @@ -571,21 +574,6 @@ pub unsafe extern "C" fn js_native_call_method_value_apply( js_native_call_method_value(object, key, args_ptr, args_len) } -#[inline] -fn root_string_arg_handle<'scope>( - scope: &'scope crate::gc::RuntimeHandleScope, - arg_handles: &[crate::gc::RuntimeHandle<'scope>], - index: usize, -) -> Option> { - let value = arg_handles.get(index)?.get_nanbox_f64(); - let ptr = crate::value::js_get_string_pointer_unified(value) as *const crate::StringHeader; - if ptr.is_null() { - None - } else { - Some(scope.root_string_ptr(ptr)) - } -} - fn throw_type_error_message(message: &[u8]) -> ! { let msg = crate::string::js_string_from_bytes(message.as_ptr(), message.len() as u32); let err = crate::error::js_typeerror_new(msg); diff --git a/crates/perry-runtime/src/object/native_call_method/dispatch_arg_coercion_tests.rs b/crates/perry-runtime/src/object/native_call_method/dispatch_arg_coercion_tests.rs new file mode 100644 index 0000000000..e827e90e19 --- /dev/null +++ b/crates/perry-runtime/src/object/native_call_method/dispatch_arg_coercion_tests.rs @@ -0,0 +1,128 @@ +//! Argument-coercion contract for `dispatch_string` (#5902): the reflective / +//! any-typed String-method dispatch must apply the observable spec coercions +//! (`ToString` on `replace`/`replaceAll` pattern+replacement, `ToLength` on +//! `padStart`/`padEnd` maxLength) to its arguments instead of silently +//! degrading non-string / non-number values. Every case here fails on the +//! pre-fix shape: `replace` extracted only already-string args (a number +//! pattern became a null pointer → receiver returned unchanged) and the pad +//! arms passed the raw NaN-boxed arg through (a string/object maxLength read +//! as NaN → target 0 → no padding). The user-code side of the same contract +//! (a `{ valueOf }` maxLength must execute, in spec order, before the fill +//! coercion) needs a JS closure and is covered by test262 +//! `built-ins/String/prototype/{replace,padStart,padEnd}` via the parity +//! sweep, not here. + +use crate::value::JSValue; + +unsafe fn call_string_method(receiver: &str, method: &str, args: &[f64]) -> f64 { + let s = crate::string::js_string_from_bytes(receiver.as_ptr(), receiver.len() as u32); + let recv = f64::from_bits(JSValue::string_ptr(s).bits()); + super::js_native_call_method( + recv, + method.as_ptr() as *const i8, + method.len(), + if args.is_empty() { + std::ptr::null() + } else { + args.as_ptr() + }, + args.len(), + ) +} + +unsafe fn assert_string_result(result: f64, expected: &str, label: &str) { + let v = JSValue::from_bits(result.to_bits()); + if v.is_string() { + let s = crate::object::has_own_helpers::str_from_string_header(v.as_string_ptr()) + .unwrap_or_default(); + assert_eq!(s, expected, "{label}"); + } else if v.is_short_string() { + let mut buf = [0u8; crate::value::SHORT_STRING_MAX_LEN]; + let n = v.short_string_to_buf(&mut buf); + assert_eq!(std::str::from_utf8(&buf[..n]).unwrap(), expected, "{label}"); + } else { + panic!( + "{label}: expected a string result, got bits {:#x}", + v.bits() + ); + } +} + +fn short(bytes: &[u8]) -> f64 { + f64::from_bits(JSValue::try_short_string(bytes).unwrap().bits()) +} + +#[test] +fn replace_coerces_a_non_string_pattern_via_to_string() { + // "a1b".replace(1, "X") — ToString(1) === "1" → "aXb". + unsafe { + let r = call_string_method("a1b", "replace", &[1.0, short(b"X")]); + assert_string_result(r, "aXb", "\"a1b\".replace(1, \"X\")"); + } +} + +#[test] +fn replace_all_coerces_a_non_string_pattern_via_to_string() { + // "1a1".replaceAll(1, "X") — ToString(1) === "1" → "XaX". + unsafe { + let r = call_string_method("1a1", "replaceAll", &[1.0, short(b"X")]); + assert_string_result(r, "XaX", "\"1a1\".replaceAll(1, \"X\")"); + } +} + +#[test] +fn replace_coerces_a_missing_replacement_to_undefined() { + // "ab".replace("b") — §22.1.3.19 runs ToString(replaceValue) even for an + // absent arg: ToString(undefined) === "undefined" → "aundefined" (Node + // agrees). The old shape substituted an empty replacement. + unsafe { + let r = call_string_method("ab", "replace", &[short(b"b")]); + assert_string_result(r, "aundefined", "\"ab\".replace(\"b\")"); + } +} + +#[test] +fn pad_end_coerces_a_string_target_length_via_to_number() { + // "abc".padEnd("11", "def") — ToLength(ToNumber("11")) === 11 → + // "abcdefdefde". + unsafe { + let r = call_string_method("abc", "padEnd", &[short(b"11"), short(b"def")]); + assert_string_result(r, "abcdefdefde", "\"abc\".padEnd(\"11\", \"def\")"); + } +} + +#[test] +fn pad_start_coerces_target_and_defaults_the_fill() { + // "5".padStart("3") — string target coerces to 3, absent fill defaults + // to " " → " 5". + unsafe { + let r = call_string_method("5", "padStart", &[short(b"3")]); + assert_string_result(r, " 5", "\"5\".padStart(\"3\")"); + } +} + +extern "C" fn undef_replacer( + _closure: *const crate::closure::ClosureHeader, + _matched: f64, + _offset: f64, + _whole: f64, +) -> f64 { + f64::from_bits(JSValue::undefined().bits()) +} + +#[test] +fn replace_fn_result_is_to_string_coerced() { + // "gnulluna".replace("null", () => undefined) — the callback result is + // ToString'd (§22.1.3.19): undefined renders as "undefined" → + // "gundefineduna" (test262 S15.5.4.11_A1_T5's shape; the old + // `call_replace_callback` dropped every non-string result to ""). + unsafe { + let func_ptr = undef_replacer as *const u8; + let closure = crate::closure::js_closure_alloc(func_ptr, 0); + assert!(!closure.is_null()); + crate::closure::js_register_closure_arity(func_ptr, 3); + let cb = crate::value::js_nanbox_pointer(closure as i64); + let r = call_string_method("gnulluna", "replace", &[short(b"null"), cb]); + assert_string_result(r, "gundefineduna", "\"gnulluna\".replace(\"null\", fn)"); + } +} diff --git a/crates/perry-runtime/src/object/native_call_method/string_methods.rs b/crates/perry-runtime/src/object/native_call_method/string_methods.rs index 2462f0f6bf..c0e0704704 100644 --- a/crates/perry-runtime/src/object/native_call_method/string_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/string_methods.rs @@ -260,6 +260,39 @@ pub(super) unsafe fn dispatch_string( } else { Some(root_scope.root_string_ptr(needle_raw)) }; + if needle_h.is_none() { + // Match Node: `s.indexOf(undefined)` → -1, includes → false. + return Some(match method_name { + "indexOf" | "lastIndexOf" => -1.0_f64, + "includes" | "startsWith" | "endsWith" => { + f64::from_bits(JSValue::bool(false).bits()) + } + _ => f64::from_bits(JSValue::undefined().bits()), + }); + } + // ToNumber(position) — the second observable coercion, AFTER + // ToString(searchString) (§22.1.3.9/§22.1.3.11 step order): a + // `{ valueOf }` position runs user code and its throw + // propagates (test262 lastIndexOf S15.5.4.8_A4_T2 — + // previously `lastIndexOf` passed the raw NaN-boxed value, + // which read as NaN and silently meant "search from the + // end"). `arg_i32` routes through the same `ToNumber`; + // `lastIndexOf` keeps the raw f64 because its NaN → end / + // clamping semantics live in the `_from` helper. + let pos_num = match method_name { + "lastIndexOf" if args_len >= 2 => Some(crate::builtins::js_number_coerce( + arg_at(1) + .unwrap_or_else(|| f64::from_bits(JSValue::undefined().bits())), + )), + "indexOf" | "includes" | "startsWith" if args_len >= 2 => { + Some(arg_i32(1) as f64) + } + "endsWith" if args_len >= 2 => Some(arg_i32(1) as f64), + _ => None, + }; + // Both coercions above can run user code and move either + // string under GC — extract the raw pointers only now, from + // their roots. let needle = needle_h .as_ref() .map(|h| h.get_raw_const_ptr::()) @@ -276,43 +309,33 @@ pub(super) unsafe fn dispatch_string( // checks both unbox these tags correctly (and Node's // `Array.prototype.includes` etc. on plain values // already use this representation). - if needle.is_null() { - // Match Node: `s.indexOf(undefined)` → -1, includes → false. - return Some(match method_name { - "indexOf" | "lastIndexOf" => -1.0_f64, - "includes" | "startsWith" | "endsWith" => { - f64::from_bits(JSValue::bool(false).bits()) - } - _ => f64::from_bits(JSValue::undefined().bits()), - }); - } return Some(match method_name { "indexOf" => { - let from = if args_len >= 2 { arg_i32(1) } else { 0 }; + let from = pos_num.unwrap_or(0.0) as i32; crate::string::js_string_index_of_from(s_ptr, needle, from) as f64 } "includes" => { - let from = if args_len >= 2 { arg_i32(1) } else { 0 }; + let from = pos_num.unwrap_or(0.0) as i32; let i = crate::string::js_string_index_of_from(s_ptr, needle, from); f64::from_bits(JSValue::bool(i >= 0).bits()) } - "lastIndexOf" => { - if args_len >= 2 { - let pos = unsafe { *args_ptr.add(1) }; + "lastIndexOf" => match pos_num { + Some(pos) => { crate::string::js_string_last_index_of_from(s_ptr, needle, pos, 1) as f64 - } else { - crate::string::js_string_last_index_of(s_ptr, needle) as f64 } - } + None => crate::string::js_string_last_index_of(s_ptr, needle) as f64, + }, "startsWith" => { - let at = if args_len >= 2 { arg_i32(1) } else { 0 }; + let at = pos_num.unwrap_or(0.0) as i32; let b = crate::string::js_string_starts_with_at(s_ptr, needle, at); f64::from_bits(JSValue::bool(b != 0).bits()) } "endsWith" => { - let len_i32 = unsafe { (*s_ptr).byte_len } as i32; - let at = if args_len >= 2 { arg_i32(1) } else { len_i32 }; + let at = match pos_num { + Some(p) => p as i32, + None => (unsafe { (*s_ptr).byte_len }) as i32, + }; let b = crate::string::js_string_ends_with_at(s_ptr, needle, at); f64::from_bits(JSValue::bool(b != 0).bits()) } @@ -459,8 +482,60 @@ pub(super) unsafe fn dispatch_string( // Function replacements route to the callback helpers so // `str.replace(x, fn)` observes Node's callback argument // shape and receiver binding. - let pat_handle = root_string_arg_handle(&root_scope, &arg_handles, 0); - let repl_handle = root_string_arg_handle(&root_scope, &arg_handles, 1); + let undefined = f64::from_bits(JSValue::undefined().bits()); + // Classify BEFORE coercing: a RegExp pattern routes to the + // regex engine un-coerced, and a callable replacement must + // not be ToString'd (§22.1.3.19 checks IsCallable first). + #[cfg(feature = "regex-engine")] + let pat_is_regex = { + let jsv = JSValue::from_bits(arg_at(0).unwrap_or(undefined).to_bits()); + jsv.is_pointer() && { + let p = jsv.as_pointer::(); + !p.is_null() && crate::regex::is_regex_pointer(p) + } + }; + #[cfg(not(feature = "regex-engine"))] + let pat_is_regex = false; + let repl_is_fn = { + let v = arg_at(1).unwrap_or(undefined); + JSValue::from_bits(v.to_bits()).is_pointer() + && crate::closure::is_closure_ptr( + (v.to_bits() & 0x0000_FFFF_FFFF_FFFF) as usize, + ) + }; + // §22.1.3.19/20 argument order: ToString(searchValue) runs + // observably — a user `toString`/`valueOf` executes and its + // throw propagates, a Symbol throws TypeError — and BEFORE + // ToString(replaceValue). The previous shape extracted only + // already-string args (`root_string_arg_handle`), silently + // degrading any other pattern/replacement to a null pointer + // (`new String(...).replace({valueOf(){throw ...}}, "x")` + // returned the receiver unchanged instead of throwing — + // test262 S15.5.4.11_A1_T12/T15/T16). + let pat_handle = if pat_is_regex { + None + } else { + let v = arg_at(0).unwrap_or(undefined); + crate::builtins::reject_symbol_to_string(v); + let raw = crate::value::js_jsvalue_to_string(v); + if raw.is_null() { + None + } else { + Some(root_scope.root_string_ptr(raw)) + } + }; + let repl_handle = if repl_is_fn { + None + } else { + let v = arg_at(1).unwrap_or(undefined); + crate::builtins::reject_symbol_to_string(v); + let raw = crate::value::js_jsvalue_to_string(v); + if raw.is_null() { + None + } else { + Some(root_scope.root_string_ptr(raw)) + } + }; let pat_str = || { pat_handle .as_ref() @@ -473,80 +548,64 @@ pub(super) unsafe fn dispatch_string( .map(|handle| handle.get_raw_const_ptr::()) .unwrap_or(std::ptr::null()) }; - if let (Some(pat_val), Some(repl_val)) = (arg_at(0), arg_at(1)) { - // `pat_jsv` is only consulted by the regex-engine-gated - // branch below (RegExp pattern + callback replacer). - #[cfg_attr(not(feature = "regex-engine"), allow(unused_variables))] - let pat_jsv = JSValue::from_bits(pat_val.to_bits()); - let repl_jsv = JSValue::from_bits(repl_val.to_bits()); - if repl_jsv.is_pointer() { - let repl_raw = (repl_val.to_bits() & 0x0000_FFFF_FFFF_FFFF) as usize; - if crate::closure::is_closure_ptr(repl_raw) { - #[cfg(feature = "regex-engine")] - if pat_jsv.is_pointer() { - let regex_ptr = - pat_jsv.as_pointer::(); - if !regex_ptr.is_null() - && crate::regex::is_regex_pointer(regex_ptr as *const u8) - { - let r = if method_name == "replaceAll" { - crate::regex::js_string_replace_all_regex_fn( - receiver_string(), - regex_ptr, - repl_val, - ) - } else { - crate::regex::js_string_replace_regex_fn( - receiver_string(), - regex_ptr, - repl_val, - ) - }; - return Some(f64::from_bits(JSValue::string_ptr(r).bits())); - } - } - let r = if method_name == "replaceAll" { - crate::regex::js_string_replace_all_string_fn( - receiver_string(), - pat_str(), - repl_val, - ) - } else { - crate::regex::js_string_replace_string_fn( - receiver_string(), - pat_str(), - repl_val, - ) - }; - return Some(f64::from_bits(JSValue::string_ptr(r).bits())); - } + if repl_is_fn { + // Re-read through the arg handles: the pattern coercion + // above may have run user code and moved the closure. + let repl_val = arg_at(1).unwrap_or(undefined); + #[cfg(feature = "regex-engine")] + if pat_is_regex { + let pat_val = arg_at(0).unwrap_or(undefined); + let regex_ptr = JSValue::from_bits(pat_val.to_bits()) + .as_pointer::(); + let r = if method_name == "replaceAll" { + crate::regex::js_string_replace_all_regex_fn( + receiver_string(), + regex_ptr, + repl_val, + ) + } else { + crate::regex::js_string_replace_regex_fn( + receiver_string(), + regex_ptr, + repl_val, + ) + }; + return Some(f64::from_bits(JSValue::string_ptr(r).bits())); } + let r = if method_name == "replaceAll" { + crate::regex::js_string_replace_all_string_fn( + receiver_string(), + pat_str(), + repl_val, + ) + } else { + crate::regex::js_string_replace_string_fn( + receiver_string(), + pat_str(), + repl_val, + ) + }; + return Some(f64::from_bits(JSValue::string_ptr(r).bits())); } - // Detect RegExp pattern: NaN-boxed pointer to a RegExpHeader. #[cfg(feature = "regex-engine")] - if let Some(v) = arg_at(0) { - let jsv = JSValue::from_bits(v.to_bits()); - if jsv.is_pointer() { - let regex_ptr = jsv.as_pointer::(); - if !regex_ptr.is_null() - && crate::regex::is_regex_pointer(regex_ptr as *const u8) - { - let r = if method_name == "replaceAll" { - crate::regex::js_string_replace_all_regex( - receiver_string(), - regex_ptr, - repl_str(), - ) - } else { - crate::regex::js_string_replace_regex( - receiver_string(), - regex_ptr, - repl_str(), - ) - }; - return Some(f64::from_bits(JSValue::string_ptr(r).bits())); - } - } + if pat_is_regex { + let pat_val = arg_at(0).unwrap_or(undefined); + let regex_ptr = JSValue::from_bits(pat_val.to_bits()) + .as_pointer::(); + let r = if method_name == "replaceAll" { + crate::regex::js_string_replace_all_regex( + receiver_string(), + regex_ptr, + repl_str(), + ) + } else { + crate::regex::js_string_replace_regex( + receiver_string(), + regex_ptr, + repl_str(), + ) + }; + return Some(f64::from_bits(JSValue::string_ptr(r).bits())); } let r = if method_name == "replaceAll" { crate::regex::js_string_replace_all_string( @@ -568,17 +627,44 @@ pub(super) unsafe fn dispatch_string( // call(boxed, …)`, routed through `string_proto_thunks` after // coercing `this` to a string) and `(s: any).padStart(…)` dynamic // dispatch resolve to the runtime helper instead of the TypeError - // catch-all. Argument coercion mirrors `lower_string_method.rs`. + // catch-all. "padStart" | "padEnd" => { - let target_len = arg_at(0).unwrap_or(0.0); - // ToString(fillString) when present and not undefined; absent / - // undefined leaves a null ptr so the helper defaults to " ". - let pad = match arg_at(1) { - Some(v) if !JSValue::from_bits(v.to_bits()).is_undefined() => { - crate::builtins::js_string_coerce(v) as *const crate::StringHeader + let undefined = f64::from_bits(JSValue::undefined().bits()); + // §22.1.3.17 StringPaddingBuiltinsImpl operation order: + // `ToLength(maxLength)` runs FIRST and observably — a + // `{ valueOf }` maxLength executes user code (whose throw + // propagates), a Symbol throws TypeError. Passing the raw + // NaN-boxed arg straight to the helper read any object as + // NaN → target 0 → receiver returned unpadded with its + // `valueOf` never invoked, and coerced the fill string + // BEFORE the length (test262 padStart/padEnd + // observable-operations.js). + let target_len = + crate::builtins::js_number_coerce(arg_at(0).unwrap_or(undefined)); + // Spec step order again: when no padding is needed + // (intMaxLength ≤ receiver length) the method returns + // before `ToString(fillString)`, so a fill-side throw must + // not fire. Mirror the helper's `to_length` clamping + // (NaN/negative → 0, fractions truncate toward zero). + let cur_len = unsafe { (*receiver_string()).utf16_len } as f64; + let pad_handle = if target_len.is_nan() || target_len.trunc() <= cur_len { + None + } else { + // ToString(fillString): `undefined`/absent → null ptr so + // the helper defaults to " "; a Symbol fill throws; a + // `{ toString }` object runs user code (root the result + // — the receiver re-reads through its handle below). + let raw = crate::string::js_string_pad_fill(arg_at(1).unwrap_or(undefined)); + if raw.is_null() { + None + } else { + Some(root_scope.root_string_ptr(raw)) } - _ => std::ptr::null(), }; + let pad = pad_handle + .as_ref() + .map(|h| h.get_raw_const_ptr::()) + .unwrap_or(std::ptr::null()); let s = receiver_string(); let r = if method_name == "padStart" { crate::string::js_string_pad_start(s, target_len, pad) diff --git a/crates/perry-runtime/src/regex/match_string.rs b/crates/perry-runtime/src/regex/match_string.rs index 09d7e76a40..d4cea24273 100644 --- a/crates/perry-runtime/src/regex/match_string.rs +++ b/crates/perry-runtime/src/regex/match_string.rs @@ -107,9 +107,13 @@ pub extern "C" fn js_string_match( let arr_handle = scope.root_raw_mut_ptr(arr); (*arr_handle.get_raw_mut_ptr::()).length = matches.len() as u32; for (i, m) in matches.iter().enumerate() { - let str_ptr = js_string_from_str(m); - let nanboxed = js_nanbox_string(str_ptr as i64); - let arr = arr_handle.get_raw_mut_ptr::(); + // `across_mut` runs the allocating pair and hands back the + // post-collection array address, so the receiver is never + // bound stale in between (#7341). + let (nanboxed, arr) = arr_handle.across_mut::(|| { + let str_ptr = js_string_from_str(m); + js_nanbox_string(str_ptr as i64) + }); // GC_STORE_AUDIT(BARRIERED): regex match array slot uses the shared array slot-store helper. crate::array::store_array_slot(arr, i, nanboxed.to_bits()); } @@ -125,9 +129,12 @@ pub extern "C" fn js_string_match( (*arr_handle.get_raw_mut_ptr::()).length = caps.len() as u32; for i in 0..caps.len() { if let Some(m) = caps.get(i) { - let str_ptr = js_string_from_str(m.as_str()); - let nanboxed = js_nanbox_string(str_ptr as i64); - let arr = arr_handle.get_raw_mut_ptr::(); + // See the sibling above (#7341). + let (nanboxed, arr) = + arr_handle.across_mut::(|| { + let str_ptr = js_string_from_str(m.as_str()); + js_nanbox_string(str_ptr as i64) + }); // GC_STORE_AUDIT(BARRIERED): regex capture array slot uses the shared array slot-store helper. crate::array::store_array_slot(arr, i, nanboxed.to_bits()); } else { diff --git a/crates/perry-runtime/src/regex/replace_fn.rs b/crates/perry-runtime/src/regex/replace_fn.rs index a5b791cc4b..ab2def54cd 100644 --- a/crates/perry-runtime/src/regex/replace_fn.rs +++ b/crates/perry-runtime/src/regex/replace_fn.rs @@ -8,7 +8,13 @@ pub(super) unsafe fn call_replace_callback(callback: f64, args: &[f64]) -> Strin let prev = crate::object::js_implicit_this_set(f64::from_bits(crate::value::TAG_UNDEFINED)); let ret = crate::closure::js_native_call_value(callback, args.as_ptr(), args.len()); crate::object::js_implicit_this_set(prev); - let ptr = crate::value::js_get_string_pointer_unified(ret) as *const StringHeader; + // §22.1.3.19 step "Let replacement be ? ToString(? Call(replaceValue, …))": + // the callback result is ToString-coerced — `undefined` renders as + // "undefined", a number stringifies, an object runs its `toString`, and a + // Symbol throws a TypeError. The old raw pointer-extract silently dropped + // every non-string result to "" (test262 replace S15.5.4.11_A1_T5/T10). + crate::builtins::reject_symbol_to_string(ret); + let ptr = crate::builtins::js_string_coerce(ret) as *const StringHeader; if is_valid_ptr(ptr) { string_as_str(ptr).to_string() } else { diff --git a/crates/perry-runtime/src/string/mod.rs b/crates/perry-runtime/src/string/mod.rs index 85c3982945..9fbfb93179 100644 --- a/crates/perry-runtime/src/string/mod.rs +++ b/crates/perry-runtime/src/string/mod.rs @@ -96,7 +96,10 @@ pub use locale::{ js_string_to_locale_lower_case, js_string_to_locale_upper_case, js_string_validate_collator_args, }; -pub use pad::{js_string_alloc_space, js_string_pad_end, js_string_pad_start, js_string_repeat}; +pub use pad::{ + js_string_alloc_space, js_string_pad_end, js_string_pad_fill, js_string_pad_start, + js_string_repeat, +}; pub use raw::js_string_raw; pub(crate) use slice_ops::is_js_whitespace; pub use slice_ops::{ diff --git a/scripts/raw_handle_debt_baseline.txt b/scripts/raw_handle_debt_baseline.txt index baccd0398f..7d802a3e71 100644 --- a/scripts/raw_handle_debt_baseline.txt +++ b/scripts/raw_handle_debt_baseline.txt @@ -1 +1 @@ -1003 +1002