Skip to content

Commit 585d2eb

Browse files
author
Ralph Küpper
committed
fix(runtime): an Array subclass in a base-typed binding was read as a raw header (#7574)
1 parent a5fddd3 commit 585d2eb

14 files changed

Lines changed: 798 additions & 17 deletions

File tree

crates/perry-codegen/src/expr/array_push.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,38 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
298298
// head — the inline path's offset-0 length read would
299299
// otherwise pick up the lower 32 bits of the
300300
// forwarding pointer (garbage).
301+
//
302+
// #7574: the same load also has to prove the receiver IS an
303+
// array. `Expr::ArrayPush` is folded from the receiver's
304+
// DECLARED type, and a declared type is a hint, never a layout
305+
// fact (CLAUDE.md, *Known Limitations*), so
306+
// `const a: number[] = new MyArr()` — a `class X extends Array`
307+
// instance, which perry models as a plain `ObjectHeader` —
308+
// reached the inline store below. `ObjectHeader` overlays
309+
// `ArrayHeader` field for field, so `length` read
310+
// `object_type` (= 1) and `capacity` read `class_id` (large):
311+
// `1 < class_id` passed the in-bounds test and the value was
312+
// stored at `handle + 8 + 1*8` — i.e. over `ObjectHeader
313+
// .keys_array`, a live GC child edge — while `length + 1`
314+
// overwrote `object_type`. The SECOND push then SIGSEGVed
315+
// (exit 139) dereferencing `keys_array`, whose bytes were now
316+
// the double `1.0` (fault address `0x3ff0000000000000`).
317+
//
318+
// Route any non-`GC_TYPE_ARRAY` receiver to `js_array_push_f64`
319+
// — the same slow arm forwarding already uses — which resolves
320+
// an array-like object receiver onto the spec-generic engine.
321+
// Strictly more restrictive than the old test: nothing that
322+
// used to take the slow arm now takes the inline store.
323+
let gc_type_addr = blk.sub(I64, &arr_handle, "8");
324+
let gc_type_ptr = blk.inttoptr(I64, &gc_type_addr);
325+
let gc_type = blk.load(I8, &gc_type_ptr);
326+
let not_array = blk.icmp_ne(I8, &gc_type, "1"); // != GC_TYPE_ARRAY
301327
let gc_flags_addr = blk.sub(I64, &arr_handle, "7");
302328
let gc_flags_ptr = blk.inttoptr(I64, &gc_flags_addr);
303329
let gc_flags = blk.load(I8, &gc_flags_ptr);
304330
let fwd_bits = blk.and(I8, &gc_flags, "128");
305-
let is_fwd = blk.icmp_ne(I8, &fwd_bits, "0");
331+
let fwd_set = blk.icmp_ne(I8, &fwd_bits, "0");
332+
let is_fwd = blk.or(I1, &not_array, &fwd_set);
306333

307334
let fwd_idx = ctx.new_block("apush.fwd");
308335
let nofwd_idx = ctx.new_block("apush.nofwd");

crates/perry-codegen/src/expr/index_get.rs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -542,16 +542,32 @@ fn lower_bounded_array_index_get(
542542
// bytes at `arr + 8 + idx*8`, so route through the slow path only when
543543
// the receiver is lazy. Issue #233: also detect FORWARDED arrays; the
544544
// slow path's `clean_arr_ptr` follows the chain.
545+
//
546+
// #7574: the test is now POSITIVE — `obj_type == GC_TYPE_ARRAY` — instead
547+
// of "not lazy". `is_array_expr` is satisfied by a DECLARED `Type::Array`,
548+
// and a declared type is a hint, never a layout fact (CLAUDE.md, *Known
549+
// Limitations*), so `const a: number[] = new MyArr()` (a `class X extends
550+
// Array` instance — a plain `ObjectHeader`) reached the raw
551+
// `gep + load double` at `handle + 8 + idx*8`, i.e. straight into
552+
// `parent_class_id ‖ field_count`, then the `keys_array` and `meta`
553+
// POINTERS — reading two live GC child edges out as user doubles. The
554+
// sibling tier in `index_get/guarded_array.rs` has always tested
555+
// `GC_TYPE_ARRAY` here; this one only excluded lazy arrays.
556+
//
557+
// Strictly more restrictive than the old test (`GC_TYPE_LAZY_ARRAY` is 9,
558+
// so `!= GC_TYPE_ARRAY` subsumes `== GC_TYPE_LAZY_ARRAY`): no receiver that
559+
// used to take the slow path now takes the fast one. It is also one
560+
// instruction CHEAPER — a single `icmp ne` replaces `icmp eq` + `or`.
545561
let gc_type_addr = blk.sub(I64, &arr_handle, "8");
546562
let gc_type_ptr = blk.inttoptr(I64, &gc_type_addr);
547563
let gc_type = blk.load(I8, &gc_type_ptr);
548-
let is_lazy = blk.icmp_eq(I8, &gc_type, "9"); // GC_TYPE_LAZY_ARRAY
564+
let not_array = blk.icmp_ne(I8, &gc_type, "1"); // != GC_TYPE_ARRAY
549565
let gc_flags_addr = blk.sub(I64, &arr_handle, "7");
550566
let gc_flags_ptr = blk.inttoptr(I64, &gc_flags_addr);
551567
let gc_flags = blk.load(I8, &gc_flags_ptr);
552568
let fwd_bits = blk.and(I8, &gc_flags, "128"); // GC_FLAG_FORWARDED
553569
let is_fwd = blk.icmp_ne(I8, &fwd_bits, "0");
554-
let needs_slow = blk.or(I1, &is_lazy, &is_fwd);
570+
let needs_slow = blk.or(I1, &not_array, &is_fwd);
555571
// Index accessors / custom attribute descriptors (`Object.defineProperty
556572
// (arr, i, { get })`) divert element reads through the descriptor tables —
557573
// the raw slot load below would bypass them (test262 sort/precise-*).

crates/perry-runtime/src/array/from_concat.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,6 +781,15 @@ unsafe fn peek_plain_array_len(arr: *const ArrayHeader) -> Option<u32> {
781781
if crate::array::array_ptr_as_proxy(arr).is_some() {
782782
return None;
783783
}
784+
// #7574: `clean_arr_ptr` now REFUSES a `GC_TYPE_OBJECT` allocation, so an
785+
// array-like object — a `class X extends Array` instance among them — would
786+
// fall into the null arm below and be mis-sized as an EMPTY array. It used
787+
// to reach the `obj_type != GC_TYPE_ARRAY` test and answer `None`
788+
// ("un-peekable, size it as 1 and take the spec-shaped per-source flow").
789+
// Keep that answer: classify BEFORE the null shortcut.
790+
if crate::array::subclass::raw_receiver_is_heap_object(arr) {
791+
return None;
792+
}
784793
let arr = clean_arr_ptr(arr);
785794
if arr.is_null() {
786795
return Some(0);
@@ -825,6 +834,16 @@ unsafe fn dense_concat_array_source(src: *const ArrayHeader) -> Option<(*const A
825834
if crate::array::array_ptr_as_proxy(src).is_some() {
826835
return None;
827836
}
837+
// #7574: same hazard as `peek_plain_array_len` — but here mis-classifying a
838+
// `class X extends Array` argument as an empty dense source would SILENTLY
839+
// DROP its elements, because this bulk path returns `Some(out)` and the
840+
// spec-shaped `append_concat_arg` flow (which has the subclass snapshot
841+
// arm) never runs. `[1, 2].concat(sub)` yielded `1,2`. Reject it here so
842+
// the caller falls through, exactly as it did before `clean_arr_ptr`
843+
// started refusing object receivers.
844+
if crate::array::subclass::raw_receiver_is_heap_object(src) {
845+
return None;
846+
}
828847
let src = clean_arr_ptr(src);
829848
if src.is_null() {
830849
return Some((src, 0));

crates/perry-runtime/src/array/header.rs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -620,13 +620,51 @@ pub(crate) fn clean_arr_ptr(arr: *const ArrayHeader) -> *const ArrayHeader {
620620
if (cleaned as usize) >= crate::gc::GC_HEADER_SIZE + 0x1000 {
621621
let gc_header =
622622
(cleaned as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader;
623-
if (*gc_header).obj_type == crate::gc::GC_TYPE_LAZY_ARRAY {
623+
let obj_type = (*gc_header).obj_type;
624+
if obj_type == crate::gc::GC_TYPE_LAZY_ARRAY {
624625
let lazy = cleaned as *mut crate::json_tape::LazyArrayHeader;
625626
if (*lazy).magic == crate::json_tape::LAZY_ARRAY_MAGIC {
626627
let materialized = crate::json_tape::force_materialize_lazy(lazy);
627628
return materialized as *const ArrayHeader;
628629
}
629630
}
631+
// #7574: a `GC_TYPE_OBJECT` / `GC_TYPE_CLOSURE` allocation is NOT
632+
// an `ArrayHeader`, and the two layouts overlay field for field —
633+
// `ArrayHeader.length` reads `ObjectHeader.object_type` (= 1),
634+
// `.capacity` reads `class_id`, and the element slots at +8/+16/+24
635+
// are `parent_class_id ‖ field_count`, `keys_array` and `meta`. The
636+
// sanity check below waves that through (1 <= class_id <= 100M), so
637+
// an element WRITE overwrites two live GC child edges with
638+
// arbitrary doubles and the collector then traces them: `class X
639+
// extends Array` in a `T[]`-annotated binding SIGSEGVs on its
640+
// second `.push()`.
641+
//
642+
// A declared TypeScript type is a hint, never a layout fact
643+
// (CLAUDE.md, *Known Limitations*), so this is reachable from every
644+
// binding form. Refusing it here makes ALL ~190 `clean_arr_ptr`
645+
// call sites fail-closed at once — each degrades through its
646+
// existing null branch instead of dereferencing a forged header —
647+
// and it is the same "resolve at the shared runtime funnel, not at
648+
// one codegen predicate at a time" shape #7573 used for Map/Set.
649+
//
650+
// Correctness (rather than mere safety) for the entry points the
651+
// declared-type tiers actually reach is layered on top: those null
652+
// branches re-enter through `array::subclass::array_object_*`,
653+
// which runs the operation on the spec-generic array-like engine.
654+
//
655+
// Costs one compare on a byte this block already loaded. Buffers
656+
// and typed arrays are `std::alloc`-backed with no `GcHeader`, so
657+
// their preceding bytes are allocator bookkeeping that can read as
658+
// any value — confirm against the registries before nulling, in the
659+
// cold arm only.
660+
if obj_type == crate::gc::GC_TYPE_OBJECT || obj_type == crate::gc::GC_TYPE_CLOSURE {
661+
let addr = cleaned as usize;
662+
if !crate::buffer::is_registered_buffer(addr)
663+
&& crate::typedarray::lookup_typed_array_kind(addr).is_none()
664+
{
665+
return std::ptr::null();
666+
}
667+
}
630668
}
631669
}
632670
// Length/capacity sanity: dense arrays have length <= capacity and

crates/perry-runtime/src/array/indexing.rs

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -660,10 +660,15 @@ pub extern "C" fn js_array_get_element_f64(arr: i64, index: i64) -> f64 {
660660
/// Use when the codegen KNOWS the pointer is a plain Array (not Map/Set/Buffer).
661661
#[no_mangle]
662662
pub extern "C" fn js_array_get_f64_unchecked(arr: *const ArrayHeader, index: u32) -> f64 {
663-
let arr = clean_arr_ptr(arr);
664-
if arr.is_null() {
663+
let cleaned = clean_arr_ptr(arr);
664+
if cleaned.is_null() {
665+
// #7574: array-like OBJECT receiver — see `js_array_get_f64`.
666+
if crate::array::subclass::array_object_receiver(arr).is_some() {
667+
return js_array_get_f64(arr, index);
668+
}
665669
return f64::NAN;
666670
}
671+
let arr = cleaned;
667672
// Index accessors / custom attrs installed via `Object.defineProperty`
668673
// need the descriptor-aware getter.
669674
if array_object_flags(arr) & crate::gc::OBJ_FLAG_ARRAY_DESCRIPTORS != 0 {
@@ -763,10 +768,17 @@ pub extern "C" fn js_array_get_f64(arr: *const ArrayHeader, index: u32) -> f64 {
763768
}
764769
}
765770
}
766-
let arr = clean_arr_ptr(arr);
767-
if arr.is_null() {
771+
let cleaned = clean_arr_ptr(arr);
772+
if cleaned.is_null() {
773+
// #7574: `a[i]` on a `class X extends Array` instance held in a
774+
// `T[]`-annotated binding. Read the object's indexed property through
775+
// the spec-generic `Get`, not the `ObjectHeader` words.
776+
if let Some(recv) = crate::array::subclass::array_object_receiver(arr) {
777+
return crate::array::subclass::array_object_index_get(recv, index);
778+
}
768779
return f64::NAN;
769780
}
781+
let arr = cleaned;
770782
// Check if this is actually a TypedArray — dispatch through typed array helper
771783
if crate::typedarray::lookup_typed_array_kind(arr as usize).is_some() {
772784
return crate::typedarray::js_typed_array_get(
@@ -1074,10 +1086,20 @@ pub extern "C" fn js_array_set_f64_extend(
10741086
) -> *mut ArrayHeader {
10751087
// Demote a uniquely-owned string source — see `js_array_set_f64`.
10761088
crate::string::js_string_addref_if_heap_string(value);
1077-
let arr = clean_arr_ptr_mut(arr);
1078-
if arr.is_null() {
1089+
let cleaned = clean_arr_ptr_mut(arr);
1090+
if cleaned.is_null() {
1091+
// #7574: `a[i] = v` on a `class X extends Array` instance held in a
1092+
// `T[]`-annotated binding. Pre-fix this stored the value into
1093+
// `ObjectHeader.keys_array` / `.meta`. Run the object `[[Set]]` plus
1094+
// the Array-exotic `length` maintenance, and return the ORIGINAL
1095+
// receiver so the caller's realloc write-back keeps the binding.
1096+
if let Some(recv) = crate::array::subclass::array_object_receiver(arr) {
1097+
crate::array::subclass::array_object_index_set(recv, index, value);
1098+
return arr;
1099+
}
10791100
return js_array_alloc(0);
10801101
}
1102+
let arr = cleaned;
10811103
// If this write targets `Array.prototype`, mark the prototype as carrying an
10821104
// indexed property so out-of-bounds element reads on ordinary arrays consult
10831105
// it (ECMA-262 OrdinaryGet → prototype chain). Cheap no-op otherwise.

crates/perry-runtime/src/array/iter_methods.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,18 @@ impl Drop for DenseThisGuard {
107107
/// Returns nothing (void)
108108
#[no_mangle]
109109
pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const ClosureHeader) {
110+
// #7574: `normalize_array_receiver` materializes an array-like OBJECT
111+
// receiver — a `class X extends Array` instance among them — into a fresh
112+
// dense snapshot. The spec passes the RECEIVER as the callback's 3rd
113+
// argument, so without this the callback saw the snapshot and
114+
// `self === sub` was false (the same "forEach's 3rd argument" obligation
115+
// #7573 hit for Map/Set). Gated on a one-load `GC_TYPE_OBJECT` header test,
116+
// so a genuine array pays a compare and never enters the registry probes.
117+
let self_override = if crate::array::subclass::raw_receiver_is_heap_object(arr) {
118+
crate::array::subclass::array_object_receiver(arr)
119+
} else {
120+
None
121+
};
110122
let arr = normalize_array_receiver(arr);
111123
if arr.is_null() {
112124
return;
@@ -142,6 +154,13 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
142154
let length = (*arr).length;
143155
let scope = crate::gc::RuntimeHandleScope::new();
144156
let rooted = RootedIterArray::new(&scope, arr);
157+
// The override is a movable `ObjectHeader` held across user callbacks
158+
// that allocate — root it for the duration of the loop.
159+
let self_handle = self_override.map(|recv| scope.root_nanbox_f64(recv));
160+
let self_value = |rooted: &RootedIterArray| match &self_handle {
161+
Some(h) => h.get_nanbox_f64(),
162+
None => rooted.receiver(),
163+
};
145164
let _tg = DenseThisGuard::bind_undefined();
146165
if crate::array::array_iteration_is_exotic(arr) {
147166
for i in 0..length as usize {
@@ -150,7 +169,7 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
150169
continue;
151170
}
152171
let element = crate::array::array_spec_get(arr, i as u32);
153-
js_closure_call3(callback, element, i as f64, rooted.receiver());
172+
js_closure_call3(callback, element, i as f64, self_value(&rooted));
154173
}
155174
return;
156175
}
@@ -162,7 +181,7 @@ pub extern "C" fn js_array_forEach(arr: *const ArrayHeader, callback: *const Clo
162181
// dispatch path supports call3 safely, so bound native
163182
// methods like `array.forEach(console.log)` can observe the
164183
// source array just like Node.
165-
js_closure_call3(callback, element, i as f64, rooted.receiver());
184+
js_closure_call3(callback, element, i as f64, self_value(&rooted));
166185
}
167186
}
168187
}

crates/perry-runtime/src/array/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ mod subclass;
2727
#[cfg(test)]
2828
mod spread_dense_tests;
2929
#[cfg(test)]
30+
mod subclass_tests;
31+
#[cfg(test)]
3032
mod tests;
3133

3234
pub(crate) use self::alloc::{array_length_range_error, js_array_alloc_pointer_elements};
@@ -141,6 +143,13 @@ pub(crate) use self::sort::object_prototype_index_get as sort_object_prototype_i
141143
pub use self::subclass::{
142144
array_subclass_dense_snapshot, array_subclass_has_iterator_override, is_array_subclass_instance,
143145
};
146+
// #7574 — array-like OBJECT receiver resolution for the raw `js_array_*` entry
147+
// points, plus the Array-exotic `length` maintenance the generic OBJECT index
148+
// store needs for a `class X extends Array` receiver.
149+
pub(crate) use self::subclass::{
150+
array_object_set_length, is_array_subclass_class_id, is_array_subclass_value,
151+
maintain_array_exotic_length, note_array_subclass_index_write,
152+
};
144153
// Issue #1572 — flatten helpers reused by `node_stream::ns_iter_flat_map`
145154
// so an `async function*` mapper return is driven through the iterator
146155
// protocol instead of being appended as a single chunk.

crates/perry-runtime/src/array/push_pop.rs

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -589,10 +589,23 @@ pub extern "C" fn js_array_push_f64(arr: *mut ArrayHeader, value: f64) -> *mut A
589589
}
590590
return arr;
591591
}
592-
let arr = clean_arr_ptr_mut(arr);
593-
if arr.is_null() {
592+
let cleaned = clean_arr_ptr_mut(arr);
593+
if cleaned.is_null() {
594+
// #7574: a `class X extends Array` instance (or any array-like object)
595+
// in a `T[]`-annotated binding. Pre-fix `clean_arr_ptr` waved its
596+
// `ObjectHeader` through and the store below overwrote `keys_array` /
597+
// `meta` — the SECOND push SIGSEGVed (exit 139). Run the spec-generic
598+
// `Array.prototype.push` on the object instead, and return the ORIGINAL
599+
// receiver so codegen's realloc write-back leaves the binding pointing
600+
// at the instance (returning a fresh empty array here is what made the
601+
// push look silently dropped).
602+
if let Some(recv) = crate::array::subclass::array_object_receiver(arr) {
603+
crate::array::subclass::array_object_method(recv, "push", &[value]);
604+
return arr;
605+
}
594606
return js_array_alloc(0);
595607
}
608+
let arr = cleaned;
596609
if array_is_frozen(arr) {
597610
throw_frozen_array_mutation();
598611
}
@@ -786,10 +799,18 @@ pub extern "C" fn js_array_pop_f64(arr: *mut ArrayHeader) -> f64 {
786799
/// here. test262 built-ins/Array length-write-on-frozen.
787800
#[no_mangle]
788801
pub extern "C" fn js_array_set_length_strict(arr: *mut ArrayHeader, new_length: f64) {
789-
let arr = clean_arr_ptr_mut(arr);
790-
if arr.is_null() {
802+
let cleaned = clean_arr_ptr_mut(arr);
803+
if cleaned.is_null() {
804+
// #7574: `a.length = n` on a `class X extends Array` instance reached
805+
// here through the `is_array_expr`-keyed `property_set` lowering and
806+
// wrote `ObjectHeader.object_type`. Perform the Array-exotic
807+
// `Set(O, "length", n, true)` on the object instead.
808+
if let Some(recv) = crate::array::subclass::array_object_receiver(arr) {
809+
crate::array::subclass::array_object_set_length(recv, new_length);
810+
}
791811
return;
792812
}
813+
let arr = cleaned;
793814
if array_object_flags(arr) & crate::gc::OBJ_FLAG_FROZEN != 0 {
794815
throw_non_writable_length();
795816
}

0 commit comments

Comments
 (0)