Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions changelog.d/honest-handle-tag-text.md
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 9 additions & 0 deletions crates/perry-codegen/src/expr/instance_misc1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,15 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// `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,
Expand Down
72 changes: 59 additions & 13 deletions crates/perry-runtime/src/gc/tests/handle_bound_method_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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::<crate::StringHeader>());
let got = std::str::from_utf8(std::slice::from_raw_parts(
interior,
(*name_ptr).byte_len as usize,
))
.unwrap_or("<non-utf8>");
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],
Expand Down Expand Up @@ -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");
}
}

Expand All @@ -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);
}
}
}
Expand Down
52 changes: 45 additions & 7 deletions crates/perry-runtime/src/hot_diag/receiver_repr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
7 changes: 0 additions & 7 deletions crates/perry-runtime/src/object/field_get_set/ic_miss.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading