From 7f398f4ffb359fc82d8f13566a54f8dbabc1aec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Sat, 11 Jul 2026 12:05:42 +0200 Subject: [PATCH] test(runtime): guard the typed-array GcHeader invariant behind plain_array_index_guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plain_array_index_guard` admits an element read to the inline plain-`ArrayHeader` raw-slot fast path, and decides that by back-reading a GcHeader at `addr - GC_HEADER_SIZE` (`gc_header_for_user_addr`) after only coarse address checks. That is sound only while every array-like receiver carries a real GcHeader. Before #6190, typed arrays under the 16 KB threshold were raw-`alloc`'d with no header, so the guard sniffed whatever heap bytes preceded the block; when they looked like GC_TYPE_ARRAY it admitted a typed array to the plain-array reader, which reinterpreted the elements as f64 denormals (~1e-312) and summed them as zero — the silent, ASLR-dependent wrong sums of #6136. #6190 closed it by making every typed array/buffer a GC-heap object. Pin the invariant so it cannot regress silently: - typed_array_alloc always yields a real GcHeader tagged GC_TYPE_TYPED_ARRAY (never GC_TYPE_ARRAY), sized to cover header + elements — covering a small Uint32Array(8), the exact shape that took the deleted off-heap tier. - the guard REJECTS typed-array receivers, both at the internal contract level and through the guard codegen actually emits. - positive control: the guard still ACCEPTS a genuine plain array. Also record the constraint at gc_header_for_user_addr. --- crates/perry-runtime/src/typed_feedback.rs | 12 ++ .../perry-runtime/src/typed_feedback/tests.rs | 119 ++++++++++++++++++ 2 files changed, 131 insertions(+) diff --git a/crates/perry-runtime/src/typed_feedback.rs b/crates/perry-runtime/src/typed_feedback.rs index c447bca1d2..503e20a295 100644 --- a/crates/perry-runtime/src/typed_feedback.rs +++ b/crates/perry-runtime/src/typed_feedback.rs @@ -1071,6 +1071,18 @@ fn finite_nonnegative_i32_index(index: f64) -> Option { } } +/// INVARIANT (load-bearing): every array-like receiver reaching this must carry +/// a real GcHeader. The address checks below are coarse (band floor, canonical +/// form, heap range) — none of them proves a header is actually there, and the +/// `addr - GC_HEADER_SIZE` back-read is taken on faith. Reintroducing an +/// off-GC-heap allocation tier for small typed arrays or buffers (the pre-#6190 +/// raw-`alloc` tier under the 16 KB threshold) therefore does not fault: it +/// silently reads whatever heap bytes precede the block, and when they happen to +/// look like `GC_TYPE_ARRAY` the caller admits a typed array to the inline +/// plain-`ArrayHeader` raw-slot path, which reinterprets its elements as f64 +/// denormals and sums them as zero (#6136; #6190 made every typed array/buffer a +/// GC-heap object). Keep that invariant or replace this with an exact registry +/// lookup — do not weaken it by adding address heuristics here. fn gc_header_for_user_addr(addr: usize) -> Option<*const crate::gc::GcHeader> { if addr < crate::gc::GC_HEADER_SIZE + 0x1000 || (addr as u64) >> 48 != 0 diff --git a/crates/perry-runtime/src/typed_feedback/tests.rs b/crates/perry-runtime/src/typed_feedback/tests.rs index 69439e4854..064452adee 100644 --- a/crates/perry-runtime/src/typed_feedback/tests.rs +++ b/crates/perry-runtime/src/typed_feedback/tests.rs @@ -2201,3 +2201,122 @@ fn normalize_raw_object_addr_rejects_inline_nanboxed_receivers() { "heap-string receiver must pass through unchanged", ); } + +// --------------------------------------------------------------------------- +// #6136 / #6190 — the typed-array GcHeader invariant behind the plain-array +// index guard. +// +// `plain_array_index_guard` decides whether an element read may take the inline +// plain-`ArrayHeader` raw-slot fast path, and it decides that by back-reading a +// `GcHeader` at `addr - GC_HEADER_SIZE` (see `gc_header_for_user_addr`). That is +// only sound while EVERY array-like receiver carries a real GcHeader. Before +// #6190, typed arrays under 16 KB were raw-`alloc`'d with no header, so the +// guard read whatever heap bytes happened to precede the block; when they looked +// like `GC_TYPE_ARRAY` it wrongly admitted a typed array to the plain-array fast +// path, which then reinterpreted the element bytes as f64 (denormals ~1e-312) +// and silently summed them as zero. These tests pin the invariant. +// --------------------------------------------------------------------------- + +/// Every typed array — including a SMALL one, below the 16 KB threshold that +/// used to select the deleted raw-`alloc` tier — must carry a real GcHeader +/// tagged `GC_TYPE_TYPED_ARRAY`, sized to cover header + elements. +#[test] +fn typed_array_alloc_always_carries_a_real_typed_array_gc_header() { + // (kind, length): Uint32Array(8) is 16 B header + 32 B data = 48 B payload, + // the exact shape that took the off-GC-heap tier and caused #6136. + let cases = [ + (crate::typedarray::KIND_UINT32, 8u32, 4usize), + (crate::typedarray::KIND_UINT8, 8, 1), + (crate::typedarray::KIND_FLOAT64, 4, 8), + ]; + for (kind, length, elem_size) in cases { + let ta = crate::typedarray::typed_array_alloc(kind, length); + assert!(!ta.is_null(), "typed_array_alloc returned null"); + + let header = unsafe { + &*((ta as *const u8).sub(crate::gc::GC_HEADER_SIZE) as *const crate::gc::GcHeader) + }; + assert_eq!( + header.obj_type, + crate::gc::GC_TYPE_TYPED_ARRAY, + "a typed array must be a GC-heap object tagged GC_TYPE_TYPED_ARRAY \ + (kind={kind}, length={length}); an off-GC-heap tier makes the \ + plain-array index guard sniff unrelated bytes at addr-8 (#6136)", + ); + assert_ne!( + header.obj_type, + crate::gc::GC_TYPE_ARRAY, + "a typed array must never be tagged GC_TYPE_ARRAY — that is exactly \ + what admits it to the inline plain-ArrayHeader raw-slot reader", + ); + + // The header's size must cover the elements, not just the header: a + // relocation that copied only the header would leave the elements zeroed. + let min_total = crate::gc::GC_HEADER_SIZE + + std::mem::size_of::() + + (length as usize) * elem_size; + assert!( + header.size as usize >= min_total, + "GcHeader.size ({}) must cover header + {} elements (>= {min_total})", + header.size, + length, + ); + } +} + +/// The load-bearing assertion: the plain-array index guard must REJECT a typed +/// array, so the read takes the boxed fallback (which dispatches typed arrays) +/// rather than the inline plain-`ArrayHeader` raw-slot path. +#[test] +fn plain_array_index_get_guard_rejects_typed_array_receivers() { + let _guard = TYPED_FEEDBACK_TEST_LOCK.lock().unwrap(); + reset_typed_feedback_for_tests(); + register(6136, TypedFeedbackSiteKind::ArrayElement, "nd.buf[i]"); + + for (kind, length) in [ + (crate::typedarray::KIND_UINT32, 8u32), + (crate::typedarray::KIND_FLOAT64, 4), + ] { + let ta = crate::typedarray::typed_array_alloc(kind, length); + let ta_box = crate::value::js_nanbox_pointer(ta as i64); + + // The internal contract check — independent of feedback-site state. + assert!( + !plain_array_index_guard(ta as *const ArrayHeader, 0, true), + "plain_array_index_guard must reject a typed array (kind={kind})", + ); + + // The guard as codegen actually calls it. + assert_eq!( + js_typed_feedback_plain_array_index_get_guard(6136, ta_box, 0, 1), + 0, + "the emitted guard must reject a typed-array receiver (kind={kind}) \ + — admitting it reads elements as f64 denormals and sums them as 0 (#6136)", + ); + } +} + +/// Positive control: the guard still ADMITS a genuine plain array, so the test +/// above proves rejection of typed arrays rather than a guard that never passes. +#[test] +fn plain_array_index_get_guard_still_accepts_plain_arrays() { + let _guard = TYPED_FEEDBACK_TEST_LOCK.lock().unwrap(); + reset_typed_feedback_for_tests(); + register(6137, TypedFeedbackSiteKind::ArrayElement, "arr[i]"); + + let arr = crate::array::js_array_alloc(4); + for i in 0..4 { + crate::array::js_array_push_f64(arr, i as f64); + } + let arr_box = crate::value::js_nanbox_pointer(arr as i64); + + assert!( + plain_array_index_guard(arr as *const ArrayHeader, 0, true), + "plain_array_index_guard must accept a plain array", + ); + assert_eq!( + js_typed_feedback_plain_array_index_get_guard(6137, arr_box, 0, 1), + 1, + "the emitted guard must keep admitting plain arrays to the fast path", + ); +}