fix(runtime): honor Array toString dispatch

changelog.d/8714-array-tostring-dispatch.md

@@ -0,0 +1,3 @@
+Fixed `Array.prototype.toString.call(...)` falling off the reflective path and statically-known array `.toString()` calls bypassing a user override. The generic prototype thunk is now real and static calls route through the live prototype method, forwarding arguments.
+
+The prototype and key addresses passed to `js_object_get_field_by_name` are rooted and produced inside scoped handle borrows (#7341). That lookup can run arbitrary user JS — an `ObjectHeader` receiver falls through to `get_field_by_name_object_tail`, whose accessor arms call `invoke_accessor_getter`, so `Object.defineProperty(Array.prototype, "toString", { get() {…} })` reaches it — which makes a raw address bound across the call the exact shape #7341 exists to remove. The interned key is rooted with `root_string_ptr` (`STRING_TAG`) rather than `root_raw_mut_ptr` (`POINTER_TAG`), so the copying and verify visitors describe it correctly.

crates/perry-codegen/src/lower_array_method.rs

@@ -45,24 +45,12 @@
 //! so `arr.slice(1, 2)`, `arr.includes(x)`, `arr.join(",")` and every no-arg
 //! method emit byte-identical IR.
 //!
-//! ## Known window deliberately NOT closed here: #7213
-//!
-//! The `toString` arm unboxes the interned `","` through
-//! `js_get_string_pointer_unified`, whose SSO branch allocates, while the
-//! receiver handle is already in a register. `js_string_materialize_to_heap`'s
-//! rustdoc records why that is unexploitable today (the alloc-point arm forces
-//! a conservative stack scan, which both makes the copying minor ineligible and
-//! finds the register) and why it is tracked as #7213 rather than papered over.
-//! Closing it needs a combinator that roots across a collection point this
-//! module *emits* rather than one it infers from an operand list; that
-//! combinator should arrive with the slice that needs it, not ahead of one.
-
 use anyhow::{bail, Result};
 use perry_hir::Expr;
 
 use crate::expr::{
     emit_root_nanbox_store_on_block, emit_write_barrier, nanbox_pointer_inline,
-    nanbox_string_inline, unbox_str_handle, unbox_to_i64, FnCtx,
+    nanbox_string_inline, unbox_to_i64, FnCtx,
 };
 use crate::nanbox::{double_literal, TAG_UNDEFINED};
 use crate::rooting;
@@ -147,10 +135,10 @@ pub(crate) fn emit_grow_mutator_writeback(
 /// migration, so no behaviour changes in this slice.
 fn lowered_arg_count(property: &str, args: &[Expr]) -> usize {
     let declared = match property {
-        // Receiver-only. `arr.toString()` and `arr.reverse()` ignore their
-        // arguments entirely today; lowering them here would be a behaviour
-        // change (an added evaluation), not a rooting fix.
-        "toString" | "reverse" => 0,
+        // Receiver-only. `arr.reverse()` ignores its arguments entirely today;
+        // lowering them here would be a behaviour change (an added evaluation),
+        // not a rooting fix.
+        "reverse" => 0,
         // One: separator, depth, callback, comparator or index.
         "join" | "flat" | "flatMap" | "sort" | "at" | "findLast" | "findLastIndex" => 1,
         // Two: callback + thisArg, value + fromIndex, start + end, source +
@@ -282,27 +270,12 @@ pub(crate) fn lower_array_method(
                 ))
             }
             "toString" => {
-                // arr.toString() == arr.join(",")
-                let key_idx = ctx.strings.intern(",");
-                let handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global);
-                let blk = ctx.block();
-                let sep_box = blk.load(DOUBLE, &handle_global);
-                let recv_handle = unbox_to_i64(blk, recv_box);
-                // Interned literal "," — heap allocated at module init, so
-                // `unbox_to_i64` would technically work, but routing through
-                // `unbox_str_handle` keeps the path uniform with the `join`
-                // arm and is robust if interning ever changes to SSO-eligible.
-                //
-                // This call is the #7213 window described in the module header:
-                // its SSO branch allocates while `recv_handle` is already in a
-                // register. Left as-is, with its reasoning recorded there.
-                let sep_handle = unbox_str_handle(blk, &sep_box);
-                let result_handle = blk.call(
-                    I64,
-                    "js_array_join",
-                    &[(I64, &recv_handle), (I64, &sep_handle)],
-                );
-                Ok(nanbox_string_inline(blk, &result_handle))
+                // A source-level method call must resolve the current property:
+                // `Array.prototype.toString` is writable, and replacing it with
+                // `Object.prototype.toString` must affect even statically-known
+                // arrays. The runtime dispatcher preserves own-property
+                // precedence and invokes the live prototype method.
+                emit_native_method_dispatch(ctx, recv_box, property, arg_vals)
             }
             "concat" => {
                 // #2805: arr.concat(...args) — spec-complete, non-mutating, variadic.
@@ -1236,7 +1209,6 @@ mod tests {
         // (property, args supplied, arguments lowered)
         let cases: &[(&str, usize, usize)] = &[
             // Receiver-only: arguments are not evaluated at all today.
-            ("toString", 2, 0),
             ("reverse", 1, 0),
             // One leading argument.
             ("join", 0, 0),
@@ -1268,6 +1240,8 @@ mod tests {
             ("pop", 2, 2),
             ("shift", 1, 1),
             ("entries", 1, 1),
+            // Live method dispatch forwards every argument to a user override.
+            ("toString", 2, 2),
             ("next", 2, 2),
             ("someUserMethod", 3, 3),
         ];

crates/perry-hir/src/lower/expr_call/intrinsics/apply_call.rs

@@ -409,6 +409,27 @@ fn is_string_prototype_generic_method(recv: &ast::Expr, method: &str) -> bool {
         )
 }
 
+/// `Array.prototype.toString` is itself a generic dispatch operation: it gets
+/// `this.join`, calls that value when callable, and otherwise calls the
+/// intrinsic `Object.prototype.toString`. Folding `.call(receiver)` into
+/// `receiver.toString()` changes all three operations, so leave this one method
+/// on the reflective runtime path.
+fn is_array_prototype_to_string(recv: &ast::Expr, method: &str) -> bool {
+    if method != "toString" {
+        return false;
+    }
+    let ast::Expr::Member(member) = recv else {
+        return false;
+    };
+    let ast::MemberProp::Ident(prop) = &member.prop else {
+        return false;
+    };
+    if prop.sym.as_ref() != "prototype" {
+        return false;
+    }
+    matches!(member.obj.as_ref(), ast::Expr::Ident(base) if base.sym.as_ref() == "Array")
+}
+
 pub(crate) fn try_builtin_prototype_method_apply_call(
     ctx: &mut LoweringContext,
     call: &ast::CallExpr,
@@ -475,6 +496,9 @@ pub(crate) fn try_builtin_prototype_method_apply_call(
             if is_string_prototype_generic_method(inner.obj.as_ref(), method_ident.sym.as_ref()) {
                 return Ok(None);
             }
+            if is_array_prototype_to_string(inner.obj.as_ref(), method_ident.sym.as_ref()) {
+                return Ok(None);
+            }
             method_ident.clone()
         }
         ast::Expr::Ident(id) => match ctx.builtin_proto_method_locals.get(id.sym.as_ref()) {
@@ -745,6 +769,9 @@ pub(crate) fn as_builtin_proto_method_ref(
     if is_string_prototype_generic_method(&member.obj, method.sym.as_ref()) {
         return None;
     }
+    if is_array_prototype_to_string(&member.obj, method.sym.as_ref()) {
+        return None;
+    }
     // For a `<Ctor>.prototype` receiver, any method ident is accepted (mirrors
     // the existing `.call`/`.apply` rewrite, which doesn't gate on the method
     // name). For an array/string literal receiver, gate on the known
@@ -834,4 +861,15 @@ mod tests {
         assert!(!is_string_prototype_generic_method(&recv, "toString"));
         assert!(!is_string_prototype_generic_method(&recv, "valueOf"));
     }
+
+    #[test]
+    fn array_to_string_stays_reflective() {
+        let recv = prototype_member("Array");
+        assert!(is_array_prototype_to_string(&recv, "toString"));
+        assert!(!is_array_prototype_to_string(&recv, "join"));
+        assert!(!is_array_prototype_to_string(
+            &prototype_member("String"),
+            "toString"
+        ));
+    }
 }

crates/perry-runtime/src/object/global_this/array_error.rs

@@ -877,6 +877,19 @@ pub(crate) extern "C" fn array_proto_join_thunk(
     let this = crate::object::js_implicit_this_get();
     crate::array::js_arraylike_join(this, sep)
 }
+
+/// Spec `Array.prototype.toString`: `ToObject(this)`, get the receiver's live
+/// `join` property, call it when callable, and otherwise use the intrinsic
+/// `Object.prototype.toString`. This must be a real thunk because keeping
+/// `Array.prototype.toString.call(x)` reflective is what preserves the generic
+/// receiver semantics; rewriting it to `x.toString()` loses the Array method.
+pub(crate) extern "C" fn array_prototype_to_string_thunk(
+    _c: *const crate::closure::ClosureHeader,
+) -> f64 {
+    let this = crate::object::js_implicit_this_get();
+    crate::value::array_prototype_to_string(this)
+}
+
 pub(crate) extern "C" fn array_prototype_concat_thunk(
     _c: *const crate::closure::ClosureHeader,
     rest: f64,

crates/perry-runtime/src/object/global_this/proto_methods.rs

@@ -30,10 +30,11 @@ const OBJECT_PROTO_METHODS: &[(&str, u32)] = &[
 /// `typeof Array.prototype.map === "function"` and `.name === "map"`
 /// agree with Node when the value is read through indirection.
 ///
-/// Two of these methods retain dedicated thunks for spec-accurate call
-/// behavior — `Array.prototype.slice` (ramda's curry/variadic helpers
+/// These methods retain dedicated thunks for spec-accurate call behavior —
+/// `Array.prototype.slice` (ramda's curry/variadic helpers
 /// reach through `Array.prototype.slice.call(args, …)` and depend on it
-/// returning a real sliced array, even via indirection) and
+/// returning a real sliced array, even via indirection),
+/// `Array.prototype.toString` (generic `join` dispatch), and
 /// `Object.prototype.toString` (ramda's `_isArguments.js` IIFE calls
 /// `Object.prototype.toString.call(arguments)` at module-init time).
 /// All other methods are noop-backed: typeof + `.name` introspection
@@ -116,6 +117,12 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj:
                 array_prototype_slice_thunk as *const u8,
                 2,
             );
+            install_proto_method(
+                proto_obj,
+                "toString",
+                array_prototype_to_string_thunk as *const u8,
+                0,
+            );
             install_noop_proto_methods(
                 proto_obj,
                 &[
@@ -127,7 +134,6 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj:
                     ("toReversed", 0),
                     ("toSorted", 1),
                     ("toSpliced", 2),
-                    ("toString", 0),
                     ("with", 2),
                 ],
             );

crates/perry-runtime/src/object/native_call_method/handle_methods.rs

@@ -268,12 +268,10 @@ pub(super) unsafe fn dispatch_handle(
                 }
                 match method_name {
                     "toString" => {
-                        let arr = raw_ptr as *const crate::array::ArrayHeader;
-                        let s = crate::array::js_array_join_value(
-                            arr,
-                            f64::from_bits(crate::value::TAG_UNDEFINED),
-                        );
-                        return Some(f64::from_bits(JSValue::string_ptr(s).bits()));
+                        return Some(crate::value::call_array_prototype_to_string_method(
+                            object_handle.get_nanbox_f64(),
+                            arg_handles,
+                        ));
                     }
                     "map" if args_len >= 1 && !args_ptr.is_null() => {
                         let arr = raw_ptr as *const crate::array::ArrayHeader;

crates/perry-runtime/src/value/mod.rs

@@ -115,7 +115,8 @@ pub use dyn_index::{js_dyn_index_get, js_dyn_index_set, js_is_undefined_or_bare_
 
 // ----- to-string conversion helpers -----
 pub(crate) use to_string::{
-    coerce_validate_radix, function_to_primitive_for_add, function_to_string_method_result,
+    array_prototype_to_string, call_array_prototype_to_string_method, coerce_validate_radix,
+    function_to_primitive_for_add, function_to_string_method_result,
     ordinary_to_primitive_for_toprimitive, ordinary_to_primitive_number_for_add,
     to_primitive_number, OrdinaryToPrimitiveOutcome,
 };