diff --git a/changelog.d/11514-callback-this-undefined.md b/changelog.d/11514-callback-this-undefined.md new file mode 100644 index 0000000000..59e31540f6 --- /dev/null +++ b/changelog.d/11514-callback-this-undefined.md @@ -0,0 +1,17 @@ +Fixed `sort` comparators and `reduce` / `reduceRight` callbacks seeing the +enclosing method's receiver as `this` (#11419). The runtime invoked them +through plain `js_closure_callN` calls without resetting the per-thread +implicit-`this` cell, so a comparator called from inside `obj.m()` read `obj` +instead of `undefined`. The `forEach` / `map` / `filter` / `find*` / `some` / +`every` / `flatMap` family already bound `undefined` around their callbacks; +the sort and reduce entry points now do the same through a new +`ImplicitThisScope::bind_undefined`, covering arrays, typed arrays (including +BigInt lanes and `toSorted`) and generic array-likes +(`Array.prototype.sort/reduce/reduceRight.call(obj, …)`). The guard restores +the caller's receiver from a rooted slot on return and on unwind. + +The structural fix (receiver as a calling-convention parameter, deleting the +cell) remains the `this`-as-parameter plan; this closes the observable leak +for these callbacks. Regression test: +`test-files/test_gap_11419_callback_this_undefined.ts` (every +surface leaks on `main`; all pass with this change). diff --git a/crates/perry-runtime/src/array/generic.rs b/crates/perry-runtime/src/array/generic.rs index 1f1d12e20f..372a6d0088 100644 --- a/crates/perry-runtime/src/array/generic.rs +++ b/crates/perry-runtime/src/array/generic.rs @@ -873,6 +873,8 @@ pub extern "C" fn js_arraylike_findLastIndex(recv: f64, cb: f64, this_arg: f64) #[no_mangle] pub extern "C" fn js_arraylike_reduce(recv: f64, cb: f64, has_init: i32, init: f64) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); + // #11419: `this` is undefined in the callback (no thisArg parameter). + let _this = crate::object::ImplicitThisScope::bind_undefined(&scope); let cb_h = scope.root_nanbox_f64(cb); let recv_h = scope.root_nanbox_f64(to_object(recv)); // Spec order: LengthOfArrayLike(O) is read *before* the IsCallable(cb) @@ -916,6 +918,8 @@ pub extern "C" fn js_arraylike_reduce(recv: f64, cb: f64, has_init: i32, init: f #[no_mangle] pub extern "C" fn js_arraylike_reduceRight(recv: f64, cb: f64, has_init: i32, init: f64) -> f64 { let scope = crate::gc::RuntimeHandleScope::new(); + // #11419: `this` is undefined in the callback (no thisArg parameter). + let _this = crate::object::ImplicitThisScope::bind_undefined(&scope); let cb_h = scope.root_nanbox_f64(cb); let recv_h = scope.root_nanbox_f64(to_object(recv)); // Spec order: LengthOfArrayLike(O) is read *before* the IsCallable(cb) diff --git a/crates/perry-runtime/src/array/generic_object.rs b/crates/perry-runtime/src/array/generic_object.rs index cbf4f74cd2..534c676ddf 100644 --- a/crates/perry-runtime/src/array/generic_object.rs +++ b/crates/perry-runtime/src/array/generic_object.rs @@ -382,6 +382,8 @@ pub(crate) fn object_sort(recv: f64, cmp_validated: *const ClosureHeader) -> f64 // Length and indexed getters may collect before comparison begins. let scope = crate::gc::RuntimeHandleScope::new(); let recv_handle = scope.root_nanbox_f64(recv); + // #11419: the comparator runs with `this` undefined. + let _this = crate::object::ImplicitThisScope::bind_undefined(&scope); let cmp_handle = scope.root_raw_const_ptr(cmp_validated); let cmp = if cmp_validated.is_null() { None diff --git a/crates/perry-runtime/src/array/iter_methods.rs b/crates/perry-runtime/src/array/iter_methods.rs index e59c1b4709..0e704d1f5c 100644 --- a/crates/perry-runtime/src/array/iter_methods.rs +++ b/crates/perry-runtime/src/array/iter_methods.rs @@ -166,7 +166,7 @@ mod rooted_iter_array_tests { fn bind_undefined_this( scope: &crate::gc::RuntimeHandleScope, ) -> crate::object::ImplicitThisScope<'_> { - crate::object::ImplicitThisScope::bind(scope, undefined_value()) + crate::object::ImplicitThisScope::bind_undefined(scope) } /// #5989/#8117: `.forEach` on a receiver codegen could not prove is a @@ -1199,6 +1199,10 @@ pub extern "C" fn js_array_reduce( has_initial: i32, initial: f64, ) -> f64 { + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); // #8137: a Buffer-backed `Uint8Array` receiver. Perry's // `new Uint8Array([…])` is a `BufferHeader`, absent from the typed-array // registry, so the `lookup_typed_array_kind` re-dispatch below never diff --git a/crates/perry-runtime/src/array/reduce_right.rs b/crates/perry-runtime/src/array/reduce_right.rs index d77ab55453..a2653d4d5c 100644 --- a/crates/perry-runtime/src/array/reduce_right.rs +++ b/crates/perry-runtime/src/array/reduce_right.rs @@ -22,6 +22,10 @@ pub extern "C" fn js_array_reduce_right( has_initial: i32, initial: f64, ) -> f64 { + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); // #8137: a Buffer-backed `Uint8Array` receiver reads as an `ArrayHeader` // below — correct `length`, GARBAGE elements. `reduceRight` is the widest // case in the family: it is wrong even for a STATICALLY typed diff --git a/crates/perry-runtime/src/array/sort.rs b/crates/perry-runtime/src/array/sort.rs index c9f4628791..afefd91184 100644 --- a/crates/perry-runtime/src/array/sort.rs +++ b/crates/perry-runtime/src/array/sort.rs @@ -767,6 +767,10 @@ pub extern "C" fn js_array_sort_with_comparator( if comparator.is_null() { return js_array_sort_default(arr); } + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); unsafe { // Runtime plain-object receiver behind a statically-Array variable — // probe the RAW pointer before the array-plausibility clean. diff --git a/crates/perry-runtime/src/object/this_binding.rs b/crates/perry-runtime/src/object/this_binding.rs index 45fe20d7c8..f996dd19f4 100644 --- a/crates/perry-runtime/src/object/this_binding.rs +++ b/crates/perry-runtime/src/object/this_binding.rs @@ -280,6 +280,15 @@ impl<'scope> ImplicitThisScope<'scope> { previous: scope.root_nanbox_f64(js_implicit_this_set(receiver)), } } + + /// Bind `this` to `undefined`: OrdinaryCallBindThis for a callback the + /// runtime invokes with no receiver (`sort` comparators, `reduce` + /// callbacks, an absent `thisArg`). Without it the callee reads whatever + /// the enclosing method dispatch left in the cell (#11419). + #[inline] + pub fn bind_undefined(scope: &'scope crate::gc::RuntimeHandleScope) -> Self { + Self::bind(scope, f64::from_bits(crate::value::TAG_UNDEFINED)) + } } impl Drop for ImplicitThisScope<'_> { diff --git a/crates/perry-runtime/src/typedarray/iterate.rs b/crates/perry-runtime/src/typedarray/iterate.rs index d782f59a1c..af0ab949d6 100644 --- a/crates/perry-runtime/src/typedarray/iterate.rs +++ b/crates/perry-runtime/src/typedarray/iterate.rs @@ -240,6 +240,10 @@ pub extern "C" fn js_typed_array_reduce( has_initial: i32, initial: f64, ) -> f64 { + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); let ta = clean_ta_ptr(ta); if ta.is_null() { if has_initial != 0 { @@ -296,6 +300,10 @@ pub extern "C" fn js_typed_array_reduce_right( has_initial: i32, initial: f64, ) -> f64 { + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); let ta = clean_ta_ptr(ta); if ta.is_null() { if has_initial != 0 { diff --git a/crates/perry-runtime/src/typedarray/transform.rs b/crates/perry-runtime/src/typedarray/transform.rs index 2b39fe2185..6c6f168dfe 100644 --- a/crates/perry-runtime/src/typedarray/transform.rs +++ b/crates/perry-runtime/src/typedarray/transform.rs @@ -187,6 +187,10 @@ pub extern "C" fn js_typed_array_sort_with_comparator( if comparator.is_null() { return js_typed_array_sort_default(ta); } + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); let ta_clean = clean_ta_ptr(ta as *const TypedArrayHeader) as *mut TypedArrayHeader; if ta_clean.is_null() { return ta_clean; @@ -277,6 +281,10 @@ pub extern "C" fn js_typed_array_to_sorted_with_comparator( if comparator.is_null() { return js_typed_array_to_sorted_default(ta); } + // #11419: the callback runs with `this` undefined, not the receiver of + // whatever method dispatch encloses this call. + let this_scope = crate::gc::RuntimeHandleScope::new(); + let _this = crate::object::ImplicitThisScope::bind_undefined(&this_scope); let ta = clean_ta_ptr(ta); if ta.is_null() { return typed_array_alloc(KIND_FLOAT64, 0); diff --git a/test-files/test_gap_11419_callback_this_undefined.ts b/test-files/test_gap_11419_callback_this_undefined.ts new file mode 100644 index 0000000000..79236f4afd --- /dev/null +++ b/test-files/test_gap_11419_callback_this_undefined.ts @@ -0,0 +1,74 @@ +"use strict"; +// #11419: a callback the runtime invokes with no receiver — a `sort` +// comparator, a `reduce` / `reduceRight` callback — and a plain call of a +// function value must see `this === undefined`, even when the call happens +// inside a method whose receiver is still sitting in the implicit-`this` cell. +// Before the fix the comparator / reducer saw the enclosing method's receiver. + +const seen: string[] = []; +function note(tag: string, self: any): void { + if (self !== undefined) seen.push(tag + ":" + typeof self); +} + +function cmp(this: any, a: number, b: number) { note("sort", this); return a - b; } +function cmpBig(this: any, a: bigint, b: bigint) { note("sortBig", this); return a < b ? -1 : a > b ? 1 : 0; } +function red(this: any, acc: number, v: number) { note("reduce", this); return acc + v; } +function zero(this: any) { return typeof this; } +function one(this: any, a: number) { return typeof this; } +function two(this: any, a: number, b: number) { return typeof this; } +function three(this: any, a: number, b: number, c: number) { return typeof this; } + +const holder: any = { + k: 1, + arrays(this: any) { + const a = [3, 1, 2]; + a.sort(cmp); + const s = [3, 1, 2].toSorted(cmp); + const r1 = [1, 2, 3].reduce(red, 0); + const r2 = [1, 2, 3].reduce(red); + const r3 = [1, 2, 3].reduceRight(red, 0); + const r4 = [1, 2, 3].reduceRight(red); + return a.join() + "|" + s.join() + "|" + r1 + "," + r2 + "," + r3 + "," + r4; + }, + typed(this: any) { + const f = new Float64Array([3, 1, 2]); + f.sort(cmp); + const t = new Int32Array([3, 1, 2]).toSorted(cmp); + const b = new BigInt64Array([3n, 1n, 2n]); + b.sort(cmpBig); + const r1 = new Int32Array([1, 2, 3]).reduce(red, 0); + const r2 = new Int32Array([1, 2, 3]).reduceRight(red); + return f.join(",") + "|" + t.join(",") + "|" + b.join(",") + "|" + r1 + "," + r2; + }, + arrayLike(this: any) { + const o: any = { length: 3, 0: 3, 1: 1, 2: 2 }; + Array.prototype.sort.call(o, cmp); + const r1 = Array.prototype.reduce.call({ length: 2, 0: 1, 1: 2 }, red, 0); + const r2 = Array.prototype.reduceRight.call({ length: 2, 0: 1, 1: 2 }, red, 0); + return o[0] + "," + o[1] + "," + o[2] + "|" + r1 + "," + r2; + }, + plain(this: any) { + const f0: any = zero; + const f1: any = one; + const f2: any = two; + const f3: any = three; + return [f0(), f1(1), f2(1, 2), f3(1, 2, 3), zero(), two(1, 2)].join(","); + }, +}; + +console.log("arrays", holder.arrays()); +console.log("typed", holder.typed()); +console.log("arrayLike", holder.arrayLike()); +console.log("plain", holder.plain()); + +class Box { + v = 7; + run() { + const out = [5, 4].sort(cmp).join() + "|" + [1, 2].reduce(red, 0); + // The method's own `this` must survive the callbacks. + return out + "|" + this.v; + } +} +console.log("class", new Box().run()); + +console.log("leaks", seen.length === 0 ? "none" : seen.join(" "));