From b60431ad4f172f7b1e9484218b245ae93a58ac91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 07:57:10 +0200 Subject: [PATCH 1/5] fix(gc): avoid stale tracked-view backing pointers --- .../perry-codegen/src/expr/buffer_access.rs | 43 +++- .../src/expr/computed_store_rooting_tests.rs | 219 ++++++++++++++++-- crates/perry-codegen/src/expr/index_get.rs | 24 +- .../perry-codegen/src/expr/masked_window.rs | 91 ++++---- .../src/expr/proven_view_access.rs | 4 +- 5 files changed, 304 insertions(+), 77 deletions(-) diff --git a/crates/perry-codegen/src/expr/buffer_access.rs b/crates/perry-codegen/src/expr/buffer_access.rs index a24064f155..bbef4054be 100644 --- a/crates/perry-codegen/src/expr/buffer_access.rs +++ b/crates/perry-codegen/src/expr/buffer_access.rs @@ -5,6 +5,7 @@ use crate::native_value::{ BufferAccessFacts, BufferAccessMode, BufferAccessProof, BufferElem, BufferEndian, BufferIndexUnit, ExpectedNativeRep, LoweredValue, MaterializationReason, }; +use crate::rooting; use crate::types::{DOUBLE, F32, I16, I32, I8, PTR}; use super::{ @@ -252,6 +253,18 @@ pub(crate) fn lower_buffer_access_proof( return Ok(None); } + // A tracked ArrayBuffer/native-arena view caches a raw backing pointer in + // `data_slot`. Moving GC rewrites `TYPED_ARRAY_VIEW_META.backing`, not that + // compiler-created alloca, and native-arena disposal can invalidate it + // outright. If evaluating the index can collect or re-enter user code, + // decline before evaluating anything; the caller's dynamic path keeps the + // receiver rooted and resolves its current backing at the consuming call. + // Fresh inline Buffer/TypedArray storage is explicitly non-movable, so its + // hot native path remains eligible. + if !view.storage_inline_proven && rooting::operand_may_collect(ctx, index_expr) { + return Ok(None); + } + // A closure-captured buffer local is hazardous even before any escape // walk stamped `buffer_hazard_reasons` — the closure may mutate/realloc // the buffer between the proof and the access. Consult the capture map @@ -457,12 +470,28 @@ pub(crate) fn lower_buffer_store( value_expr: &Expr, spec: BufferAccessSpec, ) -> Result> { + // Same cached-view rule as `lower_buffer_access_proof`, applied before the + // index is lowered: returning `None` afterward would make the caller's + // fallback evaluate the index twice. Inline-owned storage is non-movable; + // view storage must use the dynamic path when the RHS can collect. + let value_crosses_cached_view = match buffer_expr { + Expr::LocalGet(id) => ctx + .buffer_view_slots + .get(id) + .is_some_and(|view| !view.storage_inline_proven), + _ => false, + } && rooting::operand_may_collect(ctx, value_expr); + if value_crosses_cached_view { + return Ok(None); + } let Some(proof) = lower_buffer_access_proof(ctx, buffer_expr, index_expr, spec)? else { return Ok(None); }; // #7640 section E audit: `proof` contains stable slot metadata and the // lowered native index, not a receiver JSValue or raw backing-store - // pointer. Lower the RHS before `emit_buffer_access_pointer` loads either. + // pointer. The precheck above excludes movable/external views when the RHS + // collects; for inline-owned storage, lower the RHS before + // `emit_buffer_access_pointer` loads the non-movable pointer. let val_i32 = lower_value_i32(ctx, value_expr)?; let emission = emit_buffer_access_pointer(ctx, &proof, spec); let byte_val = ctx.block().trunc(I32, &val_i32, I8); @@ -695,6 +724,12 @@ pub(crate) fn lower_typed_array_store( if matches!(view.elem, BufferElem::F32 | BufferElem::F64) && !is_numeric_expr(ctx, value_expr) { return Ok(None); } + // `data_slot` is a raw cached pointer. It is stable across a collecting RHS + // only for fresh inline typed-array storage; ArrayBuffer and native-arena + // views must fall back before either index or value is evaluated. + if !view.storage_inline_proven && rooting::operand_may_collect(ctx, value_expr) { + return Ok(None); + } let Some(proof) = lower_buffer_access_proof(ctx, array_expr, index_expr, spec)? else { return Ok(None); @@ -710,9 +745,9 @@ pub(crate) fn lower_typed_array_store( // #7640 section E: do not derive `data_ptr` / `elem_ptr` until after the // RHS has been evaluated. A numeric RHS can still be a call, and a raw // backing-store pointer cannot be repaired by the JSValue rooting API. - // The view proof guarantees this binding/data slot is stable, so loading it - // here preserves the already-selected receiver without holding a raw pointer - // across the call. + // The gates above guarantee this is inline-owned, non-movable storage when + // the RHS collects, so loading the slot here does not reuse a GC-stale view + // backing pointer. let result = lower_expr_native(ctx, value_expr, expected)?; let emission = emit_buffer_access_pointer(ctx, &proof, spec); let stored = match proof.view.elem { diff --git a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs index 67ec7294aa..ca5dde464c 100644 --- a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs +++ b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs @@ -25,9 +25,12 @@ //! # What each test asserts, and why it cannot pass vacuously //! //! Each differential test builds the SAME access twice, changing one thing: -//! a later operand is either allocating or inert. Then it compares the -//! shadow-frame width the function reserves. The realloc-barrier regression at -//! the end instead names the exact SSA head consumed by its barrier. +//! a later operand is either allocating or inert. The older slice-4 tests +//! compare root-slot widths; the #7640 tests additionally trace each protected +//! call operand back to its own root slot and assert that the slot store is +//! above the allocating operand while the consuming reload is below it. The +//! realloc-barrier regression at the end instead names the exact SSA head +//! consumed by its barrier. //! //! - `Expr::Number` ⇒ `expr_may_trigger_gc` is false ⇒ `operand_protection` //! returns `Reuse` ⇒ the combinator emits nothing at all, which is the @@ -41,7 +44,9 @@ //! width measured over a store that never got emitted would be hazard 4. use perry_hir::types::Type; -use perry_hir::{Expr, Function, Module as HirModule, Param, Stmt}; +use perry_hir::{BinaryOp, Expr, Function, Module as HirModule, Param, Stmt}; + +use super::slice8_rooting_tests::{call_operand_of, producer_line}; /// Compile a one-function module and return its LLVM IR. fn compile_body(name: &str, body: Vec) -> String { @@ -99,6 +104,64 @@ fn root_slots(ir: &str) -> usize { ir.matches("alloca ptr addrspace(1)").count() + ir.matches("@js_shadow_slot_bind(").count() } +/// Assert that one specific operand consumed by `callee` is stored to a native +/// root above `window_operand`'s production and reloaded from that same slot +/// below it. The tests pin native roots so this checks the actual RS4GC IR, +/// rather than allowing an unrelated allocating expression's slot to satisfy a +/// total-width comparison. +fn assert_call_operand_rooted_across_operand( + ir: &str, + callee: &str, + protected_operand: usize, + window_operand: usize, + what: &str, +) { + let protected = call_operand_of(ir, callee, protected_operand); + let reload = producer_line(ir, &protected); + let reload_line = ir.lines().nth(reload).expect("producer line exists"); + assert!( + reload_line.contains("load ptr addrspace(1), ptr "), + "{what}: {callee} operand {protected_operand} ({protected}) is not reloaded from a \ + native root slot after the collecting operand:\n{ir}" + ); + let slot = reload_line + .rsplit_once(", ptr ") + .map(|(_, tail)| tail.split(',').next().unwrap_or(tail).trim()) + .expect("native root reload names its slot"); + + let window = call_operand_of(ir, callee, window_operand); + let window_line = producer_line(ir, &window); + assert!( + reload > window_line, + "{what}: {callee} operand {protected_operand} is reloaded at line {reload}, above \ + the collecting operand produced at line {window_line}:\n{ir}" + ); + + let store_needle = format!(", ptr {slot}"); + let store = ir + .lines() + .enumerate() + .take(window_line) + .filter(|(_, line)| { + line.contains("store ptr addrspace(1)") + && !line.contains(" null,") + && line.contains(&store_needle) + }) + .map(|(line, _)| line) + .last() + .unwrap_or_else(|| { + panic!( + "{what}: root slot {slot} is read below the window but has no non-null store \ + above it:\n{ir}" + ) + }); + assert!( + store < window_line, + "{what}: root store at line {store} must dominate the collecting operand at line \ + {window_line}:\n{ir}" + ); +} + /// An allocating RHS (`{ a: 1 }`) and an inert one (`1`), so each test can /// compare the same store under a collecting and a non-collecting window. fn allocating_value() -> Expr { @@ -109,6 +172,41 @@ fn inert_value() -> Expr { Expr::Number(1.0) } +fn calls(ir: &str, callee: &str) -> bool { + let needle = format!("@{callee}("); + ir.lines() + .any(|line| line.contains(&needle) && !line.trim_start().starts_with("declare")) +} + +/// A fixed-length Float64Array view over native-arena storage. Unlike a fresh +/// inline typed array, its cached `data_slot` can become invalid across user +/// code (arena disposal) and is not rewritten when GC rewrites side-table +/// backing pointers. +fn with_native_f64_view(tail: Stmt) -> Vec { + vec![ + Stmt::Let { + id: 10, + name: "owner".to_string(), + ty: Type::Any, + mutable: false, + init: Some(Expr::NativeArenaAlloc(Box::new(Expr::Integer(64)))), + }, + Stmt::Let { + id: 11, + name: "view".to_string(), + ty: Type::Named("Float64Array".to_string()), + mutable: false, + init: Some(Expr::NativeArenaView { + owner: Box::new(Expr::LocalGet(10)), + kind: perry_hir::TYPED_ARRAY_KIND_FLOAT64, + byte_offset: Box::new(Expr::Integer(0)), + length: Box::new(Expr::Integer(8)), + }), + }, + tail, + ] +} + /// Assert the store arm named by `callee` was emitted, then that an allocating /// RHS costs strictly more root slots than an inert one. fn assert_operands_rooted_only_when_the_window_collects( @@ -244,6 +342,7 @@ fn polymorphic_index_store_roots_both_operands_across_an_allocating_rhs() { /// consumes the receiver. #[test] fn typed_array_runtime_key_read_roots_receiver_only_when_key_collects() { + let _native_roots = crate::codegen::helpers::NativeRootsPin::native(); let compile = |label: &str, key: Expr| { compile_body_with_params( label, @@ -261,9 +360,18 @@ fn typed_array_runtime_key_read_roots_receiver_only_when_key_collects() { collecting.contains(callee) && inert.contains(callee), "both fixtures must reach the typed-array runtime-key arm:\n{collecting}\n{inert}" ); - assert!( - root_slots(&collecting) > root_slots(&inert), - "an allocating runtime key must protect the typed-array receiver" + assert_call_operand_rooted_across_operand( + &collecting, + "js_typed_array_index_get_dynamic", + 0, + 1, + "the typed-array receiver", + ); + assert_eq!( + root_slots(&collecting), + root_slots(&inert) + 1, + "the inert key must add no temporary root, while the collecting key adds exactly \ + the receiver root" ); } @@ -271,6 +379,7 @@ fn typed_array_runtime_key_read_roots_receiver_only_when_key_collects() { /// after all three JavaScript operands have been evaluated. #[test] fn typed_array_runtime_key_store_roots_operands_only_when_rhs_collects() { + let _native_roots = crate::codegen::helpers::NativeRootsPin::native(); let compile = |label: &str, value: Expr| { compile_body_with_params( label, @@ -292,9 +401,25 @@ fn typed_array_runtime_key_store_roots_operands_only_when_rhs_collects() { collecting.contains(callee) && inert.contains(callee), "both fixtures must reach the typed-array runtime-key store arm:\n{collecting}\n{inert}" ); - assert!( - root_slots(&collecting) > root_slots(&inert), - "an allocating RHS must protect the typed-array receiver and key" + assert_call_operand_rooted_across_operand( + &collecting, + "js_typed_array_index_set_dynamic", + 0, + 2, + "the typed-array receiver", + ); + assert_call_operand_rooted_across_operand( + &collecting, + "js_typed_array_index_set_dynamic", + 1, + 2, + "the typed-array property key", + ); + assert_eq!( + root_slots(&collecting), + root_slots(&inert) + 2, + "the inert RHS must add no temporary roots, while the collecting RHS adds exactly \ + the receiver and key roots" ); } @@ -303,6 +428,7 @@ fn typed_array_runtime_key_store_roots_operands_only_when_rhs_collects() { /// is consumed by the guard diamond. #[test] fn erased_receiver_inline_store_roots_receiver_and_key_across_rhs() { + let _native_roots = crate::codegen::helpers::NativeRootsPin::native(); let compile = |label: &str, value: Expr| { compile_body_with_params( label, @@ -321,10 +447,77 @@ fn erased_receiver_inline_store_roots_receiver_and_key_across_rhs() { collecting.contains(callee) && inert.contains(callee), "both fixtures must reach the #5525 inline dynamic-store arm:\n{collecting}\n{inert}" ); - let extra_slots = root_slots(&collecting).saturating_sub(root_slots(&inert)); + assert_call_operand_rooted_across_operand( + &collecting, + "js_dyn_index_set", + 0, + 2, + "the erased receiver", + ); + assert_call_operand_rooted_across_operand( + &collecting, + "js_dyn_index_set", + 1, + 2, + "the erased property key", + ); + assert_eq!( + root_slots(&collecting), + root_slots(&inert) + 2, + "the inert RHS must add no temporary roots, while the collecting RHS adds exactly \ + the erased receiver and key roots" + ); +} + +/// #7640 E follow-up — a cached `BufferViewSlot::data_slot` is safe across a +/// collecting operand only when the construction proves fresh inline storage. +/// View-backed reads/writes must decline before evaluating either operand and +/// let the rooted runtime fallback resolve the current backing pointer. +#[test] +fn collecting_native_view_operands_decline_the_cached_pointer_fast_path() { + let inert_store = compile_body( + "native_view_inert_store", + with_native_f64_view(Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(11)), + index: Box::new(Expr::Integer(0)), + value: Box::new(Expr::Number(1.0)), + })), + ); + assert!( + !calls(&inert_store, "js_typed_array_set"), + "an inert RHS on a proven fixed-length view should retain the inline store:\n{inert_store}" + ); + + let collecting_store = compile_body( + "native_view_collecting_store", + with_native_f64_view(Stmt::Expr(Expr::IndexSet { + object: Box::new(Expr::LocalGet(11)), + index: Box::new(Expr::Integer(0)), + value: Box::new(Expr::NumberCoerce(Box::new(allocating_value()))), + })), + ); + assert!( + calls(&collecting_store, "js_typed_array_set"), + "a collecting RHS must not reuse a native view's cached raw data pointer:\n\ + {collecting_store}" + ); + + let collecting_index = Expr::Binary { + op: BinaryOp::BitAnd, + left: Box::new(Expr::NumberCoerce(Box::new(allocating_value()))), + right: Box::new(Expr::Integer(0)), + }; + let collecting_load = compile_body( + "native_view_collecting_load", + with_native_f64_view(Stmt::Return(Some(Expr::IndexGet { + object: Box::new(Expr::LocalGet(11)), + index: Box::new(collecting_index), + }))), + ); assert!( - extra_slots >= 2, - "an allocating RHS must protect both erased operands; expected at least two extra slots, got {extra_slots}" + calls(&collecting_load, "js_typed_array_get"), + "a collecting proven index must not reuse a native view's cached raw data pointer:\n\ + {collecting_load}" ); } diff --git a/crates/perry-codegen/src/expr/index_get.rs b/crates/perry-codegen/src/expr/index_get.rs index 36e93ca1b2..80714a9a70 100644 --- a/crates/perry-codegen/src/expr/index_get.rs +++ b/crates/perry-codegen/src/expr/index_get.rs @@ -473,9 +473,11 @@ pub(crate) fn lower_numeric_index_get_for_number_context( if let Some(fact) = super::masked_window::masked_window_fact_for_index(ctx, *arr_id, index.as_ref()) { + let arr_box = lower_expr(ctx, object)?; + let idx_i32 = lower_expr_as_i32(ctx, index)?; return Ok(Some(super::masked_window::lower_masked_window_index_get( - ctx, *arr_id, object, index, &fact, - )?)); + ctx, *arr_id, &arr_box, &idx_i32, &fact, + ))); } } if !is_array_expr(ctx, object) || !expr_has_numeric_pointer_free_array_layout(ctx, object) { @@ -615,9 +617,11 @@ pub(crate) fn lower_unknown_local_index_get_for_number_context( // whole index window, so the read needs no per-access cache probe at all. if let Some(fact) = super::masked_window::masked_window_fact_for_index(ctx, *id, index.as_ref()) { + let arr_box = lower_expr(ctx, object)?; + let idx_i32 = lower_expr_as_i32(ctx, index)?; return Ok(Some(super::masked_window::lower_masked_window_index_get( - ctx, *id, object, index, &fact, - )?)); + ctx, *id, &arr_box, &idx_i32, &fact, + ))); } let recv_unknown = matches!( crate::type_analysis::static_type_of(ctx, object), @@ -1160,9 +1164,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { if let Some(fact) = super::masked_window::masked_window_fact_for_index(ctx, *arr_id, index.as_ref()) { + let arr_box = lower_expr(ctx, object)?; + let idx_i32 = lower_expr_as_i32(ctx, index)?; return Ok(super::masked_window::lower_masked_window_index_get( - ctx, *arr_id, object, index, &fact, - )?); + ctx, *arr_id, &arr_box, &idx_i32, &fact, + )); } } // Issue #514: when the receiver's static type is genuinely @@ -1326,9 +1332,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { *arr_id, index.as_ref(), ) { + let arr_box = lower_expr(ctx, object)?; + let idx_i32 = lower_expr_as_i32(ctx, index)?; return Ok(super::masked_window::lower_masked_window_index_get( - ctx, *arr_id, object, index, &fact, - )?); + ctx, *arr_id, &arr_box, &idx_i32, &fact, + )); } } if let (Expr::LocalGet(arr_id), Expr::LocalGet(idx_id)) = diff --git a/crates/perry-codegen/src/expr/masked_window.rs b/crates/perry-codegen/src/expr/masked_window.rs index 6aa1a6261b..9afc6e5960 100644 --- a/crates/perry-codegen/src/expr/masked_window.rs +++ b/crates/perry-codegen/src/expr/masked_window.rs @@ -16,7 +16,6 @@ use crate::nanbox::POINTER_MASK_I64; use crate::native_value::{ BoundsState, BufferAccessMode, LoweredValue, NativeFactUse, NativeRep, SemanticKind, }; -use crate::rooting; use crate::types::{DOUBLE, I32, I64}; use super::{ @@ -154,56 +153,48 @@ fn window_layout_facts(fact: &MaskedWindowArrayFact, arr_id: u32) -> (Vec, arr_id: u32, - object: &Expr, - index: &Expr, + arr_box: &str, + idx_i32: &str, fact: &MaskedWindowArrayFact, -) -> Result { - // #7640 section E: `static_index_window` admits `call() & MASK` and - // `indexGet() >>> SHIFT`, so the native-i32 index lowering can run arbitrary - // user code. Preserve the already-evaluated receiver across that custom - // lowering, then derive the raw handle only from the post-window re-read. - // Literal/local hot indexes still make `operand_protection` answer `Reuse`, - // so this adds no root traffic to the ordinary masked-loop path. - rooting::with_operands_rooted_across( - ctx, - &[object], - &[index], - |ctx| lower_expr_as_i32(ctx, index), - |ctx, vals, idx_i32| { - let value = emit_window_load_f64(ctx, &vals[0], &idx_i32, fact); - let lowered = LoweredValue { - semantic: SemanticKind::JsNumber, - rep: NativeRep::F64, - llvm_ty: DOUBLE, - value: value.clone(), - }; - let (layout_facts, layout_note) = window_layout_facts(fact, arr_id); - ctx.record_lowered_value_with_access_mode_and_facts( - "NumericArrayIndexGet", - Some(arr_id), - "packed_f64_masked_window_load", - &lowered, - Some(BoundsState::Guarded { - guard_id: fact.guard_id.clone(), - }), - None, - Some(BufferAccessMode::CheckedNative), - None, - None, - None, - layout_facts, - Vec::new(), - false, - false, - vec![ - "index_range=static_window_guarded".to_string(), - "length_range=guarded_i32".to_string(), - layout_note, - ], - ); - Ok(value) - }, - ) +) -> String { + // #7640 section E audit: `static_index_window` alone admits collecting + // operands, but every fact reaching this helper was established by + // `packed_f64_range_loop_pure_expr_collect` / `region_store_operand_collect`. + // Those walks recursively reject calls, stores, updates, closures and + // unrecognized reads, so the fast copy is call-free and its hoisted typed- + // array data pointer cannot cross a collection or disposal point. + let value = emit_window_load_f64(ctx, arr_box, idx_i32, fact); + let lowered = LoweredValue { + semantic: SemanticKind::JsNumber, + rep: NativeRep::F64, + llvm_ty: DOUBLE, + value: value.clone(), + }; + let (layout_facts, layout_note) = window_layout_facts(fact, arr_id); + ctx.record_lowered_value_with_access_mode_and_facts( + "NumericArrayIndexGet", + Some(arr_id), + "packed_f64_masked_window_load", + &lowered, + Some(BoundsState::Guarded { + guard_id: fact.guard_id.clone(), + }), + None, + Some(BufferAccessMode::CheckedNative), + None, + None, + None, + layout_facts, + Vec::new(), + false, + false, + vec![ + "index_range=static_window_guarded".to_string(), + "length_range=guarded_i32".to_string(), + layout_note, + ], + ); + value } /// True when `object[index]` matches an active i32-tier masked-window fact — diff --git a/crates/perry-codegen/src/expr/proven_view_access.rs b/crates/perry-codegen/src/expr/proven_view_access.rs index 7e8efa0a31..4c71eab0cf 100644 --- a/crates/perry-codegen/src/expr/proven_view_access.rs +++ b/crates/perry-codegen/src/expr/proven_view_access.rs @@ -231,8 +231,8 @@ pub(crate) fn try_lower_proven_view_checked_f64_load( }; // #7640 section E audit: `proven_view_for` only reads compile-time facts; // no receiver value or backing-store pointer has been materialized yet. - // Lower both user expressions first, then load `data_slot` below, so even - // a collecting proven index/value leaves no movable or raw address live. + // Lower the index expression first, then load `data_slot` below, so even a + // collecting proven index leaves no movable or raw address live. let idx_i32 = lower_expr_as_i32(ctx, index)?; let (data_ptr, len) = load_data_and_len(ctx, &view); From e8d26767f58aea059a7e540ecbd176c6e4a70e1a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 07:57:59 +0200 Subject: [PATCH 2/5] docs: add changelog for #8014 --- changelog.d/8014-tracked-view-backing.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 changelog.d/8014-tracked-view-backing.md diff --git a/changelog.d/8014-tracked-view-backing.md b/changelog.d/8014-tracked-view-backing.md new file mode 100644 index 0000000000..4f1e246141 --- /dev/null +++ b/changelog.d/8014-tracked-view-backing.md @@ -0,0 +1,10 @@ +**Fixed: tracked native views could retain a stale cached backing pointer across collecting operands (#7640).** + +ArrayBuffer and native-arena views now leave the cached-pointer fast path before +evaluating an index or stored value that can collect or re-enter user code. The +rooted runtime fallback resolves the view's current backing after that window, +while fresh inline Buffer and TypedArray storage keeps its existing fast path. + +Computed-access IR regressions now trace each protected receiver and key through +its exact root slot, and the call-free masked-window invariant is documented so +those loops retain their zero-root direct loads. From a4b5dfc1c46e265e7057da8bcd69ee42e390f4fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 09:17:08 +0200 Subject: [PATCH 3/5] fix(gc): reject collecting masked-window indexes --- changelog.d/8014-tracked-view-backing.md | 5 +- .../src/expr/computed_store_rooting_tests.rs | 95 ++++++++++++++++++- .../perry-codegen/src/expr/masked_window.rs | 9 +- crates/perry-codegen/src/stmt/loops.rs | 73 +++++++++++++- .../src/stmt/masked_window_region.rs | 37 +++++--- 5 files changed, 190 insertions(+), 29 deletions(-) diff --git a/changelog.d/8014-tracked-view-backing.md b/changelog.d/8014-tracked-view-backing.md index 4f1e246141..4a329bf02a 100644 --- a/changelog.d/8014-tracked-view-backing.md +++ b/changelog.d/8014-tracked-view-backing.md @@ -6,5 +6,6 @@ rooted runtime fallback resolves the view's current backing after that window, while fresh inline Buffer and TypedArray storage keeps its existing fast path. Computed-access IR regressions now trace each protected receiver and key through -its exact root slot, and the call-free masked-window invariant is documented so -those loops retain their zero-root direct loads. +its exact root slot. Masked-window tiers also decline collecting index +expressions before hoisting a raw typed-array pointer, while inert indexes keep +their zero-root direct loads. diff --git a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs index ca5dde464c..f0de9ee670 100644 --- a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs +++ b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs @@ -44,7 +44,9 @@ //! width measured over a store that never got emitted would be hazard 4. use perry_hir::types::Type; -use perry_hir::{BinaryOp, Expr, Function, Module as HirModule, Param, Stmt}; +use perry_hir::{ + BinaryOp, CompareOp, Expr, Function, Module as HirModule, Param, Stmt, UnaryOp, UpdateOp, +}; use super::slice8_rooting_tests::{call_operand_of, producer_line}; @@ -137,7 +139,6 @@ fn assert_call_operand_rooted_across_operand( the collecting operand produced at line {window_line}:\n{ir}" ); - let store_needle = format!(", ptr {slot}"); let store = ir .lines() .enumerate() @@ -145,7 +146,9 @@ fn assert_call_operand_rooted_across_operand( .filter(|(_, line)| { line.contains("store ptr addrspace(1)") && !line.contains(" null,") - && line.contains(&store_needle) + && line + .rsplit_once(", ptr ") + .is_some_and(|(_, tail)| tail.split(',').next().unwrap_or(tail).trim() == slot) }) .map(|(line, _)| line) .last() @@ -487,6 +490,13 @@ fn collecting_native_view_operands_decline_the_cached_pointer_fast_path() { !calls(&inert_store, "js_typed_array_set"), "an inert RHS on a proven fixed-length view should retain the inline store:\n{inert_store}" ); + assert!( + inert_store.lines().any(|line| { + line.trim_start().starts_with("store double 1.0, ptr ") && line.contains("!alias.scope") + }), + "the inert fixture must actually emit the native element store, not merely avoid the \ + runtime fallback:\n{inert_store}" + ); let collecting_store = compile_body( "native_view_collecting_store", @@ -521,6 +531,85 @@ fn collecting_native_view_operands_decline_the_cached_pointer_fast_path() { ); } +fn masked_window_coercion_loop(key_ty: Type) -> String { + let masked_index = Expr::Binary { + op: BinaryOp::BitAnd, + left: Box::new(Expr::Unary { + op: UnaryOp::Pos, + operand: Box::new(Expr::LocalGet(2)), + }), + right: Box::new(Expr::Integer(7)), + }; + compile_body_with_params( + "masked_window_coercion", + vec![param(1, "view", Type::Any), param(2, "key", key_ty)], + vec![ + Stmt::Let { + id: 3, + name: "sum".to_string(), + ty: Type::Number, + mutable: true, + init: Some(Expr::Number(0.0)), + }, + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: 4, + name: "i".to_string(), + ty: Type::Any, + mutable: true, + init: Some(Expr::Integer(0)), + })), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(4)), + right: Box::new(Expr::Integer(2)), + }), + update: Some(Expr::Update { + id: 4, + op: UpdateOp::Increment, + prefix: false, + }), + body: vec![Stmt::Expr(Expr::LocalSet( + 3, + Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(3)), + right: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(masked_index), + }), + }), + ))], + }, + Stmt::Return(Some(Expr::LocalGet(3))), + ], + ) +} + +/// #7640 E review follow-up — unary `+` over an `any` key can invoke user +/// coercion even though the surrounding mask has a static index window. Such +/// an index must decline before a typed-array tier hoists its raw data pointer; +/// the same shape with an inert i32 key proves the fast tier remains live. +#[test] +fn collecting_masked_window_index_declines_the_hoisted_pointer_tier() { + let inert = masked_window_coercion_loop(Type::Int32); + assert!( + inert.contains("for.packed_f64_range_fast_ta_i32"), + "the inert control must exercise the masked Int32Array tier:\n{inert}" + ); + + let collecting = masked_window_coercion_loop(Type::Any); + assert!( + calls(&collecting, "js_number_coerce"), + "unary + over an any key must exercise the collecting coercion witness:\n{collecting}" + ); + assert!( + !collecting.contains("for.packed_f64_range_fast_ta_i32"), + "a collecting masked index must decline before the tier hoists a raw backing pointer:\n\ + {collecting}" + ); +} + /// #7640 E — the array-grow helper may return a replacement allocation. The /// write barrier on that path must therefore shade through the returned head, /// not the raw receiver handle computed before the call. diff --git a/crates/perry-codegen/src/expr/masked_window.rs b/crates/perry-codegen/src/expr/masked_window.rs index 9afc6e5960..8dce226cdf 100644 --- a/crates/perry-codegen/src/expr/masked_window.rs +++ b/crates/perry-codegen/src/expr/masked_window.rs @@ -158,11 +158,10 @@ pub(crate) fn lower_masked_window_index_get( fact: &MaskedWindowArrayFact, ) -> String { // #7640 section E audit: `static_index_window` alone admits collecting - // operands, but every fact reaching this helper was established by - // `packed_f64_range_loop_pure_expr_collect` / `region_store_operand_collect`. - // Those walks recursively reject calls, stores, updates, closures and - // unrecognized reads, so the fast copy is call-free and its hoisted typed- - // array data pointer cannot cross a collection or disposal point. + // operands such as `(+key) & 7`. The loop and region matchers pair their + // structural walk with `masked_window_indices_are_non_collecting`, so a + // fact reaching this helper has a non-collecting index and may consume the + // tier's hoisted typed-array pointer directly. let value = emit_window_load_f64(ctx, arr_box, idx_i32, fact); let lowered = LoweredValue { semantic: SemanticKind::JsNumber, diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 6e8938f915..446343b3fa 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -788,7 +788,13 @@ fn match_packed_f64_range_loop( // read-only DENSE mode: several scalar statements, masked // statically-windowed indices, no stores, no side exits. accesses.clear(); - if !packed_f64_range_loop_dense_body_collect(body, counter_id, bound_local, &mut accesses) { + if !packed_f64_range_loop_dense_body_collect( + ctx, + body, + counter_id, + bound_local, + &mut accesses, + ) { return None; } true @@ -1060,6 +1066,7 @@ fn packed_f64_range_loop_store_collect( /// have no side exits, multi-statement bodies are safe: an iteration either /// runs entirely in the fast copy or entirely in the slow copy. fn packed_f64_range_loop_dense_body_collect( + ctx: &FnCtx<'_>, body: &[Stmt], counter_id: u32, bound_local: Option, @@ -1074,7 +1081,9 @@ fn packed_f64_range_loop_dense_body_collect( init: Some(init), .. } => { - if !packed_f64_range_loop_pure_expr_collect(init, counter_id, true, accesses) { + if !masked_window_indices_are_non_collecting(ctx, init) + || !packed_f64_range_loop_pure_expr_collect(init, counter_id, true, accesses) + { return false; } written.insert(*id); @@ -1086,7 +1095,9 @@ fn packed_f64_range_loop_dense_body_collect( if *id == counter_id || Some(*id) == bound_local { return false; } - if !packed_f64_range_loop_pure_expr_collect(value, counter_id, true, accesses) { + if !masked_window_indices_are_non_collecting(ctx, value) + || !packed_f64_range_loop_pure_expr_collect(value, counter_id, true, accesses) + { return false; } written.insert(*id); @@ -1098,7 +1109,9 @@ fn packed_f64_range_loop_dense_body_collect( written.insert(*id); } Stmt::Expr(expr) => { - if !packed_f64_range_loop_pure_expr_collect(expr, counter_id, true, accesses) { + if !masked_window_indices_are_non_collecting(ctx, expr) + || !packed_f64_range_loop_pure_expr_collect(expr, counter_id, true, accesses) + { return false; } } @@ -1110,6 +1123,58 @@ fn packed_f64_range_loop_dense_body_collect( && accesses.keys().all(|arr_id| !written.contains(arr_id)) } +/// The static-window range proof says nothing about evaluating the index. +/// Keep the masked copies that hoist a raw typed-array backing pointer behind +/// the shared collection predicate: `(+key) & 7` has a bounded range, but when +/// `key` is `any`, unary `+` can invoke user coercion and collect. Nested +/// tracked reads are checked recursively for the same reason. +pub(super) fn masked_window_indices_are_non_collecting( + ctx: &FnCtx<'_>, + expr: &perry_hir::Expr, +) -> bool { + use perry_hir::Expr; + match expr { + Expr::IndexGet { index, .. } => { + !crate::rooting::operand_may_collect(ctx, index) + && masked_window_indices_are_non_collecting(ctx, index) + } + Expr::Binary { left, right, .. } + | Expr::Compare { left, right, .. } + | Expr::Logical { left, right, .. } + | Expr::MathImul(left, right) + | Expr::MathPow(left, right) => { + masked_window_indices_are_non_collecting(ctx, left) + && masked_window_indices_are_non_collecting(ctx, right) + } + Expr::Unary { operand, .. } + | Expr::Void(operand) + | Expr::TypeOf(operand) + | Expr::NumberCoerce(operand) + | Expr::BooleanCoerce(operand) + | Expr::MathAbs(operand) + | Expr::MathSqrt(operand) + | Expr::MathFloor(operand) + | Expr::MathCeil(operand) + | Expr::MathRound(operand) + | Expr::MathTrunc(operand) + | Expr::MathSign(operand) + | Expr::MathF16round(operand) => masked_window_indices_are_non_collecting(ctx, operand), + Expr::Conditional { + condition, + then_expr, + else_expr, + } => { + masked_window_indices_are_non_collecting(ctx, condition) + && masked_window_indices_are_non_collecting(ctx, then_expr) + && masked_window_indices_are_non_collecting(ctx, else_expr) + } + Expr::MathMin(values) | Expr::MathMax(values) => values + .iter() + .all(|value| masked_window_indices_are_non_collecting(ctx, value)), + _ => true, + } +} + /// Effect-free expression walk: tracked `a[i ± c]` reads, locals, literals and /// pure arithmetic/Math only. Any store, call, update, closure, or index read /// with an unrecognized receiver/index shape bails the whole match. diff --git a/crates/perry-codegen/src/stmt/masked_window_region.rs b/crates/perry-codegen/src/stmt/masked_window_region.rs index a67c82d170..bc8cc61958 100644 --- a/crates/perry-codegen/src/stmt/masked_window_region.rs +++ b/crates/perry-codegen/src/stmt/masked_window_region.rs @@ -43,9 +43,9 @@ use anyhow::Result; use perry_hir::{Expr, Stmt}; use super::loops::{ - local_is_number_array, local_is_untyped_candidate, packed_f64_range_loop_pure_expr_collect, - packed_loop_array_binding_storage_is_addressable, record_packed_f64_range_static_access, - PackedF64RangeArrayAccess, + local_is_number_array, local_is_untyped_candidate, masked_window_indices_are_non_collecting, + packed_f64_range_loop_pure_expr_collect, packed_loop_array_binding_storage_is_addressable, + record_packed_f64_range_static_access, PackedF64RangeArrayAccess, }; use super::{emit_shadow_clears_after_stmt, lower_stmt}; use crate::expr::{ @@ -352,6 +352,9 @@ fn region_store_operand_collect( expr: &Expr, accesses: &mut std::collections::BTreeMap, ) -> bool { + if !masked_window_indices_are_non_collecting(ctx, expr) { + return false; + } match expr { Expr::IndexGet { object, index } => { let Expr::LocalGet(arr_id) = object.as_ref() else { @@ -416,12 +419,14 @@ pub(super) fn try_match_masked_window_region( let ok = match stmt { Stmt::Expr(Expr::LocalSet(id, value)) => { let mut trial = accesses.clone(); - if packed_f64_range_loop_pure_expr_collect( - value, - REGION_NO_COUNTER, - true, - &mut trial, - ) { + if masked_window_indices_are_non_collecting(ctx, value) + && packed_f64_range_loop_pure_expr_collect( + value, + REGION_NO_COUNTER, + true, + &mut trial, + ) + { accesses = trial; written.insert(*id); true @@ -452,12 +457,14 @@ pub(super) fn try_match_masked_window_region( break; } let mut trial = accesses.clone(); - if packed_f64_range_loop_pure_expr_collect( - expr, - REGION_NO_COUNTER, - true, - &mut trial, - ) { + if masked_window_indices_are_non_collecting(ctx, expr) + && packed_f64_range_loop_pure_expr_collect( + expr, + REGION_NO_COUNTER, + true, + &mut trial, + ) + { accesses = trial; true } else { From 051e3a10b467dee029596bddd5072276482d9f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 10:00:55 +0200 Subject: [PATCH 4/5] fix(gc): reject collecting masked-window expressions --- changelog.d/8014-tracked-view-backing.md | 7 +- .../src/expr/computed_store_rooting_tests.rs | 152 ++++++++++++--- crates/perry-codegen/src/stmt/loops.rs | 175 ++++++++++++++---- .../src/stmt/masked_window_region.rs | 8 +- 4 files changed, 273 insertions(+), 69 deletions(-) diff --git a/changelog.d/8014-tracked-view-backing.md b/changelog.d/8014-tracked-view-backing.md index 4a329bf02a..8d57baf89e 100644 --- a/changelog.d/8014-tracked-view-backing.md +++ b/changelog.d/8014-tracked-view-backing.md @@ -6,6 +6,7 @@ rooted runtime fallback resolves the view's current backing after that window, while fresh inline Buffer and TypedArray storage keeps its existing fast path. Computed-access IR regressions now trace each protected receiver and key through -its exact root slot. Masked-window tiers also decline collecting index -expressions before hoisting a raw typed-array pointer, while inert indexes keep -their zero-root direct loads. +its exact root slot. Masked-window tiers also prove every admitted expression +non-collecting before hoisting a raw typed-array pointer, including coercions +between element reads, while inert numeric expressions keep their zero-root +direct loads. diff --git a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs index f0de9ee670..36eff106ca 100644 --- a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs +++ b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs @@ -531,15 +531,7 @@ fn collecting_native_view_operands_decline_the_cached_pointer_fast_path() { ); } -fn masked_window_coercion_loop(key_ty: Type) -> String { - let masked_index = Expr::Binary { - op: BinaryOp::BitAnd, - left: Box::new(Expr::Unary { - op: UnaryOp::Pos, - operand: Box::new(Expr::LocalGet(2)), - }), - right: Box::new(Expr::Integer(7)), - }; +fn compile_masked_window_loop(key_ty: Type, value: Expr) -> String { compile_body_with_params( "masked_window_coercion", vec![param(1, "view", Type::Any), param(2, "key", key_ty)], @@ -569,36 +561,107 @@ fn masked_window_coercion_loop(key_ty: Type) -> String { op: UpdateOp::Increment, prefix: false, }), - body: vec![Stmt::Expr(Expr::LocalSet( - 3, - Box::new(Expr::Binary { - op: BinaryOp::Add, - left: Box::new(Expr::LocalGet(3)), - right: Box::new(Expr::IndexGet { - object: Box::new(Expr::LocalGet(1)), - index: Box::new(masked_index), - }), - }), - ))], + body: vec![Stmt::Expr(Expr::LocalSet(3, Box::new(value)))], }, Stmt::Return(Some(Expr::LocalGet(3))), ], ) } +fn masked_window_index_coercion_loop(key_ty: Type) -> String { + let masked_index = Expr::Binary { + op: BinaryOp::BitAnd, + left: Box::new(Expr::Unary { + op: UnaryOp::Pos, + operand: Box::new(Expr::LocalGet(2)), + }), + right: Box::new(Expr::Integer(7)), + }; + compile_masked_window_loop( + key_ty, + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::LocalGet(3)), + right: Box::new(Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(masked_index), + }), + }, + ) +} + +fn masked_window_rhs_coercion_loop(key_ty: Type) -> String { + let read = |index| Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::Integer(index)), + }; + compile_masked_window_loop( + key_ty, + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(read(0)), + right: Box::new(Expr::Unary { + op: UnaryOp::Pos, + operand: Box::new(Expr::LocalGet(2)), + }), + }), + right: Box::new(read(1)), + }, + ) +} + +fn masked_window_rhs_coercion_region(key_ty: Type) -> String { + let value = || { + let read = |index| Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::Integer(index)), + }; + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(Expr::Binary { + op: BinaryOp::Add, + left: Box::new(read(0)), + right: Box::new(Expr::Unary { + op: UnaryOp::Pos, + operand: Box::new(Expr::LocalGet(2)), + }), + }), + right: Box::new(read(1)), + } + }; + let mut body = vec![Stmt::Let { + id: 3, + name: "sum".into(), + ty: Type::Number, + init: Some(Expr::Number(0.0)), + mutable: true, + }]; + for _ in 0..4 { + body.push(Stmt::Expr(Expr::LocalSet(3, Box::new(value())))); + } + body.push(Stmt::Return(Some(Expr::LocalGet(3)))); + compile_body_with_params( + "masked_window_rhs_coercion_region", + vec![param(1, "view", Type::Any), param(2, "key", key_ty)], + body, + ) +} + /// #7640 E review follow-up — unary `+` over an `any` key can invoke user /// coercion even though the surrounding mask has a static index window. Such /// an index must decline before a typed-array tier hoists its raw data pointer; /// the same shape with an inert i32 key proves the fast tier remains live. #[test] fn collecting_masked_window_index_declines_the_hoisted_pointer_tier() { - let inert = masked_window_coercion_loop(Type::Int32); + let inert = masked_window_index_coercion_loop(Type::Int32); assert!( inert.contains("for.packed_f64_range_fast_ta_i32"), "the inert control must exercise the masked Int32Array tier:\n{inert}" ); - let collecting = masked_window_coercion_loop(Type::Any); + let collecting = masked_window_index_coercion_loop(Type::Any); assert!( calls(&collecting, "js_number_coerce"), "unary + over an any key must exercise the collecting coercion witness:\n{collecting}" @@ -610,6 +673,51 @@ fn collecting_masked_window_index_declines_the_hoisted_pointer_tier() { ); } +/// The same proof must cover coercion BETWEEN masked reads, not only inside an +/// index. Otherwise the second read consumes the tier's hoisted pointer after +/// `+key` has been allowed to run user code and move or dispose its backing. +#[test] +fn collecting_rhs_between_masked_reads_declines_the_hoisted_pointer_tier() { + let inert = masked_window_rhs_coercion_loop(Type::Int32); + assert!( + inert.contains("for.packed_f64_range_fast_ta_i32"), + "the inert RHS control must retain the masked Int32Array tier:\n{inert}" + ); + + let collecting = masked_window_rhs_coercion_loop(Type::Any); + assert!( + calls(&collecting, "js_number_coerce"), + "the any-typed RHS must exercise the user-coercion witness:\n{collecting}" + ); + assert!( + !collecting.contains("for.packed_f64_range_fast_ta_i32"), + "collecting coercion between masked reads must decline the hoisted-pointer tier:\n\ + {collecting}" + ); +} + +/// The straight-line masked region installs the same pointer facts, including +/// for later store operands. Exercise that caller independently of the loop +/// matcher so the shared whole-expression gate cannot regress on either path. +#[test] +fn collecting_rhs_declines_the_straight_line_masked_region() { + let inert = masked_window_rhs_coercion_region(Type::Int32); + assert!( + inert.contains("masked_region.ta_i32.preheader"), + "the inert RHS control must retain straight-line masked versioning:\n{inert}" + ); + + let collecting = masked_window_rhs_coercion_region(Type::Any); + assert!( + calls(&collecting, "js_number_coerce"), + "the any-typed region RHS must exercise the user-coercion witness:\n{collecting}" + ); + assert!( + !collecting.contains("masked_region.ta_i32.preheader"), + "collecting coercion must decline the straight-line masked region:\n{collecting}" + ); +} + /// #7640 E — the array-grow helper may return a replacement allocation. The /// write barrier on that path must therefore shade through the returned head, /// not the raw receiver handle computed before the call. diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index 446343b3fa..a97289160d 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -1081,7 +1081,7 @@ fn packed_f64_range_loop_dense_body_collect( init: Some(init), .. } => { - if !masked_window_indices_are_non_collecting(ctx, init) + if !masked_window_expression_is_non_collecting(ctx, init) || !packed_f64_range_loop_pure_expr_collect(init, counter_id, true, accesses) { return false; @@ -1095,7 +1095,7 @@ fn packed_f64_range_loop_dense_body_collect( if *id == counter_id || Some(*id) == bound_local { return false; } - if !masked_window_indices_are_non_collecting(ctx, value) + if !masked_window_expression_is_non_collecting(ctx, value) || !packed_f64_range_loop_pure_expr_collect(value, counter_id, true, accesses) { return false; @@ -1109,7 +1109,7 @@ fn packed_f64_range_loop_dense_body_collect( written.insert(*id); } Stmt::Expr(expr) => { - if !masked_window_indices_are_non_collecting(ctx, expr) + if !masked_window_expression_is_non_collecting(ctx, expr) || !packed_f64_range_loop_pure_expr_collect(expr, counter_id, true, accesses) { return false; @@ -1123,55 +1123,150 @@ fn packed_f64_range_loop_dense_body_collect( && accesses.keys().all(|arr_id| !written.contains(arr_id)) } -/// The static-window range proof says nothing about evaluating the index. -/// Keep the masked copies that hoist a raw typed-array backing pointer behind -/// the shared collection predicate: `(+key) & 7` has a bounded range, but when -/// `key` is `any`, unary `+` can invoke user coercion and collect. Nested -/// tracked reads are checked recursively for the same reason. -pub(super) fn masked_window_indices_are_non_collecting( +/// Prove that an expression lowered while masked-window facts are active cannot +/// collect. The structural matcher knows each admitted `IndexGet` becomes a +/// guarded numeric load, so the proof treats its RESULT as an inert number but +/// still checks its INDEX expression recursively: a bounded shape such as +/// `(+key) & 7` can invoke user coercion when `key` is `any`. +/// +/// Checking the WHOLE operator tree matters as much as checking indexes. In +/// `ta[0] + (+key) + ta[1]`, the tier's hoisted backing pointer crosses the +/// middle coercion before the second load. Merely proving both indexes inert +/// leaves that broader window open. +pub(super) fn masked_window_expression_is_non_collecting( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, ) -> bool { - use perry_hir::Expr; + masked_window_expression_proof(ctx, expr).is_some() +} + +/// Facts about a value whose evaluation has also been proved non-collecting. +/// `inert` means coercing the result cannot dispatch user code; `numeric` is +/// the stronger fact needed to distinguish numeric `+` from concatenation. +#[derive(Clone, Copy)] +struct MaskedWindowExpressionProof { + inert: bool, + numeric: bool, +} + +/// Prove the collection behavior of the whole expression while computing the +/// two result facts its parents need. This is deliberately an allowlist: +/// `None` is the conservative answer for forms the masked structural walkers +/// do not admit. +fn masked_window_expression_proof( + ctx: &FnCtx<'_>, + expr: &perry_hir::Expr, +) -> Option { + use perry_hir::{BinaryOp, CompareOp, Expr, UnaryOp}; + let proof = |inert, numeric| MaskedWindowExpressionProof { inert, numeric }; match expr { - Expr::IndexGet { index, .. } => { - !crate::rooting::operand_may_collect(ctx, index) - && masked_window_indices_are_non_collecting(ctx, index) + // The structural matcher separately proves this is a tracked masked + // read. Under its active fact the access itself is a guarded numeric + // load, but evaluating the index must still pass this same whole-tree + // proof before that fact may be installed. + Expr::IndexGet { object, index } => { + if !matches!(object.as_ref(), Expr::LocalGet(_)) { + return None; + } + masked_window_expression_proof(ctx, index)?; + Some(proof(true, true)) + } + Expr::Number(_) | Expr::Integer(_) => Some(proof(true, true)), + Expr::Bool(_) | Expr::Null | Expr::Undefined => Some(proof(true, false)), + Expr::LocalGet(_) => { + let inert = crate::rooting::expr_is_inert_primitive(ctx, expr); + Some(proof( + inert, + inert && crate::type_analysis::is_numeric_expr(ctx, expr), + )) } - Expr::Binary { left, right, .. } - | Expr::Compare { left, right, .. } - | Expr::Logical { left, right, .. } - | Expr::MathImul(left, right) - | Expr::MathPow(left, right) => { - masked_window_indices_are_non_collecting(ctx, left) - && masked_window_indices_are_non_collecting(ctx, right) + Expr::Binary { op, left, right } => { + let left = masked_window_expression_proof(ctx, left)?; + let right = masked_window_expression_proof(ctx, right)?; + if matches!(op, BinaryOp::Add) { + if !left.numeric || !right.numeric { + return None; + } + } else if !left.inert || !right.inert { + return None; + } + Some(proof(true, true)) + } + Expr::Compare { op, left, right } => { + let left = masked_window_expression_proof(ctx, left)?; + let right = masked_window_expression_proof(ctx, right)?; + if !matches!(op, CompareOp::Eq | CompareOp::Ne) && (!left.inert || !right.inert) { + return None; + } + Some(proof(true, false)) + } + Expr::Unary { op, operand } => { + let operand = masked_window_expression_proof(ctx, operand)?; + if !matches!(op, UnaryOp::Not) && !operand.inert { + return None; + } + Some(proof(true, !matches!(op, UnaryOp::Not))) + } + Expr::Logical { left, right, .. } => { + let left = masked_window_expression_proof(ctx, left)?; + let right = masked_window_expression_proof(ctx, right)?; + Some(proof( + left.inert && right.inert, + left.numeric && right.numeric, + )) } - Expr::Unary { operand, .. } - | Expr::Void(operand) - | Expr::TypeOf(operand) - | Expr::NumberCoerce(operand) - | Expr::BooleanCoerce(operand) - | Expr::MathAbs(operand) - | Expr::MathSqrt(operand) - | Expr::MathFloor(operand) - | Expr::MathCeil(operand) - | Expr::MathRound(operand) - | Expr::MathTrunc(operand) - | Expr::MathSign(operand) - | Expr::MathF16round(operand) => masked_window_indices_are_non_collecting(ctx, operand), Expr::Conditional { condition, then_expr, else_expr, } => { - masked_window_indices_are_non_collecting(ctx, condition) - && masked_window_indices_are_non_collecting(ctx, then_expr) - && masked_window_indices_are_non_collecting(ctx, else_expr) + masked_window_expression_proof(ctx, condition)?; + let then_expr = masked_window_expression_proof(ctx, then_expr)?; + let else_expr = masked_window_expression_proof(ctx, else_expr)?; + Some(proof( + then_expr.inert && else_expr.inert, + then_expr.numeric && else_expr.numeric, + )) } - Expr::MathMin(values) | Expr::MathMax(values) => values - .iter() - .all(|value| masked_window_indices_are_non_collecting(ctx, value)), - _ => true, + Expr::Void(value) | Expr::TypeOf(value) | Expr::BooleanCoerce(value) => { + masked_window_expression_proof(ctx, value)?; + Some(proof(true, false)) + } + Expr::NumberCoerce(value) => { + let value = masked_window_expression_proof(ctx, value)?; + value.inert.then(|| proof(true, true)) + } + Expr::MathImul(left, right) | Expr::MathPow(left, right) => { + for value in [left.as_ref(), right.as_ref()] { + if !masked_window_expression_proof(ctx, value)?.inert { + return None; + } + } + Some(proof(true, true)) + } + Expr::MathMin(values) | Expr::MathMax(values) => { + for value in values { + if !masked_window_expression_proof(ctx, value)?.inert { + return None; + } + } + Some(proof(true, true)) + } + Expr::MathAbs(value) + | Expr::MathSqrt(value) + | Expr::MathFloor(value) + | Expr::MathCeil(value) + | Expr::MathRound(value) + | Expr::MathTrunc(value) + | Expr::MathSign(value) + | Expr::MathF16round(value) => { + let value = masked_window_expression_proof(ctx, value)?; + if !value.inert { + return None; + } + Some(proof(true, true)) + } + _ => None, } } diff --git a/crates/perry-codegen/src/stmt/masked_window_region.rs b/crates/perry-codegen/src/stmt/masked_window_region.rs index bc8cc61958..8b2f6a625e 100644 --- a/crates/perry-codegen/src/stmt/masked_window_region.rs +++ b/crates/perry-codegen/src/stmt/masked_window_region.rs @@ -43,7 +43,7 @@ use anyhow::Result; use perry_hir::{Expr, Stmt}; use super::loops::{ - local_is_number_array, local_is_untyped_candidate, masked_window_indices_are_non_collecting, + local_is_number_array, local_is_untyped_candidate, masked_window_expression_is_non_collecting, packed_f64_range_loop_pure_expr_collect, packed_loop_array_binding_storage_is_addressable, record_packed_f64_range_static_access, PackedF64RangeArrayAccess, }; @@ -352,7 +352,7 @@ fn region_store_operand_collect( expr: &Expr, accesses: &mut std::collections::BTreeMap, ) -> bool { - if !masked_window_indices_are_non_collecting(ctx, expr) { + if !masked_window_expression_is_non_collecting(ctx, expr) { return false; } match expr { @@ -419,7 +419,7 @@ pub(super) fn try_match_masked_window_region( let ok = match stmt { Stmt::Expr(Expr::LocalSet(id, value)) => { let mut trial = accesses.clone(); - if masked_window_indices_are_non_collecting(ctx, value) + if masked_window_expression_is_non_collecting(ctx, value) && packed_f64_range_loop_pure_expr_collect( value, REGION_NO_COUNTER, @@ -457,7 +457,7 @@ pub(super) fn try_match_masked_window_region( break; } let mut trial = accesses.clone(); - if masked_window_indices_are_non_collecting(ctx, expr) + if masked_window_expression_is_non_collecting(ctx, expr) && packed_f64_range_loop_pure_expr_collect( expr, REGION_NO_COUNTER, From 67db2045b9aa77c7fba8b81a5ec190cb629928ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 13 Aug 2026 10:17:43 +0200 Subject: [PATCH 5/5] fix(gc): gate masked-window updates --- changelog.d/8014-tracked-view-backing.md | 4 +- .../src/expr/computed_store_rooting_tests.rs | 131 ++++++++++++++++++ crates/perry-codegen/src/stmt/loops.rs | 11 +- .../src/stmt/masked_window_region.rs | 4 +- 4 files changed, 145 insertions(+), 5 deletions(-) diff --git a/changelog.d/8014-tracked-view-backing.md b/changelog.d/8014-tracked-view-backing.md index 8d57baf89e..44b44c66e4 100644 --- a/changelog.d/8014-tracked-view-backing.md +++ b/changelog.d/8014-tracked-view-backing.md @@ -8,5 +8,5 @@ while fresh inline Buffer and TypedArray storage keeps its existing fast path. Computed-access IR regressions now trace each protected receiver and key through its exact root slot. Masked-window tiers also prove every admitted expression non-collecting before hoisting a raw typed-array pointer, including coercions -between element reads, while inert numeric expressions keep their zero-root -direct loads. +between element reads and standalone updates, while inert numeric expressions +keep their zero-root direct loads. diff --git a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs index 36eff106ca..e0ea8fa62f 100644 --- a/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs +++ b/crates/perry-codegen/src/expr/computed_store_rooting_tests.rs @@ -649,6 +649,94 @@ fn masked_window_rhs_coercion_region(key_ty: Type) -> String { ) } +fn masked_window_pair_sum() -> Expr { + let read = |index| Expr::IndexGet { + object: Box::new(Expr::LocalGet(1)), + index: Box::new(Expr::Integer(index)), + }; + Expr::Binary { + op: BinaryOp::Add, + left: Box::new(read(0)), + right: Box::new(read(1)), + } +} + +fn masked_window_standalone_update_loop(key_ty: Type) -> String { + compile_body_with_params( + "masked_window_standalone_update_loop", + vec![param(1, "view", Type::Any), param(2, "key", key_ty)], + vec![ + Stmt::Let { + id: 3, + name: "sum".into(), + ty: Type::Number, + init: Some(Expr::Number(0.0)), + mutable: true, + }, + Stmt::For { + init: Some(Box::new(Stmt::Let { + id: 4, + name: "i".into(), + ty: Type::Any, + init: Some(Expr::Integer(0)), + mutable: true, + })), + condition: Some(Expr::Compare { + op: CompareOp::Lt, + left: Box::new(Expr::LocalGet(4)), + right: Box::new(Expr::Integer(2)), + }), + update: Some(Expr::Update { + id: 4, + op: UpdateOp::Increment, + prefix: false, + }), + body: vec![ + Stmt::Expr(Expr::LocalSet(3, Box::new(masked_window_pair_sum()))), + Stmt::Expr(Expr::Update { + id: 2, + op: UpdateOp::Increment, + prefix: false, + }), + Stmt::Expr(Expr::LocalSet(3, Box::new(masked_window_pair_sum()))), + ], + }, + Stmt::Return(Some(Expr::LocalGet(3))), + ], + ) +} + +fn masked_window_standalone_update_region(key_ty: Type) -> String { + let mut body = vec![Stmt::Let { + id: 3, + name: "sum".into(), + ty: Type::Number, + init: Some(Expr::Number(0.0)), + mutable: true, + }]; + body.push(Stmt::Expr(Expr::LocalSet( + 3, + Box::new(masked_window_pair_sum()), + ))); + body.push(Stmt::Expr(Expr::Update { + id: 2, + op: UpdateOp::Increment, + prefix: false, + })); + for _ in 0..3 { + body.push(Stmt::Expr(Expr::LocalSet( + 3, + Box::new(masked_window_pair_sum()), + ))); + } + body.push(Stmt::Return(Some(Expr::LocalGet(3)))); + compile_body_with_params( + "masked_window_standalone_update_region", + vec![param(1, "view", Type::Any), param(2, "key", key_ty)], + body, + ) +} + /// #7640 E review follow-up — unary `+` over an `any` key can invoke user /// coercion even though the surrounding mask has a static index window. Such /// an index must decline before a typed-array tier hoists its raw data pointer; @@ -718,6 +806,49 @@ fn collecting_rhs_declines_the_straight_line_masked_region() { ); } +/// A standalone update is not part of an index/RHS tree, but it executes in +/// the same hoisted-pointer copy between masked reads. `any++` can run user +/// ToNumeric hooks; an inert i32 update remains call-free. +#[test] +fn collecting_standalone_update_declines_the_masked_loop() { + let inert = masked_window_standalone_update_loop(Type::Int32); + assert!( + inert.contains("for.packed_f64_range_fast_ta_i32"), + "an inert standalone update must retain the masked loop tier:\n{inert}" + ); + + let collecting = masked_window_standalone_update_loop(Type::Any); + assert!( + calls(&collecting, "js_to_numeric"), + "an any-typed standalone update must exercise collecting ToNumeric:\n{collecting}" + ); + assert!( + !collecting.contains("for.packed_f64_range_fast_ta_i32"), + "collecting standalone update must decline the hoisted-pointer loop tier:\n{collecting}" + ); +} + +/// Straight-line region matching has its own standalone-Update arm and must +/// apply the identical inert-target gate before spanning later masked reads. +#[test] +fn collecting_standalone_update_declines_the_masked_region() { + let inert = masked_window_standalone_update_region(Type::Int32); + assert!( + inert.contains("masked_region.ta_i32.preheader"), + "an inert standalone update must retain straight-line versioning:\n{inert}" + ); + + let collecting = masked_window_standalone_update_region(Type::Any); + assert!( + calls(&collecting, "js_to_numeric"), + "an any-typed region update must exercise collecting ToNumeric:\n{collecting}" + ); + assert!( + !collecting.contains("masked_region.ta_i32.preheader"), + "collecting standalone update must decline the straight-line masked region:\n{collecting}" + ); +} + /// #7640 E — the array-grow helper may return a replacement allocation. The /// write barrier on that path must therefore shade through the returned head, /// not the raw receiver handle computed before the call. diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index a97289160d..1167438cda 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -1102,10 +1102,13 @@ fn packed_f64_range_loop_dense_body_collect( } written.insert(*id); } - Stmt::Expr(Expr::Update { id, .. }) => { + Stmt::Expr(expr @ Expr::Update { id, .. }) => { if *id == counter_id || Some(*id) == bound_local { return false; } + if !masked_window_expression_is_non_collecting(ctx, expr) { + return false; + } written.insert(*id); } Stmt::Expr(expr) => { @@ -1180,6 +1183,12 @@ fn masked_window_expression_proof( inert && crate::type_analysis::is_numeric_expr(ctx, expr), )) } + // `++` / `--` execute ToNumeric before mutating their local. The + // shared inert predicate admits only a non-pointer primitive local; + // an `any` target can dispatch valueOf/Symbol.toPrimitive and collect. + Expr::Update { .. } => { + crate::rooting::expr_is_inert_primitive(ctx, expr).then(|| proof(true, true)) + } Expr::Binary { op, left, right } => { let left = masked_window_expression_proof(ctx, left)?; let right = masked_window_expression_proof(ctx, right)?; diff --git a/crates/perry-codegen/src/stmt/masked_window_region.rs b/crates/perry-codegen/src/stmt/masked_window_region.rs index 8b2f6a625e..f20dedaaa9 100644 --- a/crates/perry-codegen/src/stmt/masked_window_region.rs +++ b/crates/perry-codegen/src/stmt/masked_window_region.rs @@ -434,9 +434,9 @@ pub(super) fn try_match_masked_window_region( false } } - Stmt::Expr(Expr::Update { id, .. }) => { + Stmt::Expr(expr @ Expr::Update { id, .. }) => { written.insert(*id); - true + masked_window_expression_is_non_collecting(ctx, expr) } Stmt::Expr(expr) => { if let Some((receiver_id, index, value)) = proven_view_store_parts(ctx, expr) {