From 9d79ad272213919a1bc4c818c4bb89759458822d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 12:59:23 +0200 Subject: [PATCH] fix(intl): close the #8659 follow-up gaps; recognize captured typed-array constructors Lands #8725 and #8726. #8725 is the review tail of #8659, which landed through #8723 before these final fixes reached its head. It validates `Intl.PluralRules` digit-option bounds before flooring, roots the dynamic superclass and emitted network error values across allocating/user-code calls, rejects constructable superclasses whose `.prototype` is neither an object nor null, and restores `crate::perry_thread_local!` for the `bun:ffi` read cache. That last one is a regression I let through in #8704: a plain `thread_local!` both loses the HotTls address cache (a `_tlv_get_addr` call per read on Darwin) and escapes the root-holder census, which keys off the macro -- so the read cache was never classified. #8726 (fixes #8724) recognizes captured `ArrayBuffer` / `SharedArrayBuffer` / `DataView` constructors. `const D = DataView; new D(buf)`, `Reflect.construct(DataView, [buf])` and `class X extends DataView {}` all threw `TypeError: Constructor requires 'new'` where node succeeds; only the direct `new DataView(buf)` form worked. No version bump. --- changelog.d/8659-follow-up-review-fixes.md | 5 ++ .../8724-captured-arraybuffer-construct.md | 1 + crates/perry-runtime/src/bun_ffi/read.rs | 2 +- .../src/intl/list_relative_plural.rs | 18 ++++- .../src/object/class_registry/class_meta.rs | 13 +++ .../src/object/class_registry/state.rs | 80 +++++++++++++++---- crates/perry-stdlib/src/net/mod.rs | 5 +- ...sue_8724_captured_arraybuffer_construct.ts | 33 ++++++++ 8 files changed, 134 insertions(+), 23 deletions(-) create mode 100644 changelog.d/8659-follow-up-review-fixes.md create mode 100644 changelog.d/8724-captured-arraybuffer-construct.md create mode 100644 tests/issue_8724_captured_arraybuffer_construct.ts diff --git a/changelog.d/8659-follow-up-review-fixes.md b/changelog.d/8659-follow-up-review-fixes.md new file mode 100644 index 0000000000..9c1d5fda95 --- /dev/null +++ b/changelog.d/8659-follow-up-review-fixes.md @@ -0,0 +1,5 @@ +### Fixed + +Validate fractional `Intl.PluralRules` digit options before flooring, reject +invalid superclass prototypes, and keep emitted network errors alive across +successive listeners. diff --git a/changelog.d/8724-captured-arraybuffer-construct.md b/changelog.d/8724-captured-arraybuffer-construct.md new file mode 100644 index 0000000000..5ed9e22e9a --- /dev/null +++ b/changelog.d/8724-captured-arraybuffer-construct.md @@ -0,0 +1 @@ +Fix a regression where constructing `ArrayBuffer`, `SharedArrayBuffer`, or `DataView` through a captured value — `const D = DataView; new D(buf)`, `Reflect.construct(DataView, …)`, `class X extends DataView {}`, `new globalThis.ArrayBuffer(n)` — threw `TypeError: Constructor requires 'new'`. Their global [[Call]] behavior had moved to the shared construct-only thunk without adding that thunk to `identify_global_builtin_constructor`'s allow-list, so the dynamic-`new` path no longer recognized a captured constructor and fell through to the bare-call thunk. Minified bundles capture these globals into locals pervasively; this broke module init (e.g. the natively-compiled Claude Code cli.js bundle failed at startup on nearly every command). Direct `new DataView(buf)` was unaffected. diff --git a/crates/perry-runtime/src/bun_ffi/read.rs b/crates/perry-runtime/src/bun_ffi/read.rs index 1a5e34cc94..27602374da 100644 --- a/crates/perry-runtime/src/bun_ffi/read.rs +++ b/crates/perry-runtime/src/bun_ffi/read.rs @@ -22,7 +22,7 @@ const READERS: &[(&str, u8)] = &[ ("f64", super::types::T_F64), ]; -thread_local! { +crate::perry_thread_local! { static READ_OBJECT_CACHE: Cell = const { Cell::new(0) }; } diff --git a/crates/perry-runtime/src/intl/list_relative_plural.rs b/crates/perry-runtime/src/intl/list_relative_plural.rs index 152a0557cb..935f12ed50 100644 --- a/crates/perry-runtime/src/intl/list_relative_plural.rs +++ b/crates/perry-runtime/src/intl/list_relative_plural.rs @@ -670,18 +670,21 @@ pub(crate) extern "C" fn rtf_bound_resolved_options_thunk(closure: *const Closur // ---- Intl.PluralRules ------------------------------------------------------ +fn plural_digit_integer(number: f64, min: f64, max: f64) -> Option { + (number.is_finite() && number >= min && number <= max).then(|| number.floor()) +} + fn plural_digit_option(options: f64, key: &str, min: f64, max: f64) -> Option { let value = get_option_value(options, key); if JSValue::from_bits(value.to_bits()).is_undefined() { return None; } let number = to_number_reject_bigint(value); - let integer = number.trunc(); - if integer.is_nan() || integer < min || integer > max { + let Some(integer) = plural_digit_integer(number, min, max) else { throw_range_error(&format!( "Value {number} out of range for Intl.PluralRules options property {key}" )); - } + }; Some(integer) } @@ -929,6 +932,15 @@ pub(crate) fn plural_rules_select(obj: *const ObjectHeader, value: f64) -> f64 { mod plural_category_tests { use super::*; + #[test] + fn digit_options_validate_before_flooring() { + assert_eq!(plural_digit_integer(20.9, 1.0, 21.0), Some(20.0)); + assert_eq!(plural_digit_integer(21.9, 1.0, 21.0), None); + assert_eq!(plural_digit_integer(100.5, 0.0, 100.0), None); + assert_eq!(plural_digit_integer(-0.5, 0.0, 100.0), None); + assert_eq!(plural_digit_integer(f64::INFINITY, 0.0, 100.0), None); + } + #[test] fn locale_selectors_only_return_advertised_categories() { let samples = [ diff --git a/crates/perry-runtime/src/object/class_registry/class_meta.rs b/crates/perry-runtime/src/object/class_registry/class_meta.rs index 745de82517..e41cae59eb 100644 --- a/crates/perry-runtime/src/object/class_registry/class_meta.rs +++ b/crates/perry-runtime/src/object/class_registry/class_meta.rs @@ -186,6 +186,19 @@ pub(crate) fn identify_global_builtin_constructor(func_value: f64) -> Option<&'s let is_global_builtin_func = func_ptr == global_this_builtin_noop_thunk as *const u8 as usize || func_ptr == typed_array_constructor_call_thunk as *const u8 as usize + // ArrayBuffer / SharedArrayBuffer / DataView carry the shared + // construct-only call thunk (populate.rs). When one is captured into + // a variable and constructed — `const D = DataView; new D(buf)`, + // `Reflect.construct(DataView, …)`, `class X extends DataView` — the + // dynamic-`new` path lands here and must recognize the thunk so the + // singleton walk recovers the name and construct.rs's + // "ArrayBuffer"/"SharedArrayBuffer"/"DataView" arms build it, instead + // of falling through to invoke the bare-call thunk (which throws + // "Constructor requires 'new'"). Minified bundles capture these + // globals into locals pervasively (the Claude Code cli.js bundle + // fails at module init without this). Regression from the thunk swap + // in 06e1ab349 — before it these carried the recognized noop thunk. + || func_ptr == construct_only_builtin_call_thunk as *const u8 as usize // #4102: `Array`/`Object`/`Date` constructor *values* carry their own // coercion thunks (not the shared noop thunk), so the dynamic // `instanceof` / reflective `@@hasInstance` path could not recover diff --git a/crates/perry-runtime/src/object/class_registry/state.rs b/crates/perry-runtime/src/object/class_registry/state.rs index 6ace1cbc6d..cc78325683 100644 --- a/crates/perry-runtime/src/object/class_registry/state.rs +++ b/crates/perry-runtime/src/object/class_registry/state.rs @@ -632,6 +632,17 @@ fn install_class_decl_prototype_method_fields(proto: *mut ObjectHeader, class_id } } +fn class_parent_prototype_bits(value: f64) -> Option { + let bits = value.to_bits(); + if bits == crate::value::TAG_NULL { + return Some(bits); + } + if !unsafe { super::super::object_ops::value_is_object_like(value) } { + return None; + } + (unsafe { crate::symbol::js_is_symbol(value) } == 0).then_some(bits) +} + pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { // #7757: a specialization answers with its generic's prototype. let class_id = decl_prototype_identity_id(class_id); @@ -710,8 +721,9 @@ pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { unsafe { mirror_prototype_method_on_object(proto, &name, value_bits, enumerable) }; } - let dynamic_parent = js_get_dynamic_parent_value(class_id); - let null_heritage = dynamic_parent.to_bits() == crate::value::TAG_NULL; + let scope = crate::gc::RuntimeHandleScope::new(); + let dynamic_parent = scope.root_nanbox_f64(js_get_dynamic_parent_value(class_id)); + let null_heritage = dynamic_parent.get_nanbox_f64().to_bits() == crate::value::TAG_NULL; let parent_proto_bits = if null_heritage { // A class extending null creates a prototype object whose // [[Prototype]] is null, not Object.prototype. Record TAG_NULL @@ -719,33 +731,41 @@ pub(crate) fn class_decl_prototype_value(class_id: u32) -> f64 { // Object.prototype default. Some(crate::value::TAG_NULL) } else { - get_parent_class_id(class_id) + let registered_parent_proto = get_parent_class_id(class_id) .filter(|parent_id| *parent_id != 0 && *parent_id != class_id) .and_then(|parent_id| { let parent_proto = class_decl_prototype_value(parent_id); let parent_bits = parent_proto.to_bits(); ((parent_bits >> 48) == 0x7FFD).then_some(parent_bits) - }) + }); + if registered_parent_proto.is_some() { + registered_parent_proto + } else { // A runtime function-valued superclass (including Intl service // constructors) has no class-id edge. Link the declared prototype // to the parent's own `.prototype` exactly once, while this fresh // class prototype is initialized. Construction must never rewrite // this edge after user code mutates it. - .or_else(|| { - let parent = JSValue::from_bits(dynamic_parent.to_bits()); - if !parent.is_pointer() { - return None; - } + let parent = JSValue::from_bits(dynamic_parent.get_nanbox_f64().to_bits()); + if parent.is_pointer() { let parent_addr = parent.as_pointer::() as usize; - if !crate::closure::is_closure_ptr(parent_addr) { - return None; + if crate::closure::is_closure_ptr(parent_addr) { + let parent_proto = + crate::closure::closure_get_dynamic_prop(parent_addr, "prototype"); + if let Some(bits) = class_parent_prototype_bits(parent_proto) { + Some(bits) + } else { + super::super::object_ops::throw_object_type_error( + b"Class extends value does not have valid prototype property", + ); + } + } else { + global_object_prototype_bits() } - let parent_proto = - crate::closure::closure_get_dynamic_prop(parent_addr, "prototype"); - let bits = parent_proto.to_bits(); - ((bits >> 48) == 0x7FFD).then_some(bits) - }) - .or_else(global_object_prototype_bits) + } else { + global_object_prototype_bits() + } + } }; if let Some(bits) = parent_proto_bits { let proto = class_decl_prototype_object(class_id); @@ -905,3 +925,29 @@ mod class_dynamic_prop_store_tests { assert_eq!(stored(cid, "k"), Some(3.0)); } } + +#[cfg(test)] +mod class_parent_prototype_tests { + use super::*; + + #[test] + fn only_object_and_null_parent_prototypes_are_valid() { + let object_ptr = crate::object::js_object_alloc(0, 0); + assert!(!object_ptr.is_null()); + let object = crate::value::js_nanbox_pointer(object_ptr as i64); + assert_eq!(class_parent_prototype_bits(object), Some(object.to_bits())); + assert_eq!( + class_parent_prototype_bits(f64::from_bits(crate::value::POINTER_TAG | 0x1234)), + None + ); + assert_eq!( + class_parent_prototype_bits(f64::from_bits(crate::value::TAG_NULL)), + Some(crate::value::TAG_NULL) + ); + assert_eq!( + class_parent_prototype_bits(f64::from_bits(crate::value::TAG_UNDEFINED)), + None + ); + assert_eq!(class_parent_prototype_bits(1.0), None); + } +} diff --git a/crates/perry-stdlib/src/net/mod.rs b/crates/perry-stdlib/src/net/mod.rs index 66de8a8254..b2d832e096 100644 --- a/crates/perry-stdlib/src/net/mod.rs +++ b/crates/perry-stdlib/src/net/mod.rs @@ -1856,10 +1856,11 @@ pub unsafe extern "C" fn js_net_process_pending() -> i32 { // so user code can read `err.message`. Pre-fix the listener // received a raw NaN-boxed string and `err.message` came // back as `undefined`. - let err_f64 = build_error_object(&msg); + let scope = perry_runtime::gc::RuntimeHandleScope::new(); + let error = scope.root_nanbox_f64(build_error_object(&msg)); for cb in cbs { if cb != 0 { - js_closure_call1(cb as *const ClosureHeader, err_f64); + js_closure_call1(cb as *const ClosureHeader, error.get_nanbox_f64()); } } } diff --git a/tests/issue_8724_captured_arraybuffer_construct.ts b/tests/issue_8724_captured_arraybuffer_construct.ts new file mode 100644 index 0000000000..e0e522199a --- /dev/null +++ b/tests/issue_8724_captured_arraybuffer_construct.ts @@ -0,0 +1,33 @@ +// #8724: constructing ArrayBuffer / SharedArrayBuffer / DataView through a +// CAPTURED value (a local holding the builtin, Reflect.construct, or a +// subclass) must build the object, not throw "Constructor requires 'new'". +// Regression from routing their [[Call]] to the shared construct-only thunk +// without teaching `identify_global_builtin_constructor` about it, so the +// dynamic-`new` path stopped recognizing a captured constructor. Minified +// bundles capture these globals into locals everywhere. + +function run(label: string, fn: () => void): void { + try { + fn(); + console.log(label, "ok"); + } catch (e: any) { + console.log(label, "THREW:", e.message); + } +} + +const ab = () => new ArrayBuffer(8); + +run("direct DataView", () => { const dv = new DataView(ab()); if (dv.byteLength !== 8) throw new Error("len"); }); +run("captured DataView", () => { const D = DataView; const dv = new D(ab()); if (dv.byteLength !== 8) throw new Error("len"); }); +run("Reflect.construct DataView", () => { const dv = Reflect.construct(DataView, [ab()]); if ((dv as DataView).byteLength !== 8) throw new Error("len"); }); +run("subclass DataView", () => { class MV extends DataView {} const dv = new MV(ab()); if (dv.byteLength !== 8) throw new Error("len"); }); +run("captured ArrayBuffer", () => { const AB = ArrayBuffer; const b = new AB(8); if (b.byteLength !== 8) throw new Error("len"); }); +run("captured ArrayBuffer resizable", () => { const AB = ArrayBuffer; const b = new AB(8, { maxByteLength: 16 }); if (b.byteLength !== 8) throw new Error("len"); }); +run("captured SharedArrayBuffer", () => { const SAB = SharedArrayBuffer; const s = new SAB(8); if (s.byteLength !== 8) throw new Error("len"); }); +run("globalThis ArrayBuffer", () => { const b = new (globalThis as any).ArrayBuffer(8); if (b.byteLength !== 8) throw new Error("len"); }); + +// The captured DataView must be fully functional, not a branded-but-empty stub. +const D2 = DataView; +const dv2 = new D2(ab()); +dv2.setInt32(0, 0x41424344); +console.log("readback", dv2.getInt32(0).toString(16));