diff --git a/changelog.d/7953-weakref-receiver-shapes.md b/changelog.d/7953-weakref-receiver-shapes.md new file mode 100644 index 0000000000..a8943e5f4b --- /dev/null +++ b/changelog.d/7953-weakref-receiver-shapes.md @@ -0,0 +1,93 @@ +Gave `WeakRef` and `FinalizationRegistry` an actual runtime method surface, and +made every folded weak intrinsic brand-check its receiver. + +`WeakRef.prototype.deref` and `FinalizationRegistry.prototype.register` / +`.unregister` had **no runtime existence at all**. They were purely an HIR fold: +`pre_scan_weakref_locals` records bare local NAMES bound to +`let/const x = new WeakRef(…)` — walking module statements and the bodies of +function *declarations* only — and `expr_call/url_date_instance.rs` folds +`.deref()` to `Expr::WeakRefDeref`. Anything the fold could not +name fell through to ordinary dynamic dispatch, where nothing resolved: +`try_weak_method_dispatch` early-returned unless the receiver carried +`CLASS_ID_WEAKMAP`/`CLASS_ID_WEAKSET`, `install_collection_proto_methods` had no +arm for either wrapper (so `WeakRef.prototype` carried no `deref` property at +all), and the by-name value read in `get_field_by_name.rs` had a WeakMap/WeakSet +arm but no wrapper arm. + +Measured over twenty receiver shapes, **two worked**: a `const x = new WeakRef(…)` +at module top level, and the same inside a function *declaration*. Everything +else threw `TypeError: deref is not a function` — an array element (#7947's +report), a local copied from one, an object property, a call result, a `for…of` +binding, a function parameter, a `.map` callback, `new WeakRef(x).deref()` +inline, a `Map` value, and any binding inside an arrow function, function +expression or class method. The reflective path was equally dead: +`WeakRef.prototype.deref.call(wr)` threw "was called on a value that is not a +function", `wr.deref.bind(wr)` threw "Bind must be called on a function", +`typeof wr.deref` was `undefined`, and `wr.deref?.()` silently produced +`undefined`. `WeakMap`/`WeakSet` passed every one of those shapes both before and +after, because they have all three routes — that asymmetry is the whole bug, and +`weakref_locals.rs` already named it in a comment justifying why those sets are +exempt from the ambiguity poison pass ("have no runtime method-dispatch +fallback — they rely on the codegen fast path"). + +Three additions close it. `try_weak_method_dispatch` gains +`("deref", CLASS_ID_WEAKREF)` and +`("register" | "unregister", CLASS_ID_FINALIZATION_REGISTRY)` arms, so a *call* +on any receiver shape reaches the runtime helper. Brand-checking prototype +thunks are installed on both prototypes via +`populate_builtin_prototype_methods`, which fixes `.call`/`.apply`, method +extraction, the spec `.length` values, and makes +`WeakRef.prototype.deref.call({})` throw the `TypeError` the spec requires. And +`get_field_by_name.rs` resolves those same thunk values for an instance read, so +`typeof wr.deref === "function"` and `wr.deref === WeakRef.prototype.deref`. +`Object.prototype.toString` gained the two missing arms as well +(`[object Object]` → `[object WeakRef]` / `[object FinalizationRegistry]`). + +The same investigation turned up the silent sibling, filed as #7948 and closed +here. The fold is name-keyed and **scope-blind**, and its helpers did not +brand-check, so one genuine `const r = new WeakRef(x)` anywhere in a module +folded *every* `r.deref()` in that module onto `js_weakref_deref` — which read +`__perry_wr_target` by name off whatever it was handed and answered `undefined`. +A user class instance, an object literal, an array with an attached `deref`, and +a **function parameter** all silently returned `undefined` instead of their own +method's result, with exit code 0. `weakmap_locals`/`weakset_locals`/ +`proxy_locals` *are* subtracted by the ambiguity poison pass, but that pass only +recognises `new ()` and call/await initializers — it cannot see an +object literal, an array, or a parameter, so the identical hijack went through on +the far more common `get`/`set`/`has`/`add`/`delete`. Name poisoning can only +ever be a partial patch, because the pre-scan cannot enumerate every way a name +acquires a non-intrinsic value; parameters and destructuring bindings are not +even declarations it visits. So the fix went on the other side: +`js_weakref_deref`, `js_finreg_register`, `js_finreg_unregister`, +`js_weakmap_{set,get,has,delete}` and `js_weakset_add` now verify the receiver's +reserved `class_id` before trusting it and hand a foreign one to +`dispatch_foreign_weak_receiver`, which re-enters `js_native_call_method`. A +mis-fold degrades to the correct slow path instead of a wrong answer. Recursion +is impossible: `js_native_call_method` routes back into these helpers only via +`try_weak_method_dispatch`, which requires exactly the reserved `class_id` the +brand check just rejected. + +`try_weak_method_dispatch` and `weak_class_id_from_receiver` moved out of +`weakref.rs` (at 1988 of the 2000-line gate) into a new +`crates/perry-runtime/src/object/weakref_proto_thunks.rs` as a pure move, then +extended there. + +Deliberately left: weak-wrapper **subclassing**. `class M extends WeakMap {}` and +the WeakSet/WeakRef/FinalizationRegistry equivalents throw before *and* after this +change (`value is not a function`; `Constructor WeakRef requires 'new'`), +verified identical against a pristine `origin/main` binary so the new brand +checks cannot be blamed for it. It is a different mechanism — +constructor/prototype reification, of which `map_set_subclass` exists only for +Map/Set — and the gap test's header pins it as an explicit non-boundary so a +green run is not misread as coverage. The HIR pre-scan is also still name-keyed +and scope-blind and still does not descend into arrow bodies; that is now a +*performance* property rather than a correctness one, since an unnamed receiver +takes the dynamic path and a mis-named one brand-checks its way back to the right +method. + +`test-files/test_gap_weakref_receiver_shapes_7947.ts` pins all of it: the 14 +previously-throwing receiver shapes, the reflective and value-read paths, both +`toString` tags, the brand check, `FinalizationRegistry` through three shapes, +the six WeakRef/FinReg name-collision cells and the five WeakMap/WeakSet ones, +and the WeakMap/WeakSet shapes that already worked — so a future refactor of the +shared dispatch cannot silently drop them. diff --git a/crates/perry-runtime/src/object/collection_proto_thunks.rs b/crates/perry-runtime/src/object/collection_proto_thunks.rs index 64f9343bcf..23f68c6720 100644 --- a/crates/perry-runtime/src/object/collection_proto_thunks.rs +++ b/crates/perry-runtime/src/object/collection_proto_thunks.rs @@ -312,7 +312,7 @@ fn render_incompatible_receiver(bits: u64) -> String { if crate::map::is_registered_map(ptr) { return "#".to_string(); } - match crate::weakref::weak_class_id_from_receiver(value) { + match crate::object::weak_class_id_from_receiver(value) { Some(crate::weakref::CLASS_ID_WEAKSET) => return "#".to_string(), Some(crate::weakref::CLASS_ID_WEAKMAP) => return "#".to_string(), _ => {} @@ -417,7 +417,7 @@ fn map_receiver_or_throw(method: &str) -> *mut crate::map::MapHeader { #[inline] 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) { + match super::weak_class_id_from_receiver(receiver) { Some(cid) if cid == expected => receiver, _ => throw_incompatible_receiver(proto, method, receiver.to_bits()), } diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs index f817ca035b..5e36a5919a 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs @@ -322,7 +322,7 @@ pub extern "C" fn js_object_get_field_by_name( { unsafe { let boxed = f64::from_bits(JSValue::pointer(obj as *const u8).bits()); - if let Some(cid) = crate::weakref::weak_class_id_from_receiver(boxed) { + if let Some(cid) = crate::object::weak_class_id_from_receiver(boxed) { let name_ptr = (key as *const u8).add(std::mem::size_of::()); let name_len = (*key).byte_len as usize; let name = std::slice::from_raw_parts(name_ptr, name_len); @@ -349,6 +349,37 @@ pub extern "C" fn js_object_get_field_by_name( } } } + // #7947: the same VALUE read for a `WeakRef` / `FinalizationRegistry` + // instance — `typeof wr.deref`, `const d = wr.deref`, `wr.deref.bind(wr)`. + // These wrappers had no prototype thunks at all before #7947, so every such + // read answered `undefined` (and `.bind` threw "Bind must be called on a + // function"). Own keys keep precedence — fresh instances carry only the + // `__perry_wr_target` / `__perry_fr_*` sentinels. + if !key.is_null() + && ((obj as u64) >> 48) == 0 + && crate::value::addr_class::is_above_handle_band(obj as usize) + { + unsafe { + let boxed = f64::from_bits(JSValue::pointer(obj as *const u8).bits()); + if let Some(cid) = crate::object::weak_wrapper_class_id(boxed) { + let name_ptr = (key as *const u8).add(std::mem::size_of::()); + let name_len = (*key).byte_len as usize; + let name = std::slice::from_raw_parts(name_ptr, name_len); + if let Ok(method_name) = std::str::from_utf8(name) { + if !super::super::own_key_present(obj as *mut ObjectHeader, key) { + if let Some(v) = + super::super::weakref_proto_thunks::weakref_proto_method_value_for( + cid, + method_name, + ) + { + return JSValue::from_bits(v.to_bits()); + } + } + } + } + } + } // `class X extends Promise` instance — a value read of `then`/`catch`/ // `finally` (`p.then` / `typeof p.finally`, and codegen's `p.finally(cb)` // which reads the property first) must resolve the reified Promise prototype diff --git a/crates/perry-runtime/src/object/global_this/proto_methods.rs b/crates/perry-runtime/src/object/global_this/proto_methods.rs index 86701fd892..0cdb8f8b4d 100644 --- a/crates/perry-runtime/src/object/global_this/proto_methods.rs +++ b/crates/perry-runtime/src/object/global_this/proto_methods.rs @@ -77,6 +77,13 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: install_noop_proto_methods(proto_obj, OBJECT_PROTO_METHODS); return; } + // #7947: WeakRef / FinalizationRegistry prototypes get brand-checking + // thunks, so `WeakRef.prototype.deref.call(wr)`, `wr.deref.bind(wr)` and + // `typeof wr.deref` resolve instead of answering `undefined`. + if super::super::weakref_proto_thunks::install_weakref_proto_methods(builtin_name, proto_obj) { + install_noop_proto_methods(proto_obj, OBJECT_PROTO_METHODS); + return; + } // #4795: TC39 explicit-resource-management stacks. if super::super::disposable_proto_thunks::install_disposable_proto_methods( builtin_name, diff --git a/crates/perry-runtime/src/object/instanceof.rs b/crates/perry-runtime/src/object/instanceof.rs index 76ced8987e..d932a12088 100644 --- a/crates/perry-runtime/src/object/instanceof.rs +++ b/crates/perry-runtime/src/object/instanceof.rs @@ -1369,7 +1369,7 @@ pub extern "C" fn js_instanceof(value: f64, class_id: u32) -> f64 { const CLASS_ID_WEAKMAP_RESERVED: u32 = 0xFFFF002C; const CLASS_ID_WEAKSET_RESERVED: u32 = 0xFFFF002D; if class_id == CLASS_ID_WEAKMAP_RESERVED { - return if crate::weakref::weak_class_id_from_receiver(value) + return if crate::object::weak_class_id_from_receiver(value) == Some(crate::weakref::CLASS_ID_WEAKMAP) { true_val @@ -1378,7 +1378,7 @@ pub extern "C" fn js_instanceof(value: f64, class_id: u32) -> f64 { }; } if class_id == CLASS_ID_WEAKSET_RESERVED { - return if crate::weakref::weak_class_id_from_receiver(value) + return if crate::object::weak_class_id_from_receiver(value) == Some(crate::weakref::CLASS_ID_WEAKSET) { true_val diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 16f7a8a5e8..610fce245e 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -141,6 +141,7 @@ mod temporal_proto; mod typed_array_define; mod typed_array_proto_thunks; mod util_types; +mod weakref_proto_thunks; mod websocket_global; mod with_env; // Issue #1103 follow-up: behavior-preserving split of the residual top-level @@ -197,6 +198,12 @@ pub(crate) use typed_array_define::{ TypedArrayOwnIndex, }; pub use util_types::*; +// #7947: weak-wrapper method dispatch (moved out of `weakref.rs`, which is at +// the 2000-line gate) plus the WeakRef/FinalizationRegistry arms and thunks. +pub use weakref_proto_thunks::{ + delegate_if_not_weak_collection, dispatch_foreign_weak_receiver, is_weak_wrapper, + try_weak_method_dispatch, weak_class_id_from_receiver, weak_wrapper_class_id, +}; pub use with_env::*; // Re-exports for the residual-helper split (issue #1103 follow-up). Explicit // named re-exports keep existing `crate::object::X` / bare-name call sites in diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index d3a8dfa5ab..262e9eaa89 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -1774,7 +1774,7 @@ pub unsafe extern "C" fn js_native_call_method( // add to the js_weak* helpers instead of throwing "has is not a // function". The class_id guard + routing live in weakref.rs. if let Some(r) = - crate::weakref::try_weak_method_dispatch(obj, object(), method_name, args_ptr, args_len) + crate::object::try_weak_method_dispatch(obj, object(), method_name, args_ptr, args_len) { return r; } diff --git a/crates/perry-runtime/src/object/object_ops/prototype.rs b/crates/perry-runtime/src/object/object_ops/prototype.rs index 46970617d4..c9c60f6a63 100644 --- a/crates/perry-runtime/src/object/object_ops/prototype.rs +++ b/crates/perry-runtime/src/object/object_ops/prototype.rs @@ -256,7 +256,7 @@ pub extern "C" fn js_object_get_prototype_of(obj_value: f64) -> f64 { // comparing `class_id`, matching the `is_registered_map`/ // `is_registered_set` safety bar above. let receiver = crate::value::js_nanbox_pointer(addr as i64); - if let Some(class_id) = crate::weakref::weak_class_id_from_receiver(receiver) { + if let Some(class_id) = crate::object::weak_class_id_from_receiver(receiver) { let name = if class_id == crate::weakref::CLASS_ID_WEAKMAP { "WeakMap" } else { diff --git a/crates/perry-runtime/src/object/to_string_tag.rs b/crates/perry-runtime/src/object/to_string_tag.rs index dae554ff02..c85c93af03 100644 --- a/crates/perry-runtime/src/object/to_string_tag.rs +++ b/crates/perry-runtime/src/object/to_string_tag.rs @@ -231,11 +231,16 @@ pub unsafe extern "C" fn js_object_to_string(value: f64) -> f64 { return f64::from_bits(STRING_TAG | (str_ptr as u64 & POINTER_MASK)); } } - if let Some(cid) = crate::weakref::weak_class_id_from_receiver(value) { - let tag = if cid == crate::weakref::CLASS_ID_WEAKSET { - "WeakSet" - } else { - "WeakMap" + // #7947: `WeakRef` / `FinalizationRegistry` were missing here, so + // `Object.prototype.toString.call(new WeakRef(x))` answered + // `[object Object]` instead of `[object WeakRef]`. Same reserved-class_id + // question as WeakMap/WeakSet, just two more arms. + if let Some(cid) = crate::object::weak_wrapper_class_id(value) { + let tag = match cid { + crate::weakref::CLASS_ID_WEAKSET => "WeakSet", + crate::weakref::CLASS_ID_WEAKREF => "WeakRef", + crate::weakref::CLASS_ID_FINALIZATION_REGISTRY => "FinalizationRegistry", + _ => "WeakMap", }; let formatted = format!("[object {}]", tag); let str_ptr = diff --git a/crates/perry-runtime/src/object/weakref_proto_thunks.rs b/crates/perry-runtime/src/object/weakref_proto_thunks.rs new file mode 100644 index 0000000000..ddd86f413c --- /dev/null +++ b/crates/perry-runtime/src/object/weakref_proto_thunks.rs @@ -0,0 +1,524 @@ +//! Weak-wrapper method dispatch: `WeakMap` / `WeakSet` (moved here from +//! `weakref.rs`, which is at the 2000-line gate) plus the `WeakRef` / +//! `FinalizationRegistry` arms and prototype thunks added by #7947. +//! +//! ## Why this module exists (#7947) +//! +//! `WeakMap.prototype.get` & friends have always had two independent routes: an +//! HIR fast path that folds `wm.get(k)` straight to `js_weakmap_get`, and this +//! dynamic route, reached from `js_native_call_method` whenever the receiver is +//! anything the fold could not recognise. `WeakRef.prototype.deref` and +//! `FinalizationRegistry.prototype.register`/`.unregister` had **only** the +//! fold, and the fold keys on a bare local NAME recorded by +//! `pre_scan_weakref_locals`. Every other receiver shape — an array element, an +//! object property, a call result, a `for…of` binding, a function parameter, or +//! a `const r = new WeakRef(x)` inside an *arrow function* (the pre-scan +//! descends into function DECLARATIONS only) — resolved nothing and threw +//! `TypeError: deref is not a function`. Two of twenty receiver shapes worked. +//! +//! Three additions close that: +//! +//! * `try_weak_method_dispatch` gains `CLASS_ID_WEAKREF` / +//! `CLASS_ID_FINALIZATION_REGISTRY` arms, so a *call* on any receiver shape +//! reaches the runtime helper; +//! * `install_weakref_proto_methods` installs brand-checking +//! `WeakRef.prototype.deref` / `FinalizationRegistry.prototype.{register, +//! unregister}` thunks, so the reflective path (`.call`/`.apply`, method +//! extraction, `typeof wr.deref`) works and brand-checks `this`; +//! * `dispatch_foreign_weak_receiver` gives the *fold* a safe landing when its +//! name-keyed guess was wrong — see below. +//! +//! ## The fold's landing pad (#7948) +//! +//! `pre_scan_weakref_locals` is name-keyed and scope-blind, so a module that +//! binds `const r = new WeakRef(x)` anywhere folds EVERY `r.deref()` in that +//! module onto `js_weakref_deref` — including an `r` that is a plain object, a +//! user class instance, or a function parameter. `js_weakref_deref` used to +//! read its internal slot by name off whatever it was handed and answer +//! `undefined`: a silent wrong answer, exit code 0. The helpers now brand-check +//! their receiver and hand a foreign one to `dispatch_foreign_weak_receiver`, +//! which re-enters ordinary dynamic method dispatch. A mis-fold therefore +//! degrades to the correct slow path instead of a wrong answer. Recursion is +//! impossible: `js_native_call_method` only routes back into these helpers when +//! the receiver's `class_id` IS the weak wrapper's, which is exactly the case +//! the brand check accepts. + +use super::*; +use crate::weakref::{ + js_finreg_register, js_finreg_unregister, js_weakmap_delete, js_weakmap_get, js_weakmap_has, + js_weakmap_set, js_weakref_deref, js_weakset_add, CLASS_ID_FINALIZATION_REGISTRY, + CLASS_ID_WEAKMAP, CLASS_ID_WEAKREF, CLASS_ID_WEAKSET, +}; + +/// Dynamic-dispatch entry point for weak-wrapper method calls (issues +/// #1757/#1758 for WeakMap/WeakSet, #7947 for WeakRef/FinalizationRegistry). +/// `js_native_call_method` calls this for any heap object; it returns +/// `Some(result)` only when `obj` carries one of the reserved weak `class_id`s +/// and `method_name` is one of *that class's own* methods, and `None` otherwise +/// so the caller falls through to its normal dispatch. `receiver` is the +/// NaN-boxed f64 the `js_weak*` / `js_finreg_*` helpers expect. +/// +/// A method that isn't one of the receiver's own (e.g. `"add"` on a WeakMap, +/// `"deref"` on a WeakSet, or any name outside the per-class sets) falls +/// through to `None` so the ordinary property lookup resolves it — correctly +/// missing and raising `TypeError: ... is not a function` on a call, rather +/// than this function silently answering `undefined`. +/// +/// # Safety +/// `obj` must be a valid, readable `ObjectHeader` pointer (the caller has +/// already validated it as a live heap object). +pub unsafe fn try_weak_method_dispatch( + obj: *const ObjectHeader, + receiver: f64, + method_name: &str, + args_ptr: *const f64, + args_len: usize, +) -> Option { + let class_id = (*obj).class_id; + if !matches!( + class_id, + CLASS_ID_WEAKMAP | CLASS_ID_WEAKSET | CLASS_ID_WEAKREF | CLASS_ID_FINALIZATION_REGISTRY + ) { + return None; + } + let args: &[f64] = if !args_ptr.is_null() && args_len > 0 { + std::slice::from_raw_parts(args_ptr, args_len) + } else { + &[] + }; + // #5834: dispatch regardless of arg count, padding missing positions with + // `undefined` — mirrors calling the real thunks reflectively. Arity-gating + // these arms let `s.add()` (zero args) fall through to a no-op, skipping + // `js_weakset_add`'s CanBeHeldWeakly check entirely (it must throw + // `TypeError` since `undefined` cannot be held weakly). + // + // Also gate each method by the receiver's actual class: `"set"`/`"get"` + // only exist on WeakMap, `"add"` only on WeakSet, `"deref"` only on + // WeakRef, `"register"`/`"unregister"` only on FinalizationRegistry + // (`"has"`/`"delete"` are shared by WeakMap and WeakSet). Without this a + // WeakMap receiver could reach `js_weakset_add` for a `.add(...)` call (and + // vice versa) instead of falling through to the ordinary property lookup, + // which correctly resolves the missing method to `undefined` and throws + // `TypeError: ... is not a function`. + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + let arg = |i: usize| args.get(i).copied().unwrap_or(undef); + let result = match (method_name, class_id) { + ("set", CLASS_ID_WEAKMAP) => js_weakmap_set(receiver, arg(0), arg(1)), + ("add", CLASS_ID_WEAKSET) => js_weakset_add(receiver, arg(0)), + ("get", CLASS_ID_WEAKMAP) => js_weakmap_get(receiver, arg(0)), + ("has", CLASS_ID_WEAKMAP | CLASS_ID_WEAKSET) => js_weakmap_has(receiver, arg(0)), + ("delete", CLASS_ID_WEAKMAP | CLASS_ID_WEAKSET) => js_weakmap_delete(receiver, arg(0)), + // #7947: `deref` / `register` / `unregister` previously existed only as + // the name-keyed HIR fold, so every receiver shape it could not name + // threw `TypeError: deref is not a function`. + ("deref", CLASS_ID_WEAKREF) => js_weakref_deref(receiver), + ("register", CLASS_ID_FINALIZATION_REGISTRY) => { + js_finreg_register(receiver, arg(0), arg(1), arg(2)) + } + ("unregister", CLASS_ID_FINALIZATION_REGISTRY) => js_finreg_unregister(receiver, arg(0)), + _ => return None, + }; + Some(result) +} + +/// Return the reserved WeakMap/WeakSet `class_id` of `receiver` if it is one +/// of those collections, else `None`. Backs the reflective +/// `WeakMap.prototype.*` / `WeakSet.prototype.*` thunks so they can perform +/// the spec brand check (`TypeError` on a non-Weak* receiver) before +/// dispatching. +/// +/// Deliberately does NOT admit `CLASS_ID_WEAKREF` / +/// `CLASS_ID_FINALIZATION_REGISTRY` — callers branch on "WeakMap else WeakSet", +/// so widening it would let a `WeakRef` pass a `WeakSet` brand check. Use +/// [`weak_wrapper_class_id`] for the four-way question. +pub fn weak_class_id_from_receiver(receiver: f64) -> Option { + match weak_wrapper_class_id(receiver) { + Some(cid @ (CLASS_ID_WEAKMAP | CLASS_ID_WEAKSET)) => Some(cid), + _ => None, + } +} + +/// Return the reserved weak-wrapper `class_id` of `receiver` — WeakMap, +/// WeakSet, WeakRef or FinalizationRegistry — else `None`. +/// +/// The `GcHeader.obj_type == GC_TYPE_OBJECT` pre-filter ensures the pointer is +/// an `ObjectHeader`-backed allocation before `class_id` is read, so a +/// `Set`/`Map` pointer (different `obj_type`) or a primitive +/// (`js_nanbox_get_pointer` yields 0) safely resolves to `None`. +pub fn weak_wrapper_class_id(receiver: f64) -> Option { + let addr = crate::value::js_nanbox_get_pointer(receiver) as usize; + // #4004: reject the small-handle band (Web Fetch / node:http / timer ids + // are NaN-boxed POINTER_TAG values, not heap addresses) before + // dereferencing the GC header. The weak wrappers are ObjectHeader-backed + // allocations above the cutoff. See `value::addr_class` for the band map. + unsafe { + match crate::value::addr_class::try_read_gc_header(addr) { + Some(header) if header.obj_type == crate::gc::GC_TYPE_OBJECT => {} + _ => return None, + } + let cid = (*(addr as *const ObjectHeader)).class_id; + if matches!( + cid, + CLASS_ID_WEAKMAP | CLASS_ID_WEAKSET | CLASS_ID_WEAKREF | CLASS_ID_FINALIZATION_REGISTRY + ) { + return Some(cid); + } + } + None +} + +/// True when `receiver` is a genuine instance of the weak wrapper `class_id`. +/// The brand check the folded fast-path helpers run before trusting the HIR's +/// name-keyed guess (#7948). +pub fn is_weak_wrapper(receiver: f64, class_id: u32) -> bool { + weak_wrapper_class_id(receiver) == Some(class_id) +} + +/// The `WeakMap`/`WeakSet` form of the brand-check-and-delegate above, as one +/// call: returns `Some(result_of_the_receivers_own_method)` when `receiver` is +/// NOT a genuine weak collection, and `None` when the caller should proceed. +/// +/// `WeakMap` and `WeakSet` share three helpers (`js_weakset_has`/`_delete` +/// delegate to `js_weakmap_has`/`_delete`, and `js_weakset_add` to +/// `js_weakmap_set`), so the check admits either class id rather than the exact +/// one — the per-class method gating lives in [`try_weak_method_dispatch`] and +/// in the prototype thunks, both of which run before these helpers. +pub fn delegate_if_not_weak_collection( + receiver: f64, + method_name: &str, + args: &[f64], +) -> Option { + match weak_wrapper_class_id(receiver) { + Some(CLASS_ID_WEAKMAP | CLASS_ID_WEAKSET) => None, + _ => Some(dispatch_foreign_weak_receiver(receiver, method_name, args)), + } +} + +/// Re-dispatch `method_name` on a receiver the HIR fold mis-identified as a +/// weak wrapper (#7948). Routes back through ordinary dynamic method dispatch, +/// which resolves the receiver's own method — or, when there is none, throws +/// the same `TypeError: is not a function` node throws. +/// +/// Not reachable in a loop: `js_native_call_method` routes into the weak +/// helpers only via [`try_weak_method_dispatch`], which requires the reserved +/// `class_id` the brand check already rejected. +pub fn dispatch_foreign_weak_receiver(receiver: f64, method_name: &str, args: &[f64]) -> f64 { + unsafe { + super::js_native_call_method( + receiver, + method_name.as_ptr() as *const i8, + method_name.len(), + if args.is_empty() { + std::ptr::null() + } else { + args.as_ptr() + }, + args.len(), + ) + } +} + +// --- prototype thunks (#7947) -------------------------------------------- + +fn throw_incompatible(proto: &str, method: &str) -> ! { + let msg = format!("Method {proto}.{method} called on incompatible receiver"); + 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( + crate::value::JSValue::pointer(err as *const u8).bits(), + )) +} + +/// Resolve `IMPLICIT_THIS` to a receiver of the expected weak-wrapper class id, +/// or throw a `TypeError`. Mirrors `collection_proto_thunks`' +/// `weak_receiver_or_throw` for the WeakRef/FinalizationRegistry pair. +fn wrapper_receiver_or_throw(expected: u32, proto: &str, method: &str) -> f64 { + let receiver = f64::from_bits(IMPLICIT_THIS.with(|c| c.get())); + if is_weak_wrapper(receiver, expected) { + receiver + } else { + throw_incompatible(proto, method) + } +} + +pub(super) extern "C" fn weakref_proto_deref_thunk( + _c: *const crate::closure::ClosureHeader, +) -> f64 { + let r = wrapper_receiver_or_throw(CLASS_ID_WEAKREF, "WeakRef.prototype", "deref"); + js_weakref_deref(r) +} + +pub(super) extern "C" fn finreg_proto_register_thunk( + _c: *const crate::closure::ClosureHeader, + target: f64, + held: f64, + token: f64, +) -> f64 { + let r = wrapper_receiver_or_throw( + CLASS_ID_FINALIZATION_REGISTRY, + "FinalizationRegistry.prototype", + "register", + ); + js_finreg_register(r, target, held, token) +} + +pub(super) extern "C" fn finreg_proto_unregister_thunk( + _c: *const crate::closure::ClosureHeader, + token: f64, +) -> f64 { + let r = wrapper_receiver_or_throw( + CLASS_ID_FINALIZATION_REGISTRY, + "FinalizationRegistry.prototype", + "unregister", + ); + js_finreg_unregister(r, token) +} + +/// Install the brand-checking `.prototype` methods for `WeakRef` / +/// `FinalizationRegistry`. Returns `true` when `builtin_name` is one of those — +/// the caller then adds the shared `OBJECT_PROTO_METHODS` — and `false` +/// otherwise. Called from `global_this::populate_builtin_prototype_methods`. +/// +/// Arities are the spec `.length` values: `deref` 0, `register` 2 (the +/// unregister token is optional and does not count), `unregister` 1. +pub(super) fn install_weakref_proto_methods( + builtin_name: &str, + proto_obj: *mut ObjectHeader, +) -> bool { + use super::global_this::install_proto_method as ipm; + match builtin_name { + "WeakRef" => { + ipm( + proto_obj, + "deref", + weakref_proto_deref_thunk as *const u8, + 0, + ); + } + "FinalizationRegistry" => { + ipm( + proto_obj, + "register", + finreg_proto_register_thunk as *const u8, + 2, + ); + ipm( + proto_obj, + "unregister", + finreg_proto_unregister_thunk as *const u8, + 1, + ); + } + _ => return false, + } + true +} + +/// Resolve a `WeakRef`/`FinalizationRegistry` prototype method to the SAME +/// brand-checking thunk value installed on `.prototype`, so a VALUE +/// read off an *instance* (`typeof wr.deref`, `wr.deref.bind(wr)`, +/// `const d = wr.deref`) yields a function rather than `undefined`. Mirrors +/// `collection_proto_thunks::collection_proto_method_value`, which does this +/// for WeakMap/WeakSet. +/// +/// Returns `None` for a receiver that is not one of the two wrappers, or a +/// name that is not one of its methods, so callers keep their existing path. +pub(crate) fn weakref_proto_method_value_for(receiver_cid: u32, method_name: &str) -> Option { + let builtin = match (receiver_cid, method_name) { + (CLASS_ID_WEAKREF, "deref") => "WeakRef", + (CLASS_ID_FINALIZATION_REGISTRY, "register" | "unregister") => "FinalizationRegistry", + _ => return None, + }; + let proto = super::global_this::builtin_prototype_value(builtin); + if proto.to_bits() == crate::value::TAG_UNDEFINED { + return None; + } + let proto_ptr = crate::value::js_nanbox_get_pointer(proto) as *mut ObjectHeader; + if proto_ptr.is_null() { + return None; + } + let key = crate::string::js_string_from_bytes(method_name.as_ptr(), method_name.len() as u32); + unsafe { super::own_data_field_by_name(proto_ptr, key) } + .map(|value| f64::from_bits(value.bits())) + .filter(|v| v.to_bits() != crate::value::TAG_UNDEFINED) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::value::JSValue; + + /// Sentinel a foreign receiver's OWN method returns. Distinct from + /// `undefined`, which is exactly what the un-brand-checked helpers used to + /// answer, so the assertions below cannot pass by accident. + const FOREIGN_SENTINEL: i32 = 7947; + + extern "C" fn foreign_method_thunk(_c: *const crate::closure::ClosureHeader) -> f64 { + f64::from_bits(JSValue::int32(FOREIGN_SENTINEL).bits()) + } + + extern "C" fn foreign_method_thunk_1(_c: *const crate::closure::ClosureHeader, _a: f64) -> f64 { + f64::from_bits(JSValue::int32(FOREIGN_SENTINEL).bits()) + } + + /// A plain object carrying its own `method_name` function property — the + /// shape a name-collided fold hands to the weak helpers (`{ deref: … }`, + /// `class Cache { deref() {…} }`, an array with `.deref` attached, or a + /// function parameter). + fn plain_object_with_method(method_name: &str, func_ptr: *const u8, arity: u32) -> f64 { + let obj = crate::object::js_object_alloc(0, 0); + let closure = crate::closure::js_closure_alloc(func_ptr, 0); + assert!(!closure.is_null(), "closure alloc failed"); + crate::closure::js_register_closure_arity(func_ptr, arity); + let key = + crate::string::js_string_from_bytes(method_name.as_ptr(), method_name.len() as u32); + let value = crate::value::js_nanbox_pointer(closure as i64); + crate::object::js_object_set_field_by_name(obj, key, value); + f64::from_bits(JSValue::pointer(obj as *const u8).bits()) + } + + /// #7948: the HIR fold is keyed by bare local NAME with no scope + /// discrimination, so a module holding one genuine `new WeakRef(x)` folds + /// EVERY same-named `.deref()` onto `js_weakref_deref`. Before the brand + /// check, that read `__perry_wr_target` by name off the foreign object and + /// answered `undefined` — a wrong answer with exit code 0. + /// + /// Asserts the SUBJECT is live, not merely that nothing threw: the foreign + /// receiver's own method must actually run and its sentinel come back. + /// Removing the brand check makes every one of these return `undefined`. + #[test] + fn folded_weak_helpers_delegate_a_foreign_receiver_to_its_own_method() { + let sentinel = JSValue::int32(FOREIGN_SENTINEL).bits(); + + let deref_recv = plain_object_with_method("deref", foreign_method_thunk as *const u8, 0); + assert_eq!( + crate::weakref::js_weakref_deref(deref_recv).to_bits(), + sentinel, + "a foreign receiver's own `deref` must run, not the WeakRef intrinsic" + ); + + let key = f64::from_bits(JSValue::int32(1).bits()); + for (name, ptr) in [ + ("get", foreign_method_thunk_1 as *const u8), + ("has", foreign_method_thunk_1 as *const u8), + ("delete", foreign_method_thunk_1 as *const u8), + ] { + let recv = plain_object_with_method(name, ptr, 1); + let got = match name { + "get" => crate::weakref::js_weakmap_get(recv, key), + "has" => crate::weakref::js_weakmap_has(recv, key), + _ => crate::weakref::js_weakmap_delete(recv, key), + }; + assert_eq!( + got.to_bits(), + sentinel, + "a foreign receiver's own `{name}` must run, not the WeakMap intrinsic" + ); + } + + let add_recv = plain_object_with_method("add", foreign_method_thunk_1 as *const u8, 1); + assert_eq!( + crate::weakref::js_weakset_add(add_recv, key).to_bits(), + sentinel, + "a foreign receiver's own `add` must run, not the WeakSet intrinsic" + ); + } + + /// The other half: a GENUINE wrapper must still take the intrinsic path. + /// Without this, "brand-check everything" could pass by delegating + /// unconditionally, which would break every real weak call. + #[test] + fn genuine_wrappers_still_take_the_intrinsic_path() { + let target = crate::object::js_object_alloc(0, 0); + let target_val = f64::from_bits(JSValue::pointer(target as *const u8).bits()); + let wr = crate::weakref::js_weakref_new(target_val); + let wr_val = f64::from_bits(JSValue::pointer(wr as *const u8).bits()); + assert!( + is_weak_wrapper(wr_val, crate::weakref::CLASS_ID_WEAKREF), + "a real WeakRef must pass its own brand check" + ); + assert_eq!( + crate::weakref::js_weakref_deref(wr_val).to_bits(), + target_val.to_bits(), + "a real WeakRef must still deref to its target" + ); + + let wm = crate::weakref::js_weakmap_new(); + let wm_val = f64::from_bits(JSValue::pointer(wm as *const u8).bits()); + assert!( + delegate_if_not_weak_collection(wm_val, "get", &[]).is_none(), + "a real WeakMap must NOT be delegated away" + ); + let v = f64::from_bits(JSValue::int32(42).bits()); + crate::weakref::js_weakmap_set(wm_val, target_val, v); + assert_eq!( + crate::weakref::js_weakmap_get(wm_val, target_val).to_bits(), + v.to_bits(), + "a real WeakMap must still round-trip through the intrinsic" + ); + + // And the discriminator itself: a plain object is neither. Asserted + // through `weak_wrapper_class_id` rather than + // `delegate_if_not_weak_collection`, because the latter EAGERLY + // performs the delegated call — on a receiver with no `get` that + // re-enters dynamic dispatch and throws node's + // `TypeError: get is not a function`, which is the right production + // behaviour but terminates a unit test. The delegation itself is + // covered by `folded_weak_helpers_delegate_a_foreign_receiver_to_its_own_method`, + // whose receivers DO carry the method. + let plain = crate::object::js_object_alloc(0, 0); + let plain_val = f64::from_bits(JSValue::pointer(plain as *const u8).bits()); + assert_eq!(weak_wrapper_class_id(plain_val), None); + assert!(!is_weak_wrapper( + plain_val, + crate::weakref::CLASS_ID_WEAKMAP + )); + assert!(!is_weak_wrapper( + plain_val, + crate::weakref::CLASS_ID_WEAKREF + )); + } + + /// #7947: `deref` / `register` / `unregister` must be reachable through the + /// dynamic dispatch route, which is what every receiver shape the + /// name-keyed fold cannot see (array element, object property, call result, + /// `for…of` binding, parameter, arrow-function local) resolves through. + #[test] + fn dynamic_dispatch_reaches_weakref_and_finreg_methods() { + let target = crate::object::js_object_alloc(0, 0); + let target_val = f64::from_bits(JSValue::pointer(target as *const u8).bits()); + let wr = crate::weakref::js_weakref_new(target_val); + let wr_val = f64::from_bits(JSValue::pointer(wr as *const u8).bits()); + + let got = unsafe { + try_weak_method_dispatch( + wr as *const ObjectHeader, + wr_val, + "deref", + std::ptr::null(), + 0, + ) + }; + assert_eq!( + got.map(|v| v.to_bits()), + Some(target_val.to_bits()), + "WeakRef.deref must be reachable through dynamic dispatch (#7947)" + ); + + // A method that is not the receiver's own must still fall through to + // `None` so ordinary lookup raises `TypeError: … is not a function`. + let not_mine = unsafe { + try_weak_method_dispatch( + wr as *const ObjectHeader, + wr_val, + "add", + std::ptr::null(), + 0, + ) + }; + assert!( + not_mine.is_none(), + "`add` is not a WeakRef method — must fall through, not dispatch" + ); + } +} diff --git a/crates/perry-runtime/src/weakref.rs b/crates/perry-runtime/src/weakref.rs index 5562a700b3..4a4b5f19ae 100644 --- a/crates/perry-runtime/src/weakref.rs +++ b/crates/perry-runtime/src/weakref.rs @@ -419,6 +419,17 @@ pub extern "C" fn js_weakref_new(target: f64) -> *mut ObjectHeader { /// cleared by GC. #[no_mangle] pub extern "C" fn js_weakref_deref(weakref: f64) -> f64 { + // #7948: the HIR fold that reaches here is keyed by BARE LOCAL NAME with no + // scope discrimination, so a module holding `const r = new WeakRef(x)` + // anywhere folds EVERY `r.deref()` onto this helper — including an `r` that + // is a user class instance, an object literal or a function parameter. + // Reading the internal slot by name off a foreign object answered + // `undefined`: a wrong answer with exit code 0. Brand-check first and hand + // a foreign receiver back to ordinary dynamic dispatch, which resolves the + // receiver's own `deref` (or throws `deref is not a function` like node). + if !crate::object::is_weak_wrapper(weakref, CLASS_ID_WEAKREF) { + return crate::object::dispatch_foreign_weak_receiver(weakref, "deref", &[]); + } let ptr = js_nanbox_get_pointer(weakref) as *mut ObjectHeader; if ptr.is_null() { return f64::from_bits(TAG_UNDEFINED); @@ -516,6 +527,16 @@ fn js_finreg_record_new(target: f64, held: f64, token: f64) -> *mut ObjectHeader /// produce that token value anyway. #[no_mangle] pub extern "C" fn js_finreg_register(registry: f64, target: f64, held: f64, token: f64) -> f64 { + // #7948: brand-check the receiver BEFORE the argument checks — the fold + // that reaches here is name-keyed (see `js_weakref_deref`), so `registry` + // may be an unrelated object whose own `register` the program meant. + if !crate::object::is_weak_wrapper(registry, CLASS_ID_FINALIZATION_REGISTRY) { + return crate::object::dispatch_foreign_weak_receiver( + registry, + "register", + &[target, held, token], + ); + } if !is_valid_weak_target(target) { weakref_type_error("FinalizationRegistry.prototype.register: invalid target"); } @@ -571,6 +592,11 @@ pub extern "C" fn js_finreg_register(registry: f64, target: f64, held: f64, toke /// references — both sides are stored as POINTER_TAG-tagged f64 values. #[no_mangle] pub extern "C" fn js_finreg_unregister(registry: f64, token: f64) -> f64 { + // #7948: brand-check the receiver before the token check (see + // `js_finreg_register`). + if !crate::object::is_weak_wrapper(registry, CLASS_ID_FINALIZATION_REGISTRY) { + return crate::object::dispatch_foreign_weak_receiver(registry, "unregister", &[token]); + } if !is_valid_weak_target(token) { weakref_type_error("Invalid unregisterToken"); } @@ -1325,90 +1351,6 @@ pub const CLASS_ID_WEAKSET: u32 = 0xFFFF_0028; /// safe. Refs #6120. pub(crate) const WEAK_ENTRIES_KEY: &[u8] = b"__perry_wk_entries"; -/// Dynamic-dispatch entry point for WeakMap/WeakSet method calls (issue -/// #1757/#1758). `js_native_call_method` calls this for any heap object; -/// it returns `Some(result)` only when `obj` carries the reserved -/// WeakMap/WeakSet `class_id` and `method_name` is one of *that class's own* -/// methods, and `None` otherwise so the caller falls through to its normal -/// dispatch. `receiver` is the NaN-boxed f64 the `js_weak*` helpers expect. -/// -/// A method that isn't one of the receiver's own (e.g. `"add"` on a WeakMap, -/// or any name outside `set`/`add`/`get`/`has`/`delete`) falls through to -/// `None` so the ordinary property lookup resolves it — correctly missing -/// and raising `TypeError: ... is not a function` on a call, rather than -/// this function silently answering `undefined`. -/// -/// # Safety -/// `obj` must be a valid, readable `ObjectHeader` pointer (the caller has -/// already validated it as a live heap object). -pub unsafe fn try_weak_method_dispatch( - obj: *const ObjectHeader, - receiver: f64, - method_name: &str, - args_ptr: *const f64, - args_len: usize, -) -> Option { - let class_id = (*obj).class_id; - if class_id != CLASS_ID_WEAKMAP && class_id != CLASS_ID_WEAKSET { - return None; - } - let args: &[f64] = if !args_ptr.is_null() && args_len > 0 { - std::slice::from_raw_parts(args_ptr, args_len) - } else { - &[] - }; - // #5834: dispatch regardless of arg count, padding missing positions with - // `undefined` — mirrors calling the real thunks reflectively. Arity-gating - // these arms let `s.add()` (zero args) fall through to a no-op, skipping - // `js_weakset_add`'s CanBeHeldWeakly check entirely (it must throw - // `TypeError` since `undefined` cannot be held weakly). - // - // Also gate each method by the receiver's actual class: `"set"`/`"get"` - // only exist on WeakMap, `"add"` only on WeakSet (`"has"`/`"delete"` are - // shared). Without this a WeakMap receiver could reach `js_weakset_add` - // for a `.add(...)` call (and vice versa) instead of falling through to - // the ordinary property lookup, which correctly resolves the missing - // method to `undefined` and throws `TypeError: ... is not a function`. - let undef = f64::from_bits(TAG_UNDEFINED); - let arg = |i: usize| args.get(i).copied().unwrap_or(undef); - let result = match (method_name, class_id) { - ("set", CLASS_ID_WEAKMAP) => js_weakmap_set(receiver, arg(0), arg(1)), - ("add", CLASS_ID_WEAKSET) => js_weakset_add(receiver, arg(0)), - ("get", CLASS_ID_WEAKMAP) => js_weakmap_get(receiver, arg(0)), - ("has", _) => js_weakmap_has(receiver, arg(0)), - ("delete", _) => js_weakmap_delete(receiver, arg(0)), - _ => return None, - }; - Some(result) -} - -/// Return the reserved WeakMap/WeakSet `class_id` of `receiver` if it is one -/// of those collections, else `None`. Backs the reflective -/// `WeakMap.prototype.*` / `WeakSet.prototype.*` thunks so they can perform -/// the spec brand check (`TypeError` on a non-Weak* receiver) before -/// dispatching. The `GcHeader.obj_type == GC_TYPE_OBJECT` pre-filter ensures -/// the pointer is an `ObjectHeader`-backed allocation before `class_id` is -/// read, so a `Set`/`Map` pointer (different `obj_type`) or a primitive -/// (`js_nanbox_get_pointer` yields 0) safely resolves to `None`. -pub fn weak_class_id_from_receiver(receiver: f64) -> Option { - let addr = js_nanbox_get_pointer(receiver) as usize; - // #4004: reject the small-handle band (Web Fetch / node:http / timer ids - // are NaN-boxed POINTER_TAG values, not heap addresses) before - // dereferencing the GC header. WeakMap/WeakSet are ObjectHeader-backed - // allocations above the cutoff. See `value::addr_class` for the band map. - unsafe { - match crate::value::addr_class::try_read_gc_header(addr) { - Some(header) if header.obj_type == crate::gc::GC_TYPE_OBJECT => {} - _ => return None, - } - let cid = (*(addr as *const ObjectHeader)).class_id; - if cid == CLASS_ID_WEAKMAP || cid == CLASS_ID_WEAKSET { - return Some(cid); - } - } - None -} - unsafe fn entries_array(reg: *mut ObjectHeader) -> *mut ArrayHeader { // #6136: `js_string_from_bytes` allocates and can fire a moving minor GC, // which relocates the (movable, GcHeader-backed) WeakMap/WeakSet `reg`. @@ -1550,6 +1492,15 @@ fn throw_invalid_weakset_value() -> ! { #[no_mangle] pub extern "C" fn js_weakmap_set(map: f64, key: f64, value: f64) -> f64 { + // #7948: brand-check the receiver — the HIR fold that reaches here is keyed + // by BARE LOCAL NAME with no scope discrimination, so `map` may be an + // unrelated object (a literal, a user class instance, a parameter) whose own + // `set` the program meant. Reading the weak entries array by name off a + // foreign object answered `undefined`/`false` — a wrong answer with exit + // code 0. Hand it back to ordinary dynamic dispatch instead. + if let Some(v) = crate::object::delegate_if_not_weak_collection(map, "set", &[key, value]) { + return v; + } // #2772: WeakMap keys must be values that "CanBeHeldWeakly" (ES2023): // objects/handles AND non-registered Symbols (a fresh `Symbol()` or a // well-known symbol). Only `Symbol.for(...)` registered symbols, and @@ -1657,6 +1608,15 @@ pub extern "C" fn js_weakmap_set(map: f64, key: f64, value: f64) -> f64 { #[no_mangle] pub extern "C" fn js_weakmap_get(map: f64, key: f64) -> f64 { + // #7948: brand-check the receiver — the HIR fold that reaches here is keyed + // by BARE LOCAL NAME with no scope discrimination, so `map` may be an + // unrelated object (a literal, a user class instance, a parameter) whose own + // `get` the program meant. Reading the weak entries array by name off a + // foreign object answered `undefined`/`false` — a wrong answer with exit + // code 0. Hand it back to ordinary dynamic dispatch instead. + if let Some(v) = crate::object::delegate_if_not_weak_collection(map, "get", &[key]) { + return v; + } let map_ptr = js_nanbox_get_pointer(map) as *mut ObjectHeader; if map_ptr.is_null() { return f64::from_bits(TAG_UNDEFINED); @@ -1690,6 +1650,15 @@ pub extern "C" fn js_weakmap_get(map: f64, key: f64) -> f64 { #[no_mangle] pub extern "C" fn js_weakmap_has(map: f64, key: f64) -> f64 { + // #7948: brand-check the receiver — the HIR fold that reaches here is keyed + // by BARE LOCAL NAME with no scope discrimination, so `map` may be an + // unrelated object (a literal, a user class instance, a parameter) whose own + // `has` the program meant. Reading the weak entries array by name off a + // foreign object answered `undefined`/`false` — a wrong answer with exit + // code 0. Hand it back to ordinary dynamic dispatch instead. + if let Some(v) = crate::object::delegate_if_not_weak_collection(map, "has", &[key]) { + return v; + } let map_ptr = js_nanbox_get_pointer(map) as *mut ObjectHeader; if map_ptr.is_null() { return f64::from_bits(TAG_FALSE); @@ -1720,6 +1689,15 @@ pub extern "C" fn js_weakmap_has(map: f64, key: f64) -> f64 { #[no_mangle] pub extern "C" fn js_weakmap_delete(map: f64, key: f64) -> f64 { + // #7948: brand-check the receiver — the HIR fold that reaches here is keyed + // by BARE LOCAL NAME with no scope discrimination, so `map` may be an + // unrelated object (a literal, a user class instance, a parameter) whose own + // `delete` the program meant. Reading the weak entries array by name off a + // foreign object answered `undefined`/`false` — a wrong answer with exit + // code 0. Hand it back to ordinary dynamic dispatch instead. + if let Some(v) = crate::object::delegate_if_not_weak_collection(map, "delete", &[key]) { + return v; + } if js_nanbox_get_pointer(map) == 0 { return f64::from_bits(TAG_FALSE); } @@ -1862,6 +1840,15 @@ pub extern "C" fn js_weakset_init_iterable(set: f64, iterable: f64) -> f64 { #[no_mangle] pub extern "C" fn js_weakset_add(set: f64, value: f64) -> f64 { + // #7948: brand-check the receiver — the HIR fold that reaches here is keyed + // by BARE LOCAL NAME with no scope discrimination, so `set` may be an + // unrelated object (a literal, a user class instance, a parameter) whose own + // `add` the program meant. Reading the weak entries array by name off a + // foreign object answered `undefined`/`false` — a wrong answer with exit + // code 0. Hand it back to ordinary dynamic dispatch instead. + if let Some(v) = crate::object::delegate_if_not_weak_collection(set, "add", &[value]) { + return v; + } // #2772: WeakSet members must "CanBeHeldWeakly" (ES2023): objects/handles // AND non-registered Symbols. Throw the WeakSet-specific message *before* // delegating (js_weakmap_set throws the weak-map-key message, which is wrong diff --git a/test-files/test_gap_weakref_receiver_shapes_7947.ts b/test-files/test_gap_weakref_receiver_shapes_7947.ts new file mode 100644 index 0000000000..8e77699916 --- /dev/null +++ b/test-files/test_gap_weakref_receiver_shapes_7947.ts @@ -0,0 +1,248 @@ +// #7947: `WeakRef.prototype.deref` and `FinalizationRegistry.prototype. +// register`/`.unregister` existed ONLY as an HIR fold keyed on a bare local +// NAME recorded by `pre_scan_weakref_locals`. Unlike WeakMap/WeakSet they had +// no runtime method-dispatch fallback and no prototype thunks, so exactly two +// receiver shapes worked — a `const x = new WeakRef(...)` at module top level +// or inside a function DECLARATION. Every other shape threw +// `TypeError: deref is not a function`: an array element, an object property, +// a call result, a `for…of` binding, a function parameter, a `.map` callback, +// and any binding inside an arrow function / function expression / class method +// (the pre-scan descends into function declarations only). +// +// #7948: the fold is also scope-blind, so a module holding one genuine +// `new WeakRef(x)` under some name folded EVERY same-named receiver's +// `.deref()` onto `js_weakref_deref`, which read its internal slot by name off +// the foreign object and answered `undefined` — a wrong answer with exit code +// 0. The helpers now brand-check and fall back to ordinary dynamic dispatch. +// +// The `WeakMap`/`WeakSet` halves already worked through every receiver shape +// (they have both the dispatch arm and the prototype thunks) and are asserted +// here so a future refactor of the shared dispatch cannot silently drop them — +// but their #7948 collision half was equally broken, with far more common +// method names (`get`/`set`/`has`/`add`/`delete`), and is asserted too. +// +// Boundary this test pins DELIBERATELY as still-broken (so nobody reads a green +// run as coverage): weak-wrapper SUBCLASSING — `class M extends WeakMap {}` and +// the WeakRef/WeakSet/FinalizationRegistry equivalents — throws on `main` both +// before and after this change, and is out of scope here. + +const target = { tag: "T" }; +const key1 = { k: 1 }; + +// --- receiver shapes that used to throw -------------------------------- +const arrA = [new WeakRef(target)]; +console.log("array-element:", (arrA[0].deref() as any).tag); + +const wrFromElement = arrA[0]; +console.log("local-from-element:", (wrFromElement.deref() as any).tag); + +console.log("new-inline:", (new WeakRef(target).deref() as any).tag); + +const holder = { r: new WeakRef(target) }; +console.log("object-property:", (holder.r.deref() as any).tag); + +const wrFromProperty = holder.r; +console.log("local-from-property:", (wrFromProperty.deref() as any).tag); + +function makeRef(): WeakRef { + return new WeakRef(target); +} +console.log("function-return:", (makeRef().deref() as any).tag); + +const wrFromCall = makeRef(); +console.log("local-from-call:", (wrFromCall.deref() as any).tag); + +let forOfOut = ""; +for (const wrLoop of arrA) { + forOfOut = (wrLoop.deref() as any).tag; +} +console.log("for-of-binding:", forOfOut); + +function viaParam(w: WeakRef): string { + return (w.deref() as any).tag; +} +console.log("function-param:", viaParam(new WeakRef(target))); + +console.log("map-callback:", arrA.map((w) => (w.deref() as any).tag).join("")); + +const mapOfRefs = new Map>(); +mapOfRefs.set("a", new WeakRef(target)); +console.log("map-value:", (mapOfRefs.get("a")!.deref() as any).tag); + +// The pre-scan visits function DECLARATIONS but not arrow bodies, function +// expressions, or class methods — those threw even for a directly-named local. +const arrowLocal = (): string => { + const wrArrow = new WeakRef(target); + return (wrArrow.deref() as any).tag; +}; +console.log("arrow-fn-local:", arrowLocal()); + +const fnExprLocal = function (): string { + const wrExpr = new WeakRef(target); + return (wrExpr.deref() as any).tag; +}; +console.log("fn-expr-local:", fnExprLocal()); + +class MethodHolder { + go(): string { + const wrMethod = new WeakRef(target); + return (wrMethod.deref() as any).tag; + } +} +console.log("class-method-local:", new MethodHolder().go()); + +// The two shapes that already worked — kept so the fix cannot regress them. +const wrTopLevel = new WeakRef(target); +console.log("direct-local:", (wrTopLevel.deref() as any).tag); +function declFnLocal(): string { + const wrDecl = new WeakRef(target); + return (wrDecl.deref() as any).tag; +} +console.log("decl-fn-local:", declFnLocal()); + +// --- the reflective / value-read path ---------------------------------- +console.log("typeof-method:", typeof wrTopLevel.deref); +console.log("typeof-method-element:", typeof arrA[0].deref); +console.log("reflective-call:", ((WeakRef.prototype.deref as any).call(wrTopLevel) as any).tag); +const boundDeref = wrTopLevel.deref.bind(wrTopLevel); +console.log("method-extract:", (boundDeref() as any).tag); +console.log("proto-identity:", wrTopLevel.deref === WeakRef.prototype.deref); +console.log("optional-call:", (wrTopLevel.deref?.() as any).tag); +console.log("deref-length:", WeakRef.prototype.deref.length); +console.log("toString-tag:", Object.prototype.toString.call(wrTopLevel)); +try { + (WeakRef.prototype.deref as any).call({}); + console.log("brand-check:", "NO THROW"); +} catch (e) { + console.log("brand-check:", (e as Error).name); +} + +// --- FinalizationRegistry through the same shapes ---------------------- +const regArr = [new FinalizationRegistry(() => {})]; +regArr[0].register(target, 1, target); +console.log("finreg-array-element:", regArr[0].unregister(target)); + +const regHolder = { f: new FinalizationRegistry(() => {}) }; +regHolder.f.register(target, 2, target); +console.log("finreg-object-property:", regHolder.f.unregister(target)); + +const finregArrow = (): boolean => { + const regArrow = new FinalizationRegistry(() => {}); + regArrow.register(target, 3, target); + return regArrow.unregister(target); +}; +console.log("finreg-arrow-fn-local:", finregArrow()); +console.log("finreg-register-length:", (FinalizationRegistry.prototype.register as any).length); +console.log( + "finreg-toString-tag:", + Object.prototype.toString.call(new FinalizationRegistry(() => {})), +); + +// --- #7948: a same-named foreign receiver must keep its OWN method ------ +class Cache { + v: number; + constructor(v: number) { + this.v = v; + } + deref(): number { + return this.v * 10; + } + register(a: number): string { + return "user-register:" + a; + } + unregister(a: number): string { + return "user-unregister:" + a; + } +} + +// `wrTopLevel` above is the genuine WeakRef binding that makes the name-keyed +// pre-scan fold every same-named `.deref()` in this module. These four bind the +// SAME identifier to something else; each must resolve its own method. +function collideObjectLiteral(): unknown { + const wrTopLevel = { deref: () => 40 }; + return wrTopLevel.deref(); +} +function collideUserClass(): unknown { + const wrTopLevel = new Cache(4); + return wrTopLevel.deref(); +} +function collideArrayProp(): unknown { + const wrTopLevel: any = [1, 2, 3]; + wrTopLevel.deref = () => 41; + return wrTopLevel.deref(); +} +function collideParam(wrTopLevel: { deref: () => number }): unknown { + return wrTopLevel.deref(); +} +console.log("collide-object-literal:", collideObjectLiteral()); +console.log("collide-user-class:", collideUserClass()); +console.log("collide-array-prop:", collideArrayProp()); +console.log("collide-param:", collideParam({ deref: () => 42 })); + +// Same for the FinalizationRegistry names (`regArrow` is the genuine binding). +function collideRegister(): unknown { + const regArrow = new Cache(1); + return regArrow.register(3); +} +function collideUnregister(): unknown { + const regArrow = new Cache(1); + return regArrow.unregister(4); +} +console.log("collide-register:", collideRegister()); +console.log("collide-unregister:", collideUnregister()); + +// --- boundary: WeakMap / WeakSet already worked; keep them working ----- +const wmArr = [new WeakMap()]; +wmArr[0].set(key1, 7); +console.log("weakmap-array-element:", wmArr[0].get(key1), wmArr[0].has(key1)); +console.log("weakmap-delete:", wmArr[0].delete(key1), wmArr[0].has(key1)); + +const wsArr = [new WeakSet()]; +wsArr[0].add(key1); +console.log("weakset-array-element:", wsArr[0].has(key1)); + +const wmReflective = new WeakMap(); +wmReflective.set(key1, 3); +console.log("weakmap-reflective:", (WeakMap.prototype.get as any).call(wmReflective, key1)); + +// #7948 for the weak COLLECTIONS: `wmReflective`/`wsArr` above are the genuine +// bindings that make the name-keyed fold claim these identifiers module-wide. +// The poison pass only recognises `new OtherClass()` and call/await initializers, +// so an object literal, an array, or a parameter slipped straight through to +// `js_weakmap_get` and answered `undefined`. +function collideLiteralGet(): unknown { + const wmReflective = { get: (k: string) => "lit:" + k }; + return wmReflective.get("a"); +} +function collideParamGet(wmReflective: { get: (k: string) => string }): unknown { + return wmReflective.get("b"); +} +function collideArrayGet(): unknown { + const wmReflective: any = []; + wmReflective.get = (k: string) => "arr:" + k; + return wmReflective.get("c"); +} +function collideLiteralSetHasDelete(): string { + const wmReflective = { + set: (k: string, v: number) => "set:" + k + v, + has: (k: string) => "has:" + k, + delete: (k: string) => "del:" + k, + }; + return ( + wmReflective.set("x", 1) + "/" + wmReflective.has("y") + "/" + wmReflective.delete("z") + ); +} +function collideLiteralAdd(): unknown { + const wsArr = { add: (v: number) => "add:" + v }; + return wsArr.add(9); +} +console.log("collide-literal-get:", collideLiteralGet()); +console.log("collide-param-get:", collideParamGet({ get: (k: string) => "p:" + k })); +console.log("collide-array-get:", collideArrayGet()); +console.log("collide-literal-set-has-delete:", collideLiteralSetHasDelete()); +console.log("collide-literal-add:", collideLiteralAdd()); +console.log("weakmap-toString-tag:", Object.prototype.toString.call(wmReflective)); +console.log("weakset-toString-tag:", Object.prototype.toString.call(new WeakSet())); + +// The deref result is still the live object, not a copy. +console.log("identity:", arrA[0].deref() === target);