diff --git a/changelog.d/honest-handle-tag-text.md b/changelog.d/honest-handle-tag-text.md new file mode 100644 index 00000000000..ceb14c7ba43 --- /dev/null +++ b/changelog.d/honest-handle-tag-text.md @@ -0,0 +1,31 @@ +`TextEncoder` and `TextDecoder` instances are now ordinary objects with their +own identity. They used to travel as small registry integers under +`POINTER_TAG`, so a value's identity was its registry id: `new TextEncoder() +=== new TextEncoder()` was `true` (every encoder shared one stateless sentinel +id), two encoders collapsed into a single `Map` or `Set` key, and a `WeakMap` +entry stored under one encoder was readable through an unrelated one (#10821). + +An instance is now a `GC_TYPE_OBJECT` with a real ShapeId, a family class id and +a per-family prototype — identical in kind to an object TypeScript itself +creates — so the whole object surface matches node without a per-kind arm +anywhere: `typeof` is `"object"`, `Object.keys` / `getOwnPropertyNames` are `[]`, +`JSON.stringify` is `{}` (it was `null`), `{...d}` is empty, `for...in` yields +the WebIDL-enumerable prototype members, `instanceof` works through the ordinary +prototype walk, `Object.getPrototypeOf(d) === TextDecoder.prototype`, and +`TextDecoder.prototype.decode.call({})` throws a `TypeError` on a foreign +receiver. `decode` / `encode` / `encodeInto` are real methods on the prototype +and `encoding` / `fatal` / `ignoreBOM` are real accessors, all found by ordinary +lookup instead of a handle-dispatch table. + +A decoder's entire state — which of the ~40 fixed encodings, `fatal`, +`ignoreBOM` — packs into one word in the object's own meta record, so +`DECODER_REGISTRY` is deleted: it was a mutex-guarded map that grew one entry per +`new TextDecoder()` and was never pruned, i.e. a leak and a lock on the decode +path, both now gone. + +This is the first family of the "honest tags" invariant (#340/#341): a +`POINTER_TAG` value should always be a dereferenceable GC cell, never a small +registry integer. No codegen change — these natives already return an `i64` that +codegen boxes with `POINTER_TAG`, and returning the object address instead of an +id is identical at the C ABI. The emitted small-handle guards and the `addr_class` +band predicates stay exactly as they are until every family has moved. diff --git a/crates/perry-codegen/src/expr/instance_misc1.rs b/crates/perry-codegen/src/expr/instance_misc1.rs index 536058101e4..1596a9a7c2c 100644 --- a/crates/perry-codegen/src/expr/instance_misc1.rs +++ b/crates/perry-codegen/src/expr/instance_misc1.rs @@ -594,6 +594,15 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { // `class X extends EventTarget` instance reaches it through the // parent edge the class registry wires at definition time. "EventTarget" | "globalThis.EventTarget" => 0xFFFF2406u32, + // #340/#341. Keep in sync with TEXT_{ENCODER,DECODER}_CLASS_ID + // in perry-runtime/src/text.rs. These instances are ordinary + // objects carrying the class id on their header, so the runtime + // class-id chain matches them with no probe -- the same shape as + // EventTarget above. Before they became ordinary objects there + // was no id to name here, so `e instanceof TextEncoder` folded + // to `js_instanceof(_, 0)` == false where node says true. + "TextEncoder" | "globalThis.TextEncoder" => 0xFFFF2407u32, + "TextDecoder" | "globalThis.TextDecoder" => 0xFFFF2408u32, // node:fs constructor exports. Keep these ids in sync with // perry-runtime/src/fs/mod.rs and instanceof.rs. "fs.Dir" => 0xFFFF0086u32, diff --git a/crates/perry-runtime/src/gc/tests/handle_bound_method_name.rs b/crates/perry-runtime/src/gc/tests/handle_bound_method_name.rs index a299a8d6ff0..8c57da8f393 100644 --- a/crates/perry-runtime/src/gc/tests/handle_bound_method_name.rs +++ b/crates/perry-runtime/src/gc/tests/handle_bound_method_name.rs @@ -24,10 +24,13 @@ //! * `get_field_by_name_tail.rs` — the timer-handle arm, twice (a NaN-boxed //! small-handle receiver and an already-stripped handle-band one). //! * `text.rs`'s `text_handle_property` — `TextDecoder.prototype.decode` and -//! `TextEncoder.prototype.encode`/`encodeInto` read as VALUES. That -//! function's own docstring says value reads are the reason it exists +//! `TextEncoder.prototype.encode`/`encodeInto` read as VALUES //! (`K.decode.bind(K)`, "the shape a minified SDK's cached decodeText helper -//! takes"), so this is the intended hot path, not an edge. +//! takes"), so this was the intended hot path, not an edge. #340/#341 made +//! the text family ordinary objects and deleted that reifier: those reads now +//! resolve on the family prototype and bind nothing, so the two text tests +//! below assert the stronger property instead (see +//! `assert_names_the_installed_method`). //! * `ic_miss.rs` — the inline-cache MISS mirror, whose own comment says "the //! IC fast path funnels small handles here, bypassing the identical block in //! `js_object_get_field_by_name`, so it must be mirrored". A separate live @@ -80,6 +83,50 @@ unsafe fn heap_key(name: &str) -> (*mut crate::string::StringHeader, *const u8) /// `&'static [u8]`s that the linker is free to leave at different addresses, /// and it does. Asserting against a locally written literal fails on a correct /// implementation — measured, before this comment existed. +/// #340/#341: for a family whose instances are ORDINARY objects the read +/// resolves the method on the family prototype and returns that prototype's +/// own closure. `js_class_method_bind` never runs, so there is no per-read +/// captured key pointer left to get wrong — the hazard is unwritable for this +/// family rather than merely fixed, and `captured_name` reads capture slots a +/// prototype closure does not have. +/// +/// What still has to hold, and still fails if the lookup regresses: the read +/// yields the REAL installed method (a placeholder or a miss fails here), and +/// the name it carries is the install-time string rather than the caller's +/// movable key-string interior — the same property `assert_names_the_literal` +/// asserts for the bound form. +unsafe fn assert_names_the_installed_method( + value: crate::value::JSValue, + expected: &str, + key_interior: *const u8, + what: &str, +) { + let closure = crate::value::js_nanbox_get_pointer(f64::from_bits(value.bits())) + as *const crate::ClosureHeader; + assert!(!closure.is_null(), "{what}: the read must produce a callable"); + let name = crate::closure::closure_get_dynamic_prop(closure as usize, "name"); + let name_value = crate::value::JSValue::from_bits(name.to_bits()); + assert!( + name_value.is_string(), + "{what}: the read must yield the installed named method, got {:#018x}", + name.to_bits() + ); + let name_ptr = (name.to_bits() & crate::value::POINTER_MASK) as *const crate::StringHeader; + assert!(!name_ptr.is_null(), "{what}: the method name must be a string"); + let interior = (name_ptr as *const u8).add(std::mem::size_of::()); + let got = std::str::from_utf8(std::slice::from_raw_parts( + interior, + (*name_ptr).byte_len as usize, + )) + .unwrap_or(""); + assert_eq!(got, expected, "{what}: wrong method name"); + assert_ne!( + interior, key_interior, + "{what}: the value names the KEY STRING's interior — that allocation \ + is movable and unreachable after this read" + ); +} + unsafe fn assert_names_the_literal( bound: crate::value::JSValue, expected: &'static [u8], @@ -207,17 +254,17 @@ fn a_bound_text_decoder_decode_never_captures_the_key_strings_interior() { unsafe { let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); let id = crate::text::js_text_decoder_new(undefined, undefined, undefined); - assert!( - crate::text::is_known_text_decoder_id(id), - "the decoder arm is gated on `is_known_text_decoder_id`" - ); let (key, key_interior) = heap_key("decode"); + // #340/#341: a decoder is an ordinary object now, so this read resolves + // `decode` on `TextDecoder.prototype` and returns the prototype's own + // closure. No `js_class_method_bind` runs, so there is no captured key + // pointer for #8133 to be about — the hazard is now unwritable for this + // family rather than merely fixed. Assert the stronger property: the + // value is a callable that does NOT alias the heap key's interior. let bound = crate::object::js_object_get_field_by_name(id as *const crate::ObjectHeader, key); - let expected = crate::text::text_decoder_method_name_static(b"decode") - .expect("decode is a TextDecoder method"); - assert_names_the_literal(bound, expected, key_interior, "TextDecoder.decode"); + assert_names_the_installed_method(bound, "decode", key_interior, "TextDecoder.decode"); } } @@ -229,11 +276,10 @@ fn a_bound_text_encoder_method_never_captures_the_key_strings_interior() { let id = crate::text::js_text_encoder_new(); for name in ["encode", "encodeInto"] { let (key, key_interior) = heap_key(name); + // See the decoder sibling: ordinary prototype lookup, no bind. let bound = crate::object::js_object_get_field_by_name(id as *const crate::ObjectHeader, key); - let expected = crate::text::text_encoder_method_name_static(name.as_bytes()) - .expect("a TextEncoder method"); - assert_names_the_literal(bound, expected, key_interior, name); + assert_names_the_installed_method(bound, name, key_interior, name); } } } diff --git a/crates/perry-runtime/src/hot_diag/receiver_repr.rs b/crates/perry-runtime/src/hot_diag/receiver_repr.rs index e7614939fd4..3cf951bb2ba 100644 --- a/crates/perry-runtime/src/hot_diag/receiver_repr.rs +++ b/crates/perry-runtime/src/hot_diag/receiver_repr.rs @@ -202,11 +202,11 @@ fn observe_pointer(addr: usize) { if crate::timer::is_known_timer_id(addr as i64) { mark_old(ReceiverReprFamily::Timer); } - if addr as i64 == crate::text::TEXT_ENCODER_SENTINEL_ID - || crate::text::is_known_text_decoder_id(addr as i64) - { - mark_old(ReceiverReprFamily::Text); - } + // #340/#341 GATE A: `text` has migrated to ordinary objects, so no text + // receiver can be a small band id any more and this family can never be + // marked old again. The fixture below asserts `observed_old == 0` for it; + // that inversion is the per-family record that the migration landed. + // Each remaining family deletes its arm here as it moves. if crate::tui::is_known_handle(addr as i64) { mark_old(ReceiverReprFamily::Tui); } @@ -364,6 +364,42 @@ mod tests { assert_eq!(wrapped, 0, "PR 1 must not create wrappers"); } + /// #340/#341 GATE A. A family that has migrated to ordinary objects can no + /// longer hand a small band id to any funnel, so its `observed_old` bucket + /// must stay 0 while `constructed` keeps moving. This is the per-family + /// record that the migration landed, and it cannot pass by accident: on an + /// unmigrated tree `observe_pointer` still marks the family old and the + /// `assert_eq!(observed, 0)` fails; if a producer ever returns a small id + /// again, the band assertion fails first. + /// + /// It is a gate only from HERE, because the fixture calls the funnels + /// directly. A compiled program's `observed_old` proves nothing for this + /// family — a statically lowered `d.encoding` (codegen's + /// `Expr::TextDecoderEncoding`) never reaches an instrumented funnel, so + /// the counter read 0 before the migration too. Gate B (the producer-side + /// band assertion in `text.rs`'s own tests) covers those reads. + fn assert_fixture_migrated(family: ReceiverReprFamily, construct: impl FnOnce() -> usize) { + receiver_repr_test_reset(); + receiver_repr_test_arm(true); + let value = construct(); + assert!( + !crate::value::addr_class::is_handle_band(value), + "{family:?} producer still returns a small band id ({value:#x})" + ); + receiver_repr_note_decoded_pointer(value); + let (constructed, observed, wrapped) = receiver_repr_test_snapshot(family); + assert!( + constructed > 0, + "{family:?} constructor did not move its bucket" + ); + assert_eq!( + observed, 0, + "{family:?} has migrated to ordinary objects: no receiver of it can \ + be a band id any more" + ); + assert_eq!(wrapped, 0, "PR 1 must not create wrappers"); + } + #[test] fn receiver_repr_family_fixtures_move_constructed_and_observed_old() { // These three producers live in perry-stdlib, below perry-runtime in @@ -397,8 +433,10 @@ mod tests { false, ) }); - assert_fixture(ReceiverReprFamily::Text, || { - (crate::text::js_text_encoder_new() as usize, false) + // #340/#341: `text` is migrated — gate A, inverted. Every other family + // still asserts the old representation above and below. + assert_fixture_migrated(ReceiverReprFamily::Text, || { + crate::text::js_text_encoder_new() as usize }); assert_fixture(ReceiverReprFamily::Tui, || { let mut handle = crate::tui::state::js_perry_tui_state_alloc(0.0); 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 27241ffd51d..e5b0a0715de 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 @@ -1018,11 +1018,6 @@ pub extern "C" fn js_object_get_field_by_name( return JSValue::from_bits(result.to_bits()); } } - // TextDecoder/TextEncoder registry handles — see - // `text_handle_property` (text.rs). - if let Some(v) = crate::text::text_handle_property(raw, key_bytes) { - return v; - } if key_bytes == b"constructor" { let null_obj_ptr = &NULL_OBJECT_BYTES as *const NullObjectBytes as *mut u8; return JSValue::from_bits(JSValue::pointer(null_obj_ptr).bits()); diff --git a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs index f516a45b58a..f1e87f99ab8 100644 --- a/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs +++ b/crates/perry-runtime/src/object/field_get_set/get_field_by_name_tail.rs @@ -75,10 +75,6 @@ pub(crate) fn get_field_by_name_object_tail( return JSValue::from_bits(result.to_bits()); } } - if let Some(v) = crate::text::text_handle_property(raw as usize, key_bytes) - { - return v; - } } // Drizzle-sqlite blocker: synth `data.constructor` for // small-handle native instances so drizzle's @@ -157,9 +153,6 @@ pub(crate) fn get_field_by_name_object_tail( return JSValue::from_bits(result.to_bits()); } } - if let Some(v) = crate::text::text_handle_property(obj as usize, key_bytes) { - return v; - } } if let Some(dispatch) = handle_property_dispatch() { unsafe { diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs index 4005e85cf22..81df15db2aa 100644 --- a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs @@ -765,13 +765,6 @@ pub(super) fn get_field_ic_miss_impl( ); } } - // TextDecoder/TextEncoder registry handles — IC-miss mirror of - // the arms in `js_object_get_field_by_name` / - // `get_field_by_name_object_tail`; static-name reads (`td.decode`, - // `td.encoding`) funnel here. See `text_handle_property`. - if let Some(v) = crate::text::text_handle_property(obj as usize, key_bytes) { - return f64::from_bits(v.bits()); - } } // Drizzle-sqlite blocker: synth `data.constructor` for small-handle // receivers — IC-miss path mirror of the constructor intercept in 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 fa3f8c52312..9788c290e14 100644 --- a/crates/perry-runtime/src/object/global_this/proto_methods.rs +++ b/crates/perry-runtime/src/object/global_this/proto_methods.rs @@ -209,6 +209,67 @@ extern "C" fn object_prototype_dunder_proto_setter_thunk( f64::from_bits(crate::value::TAG_UNDEFINED) } +/// Install a native accessor (getter only) on a builtin prototype. The getter +/// is an ordinary `ClosureHeader` that reads its receiver from `IMPLICIT_THIS` +/// and brand-checks it, so `Object.getOwnPropertyDescriptor(P, k).get.call({})` +/// throws like node's. +/// +/// Two things this must NOT do, both learned rather than assumed: +/// +/// * it must not use `install_fresh_accessor_property`, which flips the +/// process-wide `GLOBAL_DESCRIPTORS_IN_USE` gate (#6809) and would push every +/// later dynamic property write in ANY program that merely constructs a +/// `TextDecoder` onto the descriptor-interception slow walk; +/// * it must put the key in the prototype's keys array, or +/// `Object.getOwnPropertyNames(TextDecoder.prototype)` and `for...in` on an +/// instance would not see it. +/// +/// `install_builtin_getter` does both. It records the ECMA-262 builtin shape +/// (`enumerable: false`), which is right for its other callers +/// (`ArrayBuffer.prototype.byteLength`), so the WebIDL flags are restated on +/// the same entry afterwards: interface members are +/// `{ enumerable: true, configurable: true }`, and node prints +/// `for-in d: decode,encoding,fatal,ignoreBOM`. +#[cfg(feature = "global-text")] +fn install_text_accessor(proto_obj: *mut ObjectHeader, name: &str, getter: *const u8) { + unsafe { + crate::closure::js_register_closure_arity(getter, 0); + let closure = crate::closure::js_closure_alloc(getter, 0); + if closure.is_null() { + return; + } + let get_bits = crate::value::js_nanbox_pointer(closure as i64).to_bits(); + // Spec: an accessor getter's own `.name` is `"get " + key` and its + // `.length` is 0, and it is not constructable. + super::super::native_module::set_bound_native_closure_name(closure, &format!("get {name}")); + super::super::native_module::set_builtin_closure_length(closure as usize, 0); + super::super::native_module::set_builtin_closure_non_constructable(closure as usize); + install_builtin_getter(proto_obj, name, get_bits); + super::super::set_builtin_accessor_descriptor( + proto_obj as usize, + name.to_string(), + super::super::AccessorDescriptor { + get: get_bits, + set: 0, + }, + super::super::PropertyAttrs::new(true, true, true), + ); + } +} + +/// Mark an already-installed prototype method enumerable. WebIDL interface +/// members are enumerable; `install_proto_method` records the ECMA-262 builtin +/// shape (`enumerable: false`), which is right for its other callers, so the +/// text family restates its own entries here instead. +#[cfg(feature = "global-text")] +fn make_proto_member_enumerable(proto_obj: *mut ObjectHeader, name: &str) { + super::super::set_builtin_property_attrs( + proto_obj as usize, + name.to_string(), + super::super::PropertyAttrs::new(true, true, true), + ); +} + pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: *mut ObjectHeader) { if proto_obj.is_null() { return; @@ -824,15 +885,60 @@ pub(crate) fn populate_builtin_prototype_methods(builtin_name: &str, proto_obj: ); install_noop_proto_methods(proto_obj, OBJECT_PROTO_METHODS); } + // #340/#341: these carried `install_noop_proto_methods` placeholders — + // the prototype looked right to reflection while the real work went + // through the handle-dispatch table. Instances are ordinary objects + // now, so the prototype carries the real implementations and an + // ordinary property lookup finds them. #[cfg(feature = "global-text")] "TextEncoder" => { - install_noop_proto_methods(proto_obj, &[("encode", 1), ("encodeInto", 2)]); + install_proto_method( + proto_obj, + "encode", + crate::text::text_encoder_encode_thunk as *const u8, + 1, + ); + install_proto_method( + proto_obj, + "encodeInto", + crate::text::text_encoder_encode_into_thunk as *const u8, + 2, + ); + install_text_accessor( + proto_obj, + "encoding", + crate::text::text_encoder_encoding_getter as *const u8, + ); + for name in ["encode", "encodeInto"] { + make_proto_member_enumerable(proto_obj, name); + } install_noop_proto_methods(proto_obj, OBJECT_PROTO_METHODS); unsafe { install_web_builtin_to_string_tag(proto_obj, "TextEncoder") }; } #[cfg(feature = "global-text")] "TextDecoder" => { - install_noop_proto_methods(proto_obj, &[("decode", 1)]); + install_proto_method( + proto_obj, + "decode", + crate::text::text_decoder_decode_thunk as *const u8, + 1, + ); + install_text_accessor( + proto_obj, + "encoding", + crate::text::text_decoder_encoding_getter as *const u8, + ); + install_text_accessor( + proto_obj, + "fatal", + crate::text::text_decoder_fatal_getter as *const u8, + ); + install_text_accessor( + proto_obj, + "ignoreBOM", + crate::text::text_decoder_ignore_bom_getter as *const u8, + ); + make_proto_member_enumerable(proto_obj, "decode"); install_noop_proto_methods(proto_obj, OBJECT_PROTO_METHODS); unsafe { install_web_builtin_to_string_tag(proto_obj, "TextDecoder") }; } diff --git a/crates/perry-runtime/src/object/meta_accessors.rs b/crates/perry-runtime/src/object/meta_accessors.rs index 6f16068a05e..71d428e1752 100644 --- a/crates/perry-runtime/src/object/meta_accessors.rs +++ b/crates/perry-runtime/src/object/meta_accessors.rs @@ -30,6 +30,7 @@ pub(crate) unsafe fn object_meta_ensure_for_cell(user_ptr: usize) -> Option<*mut (*meta).flags = 0; (*meta).spill = 0; (*meta).private_evaluation_brand = 0; + (*meta).native_state = 0; (*meta).array_subclass_named_prefix_token = 0; (*meta).array_tail_object_hot = 0; (*meta).array_subclass_dense_key = 0; @@ -74,6 +75,7 @@ pub(crate) unsafe fn object_meta_ensure(obj: *mut ObjectHeader) -> *mut ObjectMe (*meta).flags = 0; (*meta).spill = 0; (*meta).private_evaluation_brand = 0; + (*meta).native_state = 0; (*meta).array_subclass_named_prefix_token = 0; (*meta).array_tail_object_hot = 0; (*meta).array_subclass_dense_key = 0; diff --git a/crates/perry-runtime/src/object/mod.rs b/crates/perry-runtime/src/object/mod.rs index 8d2b99c02db..b6e8d86121e 100644 --- a/crates/perry-runtime/src/object/mod.rs +++ b/crates/perry-runtime/src/object/mod.rs @@ -1605,6 +1605,27 @@ pub struct ObjectMeta { /// Distinct descriptor-key count, saturating at 2. See /// [`ObjectMeta::descriptor_key_hash`]. pub descriptor_key_count: u64, + /// #340/#341 honest tags: packed state for a runtime class whose instances + /// are ORDINARY objects rather than small registry handles + /// (`TextEncoder` / `TextDecoder` today; the other twelve families follow). + /// + /// The layout is private to the owning family — `text.rs` packs + /// `(present | encoding index | fatal | ignoreBOM)` here — and `0` means + /// "this object has no native state". It lives in the meta record rather + /// than an inline slot for the same reason as + /// [`ObjectMeta::private_evaluation_brand`]: it must not consume a user + /// field slot, alter the ShapeId or key order, or become visible to + /// enumeration. An inline slot would also be handed to the first user + /// expando (`decoder.mine = 1`) by the slot allocator and overwritten. + /// + /// POD. Never a managed-heap edge — the GC trace arm visits this record's + /// child edges explicitly and this word is not one of them, exactly like + /// `array_tail_object_hot`. + /// + /// LAST FIELD ON PURPOSE: this record carries `offset_of!` assertions for + /// the words codegen and the spill lanes address by index, so a new field + /// may only be appended. + pub native_state: u64, } pub(crate) const OBJECT_META_FLAG_PROTO_DIVERGED: u64 = 1; @@ -1654,6 +1675,11 @@ const _: () = assert!(std::mem::offset_of!(ObjectMeta, array_tail_object_hot) == // `ObjectHeader.meta` then this word (perry-codegen `expr/index_get` and // `property_get/composed_ics.rs`). Keep in lock-step. const _: () = assert!(std::mem::offset_of!(ObjectMeta, elements) == 96); +// #340/#341: `native_state` must stay the LAST word. The offsets above are +// addressed by index from emitted code, so a field inserted mid-struct moves +// them silently; this pins the append instead of trusting the comment. +const _: () = + assert!(std::mem::offset_of!(ObjectMeta, native_state) + 8 == std::mem::size_of::()); const _: () = assert!(std::mem::offset_of!(ObjectHeader, meta) == 8); const _: () = assert!(std::mem::size_of::() == 8); diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 6c12f2f7234..c6406187505 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -1465,46 +1465,12 @@ pub unsafe extern "C-unwind" fn js_native_call_method( args_len > 0 && !args_ptr.is_null() && { crate::value::js_is_truthy(*args_ptr) != 0 }; return js_using_check_disposable(object(), want_async); } - // TextDecoder / TextEncoder registry handles on a type-erased receiver — - // same wall class as the URLSearchParams / AbortSignal blocks below: the - // statically-typed `td.decode(buf)` lowers straight to - // `js_text_decoder_decode_llvm`, but a fused dynamic call (through an - // untyped local, or via the bound method the VALUE read in - // `get_field_by_name_tail.rs` reifies for `K.decode.bind(K)` — the shape - // a minified SDK's cached decodeText helper takes) lands here, and the - // generic field-scan would miss and throw "is not a function". - if matches!(method_name, "decode" | "encode" | "encodeInto") && jsval().is_pointer() { - let raw = (object().to_bits() & 0x0000_FFFF_FFFF_FFFF) as usize; - if crate::value::addr_class::is_small_handle(raw) { - let undef = f64::from_bits(crate::value::TAG_UNDEFINED); - let arg0 = if args_len > 0 && !args_ptr.is_null() { - *args_ptr - } else { - undef - }; - if method_name == "decode" && crate::text::is_known_text_decoder_id(raw as i64) { - let sp = crate::text::js_text_decoder_decode_llvm(object(), arg0); - return f64::from_bits( - JSValue::string_ptr(sp as *mut crate::string::StringHeader).bits(), - ); - } - if raw as i64 == crate::text::TEXT_ENCODER_SENTINEL_ID { - if method_name == "encode" { - let bp = crate::text::js_text_encoder_encode_llvm(arg0); - return crate::value::js_nanbox_pointer(bp); - } - if method_name == "encodeInto" { - let arg1 = if args_len > 1 && !args_ptr.is_null() { - *args_ptr.add(1) - } else { - undef - }; - let rp = crate::text::js_text_encoder_encode_into_llvm(arg0, arg1); - return crate::value::js_nanbox_pointer(rp); - } - } - } - } + // #340/#341: TextDecoder / TextEncoder no longer need an arm here. Their + // instances are ordinary objects linked to a per-family prototype, so a + // fused dynamic call on a type-erased receiver resolves `decode` / `encode` + // / `encodeInto` through the generic prototype walk below, exactly as it + // does for a user class. The arm this replaces existed only because a + // small-handle receiver had no prototype to walk. // #5961/#6710: native URLSearchParams (class_id == 0, leading `_entries` // slot) AND `class X extends URLSearchParams` subclass instances resolve // their method surface via static type-directed lowering. A fused dynamic diff --git a/crates/perry-runtime/src/object/tests.rs b/crates/perry-runtime/src/object/tests.rs index 43f8270dac6..4b6221928d2 100644 --- a/crates/perry-runtime/src/object/tests.rs +++ b/crates/perry-runtime/src/object/tests.rs @@ -1311,9 +1311,9 @@ fn wide_object_own_key_present_uses_index_and_object_values_is_complete() { /// `Headers`/`Request`/`Response`/`Blob` registry id, or any other small native /// handle) as a heap pointer -- `id - 8` / `id` faults on unmapped low memory. /// This is the `claude -p` SIGSEGV (`EXC_BAD_ACCESS` at `0x3FFFB` == -/// `0x40003 - 8`). Every id here is unclaimed in a bare unit-test process -/// (not `TEXT_ENCODER_SENTINEL_ID` either -- see the sibling test below), so -/// the brand must fall through to the generic `[object Object]` tag. +/// `0x40003 - 8`). Every id here is unclaimed in a bare unit-test process, so +/// the brand must fall through to the generic `[object Object]` tag. (The text +/// family no longer mints ids at all -- see the sibling test below.) #[test] fn object_to_string_rejects_handle_band_ids() { use crate::value::addr_class; @@ -1324,11 +1324,6 @@ fn object_to_string_rejects_handle_band_ids() { 3usize, // common native handle, unclaimed ] { assert!(addr_class::is_handle_band(id)); - assert_ne!( - id, - crate::text::TEXT_ENCODER_SENTINEL_ID as usize, - "must not pick an id #10555 gives real meaning to" - ); let handle = crate::value::js_nanbox_pointer(id as i64); // Must return a string brand without dereferencing the bogus pointer. let result = unsafe { js_object_to_string(handle) }; @@ -1340,12 +1335,14 @@ fn object_to_string_rejects_handle_band_ids() { } } -/// #10555: `TEXT_ENCODER_SENTINEL_ID` is the id every `TextEncoder` shares -- -/// unlike the ids above, `js_object_to_string` must brand it `TextEncoder` -/// unconditionally, matching the runtime's own treatment of that id. +/// #10555 / #340 / #341: there is no `TextEncoder` sentinel id any more — an +/// encoder is an ordinary object linked to `TextEncoder.prototype`, so the +/// brand comes from the `Symbol.toStringTag` installed there and the generic +/// own/inherited walk finds it with no special arm. Same observable answer, +/// reached the ordinary way. #[test] -fn object_to_string_brands_the_text_encoder_sentinel() { - let handle = crate::value::js_nanbox_pointer(crate::text::TEXT_ENCODER_SENTINEL_ID); +fn object_to_string_brands_a_real_text_encoder() { + let handle = crate::value::js_nanbox_pointer(crate::text::js_text_encoder_new()); let result = unsafe { js_object_to_string(handle) }; let s = js_string_to_rust(JSValue::from_bits(result.to_bits())); assert_eq!(s, "[object TextEncoder]"); diff --git a/crates/perry-runtime/src/object/to_string_tag.rs b/crates/perry-runtime/src/object/to_string_tag.rs index 1b17f6b5ab4..16b18a1df93 100644 --- a/crates/perry-runtime/src/object/to_string_tag.rs +++ b/crates/perry-runtime/src/object/to_string_tag.rs @@ -54,15 +54,10 @@ pub(crate) fn web_builtin_to_string_tag(value: f64) -> Option<&'static str> { _ => {} } } - // `TextEncoder` is a single stateless sentinel id; `TextDecoder` - // instances are `DECODER_REGISTRY` members. Neither overlaps the - // Web Fetch band (`FETCH_HANDLE_BAND_START` starts well above 2). - if addr == crate::text::TEXT_ENCODER_SENTINEL_ID as usize { - return Some("TextEncoder"); - } - if crate::text::is_known_text_decoder_id(addr as i64) { - return Some("TextDecoder"); - } + // #340/#341: TextEncoder/TextDecoder used to be sentinel ids handled + // here. They are ordinary objects linked to their prototypes now, so + // the generic own/inherited walk finds the `Symbol.toStringTag` + // installed there and no arm is needed. return None; } // #10555 lint: `is_valid_obj_ptr` alone is not a sufficient handle-band diff --git a/crates/perry-runtime/src/text.rs b/crates/perry-runtime/src/text.rs index 7054ed3c57f..02d24299045 100644 --- a/crates/perry-runtime/src/text.rs +++ b/crates/perry-runtime/src/text.rs @@ -8,11 +8,23 @@ //! the IEEE-754 byte pattern of the first byte instead of the byte itself //! (issue #584). //! -//! `TextEncoder` / `TextDecoder` are stateless wrappers — the encoder is -//! always UTF-8, so we return a small sentinel integer NaN-boxed as a -//! pointer on the codegen side. The runtime doesn't need per-instance state. +//! `TextEncoder` / `TextDecoder` instances are ORDINARY objects — a +//! `GC_TYPE_OBJECT` with a real ShapeId, a family class id and a per-family +//! prototype — identical in kind to an object TypeScript itself creates. That +//! is the honest-tag invariant of #340/#341: a `POINTER_TAG` value is always a +//! dereferenceable GC cell, and a runtime-made object is not a second kind of +//! thing. +//! +//! They used to be small registry integers wearing `POINTER_TAG`, which made a +//! value's identity its registry id: every `TextEncoder` shared one sentinel, +//! so two of them were `===`, two collapsed into one `Map`/`Set`/`WeakMap` key, +//! and `JSON.stringify` gave `null` where node gives `{}` (#10821). +//! +//! A decoder's entire state is three scalars, so it rides in the object's own +//! `ObjectMeta.native_state` word (packing below) and `DECODER_REGISTRY` — a +//! mutex-guarded map that grew one entry per `new TextDecoder()` and was never +//! pruned — is gone outright, taking a leak and a lock off the decode path. -use std::collections::HashMap; use std::sync::Mutex; use crate::buffer::{buffer_alloc, buffer_data_mut, mark_as_uint8array, BufferHeader}; @@ -37,6 +49,7 @@ enum DecoderEncoding { Utf16Le, } +#[derive(Clone, Copy)] struct DecoderState { encoding: DecoderEncoding, /// WHATWG-canonical label reported by `decoder.encoding`. @@ -45,9 +58,180 @@ struct DecoderState { ignore_bom: bool, } +// --------------------------------------------------------------------------- +// Honest tags (#340/#341, #10821): instances are ORDINARY objects. +// +// `js_object_alloc(CLASS_ID, 0)` gives a `GC_TYPE_OBJECT` with a real ShapeId +// and ZERO own keys, linked to the per-family prototype that carries +// `decode` / `encode` / `encodeInto` as methods and `encoding` / `fatal` / +// `ignoreBOM` as accessors. Reads take the ordinary prototype path — this +// family has no handle-dispatch arm any more — so node parity follows from the +// object's shape rather than from a dispatch table: `typeof` is `"object"`, +// `Object.keys` is `[]`, `JSON.stringify` is `{}`, `instanceof` works, and two +// instances are two objects (#10821). +// +// Per-instance state lives in `ObjectMeta.native_state`, packed as +// bit 0 present +// bit 1 fatal +// bit 2 ignoreBOM +// bits 8.. index into `ENCODINGS` +// +// `ENCODINGS` interns the DISTINCT encodings a program actually names. It is +// bounded by the compile-time label set (~40 entries), never grows per decoder, +// and therefore needs no pruning when a decoder dies. It replaces +// `DECODER_REGISTRY`, which grew one entry per `new TextDecoder()` and was +// never released — so this also removes a leak and a mutex from `decode`. +// --------------------------------------------------------------------------- + +/// Class ids in the web-builtin block (`0xFFFF_24xx`); `0x2401..=0x2406` are +/// AbortController/AbortSignal/Event/CustomEvent/DOMException/EventTarget. +pub(crate) const TEXT_ENCODER_CLASS_ID: u32 = 0xFFFF_2407; +pub(crate) const TEXT_DECODER_CLASS_ID: u32 = 0xFFFF_2408; + +const STATE_PRESENT: u64 = 1; +const STATE_FATAL: u64 = 1 << 1; +const STATE_IGNORE_BOM: u64 = 1 << 2; +const STATE_ENCODING_SHIFT: u32 = 8; + lazy_static::lazy_static! { - static ref DECODER_REGISTRY: Mutex> = Mutex::new(HashMap::new()); - static ref NEXT_DECODER_ID: Mutex = Mutex::new(2); + /// Distinct `(encoding, canonical label)` pairs named so far. Append-only, + /// bounded by the compile-time label set — NOT a per-instance registry. + static ref ENCODINGS: Mutex> = + Mutex::new(vec![(DecoderEncoding::Utf8, "utf-8")]); +} + +fn intern_encoding(encoding: DecoderEncoding, label: &'static str) -> u64 { + let mut table = ENCODINGS.lock().unwrap(); + // Compared by VALUE: two occurrences of the same canonical label are not + // guaranteed to be one address (thin LTO may duplicate a literal), and an + // address compare would then mint a second row for one encoding. + if let Some(index) = table.iter().position(|(e, l)| *e == encoding && *l == label) { + return index as u64; + } + table.push((encoding, label)); + (table.len() - 1) as u64 +} + +fn encoding_at(index: u64) -> (DecoderEncoding, &'static str) { + ENCODINGS + .lock() + .unwrap() + .get(index as usize) + .copied() + .unwrap_or((DecoderEncoding::Utf8, "utf-8")) +} + +fn pack_decoder_state(state: &DecoderState) -> u64 { + let index = intern_encoding(state.encoding, state.label); + let mut word = STATE_PRESENT | (index << STATE_ENCODING_SHIFT); + if state.fatal { + word |= STATE_FATAL; + } + if state.ignore_bom { + word |= STATE_IGNORE_BOM; + } + word +} + +fn unpack_decoder_state(word: u64) -> Option { + if word & STATE_PRESENT == 0 { + return None; + } + let (encoding, label) = encoding_at(word >> STATE_ENCODING_SHIFT); + Some(DecoderState { + encoding, + label, + fatal: word & STATE_FATAL != 0, + ignore_bom: word & STATE_IGNORE_BOM != 0, + }) +} + +/// The `native_state` word of a text instance, or `None` for any other value. +/// Gated on the class id in the object header, so a foreign receiver +/// (`TextDecoder.prototype.decode.call({})`) is rejected rather than +/// misinterpreted — the same brand check node performs. +fn text_native_state(value: f64, class_id: u32) -> Option { + let bits = value.to_bits(); + if (bits & crate::value::TAG_MASK) != crate::value::POINTER_TAG { + return None; + } + let addr = (bits & crate::value::POINTER_MASK) as usize; + let header = unsafe { crate::value::addr_class::try_read_gc_header(addr)? }; + if header.obj_type != crate::gc::GC_TYPE_OBJECT { + return None; + } + let obj = addr as *mut ObjectHeader; + unsafe { + if (*obj).class_id != class_id { + return None; + } + let meta = (*obj).meta; + if meta.is_null() { + return Some(0); + } + Some((*meta).native_state) + } +} + +/// Decoder state for a receiver, or the utf-8 non-fatal default for a receiver +/// that carries none (matching the previous "unknown handle" behaviour). +fn decoder_state_of(value: f64) -> DecoderState { + text_native_state(value, TEXT_DECODER_CLASS_ID) + .and_then(unpack_decoder_state) + .unwrap_or(DecoderState { + encoding: DecoderEncoding::Utf8, + label: "utf-8", + fatal: false, + ignore_bom: false, + }) +} + +/// Allocate a text instance: an ordinary object with no own keys, linked to +/// its family prototype so methods and accessors resolve by ordinary lookup. +/// +/// The prototype is where this family's whole surface lives, so it is not +/// optional: without it `d.decode` and `d.encoding` are `undefined`. It is +/// built by `populate_builtin_prototype_methods` under `global-text`, which the +/// compiler's feature analysis must therefore enable for any program that +/// constructs one — see the `TextEncoder`/`TextDecoder` arm of +/// `collect_modules/feature_detect.rs`, which matches the folded HIR nodes and +/// not just a quoted type name for this reason. +fn alloc_text_object(class_id: u32, builtin_name: &str, state: u64) -> i64 { + let obj = js_object_alloc(class_id, 0); + if obj.is_null() { + return 0; + } + // Everything below can allocate -- materializing the prototype builds + // `globalThis` lazily and the meta record is its own allocation -- and + // `GC_TYPE_OBJECT` is movable, so the instance is re-read through its + // handle after each one rather than carried as a raw pointer. + let scope = crate::gc::RuntimeHandleScope::new(); + let obj_handle = scope.root_raw_mut_ptr(obj); + let proto_value = crate::object::builtin_prototype_value(builtin_name); + debug_assert!( + crate::value::JSValue::from_bits(proto_value.to_bits()).is_pointer(), + "{builtin_name}.prototype is missing: the instance would have no methods" + ); + if crate::value::JSValue::from_bits(proto_value.to_bits()).is_pointer() { + crate::object::prototype_chain::object_link_class_default_prototype( + obj_handle.get_raw_mut_ptr::() as usize, + proto_value.to_bits(), + ); + } + if state != 0 { + unsafe { + let meta = + crate::object::object_meta_ensure(obj_handle.get_raw_mut_ptr::()); + // A decoder whose state word never landed would read back as the + // lenient utf-8 default -- wrong label, wrong `fatal` -- rather than + // fail, so the only way this stays honest is that the meta exists. + debug_assert!(!meta.is_null(), "a text instance must carry its meta"); + if !meta.is_null() { + (*meta).native_state = state; + } + } + } + obj_handle.get_raw_mut_ptr::() as i64 } /// Map a user-supplied encoding label to (enum, canonical-name). @@ -96,37 +280,23 @@ pub(crate) fn text_encoder_string_ptr(value: f64) -> *const StringHeader { crate::value::js_jsvalue_to_string(value) as *const StringHeader } -/// `new TextEncoder()` — returns a non-null sentinel integer pointer. -/// -/// The returned value is a small integer (`1`) that the codegen NaN-boxes -/// with `POINTER_TAG`. TextEncoder has no state beyond "I encode UTF-8", -/// so any non-null sentinel works. We use a distinct value from the -/// decoder sentinel purely for debuggability. +/// `new TextEncoder()` — an ordinary object with no own keys, linked to +/// `TextEncoder.prototype`. It carries no state (it always encodes UTF-8), but +/// it is still one object per construction: every encoder used to share one +/// sentinel id, which made `new TextEncoder() === new TextEncoder()` true and +/// collapsed two encoders into one `Map` key (#10821). #[no_mangle] pub extern "C" fn js_text_encoder_new() -> i64 { if crate::hot_diag::receiver_repr_on() { crate::hot_diag::receiver_repr_note_constructed(crate::hot_diag::ReceiverReprFamily::Text); } - TEXT_ENCODER_SENTINEL_ID + alloc_text_object(TEXT_ENCODER_CLASS_ID, "TextEncoder", 0) } -/// The stateless `TextEncoder` sentinel handle returned by -/// `js_text_encoder_new` — see its doc comment. -pub const TEXT_ENCODER_SENTINEL_ID: i64 = 1; - -/// Whether `id` is a live `TextDecoder` registry handle. Used by the -/// dynamic method-call / property-GET handle arms -/// (`native_call_method.rs` / `get_field_by_name_tail.rs`) to route -/// `decode`/`encoding`/… on a type-erased receiver to the text natives. -pub fn is_known_text_decoder_id(id: i64) -> bool { - DECODER_REGISTRY.lock().unwrap().contains_key(&id) -} - -/// `new TextDecoder(label?, { fatal?, ignoreBOM? })` — validates the -/// label, stores per-instance decode state in `DECODER_REGISTRY`, and -/// returns a small-int handle that the codegen NaN-boxes with -/// `POINTER_TAG`. An unsupported label throws a `RangeError` -/// (`ERR_ENCODING_NOT_SUPPORTED`). +/// `new TextDecoder(label?, { fatal?, ignoreBOM? })` — validates the label and +/// returns an ordinary object linked to `TextDecoder.prototype`, carrying its +/// decode state in the object's own meta word. An unsupported label throws a +/// `RangeError` (`ERR_ENCODING_NOT_SUPPORTED`). /// /// `label` arrives as a NaN-boxed f64 (`undefined` for the no-arg form); /// `fatal` / `ignore_bom` arrive as NaN-boxed booleans (truthy → on). @@ -168,56 +338,31 @@ fn register_decoder( fatal: crate::value::js_is_truthy(fatal) != 0, ignore_bom: crate::value::js_is_truthy(ignore_bom) != 0, }; - let id = { - let mut next = NEXT_DECODER_ID.lock().unwrap(); - let id = *next; - *next += 1; - id - }; - DECODER_REGISTRY.lock().unwrap().insert(id, state); if crate::hot_diag::receiver_repr_on() { crate::hot_diag::receiver_repr_note_constructed(crate::hot_diag::ReceiverReprFamily::Text); } - id + // The whole per-instance state is three scalars, so it rides in the + // object's own meta word — there is no registry entry to allocate here and + // nothing to release when the decoder dies. + alloc_text_object( + TEXT_DECODER_CLASS_ID, + "TextDecoder", + pack_decoder_state(&state), + ) } -fn decoder_handle_id(handle: f64) -> i64 { - let bits = handle.to_bits(); - const POINTER_TAG: u64 = 0x7FFD_0000_0000_0000; - const POINTER_MASK: u64 = 0x0000_FFFF_FFFF_FFFF; - const TAG_MASK: u64 = 0xFFFF_0000_0000_0000; - if (bits & TAG_MASK) == POINTER_TAG { - (bits & POINTER_MASK) as i64 - } else if !handle.is_nan() && bits != 0 && bits < 0x0001_0000_0000_0000 { - bits as i64 - } else { - 0 - } -} /// `decoder.encoding` — WHATWG-canonical label. #[no_mangle] pub extern "C" fn js_text_decoder_encoding(handle: f64) -> *mut StringHeader { - let id = decoder_handle_id(handle); - let label = DECODER_REGISTRY - .lock() - .unwrap() - .get(&id) - .map(|s| s.label) - .unwrap_or("utf-8"); + let label = decoder_state_of(handle).label; js_string_from_bytes(label.as_ptr(), label.len() as u32) } /// `decoder.fatal` — boolean (NaN-boxed by codegen). #[no_mangle] pub extern "C" fn js_text_decoder_fatal(handle: f64) -> f64 { - let id = decoder_handle_id(handle); - let fatal = DECODER_REGISTRY - .lock() - .unwrap() - .get(&id) - .map(|s| s.fatal) - .unwrap_or(false); + let fatal = decoder_state_of(handle).fatal; if fatal { f64::from_bits(0x7FFC_0000_0000_0004) // TAG_TRUE } else { @@ -228,13 +373,7 @@ pub extern "C" fn js_text_decoder_fatal(handle: f64) -> f64 { /// `decoder.ignoreBOM` — boolean (NaN-boxed by codegen). #[no_mangle] pub extern "C" fn js_text_decoder_ignore_bom(handle: f64) -> f64 { - let id = decoder_handle_id(handle); - let ignore = DECODER_REGISTRY - .lock() - .unwrap() - .get(&id) - .map(|s| s.ignore_bom) - .unwrap_or(false); + let ignore = decoder_state_of(handle).ignore_bom; if ignore { f64::from_bits(0x7FFC_0000_0000_0004) // TAG_TRUE } else { @@ -475,13 +614,8 @@ pub extern "C" fn js_text_encoder_encode_into_llvm(source: f64, dest: f64) -> i6 pub extern "C" fn js_text_decoder_decode_llvm(handle: f64, value: f64) -> i64 { // Pull the decoder state (encoding / fatal). Unknown handle → utf-8, // non-fatal (matches the old stateless default). - let id = decoder_handle_id(handle); - let (encoding, fatal, label) = DECODER_REGISTRY - .lock() - .unwrap() - .get(&id) - .map(|s| (s.encoding, s.fatal, s.label)) - .unwrap_or((DecoderEncoding::Utf8, false, "utf-8")); + let state = decoder_state_of(handle); + let (encoding, fatal, label) = (state.encoding, state.fatal, state.label); // Node `TextDecoder.prototype.decode(input)` input contract: // - omitted / undefined → decode empty (returns ""). @@ -642,86 +776,320 @@ static KEEP_TEXT_DECODER_IGNORE_BOM: extern "C" fn(f64) -> f64 = js_text_decoder /// "Bind must be called on a function"). Methods reify as bound methods — /// the dynamic-call arm in `native_call_method.rs` executes them on call — /// and accessors return their values directly. -/// A `TextDecoder.prototype` method key, as a `'static` byte string. -/// -/// #8133 — the bind below used to hand `js_class_method_bind` the caller's -/// `key_ptr`, which every caller derives as -/// `key_string + size_of::()`: the interior of a movable GC heap -/// string, unreachable the moment the read returns. The closure captures that -/// pointer and `dispatch_bound_method` re-reads it at CALL time, so a copying -/// minor could relocate or reclaim the bytes the closure names. #7747 fixed the -/// same defect on the Buffer path. -/// -/// `text_handle_property` no longer TAKES the caller's pointer at all, so the -/// bug is not merely fixed here, it is unwritable. -pub(crate) fn text_decoder_method_name_static(key: &[u8]) -> Option<&'static [u8]> { - // A named static has one identity across codegen units. Anonymous byte - // literals may be duplicated by thin LTO even within this lookup. - static DECODE: [u8; 6] = *b"decode"; - match key { - b"decode" => Some(&DECODE), - _ => None, - } -} - -/// A `TextEncoder.prototype` method key, as a `'static` byte string. See -/// [`text_decoder_method_name_static`]. -pub(crate) fn text_encoder_method_name_static(key: &[u8]) -> Option<&'static [u8]> { - static ENCODE: [u8; 6] = *b"encode"; - static ENCODE_INTO: [u8; 10] = *b"encodeInto"; - match key { - b"encode" => Some(&ENCODE), - b"encodeInto" => Some(&ENCODE_INTO), - _ => None, - } -} - -/// Bind `method` — always a `'static` literal from the two lookups above — as a -/// bound-method value on `this_f64`. -fn bind_static_method(this_f64: f64, method: &'static [u8]) -> crate::value::JSValue { - let result = crate::object::js_class_method_bind(this_f64, method.as_ptr(), method.len()); - crate::value::JSValue::from_bits(result.to_bits()) -} - -pub(crate) unsafe fn text_handle_property( - raw: usize, - key_bytes: &[u8], -) -> Option { - let this_f64 = f64::from_bits(crate::value::js_nanbox_pointer(raw as i64).to_bits()); - if is_known_text_decoder_id(raw as i64) { - if let Some(method) = text_decoder_method_name_static(key_bytes) { - return Some(bind_static_method(this_f64, method)); - } - match key_bytes { - b"encoding" => { - let s = js_text_decoder_encoding(this_f64); - return Some(crate::value::JSValue::string_ptr(s)); - } - b"fatal" => { - return Some(crate::value::JSValue::from_bits( - js_text_decoder_fatal(this_f64).to_bits(), - )); - } - b"ignoreBOM" => { - return Some(crate::value::JSValue::from_bits( - js_text_decoder_ignore_bom(this_f64).to_bits(), - )); - } - _ => {} - } - } - if raw as i64 == TEXT_ENCODER_SENTINEL_ID { - if let Some(method) = text_encoder_method_name_static(key_bytes) { - return Some(bind_static_method(this_f64, method)); - } +// --------------------------------------------------------------------------- +// Prototype thunks. +// +// A native method or accessor installed on a shared prototype is a +// `ClosureHeader` whose ABI carries no receiver; it reads one from +// `js_implicit_this_get()`. `temporal_proto_getter_thunk` is the template. +// Each thunk brand-checks its receiver against the family class id, so +// `TextDecoder.prototype.decode.call({})` throws a TypeError exactly as node +// does, instead of silently decoding as utf-8. +// --------------------------------------------------------------------------- + +/// Class ids whose instances are ordinary objects carrying native state that +/// cannot cross a thread boundary (#340/#341). A contiguous range so the +/// remaining handle families join it without touching the transfer path again. +pub(crate) fn is_native_backed_class_id(class_id: u32) -> bool { + (TEXT_ENCODER_CLASS_ID..=TEXT_DECODER_CLASS_ID).contains(&class_id) +} + +#[cfg(feature = "global-text")] +fn require_text_brand(value: f64, class_id: u32, message: &[u8]) { + if text_native_state(value, class_id).is_none() { + throw_type_error(message); } - None +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_decoder_decode_thunk( + _closure: *const crate::closure::ClosureHeader, + input: f64, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_DECODER_CLASS_ID, + b"TextDecoder.prototype.decode called on an incompatible receiver", + ); + let s = js_text_decoder_decode_llvm(this, input); + f64::from_bits(crate::value::JSValue::string_ptr(s as *mut StringHeader).bits()) +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_decoder_encoding_getter( + _closure: *const crate::closure::ClosureHeader, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_DECODER_CLASS_ID, + b"TextDecoder.prototype.encoding called on an incompatible receiver", + ); + let s = js_text_decoder_encoding(this); + f64::from_bits(crate::value::JSValue::string_ptr(s).bits()) +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_decoder_fatal_getter( + _closure: *const crate::closure::ClosureHeader, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_DECODER_CLASS_ID, + b"TextDecoder.prototype.fatal called on an incompatible receiver", + ); + js_text_decoder_fatal(this) +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_decoder_ignore_bom_getter( + _closure: *const crate::closure::ClosureHeader, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_DECODER_CLASS_ID, + b"TextDecoder.prototype.ignoreBOM called on an incompatible receiver", + ); + js_text_decoder_ignore_bom(this) +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_encoder_encoding_getter( + _closure: *const crate::closure::ClosureHeader, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_ENCODER_CLASS_ID, + b"TextEncoder.prototype.encoding called on an incompatible receiver", + ); + let s = js_string_from_bytes(b"utf-8".as_ptr(), 5); + f64::from_bits(crate::value::JSValue::string_ptr(s).bits()) +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_encoder_encode_thunk( + _closure: *const crate::closure::ClosureHeader, + input: f64, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_ENCODER_CLASS_ID, + b"TextEncoder.prototype.encode called on an incompatible receiver", + ); + crate::value::js_nanbox_pointer(js_text_encoder_encode_llvm(input)) +} + +#[cfg(feature = "global-text")] +pub(crate) extern "C" fn text_encoder_encode_into_thunk( + _closure: *const crate::closure::ClosureHeader, + source: f64, + dest: f64, +) -> f64 { + let this = crate::object::js_implicit_this_get(); + require_text_brand( + this, + TEXT_ENCODER_CLASS_ID, + b"TextEncoder.prototype.encodeInto called on an incompatible receiver", + ); + crate::value::js_nanbox_pointer(js_text_encoder_encode_into_llvm(source, dest)) } #[cfg(test)] mod tests { use super::*; + /// #10821 / #340 / #341 -- instances are ORDINARY objects. + /// + /// The representation invariant: a text receiver is a `GC_TYPE_OBJECT` + /// with the family class id, above the small-handle band, carrying ZERO + /// own keys. That last part is what keeps `Object.keys` / `JSON.stringify` + /// matching node without a per-kind arm anywhere. + #[test] + fn text_instances_are_ordinary_objects_with_no_own_keys() { + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + let cases = [ + (js_text_encoder_new(), TEXT_ENCODER_CLASS_ID), + (js_text_decoder_new(undef, undef, undef), TEXT_DECODER_CLASS_ID), + ]; + for (raw, class_id) in cases { + let addr = raw as usize; + assert!( + !crate::value::addr_class::is_handle_band(addr), + "an instance must be a real heap object, got {addr:#x}" + ); + let header = unsafe { crate::value::addr_class::try_read_gc_header(addr) } + .expect("instances carry a GcHeader"); + assert_eq!(header.obj_type, crate::gc::GC_TYPE_OBJECT); + let obj = addr as *mut ObjectHeader; + assert_eq!(unsafe { (*obj).class_id }, class_id); + let keys = unsafe { crate::object::object_keys_array(obj) }; + let key_count = if keys.is_null() { + 0 + } else { + unsafe { (*keys).length } + }; + assert_eq!(key_count, 0, "an instance must have no own keys"); + } + } + + /// The COMPILED read path, which is not the one the other tests take. + /// An emitted `td.decode` value read is a per-site inline cache whose miss + /// edge calls `js_object_get_field_ic_slow` with the receiver's 48-bit + /// payload — not `js_object_get_field_by_name`. The method has to resolve + /// on the family prototype through THAT entry too, and nothing else in this + /// file would notice if it did not: measured, a program whose reads took + /// this edge answered `undefined` while every by-name test passed. + #[test] + fn a_method_value_read_resolves_through_the_ic_miss_entry() { + use crate::object::{PicCache, PicCacheSlot, PIC_CACHE_WORDS}; + use std::sync::atomic::AtomicU64; + + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + for (raw, name) in [ + (js_text_decoder_new(undef, undef, undef), "decode"), + (js_text_encoder_new(), "encode"), + (js_text_encoder_new(), "encodeInto"), + ] { + let key = crate::string::js_string_from_bytes(name.as_ptr(), name.len() as u32); + let mut cache: PicCache = [0; PIC_CACHE_WORDS]; + let mut slot: PicCacheSlot = &mut cache; + let packed = AtomicU64::new(0); + let value = crate::object::js_object_get_field_ic_slow( + (raw as u64 & crate::value::POINTER_MASK) as i64, + key, + &mut slot, + &packed, + ); + let jsv = crate::value::JSValue::from_bits(value.to_bits()); + assert!( + jsv.is_pointer(), + "{name}: the IC miss edge must answer the prototype method, got {:#018x}", + value.to_bits() + ); + let ptr = (value.to_bits() & crate::value::POINTER_MASK) as usize; + assert!( + crate::closure::is_closure_ptr(ptr), + "{name}: the answer must be callable" + ); + } + } + + /// Two constructions are two objects. Every `TextEncoder` used to be the + /// same sentinel id, which made `e1 === e2` true and collapsed two + /// encoders into one `Map` key (#10821). + #[test] + fn text_instances_are_distinct_objects() { + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + assert_ne!(js_text_encoder_new(), js_text_encoder_new()); + assert_ne!( + js_text_decoder_new(undef, undef, undef), + js_text_decoder_new(undef, undef, undef) + ); + } + + /// The whole per-instance state round-trips through the meta word, so + /// there is no registry entry behind a decoder and nothing to release. + #[test] + fn decoder_state_round_trips_through_the_meta_word() { + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + let truthy = f64::from_bits(crate::value::TAG_TRUE); + let label = crate::string::js_string_from_bytes(b"latin1".as_ptr(), 6); + let label_value = f64::from_bits(crate::value::JSValue::string_ptr(label).bits()); + let raw = js_text_decoder_new(label_value, undef, truthy); + let boxed = crate::value::js_nanbox_pointer(raw); + + let state = decoder_state_of(boxed); + assert_eq!(state.label, "windows-1252"); + assert!(!state.fatal); + assert!(state.ignore_bom); + + // …and through the public natives the prototype accessors call. + let encoding = js_text_decoder_encoding(boxed); + let got = unsafe { + let len = (*encoding).byte_len as usize; + let data = (encoding as *const u8).add(std::mem::size_of::()); + std::str::from_utf8(std::slice::from_raw_parts(data, len)) + .expect("ASCII") + .to_string() + }; + assert_eq!(got, "windows-1252"); + assert_eq!(js_text_decoder_ignore_bom(boxed).to_bits(), crate::value::TAG_TRUE); + assert_eq!(js_text_decoder_fatal(boxed).to_bits(), crate::value::TAG_FALSE); + } + + /// The encoding intern is bounded by the compile-time label set: naming the + /// same encoding again must NOT grow it. This is the property that lets + /// `DECODER_REGISTRY` go away -- that table grew one entry per decoder and + /// was never pruned, so a long-running program leaked one entry per + /// `new TextDecoder()`. + #[test] + fn the_encoding_intern_does_not_grow_per_decoder() { + let undef = f64::from_bits(crate::value::TAG_UNDEFINED); + let decoder = |label: &str| { + let s = crate::string::js_string_from_bytes(label.as_ptr(), label.len() as u32); + let value = f64::from_bits(crate::value::JSValue::string_ptr(s).bits()); + js_text_decoder_new(value, undef, undef) + }; + // Warm every label this test names, so the measurement below is only + // about REPEATS. + let _ = js_text_decoder_new(undef, undef, undef); + for label in ["latin1", "utf-16le", "ibm866"] { + let _ = decoder(label); + } + let before = ENCODINGS.lock().unwrap().len(); + for _ in 0..64 { + let _ = js_text_decoder_new(undef, undef, undef); + for label in ["latin1", "utf-16le", "ibm866", "windows-1252"] { + let _ = decoder(label); + } + } + assert_eq!( + ENCODINGS.lock().unwrap().len(), + before, + "interning is per ENCODING, not per decoder" + ); + // …and the counter-direction: a label the program has not named yet + // MUST add exactly one row, or the assertion above would pass on an + // implementation that simply never grows. + assert!( + resolve_decoder_label("koi8-r").is_some(), + "koi8-r has to be a supported label for the rest of this to mean anything" + ); + let fresh = decoder("koi8-r"); + assert_ne!(fresh, 0); + assert_eq!( + ENCODINGS.lock().unwrap().len(), + before + 1, + "a newly named encoding interns exactly once" + ); + let _ = decoder("koi8-r"); + assert_eq!( + ENCODINGS.lock().unwrap().len(), + before + 1, + "…and naming it again does not" + ); + } + + /// A foreign receiver is refused rather than silently decoded as utf-8 -- + /// the brand check the prototype thunks make, matching node's + /// `TextDecoder.prototype.decode.call({})`. + #[test] + fn text_native_state_rejects_a_foreign_receiver() { + let plain = crate::object::js_object_alloc(0, 0); + let boxed = crate::value::js_nanbox_pointer(plain as i64); + assert!(text_native_state(boxed, TEXT_DECODER_CLASS_ID).is_none()); + assert!(text_native_state(boxed, TEXT_ENCODER_CLASS_ID).is_none()); + // …and an encoder is not a decoder. + let enc = crate::value::js_nanbox_pointer(js_text_encoder_new()); + assert!(text_native_state(enc, TEXT_DECODER_CLASS_ID).is_none()); + assert!(text_native_state(enc, TEXT_ENCODER_CLASS_ID).is_some()); + } + /// `TextDecoder.decode(dataView)` must read the backing store, not the /// DataView's construction-time snapshot. A `Uint32Array` over the same /// `ArrayBuffer` writes straight into the backing, so the snapshot decoded diff --git a/crates/perry-runtime/src/thread.rs b/crates/perry-runtime/src/thread.rs index 983fcb83093..07348cc79ee 100644 --- a/crates/perry-runtime/src/thread.rs +++ b/crates/perry-runtime/src/thread.rs @@ -446,6 +446,16 @@ pub unsafe fn serialize_nanbox_for_thread(bits: u64) -> SerializedValue { if fs_thread_codec().is_some_and(|codec| (codec.is_filehandle)(value)) { return SerializedValue::DetachedFileHandle; } + // #340/#341: a native-backed builtin (TextEncoder/TextDecoder + // today) is an ORDINARY object now, so the GC kind no longer + // rejects it the way kinds 13-16 do below. Refuse it by class + // id instead — deep-copying one would hand the other thread a + // plain `{}` with no native state, which is exactly the silent + // shape #6185 made these surface a named TypeError for. + let class_id = (*(raw_ptr as *const crate::object::ObjectHeader)).class_id; + if crate::text::is_native_backed_class_id(class_id) { + return SerializedValue::Unsupported("native handle"); + } return serialize_object(raw_ptr as *const crate::object::ObjectHeader); } gc::GC_TYPE_CLOSURE => { diff --git a/crates/perry/src/commands/compile/collect_modules/feature_detect.rs b/crates/perry/src/commands/compile/collect_modules/feature_detect.rs index c16f1b4644d..15e917e49f6 100644 --- a/crates/perry/src/commands/compile/collect_modules/feature_detect.rs +++ b/crates/perry/src/commands/compile/collect_modules/feature_detect.rs @@ -448,7 +448,23 @@ pub(super) fn detect_optional_feature_usage( if hir_debug.contains("\"URL") { ctx.uses_global_url = true; } - if hir_debug.contains("\"Text") { + // Both spellings, and the second one is load-bearing since #340/#341: + // a quoted type name (`(globalThis as any).TextDecoder`, `"TextDecoder"`) + // AND the HIR nodes the text lowering folds a construction into + // (`TextEncoderNew`, `TextDecoderNew`, `TextDecoderDecode`, …), which + // carry no quotes at all. A text instance is now an ordinary object + // linked to `TextEncoder.prototype` / `TextDecoder.prototype`, and those + // prototypes — carrying the family's methods and accessors — are built + // by `populate_builtin_prototype_methods` only under this feature. So a + // program that constructs a decoder without ever naming the type in + // quotes used to get an instance with no prototype, and every + // `d.decode` / `d.encoding` read on it answered `undefined`. Measured; + // `crates/perry/tests/text_decoder_dynamic_surface.rs` is exactly that + // program shape. + if hir_debug.contains("\"Text") + || hir_debug.contains("TextEncoder") + || hir_debug.contains("TextDecoder") + { ctx.uses_global_text = true; } if hir_debug.contains("\"WebSocket\"") { diff --git a/crates/perry/tests/native_handle_identity.rs b/crates/perry/tests/native_handle_identity.rs new file mode 100644 index 00000000000..91bc055bcb2 --- /dev/null +++ b/crates/perry/tests/native_handle_identity.rs @@ -0,0 +1,159 @@ +//! #10821 -- object identity for native-handle receivers. +//! +//! A native-module handle used to travel as a small registry integer under +//! `POINTER_TAG`, so a value's identity was its registry id rather than its +//! object identity. `TextEncoder` is the extreme case: it is stateless, so +//! every instance shared one sentinel id and *every* `TextEncoder` in a program +//! was literally the same value -- `a === b` was true, two of them collapsed +//! into one `Map` key, and a `WeakMap` entry stored under `a` was readable +//! through an unrelated `b`. +//! +//! With honest tags a handle is a real GC cell and identity is the cell +//! address, which is what all of these operations actually want. +//! +//! These tests are the identity contract every migrated family must satisfy: +//! two distinct resources are `!==`, the same resource reached twice is `===`, +//! and both hold as `Map` / `Set` / `WeakMap` keys. + +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.code(), + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +/// Every row here was wrong before the change; the expected strings are node's +/// (node 26.8.1) output for the same program. +#[test] +fn distinct_text_handles_are_distinct_objects() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +const a: any = new TextEncoder(); +const b: any = new TextEncoder(); +console.log(a === b, Object.is(a, b), a === a); +const m = new Map([[a, "one"], [b, "two"]]); +console.log(m.size, m.get(a), m.get(b)); +console.log(new Set([a, b]).size); +const wm = new WeakMap(); +wm.set(a, "A"); +console.log(wm.get(b), wm.get(a)); +console.log([a].indexOf(b), [a].indexOf(a)); +const d1: any = new TextDecoder(); +const d2: any = new TextDecoder(); +console.log(d1 === d2, d1 === d1); +"#, + ); + assert_eq!( + stdout, + "false false true\n\ + 2 one two\n\ + 2\n\ + undefined A\n\ + -1 0\n\ + false true\n" + ); +} + +/// The other half: one resource reached through several routes is ONE object. +/// A family whose registry hands out a fresh cell per lookup would fail here. +#[test] +fn the_same_text_handle_reached_twice_is_the_same_object() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +const d: any = new TextDecoder("latin1"); +const holder: any = { d }; +const viaField = holder.d; +const viaFn = ((x: any) => x)(d); +const viaArray = [d][0]; +console.log(viaField === d, viaFn === d, viaArray === d); +const m = new Map(); +m.set(d, "state"); +console.log(m.get(viaField), m.get(viaFn), m.get(viaArray), m.size); +const wm = new WeakMap(); +wm.set(d, "private"); +console.log(wm.get(viaField)); +// identity must survive a collection, and so must the registry entry +let sink = 0; +for (let i = 0; i < 200000; i++) { const o = { a: i }; sink += o.a; } +console.log(viaField === d, d.encoding, sink); +"#, + ); + assert_eq!( + stdout, + "true true true\n\ + state state state 1\n\ + private\n\ + true windows-1252 19999900000\n" + ); +} + +/// The honest representation must not disturb the surface the family already +/// had: #8133's `K.decode.bind(K)` shape, the accessors, and encode/decode. +#[test] +fn text_handle_surface_survives_the_representation_change() { + let dir = tempfile::tempdir().expect("tempdir"); + let stdout = compile_and_run( + dir.path(), + r#" +const e: any = new TextEncoder(); +const d: any = new TextDecoder(); +console.log(typeof e, typeof d, typeof d.decode, typeof e.encode); +console.log(Object.prototype.toString.call(e), Object.prototype.toString.call(d)); +console.log(e.encoding, d.encoding, d.fatal, d.ignoreBOM); +const bound = d.decode.bind(d); +console.log(bound(new Uint8Array([104, 105])), d.decode(new Uint8Array([111, 107]))); +const u8 = e.encode("hi"); +console.log(u8 instanceof Uint8Array, u8[0], u8[1]); +const dest = new Uint8Array(4); +const r = e.encodeInto("ab", dest); +console.log(r.read, r.written, dest[0], dest[1]); +"#, + ); + assert_eq!( + stdout, + "object object function function\n\ + [object TextEncoder] [object TextDecoder]\n\ + utf-8 utf-8 false false\n\ + hi ok\n\ + true 104 105\n\ + 2 2 97 98\n" + ); +} diff --git a/crates/perry/tests/text_decoder_dynamic_surface.rs b/crates/perry/tests/text_decoder_dynamic_surface.rs index e0542249855..2f290ba224a 100644 --- a/crates/perry/tests/text_decoder_dynamic_surface.rs +++ b/crates/perry/tests/text_decoder_dynamic_surface.rs @@ -19,10 +19,13 @@ //! `TypeError: Bind must be called on a function`, killing every streamed //! (SSE) API response in a large esbuild-bundled CLI app. //! -//! Fix: `text_handle_property` (get_field_by_name_tail.rs) reifies the -//! method/accessor surface for VALUE reads at all three funnels, and a -//! dispatch arm in `native_call_method.rs` routes `decode` / `encode` / -//! `encodeInto` on text registry handles to the text natives. +//! Fix, first as `text_handle_property` (a reifier for VALUE reads at all three +//! funnels) plus a `native_call_method.rs` dispatch arm; since #340/#341 both +//! are DELETED and the surface comes from the representation instead: an +//! instance is an ordinary object linked to `TextDecoder.prototype`, so the +//! same reads and fused calls resolve through the generic prototype walk. The +//! tests below are unchanged and still pin the behaviour — which is the point +//! of having written them against behaviour rather than representation. use std::path::PathBuf; use std::process::Command; diff --git a/scripts/gc_runtime_root_holders.json b/scripts/gc_runtime_root_holders.json index 265e1d82a37..a46b39241f1 100644 --- a/scripts/gc_runtime_root_holders.json +++ b/scripts/gc_runtime_root_holders.json @@ -1117,12 +1117,6 @@ "verdict": "not_a_gc_pointer", "why": "Memoizes the sym_key of the Symbol.for('NextInternalRequestMeta') REGISTERED symbol. Registered / well-known symbols are Box::leak'd (symbol.rs's SYMBOL_REGISTRY / WELL_KNOWN_SYMBOLS), so they live outside the GC arena and their addresses are stable for the process. A FRESH Symbol() would not be \u2014 see #7246." }, - { - "file": "crates/perry-runtime/src/text.rs", - "name": "DECODER_REGISTRY", - "verdict": "not_a_gc_pointer", - "why": "Keyed by decoder handle id; DecoderState is Rust-owned (encoding enum, label, flags), no JS values." - }, { "file": "crates/perry-runtime/src/thread.rs", "name": "ACTIVE_THREAD_JOBS",