diff --git a/crates/perry-runtime/src/object/collection_proto_thunks.rs b/crates/perry-runtime/src/object/collection_proto_thunks.rs index 0e68701728..62a3eff3ff 100644 --- a/crates/perry-runtime/src/object/collection_proto_thunks.rs +++ b/crates/perry-runtime/src/object/collection_proto_thunks.rs @@ -260,11 +260,132 @@ fn install_collection_size_getter(proto_obj: *mut ObjectHeader, name: &str, func } } -/// Throw `TypeError: Method . called on incompatible receiver`. -/// Mirrors V8's wording closely; Test262's brand-check tests assert only the -/// error *type*, so the exact message is informational. Never returns. -fn throw_incompatible_receiver(proto: &str, method: &str) -> ! { - let msg = format!("Method {proto}.{method} called on incompatible receiver"); +/// Owned UTF-8 copy of a heap `StringHeader`'s bytes. +fn string_header_owned(sp: *const crate::string::StringHeader) -> String { + if sp.is_null() { + return String::new(); + } + unsafe { + let bytes = (sp as *const u8).add(std::mem::size_of::()); + let len = (*sp).byte_len as usize; + std::str::from_utf8(std::slice::from_raw_parts(bytes, len)) + .unwrap_or("") + .to_string() + } +} + +/// Render the brand-check receiver the way V8's `NoSideEffectsToString` does — +/// node APPENDS it to the incompatible-receiver TypeError (`Method +/// Set.prototype.add called on incompatible receiver #` / +/// `... receiver undefined` / `... receiver [object Array]`), and #6658's +/// parity fixtures assert the full message. V8 never CALLS a potentially +/// side-effecting user `toString` here: primitives print their ToString +/// (`undefined`, `null`, `5.5`, the raw string chars, `Symbol(desc)`, bigint +/// digits); objects whose `toString` is the default `Object.prototype.toString` +/// print `#` (`#`, `#`, `#`, `#`, +/// `#`); objects that override it (Array, Date, RegExp, an own user +/// `toString`) print the `Object.prototype.toString` tag instead; errors print +/// `Name: message`. node prints a function receiver's SOURCE — perry keeps no +/// source, so the native-code form is the closest stable rendering. +/// Unclassifiable heap values fall back to `#`. +fn render_incompatible_receiver(bits: u64) -> String { + use crate::value::JSValue; + let value = f64::from_bits(bits); + let jv = JSValue::from_bits(bits); + if !jv.is_pointer() { + // undefined / null / bool / number / bigint / heap+SSO string: plain + // side-effect-free ToString. + return string_header_owned(crate::value::js_jsvalue_to_string(value)); + } + let ptr = (bits & crate::value::POINTER_MASK) as usize; + if crate::symbol::is_registered_symbol(ptr) { + // "Symbol(desc)" — Symbol.prototype.toString is side-effect free. + return string_header_owned(unsafe { crate::symbol::js_symbol_to_string(value) } + as *const crate::string::StringHeader); + } + if crate::set::is_registered_set(ptr) { + return "#".to_string(); + } + if crate::map::is_registered_map(ptr) { + return "#".to_string(); + } + match crate::weakref::weak_class_id_from_receiver(value) { + Some(crate::weakref::CLASS_ID_WEAKSET) => return "#".to_string(), + Some(crate::weakref::CLASS_ID_WEAKMAP) => return "#".to_string(), + _ => {} + } + if crate::date::is_registered_date_bits(bits) { + return "[object Date]".to_string(); + } + if crate::regex::is_registered_regex(ptr) { + return "[object RegExp]".to_string(); + } + // Heap-header classification — only a real heap pointer above the + // synthetic handle band may be dereferenced (mirrors `as_real_array`). + if crate::value::addr_class::is_above_handle_band(ptr) + && crate::object::is_valid_obj_ptr(ptr as *const u8) + { + let obj_type = unsafe { + (*((ptr as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader)) + .obj_type + }; + match obj_type { + crate::gc::GC_TYPE_ARRAY | crate::gc::GC_TYPE_LAZY_ARRAY => { + return "[object Array]".to_string(); + } + crate::gc::GC_TYPE_PROMISE => return "#".to_string(), + crate::gc::GC_TYPE_ERROR => { + let err = ptr as *mut crate::error::ErrorHeader; + let name = string_header_owned(crate::error::js_error_get_name(err)); + let message = string_header_owned(crate::error::js_error_get_message(err)); + return if message.is_empty() { + name + } else { + format!("{name}: {message}") + }; + } + crate::gc::GC_TYPE_CLOSURE => { + return "function () { [native code] }".to_string(); + } + crate::gc::GC_TYPE_OBJECT => { + // An OWN user `toString` overrides the default — V8 refuses to + // call it and falls back to the Object.prototype.toString tag. + // js_string_from_bytes allocates and can trigger a GC that + // relocates the receiver: root it and re-derive the pointer + // through the (rewritten) handle after the allocation. + let scope = crate::gc::RuntimeHandleScope::new(); + let handle = scope.root_nanbox_f64(f64::from_bits(bits)); + let key = crate::string::js_string_from_bytes(b"toString".as_ptr(), 8); + let obj = crate::value::JSValue::from_bits(handle.get_nanbox_f64().to_bits()) + .as_pointer::() as *mut ObjectHeader; + if unsafe { super::own_data_field_by_name(obj, key) }.is_some() { + return "[object Object]".to_string(); + } + let cid = unsafe { (*obj).class_id }; + if cid != 0 { + if let Some(name) = crate::object::class_name_for_id(cid) { + if !name.is_empty() { + return format!("#<{name}>"); + } + } + } + return "#".to_string(); + } + _ => {} + } + } + "#".to_string() +} + +/// Throw `TypeError: Method . called on incompatible receiver +/// `. Matches V8/node's wording INCLUDING the appended receiver +/// rendering (#6658 asserts the full message; Test262's brand-check tests +/// assert only the error *type*). Never returns. +fn throw_incompatible_receiver(proto: &str, method: &str, receiver_bits: u64) -> ! { + let msg = format!( + "Method {proto}.{method} called on incompatible receiver {}", + render_incompatible_receiver(receiver_bits) + ); let s = crate::string::js_string_from_bytes(msg.as_ptr(), msg.len() as u32); let err = crate::error::js_typeerror_new(s); crate::exception::js_throw(f64::from_bits( @@ -277,7 +398,7 @@ fn set_receiver_or_throw(method: &str) -> *mut crate::set::SetHeader { let bits = IMPLICIT_THIS.with(|c| c.get()); match crate::set::set_ptr_from_receiver_bits(bits) { Some(p) => p, - None => throw_incompatible_receiver("Set.prototype", method), + None => throw_incompatible_receiver("Set.prototype", method, bits), } } @@ -286,7 +407,7 @@ fn map_receiver_or_throw(method: &str) -> *mut crate::map::MapHeader { let bits = IMPLICIT_THIS.with(|c| c.get()); match crate::map::map_ptr_from_receiver_bits(bits) { Some(p) => p, - None => throw_incompatible_receiver("Map.prototype", method), + None => throw_incompatible_receiver("Map.prototype", method, bits), } } @@ -295,7 +416,7 @@ fn weak_receiver_or_throw(expected: u32, proto: &str, method: &str) -> f64 { let receiver = f64::from_bits(IMPLICIT_THIS.with(|c| c.get())); match crate::weakref::weak_class_id_from_receiver(receiver) { Some(cid) if cid == expected => receiver, - _ => throw_incompatible_receiver(proto, method), + _ => throw_incompatible_receiver(proto, method, receiver.to_bits()), } } diff --git a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs index a383d0e948..1fec3fc37e 100644 --- a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs @@ -219,6 +219,45 @@ pub(super) unsafe fn dispatch_handle( return Some(result); } } + // #6658: an explicit `thisArg` (2nd argument) must bind the + // callback's `this`. The dense helpers the arms below dispatch + // to deliberately bind `undefined` (spec: absent thisArg) and + // take no thisArg parameter, so routing a 2-arg call through + // them silently DROPS the thisArg — an extracted builtin + // method used as the callback (`arr.forEach(set.add, set)`, + // @babel/types' alias-expansion loop) then brand-checks + // `this === undefined` and throws "Method Set.prototype.add + // called on incompatible receiver". Mirror the static lowering + // rule (lower_array_method.rs): with a thisArg present, route + // through the spec-generic array-like engine, which binds it + // for each callback call (real-array receivers keep a fast + // element path there). `flatMap` has no engine entry (the + // static path shares that thisArg gap) and falls through + // unchanged. + if args_len >= 2 + && !args_ptr.is_null() + && matches!( + method_name, + "forEach" + | "map" + | "filter" + | "some" + | "every" + | "find" + | "findIndex" + | "findLast" + | "findLastIndex" + ) + { + if let Some(result) = crate::array::dispatch_arraylike_read_method( + object, + method_name, + args_ptr, + args_len, + ) { + return Some(result); + } + } match method_name { "toString" => { let arr = raw_ptr as *const crate::array::ArrayHeader; diff --git a/crates/perry/tests/issue_6658_extracted_builtin_method_thisarg.rs b/crates/perry/tests/issue_6658_extracted_builtin_method_thisarg.rs new file mode 100644 index 0000000000..d3b6fec4f3 --- /dev/null +++ b/crates/perry/tests/issue_6658_extracted_builtin_method_thisarg.rs @@ -0,0 +1,187 @@ +//! Regression tests for #6658 (pi wall #7): an extracted builtin method used +//! as a callback with an explicit `thisArg` — `arr.forEach(set.add, set)` — +//! must invoke the builtin with `this = thisArg`, exactly as node does. +//! +//! Trigger in the wild: @babel/types' alias-expansion loop +//! (`e5 ? e5.forEach(t4.add, t4) : t4.add(r4)`, pi-bundle.mjs:203827) threw +//! "TypeError: Method Set.prototype.add called on incompatible receiver" +//! during pi-native module init. +//! +//! Root cause: the DYNAMIC method-dispatch tower (`js_native_call_method` → +//! the dense-array arms in `native_call_method/handle_methods.rs`) dropped +//! `args[1]` for the whole Array.prototype callback family and dispatched to +//! the dense helpers, which bind the callback's `this` to undefined (the spec +//! rule for an ABSENT thisArg). The STATIC lowering already routed +//! explicit-thisArg calls through the this-binding `js_arraylike_*` engine — +//! a receiver only reaches the dynamic tower when codegen can't prove its +//! type (here: an object member read through a DYNAMIC key), which is why +//! only the combined @babel/types shape reproduced and every +//! statically-provable simplification worked. The fix mirrors the static +//! rule in the dynamic tower: with a thisArg present, route through +//! `dispatch_arraylike_read_method`. +//! +//! Also pinned: node/V8's brand-check TypeError message, receiver rendering +//! included (`NoSideEffectsToString`: "#", "undefined", "5.5", ...). +//! +//! Expected outputs are node v26's, byte for byte. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(dir: &std::path::Path, source: &str) -> String { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +/// The issue's combined shape verbatim, the five previously-ruled-out simpler +/// shapes as anchors, and the minimal dynamic-tower trigger (a dynamic-key +/// member read defeats flow typing, so `.forEach` dispatches through the +/// runtime tower that dropped the thisArg) with Set.prototype.add, +/// Map.prototype.set, and Array.prototype.push extracted as callbacks. +#[test] +fn extracted_builtin_method_callback_thisarg_matches_node() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +const FLIPPED: any = { A: ["x", "y"], B: null }; +const allExpandedTypes: any[] = [ + { types: ["A", "B"], set: new Set() }, + { types: ["B"], set: new Set() }, +]; +for (const { types: e4, set: t4 } of allExpandedTypes) { + for (const r4 of e4) { + const e5 = FLIPPED[r4]; + e5 ? e5.forEach(t4.add, t4) : t4.add(r4); + } +} +console.log("sizes:", allExpandedTypes[0].set.size, allExpandedTypes[1].set.size); + +const s = new Set(); +["a", "b"].forEach(s.add, s); +const t4: any = new Set(); +["a"].forEach(t4.add, t4); +const add = t4.add; +add.call(t4, "q"); +const rows: any[] = [{ set: new Set() }]; +for (const { set: r } of rows) ["a"].forEach(r.add, r); +const e5t: any = ["z1", "z2"]; +const t5: any = new Set(); +e5t ? e5t.forEach(t5.add, t5) : t5.add("z"); +console.log("anchors:", s.size, t4.size, rows[0].set.size, t5.size); + +const SRC: any = { A: ["x", "y"] }; +const KEYS = ["A"]; +const dyn = SRC[KEYS[0]]; +const ds: any = new Set(); +dyn.forEach(ds.add, ds); +const m: any = new Map(); +dyn.forEach(m.set, m); +const a2: any = []; +dyn.forEach(a2.push, a2); +console.log("dynamic:", ds.size, m.size, m.get("x"), m.get("y"), a2.length); + +const marker = { tag: "T" }; +const seen: boolean[] = []; +function observe(this: any): boolean { + seen.push(this === marker); + return true; +} +dyn.forEach(observe, marker); +dyn.map(observe, marker); +dyn.filter(observe, marker); +dyn.every(observe, marker); +dyn.some(function (this: any) { seen.push(this === marker); return false; }, marker); +dyn.find(function (this: any) { seen.push(this === marker); return false; }, marker); +dyn.findIndex(function (this: any) { seen.push(this === marker); return false; }, marker); +dyn.findLast(function (this: any) { seen.push(this === marker); return false; }, marker); +dyn.findLastIndex(function (this: any) { seen.push(this === marker); return false; }, marker); +console.log("family this-bound:", seen.length, seen.every(Boolean)); +"#, + ); + assert_eq!( + stdout, + "sizes: 3 1\n\ + anchors: 2 2 1 2\n\ + dynamic: 2 2 0 1 6\n\ + family this-bound: 18 true\n" + ); +} + +/// The brand-check TypeError cases where node DOES throw — message-identical, +/// including V8's `NoSideEffectsToString` receiver rendering. +#[test] +fn incompatible_receiver_message_matches_node() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +function thrown(fn: () => void): string { + try { + fn(); + return "NO_THROW"; + } catch (e: any) { + return e.constructor.name + ": " + e.message; + } +} +const SRC: any = { A: ["x"] }; +const KEYS = ["A"]; +const dyn = SRC[KEYS[0]]; +const t4: any = new Set(); +const m: any = new Map(); +console.log("undef:", thrown(() => dyn.forEach(t4.add))); +console.log("obj:", thrown(() => dyn.forEach(t4.add, {}))); +console.log("cross:", thrown(() => dyn.forEach(m.set, t4))); +console.log("num:", thrown(() => dyn.forEach(t4.add, 5.5 as any))); +console.log("str:", thrown(() => dyn.forEach(t4.add, "abc" as any))); +console.log("null:", thrown(() => (Set.prototype.add as any).call(null, 1))); +class Foo {} +console.log("inst:", thrown(() => (Set.prototype.add as any).call(new Foo(), 1))); +console.log("arr:", thrown(() => (Set.prototype.add as any).call([1, 2], 1))); +"#, + ); + assert_eq!( + stdout, + "undef: TypeError: Method Set.prototype.add called on incompatible receiver undefined\n\ + obj: TypeError: Method Set.prototype.add called on incompatible receiver #\n\ + cross: TypeError: Method Map.prototype.set called on incompatible receiver #\n\ + num: TypeError: Method Set.prototype.add called on incompatible receiver 5.5\n\ + str: TypeError: Method Set.prototype.add called on incompatible receiver abc\n\ + null: TypeError: Method Set.prototype.add called on incompatible receiver null\n\ + inst: TypeError: Method Set.prototype.add called on incompatible receiver #\n\ + arr: TypeError: Method Set.prototype.add called on incompatible receiver [object Array]\n" + ); +} diff --git a/test-parity/node-suite/object/extracted-builtin-method-callback-thisarg.ts b/test-parity/node-suite/object/extracted-builtin-method-callback-thisarg.ts new file mode 100644 index 0000000000..8c3e28058d --- /dev/null +++ b/test-parity/node-suite/object/extracted-builtin-method-callback-thisarg.ts @@ -0,0 +1,147 @@ +// Regression (#6658, pi wall #7): an extracted builtin method used as a +// callback with an explicit `thisArg` — `arr.forEach(set.add, set)` — must +// invoke the builtin with `this = thisArg`, exactly as node does. +// Trigger in the wild: @babel/types' alias-expansion loop +// (`e5 ? e5.forEach(t4.add, t4) : t4.add(r4)`, pi-bundle.mjs:203827) threw +// "TypeError: Method Set.prototype.add called on incompatible receiver" +// during pi-native module init. +// +// Root cause: the DYNAMIC method-dispatch tower (js_native_call_method's +// dense-array arms) dropped args[1] for the whole Array.prototype callback +// family (forEach/map/filter/some/every/find/findIndex/findLast/ +// findLastIndex) and dispatched to the dense helpers, which bind the +// callback's `this` to undefined (the spec rule for an ABSENT thisArg). The +// STATIC lowering already routed explicit-thisArg calls through the +// this-binding array-like engine — a receiver only reaches the dynamic tower +// when codegen can't prove its type (here: an object member read through a +// DYNAMIC key), which is why only the combined @babel/types shape reproduced +// and every statically-provable simplification worked. +// +// Also pinned: node's V8 brand-check TypeError message, receiver rendering +// included ("... called on incompatible receiver #"). + +// --- 1. the issue's combined shape (verbatim) ------------------------------- +{ + const FLIPPED: any = { A: ["x", "y"], B: null }; + const allExpandedTypes: any[] = [ + { types: ["A", "B"], set: new Set() }, + { types: ["B"], set: new Set() }, + ]; + for (const { types: e4, set: t4 } of allExpandedTypes) { + for (const r4 of e4) { + const e5 = FLIPPED[r4]; + e5 ? e5.forEach(t4.add, t4) : t4.add(r4); + } + } + console.log("sizes:", allExpandedTypes[0].set.size, allExpandedTypes[1].set.size); +} + +// --- 2. the five previously-ruled-out simpler shapes (anchors) -------------- +{ + // 2a. typed receiver + const s = new Set(); + ["a", "b"].forEach(s.add, s); + console.log("anchor typed:", s.size); + + // 2b. Any receiver + const t4: any = new Set(); + ["a"].forEach(t4.add, t4); + console.log("anchor any:", t4.size); + + // 2c. extraction + .call + const add = t4.add; + add.call(t4, "q"); + console.log("anchor call:", t4.size); + + // 2d. destructured for-of + const rows: any[] = [{ set: new Set() }]; + for (const { set: r } of rows) ["a"].forEach(r.add, r); + console.log("anchor destructured:", rows[0].set.size); + + // 2e. ternary shape + const e5: any = ["z1", "z2"]; + const t5: any = new Set(); + e5 ? e5.forEach(t5.add, t5) : t5.add("z"); + console.log("anchor ternary:", t5.size); +} + +// --- 3. minimal dynamic-tower trigger (dynamic-key member read) ------------- +// SRC[key] with a non-literal key defeats flow typing, so `.forEach` on the +// result dispatches through the runtime tower — the path that dropped thisArg. +const SRC: any = { A: ["x", "y"] }; +const KEYS = ["A"]; +const dyn = SRC[KEYS[0]]; // e5's provenance in the bundle + +{ + const t4: any = new Set(); + dyn.forEach(t4.add, t4); + console.log("dynamic Set.add:", t4.size, t4.has("x"), t4.has("y")); +} + +// --- 4. other builtin methods extracted the same way ------------------------ +{ + // Map.prototype.set — forEach passes (value, index), so m.set("x", 0) etc. + const m: any = new Map(); + dyn.forEach(m.set, m); + console.log("dynamic Map.set:", m.size, m.get("x"), m.get("y")); + + // Array.prototype.push — forEach passes (value, index, array): 3 pushes/el. + const a2: any = []; + dyn.forEach(a2.push, a2); + console.log("dynamic Array.push:", a2.length, a2[0], a2[1] === 0, a2[3]); + + // Set.prototype.forEach with an extracted add (collection engine, not the + // array tower — must bind thisArg the same way). + const src: any = new Set(["p", "q"]); + const dst: any = new Set(); + src.forEach(dst.add, dst); + console.log("set-forEach Set.add:", dst.size, dst.has("p"), dst.has("q")); +} + +// --- 5. the whole dynamic callback family binds thisArg --------------------- +{ + const marker = { tag: "T" }; + const seen: boolean[] = []; + function observe(this: any): boolean { + seen.push(this === marker); + return true; + } + dyn.forEach(observe, marker); + dyn.map(observe, marker); + dyn.filter(observe, marker); + dyn.some(function (this: any) { seen.push(this === marker); return false; }, marker); + dyn.every(observe, marker); + dyn.find(function (this: any) { seen.push(this === marker); return false; }, marker); + dyn.findIndex(function (this: any) { seen.push(this === marker); return false; }, marker); + dyn.findLast(function (this: any) { seen.push(this === marker); return false; }, marker); + dyn.findLastIndex(function (this: any) { seen.push(this === marker); return false; }, marker); + console.log("family this-bound:", seen.length, seen.every(Boolean)); +} + +// --- 6. cases where node DOES throw (message-identical) --------------------- +function thrown(fn: () => void): string { + try { + fn(); + return "NO_THROW"; + } catch (e: any) { + return e.constructor.name + ": " + e.message; + } +} + +{ + const t4: any = new Set(); + const m: any = new Map(); + // absent thisArg: callback `this` is undefined → brand check throws + console.log("throw undef:", thrown(() => dyn.forEach(t4.add))); + // wrong-brand thisArg: plain object + console.log("throw obj:", thrown(() => dyn.forEach(t4.add, {}))); + // cross-brand: Map.prototype.set with a Set thisArg + console.log("throw cross:", thrown(() => dyn.forEach(m.set, t4))); + // primitive thisArgs render per V8's NoSideEffectsToString + console.log("throw num:", thrown(() => dyn.forEach(t4.add, 5.5 as any))); + console.log("throw str:", thrown(() => dyn.forEach(t4.add, "abc" as any))); + // reflective .call keeps the same message family + console.log("throw call:", thrown(() => (Set.prototype.add as any).call(null, 1))); + class Foo {} + console.log("throw inst:", thrown(() => (Set.prototype.add as any).call(new Foo(), 1))); +}