diff --git a/changelog.d/9313-train13-followup.md b/changelog.d/9313-train13-followup.md new file mode 100644 index 0000000000..2222a28e7b --- /dev/null +++ b/changelog.d/9313-train13-followup.md @@ -0,0 +1,11 @@ +### Internal + +- **Keeps `ic_miss.rs` and `array/tests.rs` under the 2000-line file gate.** + #9302 and #9307 each landed on a file already within ~35 lines of the cap. + The C3C PIC test module and the `Array.prototype` method-discriminator tests + move to sibling files; no behaviour change. + +- **Drops the dead catch-all left behind the combined accumulator arm.** #9303 + added a combined `_ =>` arm without removing the `_ => false` it supersedes, + which is an unreachable pattern and so a `-D warnings` failure. This is the + deletion #9308 identified. diff --git a/changelog.d/dense-accumulator-masked-reads.md b/changelog.d/dense-accumulator-masked-reads.md new file mode 100644 index 0000000000..e8b12f2b46 --- /dev/null +++ b/changelog.d/dense-accumulator-masked-reads.md @@ -0,0 +1,32 @@ +**A float accumulator over masked reads now earns the dense range clone** +(`17_loop_data_dependent`: 475 ms → 219 ms against node's 220 ms on an idle +machine — parity, from 2.16×). + +`sum = sum * x[i & 63] + x[(i * 7) & 63]` was rejected by the dense range tier +while `sum = sum * x[i & 63]` was admitted. The discriminator was the +accumulator's static numeric proof: `+` can be concatenation, so the +per-statement proof demands both operands numeric, and a reassigned +accumulator has no such proof — its own writes read the guarded array, whose +element proof only exists once the guard has run. A chicken-and-egg that `*` +never faces, because multiplication needs only the weaker inert fact. + +The matcher now peels the accumulator: when the proof fails on the `LocalSet` +target of a self-accumulating write, it retries with the target treated as +numeric BY CONTRACT, records it pending, and then verifies every pending +local with the same collector the lowering runs — rejecting the whole dense +match (with its own named trace reasons) if the two disagree, so the clone can +never contain a dynamic `+` under facts that forbid one. The contract is +enforced at run time twice over: the clone's entry emits a genuine-double tag +check on the accumulator, and the dense entry guard validates the whole masked +window hole-free. A string-seeded accumulator and a string element both route +to the slow copy and produce node's concatenation, verified under forced +evacuation. + +Along the way the accumulator walk's index leaf learned masked reads — and +fixed a match-arm reachability bug while doing so: `_ if offset_reads_inlined` +was a guarded catch-all, so any arm placed after it was unreachable whenever +the flag was set. Admitted masked-only single arrays now qualify for +accumulator admission (counter-bearing arrays keep priority; multiple arrays +still decline), and `MaskedWindowArrayFact` carries the admitted accumulators +so `is_numeric_expr` can see them while the clone lowers, mirroring the +string-window fact's field. diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index e7d4a58cdd..749ee678a1 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2064,6 +2064,12 @@ pub(crate) struct MaskedWindowArrayFact { /// element type is exactly i32 (Int32Array tier), so loads may /// materialize elements as native `i32`. pub values_i32: bool, + /// Accumulator locals admitted by the entry tag check for THIS clone: + /// every in-clone write is numeric-preserving (verified by the + /// accumulator walk), so `is_numeric_expr` may treat them as Numbers + /// while the fact is live. Mirrors `StringWindowArrayFact`'s + /// `numeric_accumulator` (#9160) and `PackedF64LoopFact`'s vec. + pub numeric_accumulators: Vec, /// Storage layout the guard proved — selects the inline load shape. pub elem: MaskedWindowElem, /// True only in a dense fast-loop scope whose matcher admitted masked diff --git a/crates/perry-codegen/src/stmt/loops.rs b/crates/perry-codegen/src/stmt/loops.rs index a795e80256..b31746711d 100644 --- a/crates/perry-codegen/src/stmt/loops.rs +++ b/crates/perry-codegen/src/stmt/loops.rs @@ -572,17 +572,17 @@ fn emit_range_loop_accumulator_admission( slow_pre_label: &str, block_prefix: &str, ) -> PackedAccumulatorScope { - let mut counter_arrays = matched - .arrays - .iter() - .filter(|access| access.counter.is_some()) - .map(|access| access.array_id); - let Some(array_id) = counter_arrays.next() else { - return PackedAccumulatorScope::empty(); + // One array may carry the accumulator proof: with several, no single + // admitted leaf spans them all. Counter-bearing arrays keep priority; + // a masked-only array (dense mode) now qualifies too, because the dense + // entry guard validates its whole window hole-free, making every + // in-window read a Number the accumulator walk may lean on. + let mut single = matched.arrays.iter().filter(|a| a.counter.is_some()); + let (array_id, masked_reads_validated) = match (single.next(), single.next()) { + (Some(access), None) => (access.array_id, access.stat.is_some()), + (None, _) if matched.arrays.len() == 1 => (matched.arrays[0].array_id, true), + _ => return PackedAccumulatorScope::empty(), }; - if counter_arrays.next().is_some() { - return PackedAccumulatorScope::empty(); - } emit_packed_numeric_accumulator_admission( ctx, body, @@ -594,6 +594,7 @@ fn emit_range_loop_accumulator_admission( // hole-checked, so an `a[i +/- c]` read is lowered inline and yields a // Number. See `accumulator_rhs_is_numeric`. true, + masked_reads_validated, ) } @@ -907,6 +908,7 @@ fn emit_packed_numeric_accumulator_admission( slow_pre_label: &str, block_prefix: &str, offset_reads_inlined: bool, + masked_reads_validated: bool, ) -> PackedAccumulatorScope { let accumulators = super::stable_packed_accumulator::collect_numeric_accumulators( ctx, @@ -914,6 +916,7 @@ fn emit_packed_numeric_accumulator_admission( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, ); // Integer (`c++`) accumulators admit independently of the float set — // a pure count loop has no float accumulator at all. @@ -1099,6 +1102,9 @@ fn lower_packed_f64_versioned_for( // read takes the generic path — which can produce `undefined`. #9259 // is the work that would make an offset read inline here. false, + // No masked windows either: this tier's guard proves the receiver and + // the length bound, not a hole-free window. + false, ); acc_scope.hoist_receivers(ctx, &[matched.array_id]); ctx.packed_f64_loop_facts.push(PackedF64LoopFact { @@ -1373,15 +1379,35 @@ fn match_packed_f64_range_loop( // read-only DENSE mode: several scalar statements, masked // statically-windowed indices, no stores, no side exits. accesses.clear(); + let mut pending_accumulators = std::collections::BTreeSet::new(); if !packed_f64_range_loop_dense_body_collect( ctx, body, counter_id, bound_local, &mut accesses, + &mut pending_accumulators, ) { return range_loop_reject("body_not_admissible"); } + if !pending_accumulators.is_empty() { + // The peel above assumed each pending local numeric; that holds + // only if the lowering will actually admit it (entry tag check + + // numeric-preserving writes). Verify with the SAME collector and + // the SAME array selection `emit_range_loop_accumulator_admission` + // uses -- if the two disagree, the clone would contain a dynamic + // `+` (a collecting call) under facts that forbid one. + if accesses.len() != 1 { + return range_loop_reject("accumulator_needs_single_array"); + } + let array_id = *accesses.keys().next().expect("len checked"); + let admitted = super::stable_packed_accumulator::collect_numeric_accumulators( + ctx, body, array_id, counter_id, true, true, + ); + if !pending_accumulators.iter().all(|id| admitted.contains(id)) { + return range_loop_reject("accumulator_not_provable"); + } + } true }; if accesses.is_empty() { @@ -1818,6 +1844,7 @@ fn packed_f64_range_loop_dense_body_collect( counter_id: u32, bound_local: Option, accesses: &mut std::collections::BTreeMap, + pending_accumulators: &mut std::collections::BTreeSet, ) -> bool { let mut written: std::collections::HashSet = std::collections::HashSet::new(); packed_f64_range_loop_dense_stmts_collect( @@ -1827,6 +1854,7 @@ fn packed_f64_range_loop_dense_body_collect( bound_local, accesses, &mut written, + pending_accumulators, ) // Written arrays are allowed (masked stores above); a scalar `let`/set // shadowing a tracked array id still rejects. @@ -1847,6 +1875,7 @@ fn packed_f64_range_loop_dense_stmts_collect( bound_local: Option, accesses: &mut std::collections::BTreeMap, written: &mut std::collections::HashSet, + pending_accumulators: &mut std::collections::BTreeSet, ) -> bool { use perry_hir::Expr; for stmt in body { @@ -1872,10 +1901,20 @@ fn packed_f64_range_loop_dense_stmts_collect( if *id == counter_id || Some(*id) == bound_local { return false; } - if !masked_window_expression_is_non_collecting(ctx, value) - || !packed_f64_range_loop_pure_expr_collect( - value, counter_id, true, accesses, None, - ) + // First try the plain proof; a self-accumulating write whose + // only unprovable leaf is the target itself retries with the + // target treated as numeric and records it as PENDING. The + // caller then verifies every pending id against + // `collect_numeric_accumulators` -- the same walk the lowering + // runs -- and rejects the whole dense match otherwise, so the + // clone never contains a write this assumption cannot cover. + if masked_window_expression_proof(ctx, value, None).is_none() { + if masked_window_expression_proof(ctx, value, Some(*id)).is_none() { + return false; + } + pending_accumulators.insert(*id); + } + if !packed_f64_range_loop_pure_expr_collect(value, counter_id, true, accesses, None) { return false; } @@ -1971,6 +2010,7 @@ fn packed_f64_range_loop_dense_stmts_collect( bound_local, accesses, written, + pending_accumulators, ) { return false; } @@ -1982,6 +2022,7 @@ fn packed_f64_range_loop_dense_stmts_collect( bound_local, accesses, written, + pending_accumulators, ) { return false; } @@ -2052,7 +2093,7 @@ pub(super) fn masked_window_expression_is_non_collecting( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, ) -> bool { - masked_window_expression_proof(ctx, expr).is_some() + masked_window_expression_proof(ctx, expr, None).is_some() } /// Facts about a value whose evaluation has also been proved non-collecting. @@ -2071,6 +2112,7 @@ struct MaskedWindowExpressionProof { fn masked_window_expression_proof( ctx: &FnCtx<'_>, expr: &perry_hir::Expr, + accumulator: Option, ) -> Option { use perry_hir::{BinaryOp, CompareOp, Expr, UnaryOp}; let proof = |inert, numeric| MaskedWindowExpressionProof { inert, numeric }; @@ -2083,12 +2125,22 @@ fn masked_window_expression_proof( if !matches!(object.as_ref(), Expr::LocalGet(_)) { return None; } - masked_window_expression_proof(ctx, index)?; + masked_window_expression_proof(ctx, index, accumulator)?; Some(proof(true, true)) } Expr::Number(_) | Expr::Integer(_) => Some(proof(true, true)), Expr::Bool(_) | Expr::Null | Expr::Undefined => Some(proof(true, false)), - Expr::LocalGet(_) => { + Expr::LocalGet(id) => { + // A pending accumulator is numeric BY CONTRACT, not by static + // proof: the clone's entry emits a genuine-double tag check on it + // and branches to the slow copy otherwise, and the caller verifies + // (with the same collector the lowering runs) that every in-clone + // write keeps it numeric. Static analysis cannot see this because + // the accumulator's own writes read the guarded array, whose + // element proof only exists once the guard has run. + if accumulator == Some(*id) { + return Some(proof(true, true)); + } let inert = crate::rooting::expr_is_inert_primitive(ctx, expr); Some(proof( inert, @@ -2102,8 +2154,8 @@ fn masked_window_expression_proof( 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)?; + let left = masked_window_expression_proof(ctx, left, accumulator)?; + let right = masked_window_expression_proof(ctx, right, accumulator)?; if matches!(op, BinaryOp::Add) { if !left.numeric || !right.numeric { return None; @@ -2114,23 +2166,23 @@ fn masked_window_expression_proof( 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)?; + let left = masked_window_expression_proof(ctx, left, accumulator)?; + let right = masked_window_expression_proof(ctx, right, accumulator)?; 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)?; + let operand = masked_window_expression_proof(ctx, operand, accumulator)?; 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)?; + let left = masked_window_expression_proof(ctx, left, accumulator)?; + let right = masked_window_expression_proof(ctx, right, accumulator)?; Some(proof( left.inert && right.inert, left.numeric && right.numeric, @@ -2141,25 +2193,25 @@ fn masked_window_expression_proof( then_expr, 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)?; + masked_window_expression_proof(ctx, condition, accumulator)?; + let then_expr = masked_window_expression_proof(ctx, then_expr, accumulator)?; + let else_expr = masked_window_expression_proof(ctx, else_expr, accumulator)?; Some(proof( then_expr.inert && else_expr.inert, then_expr.numeric && else_expr.numeric, )) } Expr::Void(value) | Expr::TypeOf(value) | Expr::BooleanCoerce(value) => { - masked_window_expression_proof(ctx, value)?; + masked_window_expression_proof(ctx, value, accumulator)?; Some(proof(true, false)) } Expr::NumberCoerce(value) => { - let value = masked_window_expression_proof(ctx, value)?; + let value = masked_window_expression_proof(ctx, value, accumulator)?; 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 { + if !masked_window_expression_proof(ctx, value, accumulator)?.inert { return None; } } @@ -2167,7 +2219,7 @@ fn masked_window_expression_proof( } Expr::MathMin(values) | Expr::MathMax(values) => { for value in values { - if !masked_window_expression_proof(ctx, value)?.inert { + if !masked_window_expression_proof(ctx, value, accumulator)?.inert { return None; } } @@ -2181,7 +2233,7 @@ fn masked_window_expression_proof( | Expr::MathTrunc(value) | Expr::MathSign(value) | Expr::MathF16round(value) => { - let value = masked_window_expression_proof(ctx, value)?; + let value = masked_window_expression_proof(ctx, value, accumulator)?; if !value.inert { return None; } @@ -2497,6 +2549,7 @@ fn push_packed_f64_range_facts( values_i32, elem: crate::expr::MaskedWindowElem::PlainF64, allows_stores: allow_masked_stores, + numeric_accumulators: numeric_accumulators.to_vec(), }); } } @@ -2599,6 +2652,7 @@ fn lower_masked_window_ta_tier( values_i32, elem, allows_stores: false, + numeric_accumulators: Vec::new(), }); } lower_for_after_init_with_i32_bound( diff --git a/crates/perry-codegen/src/stmt/masked_window_region.rs b/crates/perry-codegen/src/stmt/masked_window_region.rs index 78a77ba55c..15da4ed148 100644 --- a/crates/perry-codegen/src/stmt/masked_window_region.rs +++ b/crates/perry-codegen/src/stmt/masked_window_region.rs @@ -887,6 +887,7 @@ pub(super) fn lower_masked_window_region( values_i32: true, allows_stores: false, elem: MaskedWindowElem::TaI32 { data_ptr: data_i64 }, + numeric_accumulators: Vec::new(), }); } let privatize = ctx.try_depth == 0; @@ -1003,6 +1004,7 @@ pub(super) fn lower_masked_window_region( values_i32: true, allows_stores: false, elem: MaskedWindowElem::TaI32 { data_ptr }, + numeric_accumulators: Vec::new(), }); } let privatize = ctx.try_depth == 0; @@ -1037,6 +1039,7 @@ pub(super) fn lower_masked_window_region( values_i32: false, allows_stores: false, elem: MaskedWindowElem::PlainF64, + numeric_accumulators: Vec::new(), }); } lower_region_copy( diff --git a/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs b/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs index 2edce30b26..08d4ce3591 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_accumulator.rs @@ -39,6 +39,7 @@ fn accumulator_rhs_is_numeric( array_id: u32, counter_id: u32, offset_reads_inlined: bool, + masked_reads_validated: bool, candidates: &std::collections::BTreeSet, ) -> bool { match expr { @@ -67,9 +68,21 @@ fn accumulator_rhs_is_numeric( // expression lowers to a tag-test diamond over // `js_dynamic_string_or_number_add` — the same cost #9060 and // #9091 removed for the bare-counter form. - _ if offset_reads_inlined => crate::expr::packed_f64_loop_index_parts(index) - .is_some_and(|(i, _)| i == counter_id), - _ => false, + // `_ if offset_reads_inlined` used to sit above the masked + // arm as its own guarded catch-all — which swallowed every + // non-offset index whenever the flag was set, so the masked + // test below it was unreachable. One combined catch-all keeps + // both reachable. + _ => { + (offset_reads_inlined + && crate::expr::packed_f64_loop_index_parts(index) + .is_some_and(|(i, _)| i == counter_id)) + // Dense masked mode: the entry guard validated the + // union of every static window hole-free, so an + // in-window read is a genuine Number. + || (masked_reads_validated + && crate::collectors::static_index_window(index).is_some()) + } } } Expr::LocalGet(id) => { @@ -82,6 +95,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) && accumulator_rhs_is_numeric( ctx, @@ -89,6 +103,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) } @@ -98,6 +113,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ), Expr::Unary { op, operand } => { @@ -110,6 +126,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) } @@ -126,6 +143,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ), Expr::MathImul(l, r) | Expr::MathPow(l, r) => { @@ -135,6 +153,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) && accumulator_rhs_is_numeric( ctx, @@ -142,6 +161,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) } @@ -152,6 +172,7 @@ fn accumulator_rhs_is_numeric( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, candidates, ) }), @@ -305,6 +326,7 @@ pub(super) fn collect_numeric_accumulators( array_id: u32, counter_id: u32, offset_reads_inlined: bool, + masked_reads_validated: bool, ) -> Vec { if !packed_loop_numeric_accumulators_enabled() { return Vec::new(); @@ -337,6 +359,7 @@ pub(super) fn collect_numeric_accumulators( array_id, counter_id, offset_reads_inlined, + masked_reads_validated, &candidates, ), // `Update` (++/--): ToNumeric(Number) ± 1 is a Number. diff --git a/crates/perry-codegen/src/stmt/stable_packed_loop.rs b/crates/perry-codegen/src/stmt/stable_packed_loop.rs index 98ad003f04..3d048b2763 100644 --- a/crates/perry-codegen/src/stmt/stable_packed_loop.rs +++ b/crates/perry-codegen/src/stmt/stable_packed_loop.rs @@ -1659,7 +1659,14 @@ pub(super) fn lower( let numeric_accumulators = if candidate.numeric_elements { // The stable-packed tier is left as it was; widening it needs its own // proof that an offset read lowers inline here. - collect_numeric_accumulators(ctx, body, candidate.array_id, candidate.counter_id, false) + collect_numeric_accumulators( + ctx, + body, + candidate.array_id, + candidate.counter_id, + false, + false, + ) } else { Vec::new() }; diff --git a/crates/perry-codegen/src/type_analysis/numeric.rs b/crates/perry-codegen/src/type_analysis/numeric.rs index b060149ae1..850f324cfc 100644 --- a/crates/perry-codegen/src/type_analysis/numeric.rs +++ b/crates/perry-codegen/src/type_analysis/numeric.rs @@ -189,6 +189,13 @@ pub(crate) fn is_numeric_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { // after an entry tag check, and its sole write adds a proven // string length. The fact exists only while lowering that // clone, so the slow copy retains dynamic `+` semantics. + // The dense masked-window clone's twin: same entry tag + // check, same numeric-preserving write proof. + || ctx + .masked_window_array_facts + .iter() + .rev() + .any(|fact| fact.numeric_accumulators.contains(id)) || ctx .string_window_array_facts .iter() diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index 1b72c5d54c..684920efee 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -1906,98 +1906,6 @@ fn push_built_array_gets_and_keeps_dense_raw_f64_flag() { /// `built-ins/String/prototype/concat/S15.5.4.6_A4_T1`). The discriminator is /// the closure's function pointer; this test pins both directions of it, so a /// regression that re-broadens (or over-narrows) the predicate fails here -/// rather than only in the parity sweep. -#[test] -fn array_prototype_method_discriminator_separates_foreign_builtins() { - // Reads realm intrinsics and holds raw pointers across allocating calls, - // and libtest gives each test its own thread where `GLOBAL_THIS_PTR` can be - // re-created — so resolve the whole snapshot inside one iteration and retry - // until a GC-quiet pass yields a self-consistent view. Mirrors - // `array_literal_shares_the_realm_array_prototype`'s loop above. - let mut checked = false; - for _ in 0..256 { - let global = crate::object::js_get_global_this(); - let global_ptr = - crate::value::js_nanbox_get_pointer(global) as *const crate::object::ObjectHeader; - if global_ptr.is_null() { - std::thread::yield_now(); - continue; - } - - let proto_of = |ctor_name: &[u8]| -> Option { - let ctor = - crate::object::js_object_get_field_by_name(global_ptr, string_key(ctor_name)); - if !ctor.is_pointer() { - return None; - } - let ctor_ptr = - crate::value::js_nanbox_get_pointer(f64::from_bits(ctor.bits())) as usize; - if ctor_ptr == 0 { - return None; - } - Some(crate::closure::closure_get_dynamic_prop( - ctor_ptr, - "prototype", - )) - }; - - let (Some(array_proto), Some(string_proto)) = (proto_of(b"Array"), proto_of(b"String")) - else { - std::thread::yield_now(); - continue; - }; - let array_proto_ptr = - crate::value::js_nanbox_get_pointer(array_proto) as *const crate::object::ObjectHeader; - let string_proto_ptr = - crate::value::js_nanbox_get_pointer(string_proto) as *const crate::object::ObjectHeader; - if array_proto_ptr.is_null() || string_proto_ptr.is_null() { - std::thread::yield_now(); - continue; - } - let array_concat = - crate::object::js_object_get_field_by_name_f64(array_proto_ptr, string_key(b"concat")); - let string_concat = - crate::object::js_object_get_field_by_name_f64(string_proto_ptr, string_key(b"concat")); - if !crate::value::JSValue::from_bits(array_concat.to_bits()).is_pointer() - || !crate::value::JSValue::from_bits(string_concat.to_bits()).is_pointer() - { - std::thread::yield_now(); - continue; - } - - // The Array borrow must still be claimed — this is the behavior the - // original classification existed to protect (`obj.concat = - // Array.prototype.concat` has to run the array engine on `obj`). - assert!( - crate::object::is_array_prototype_method_value(array_concat, "concat"), - "Array.prototype.concat must be recognized as an Array builtin" - ); - // The foreign borrow must NOT be claimed. - assert!( - !crate::object::is_array_prototype_method_value(string_concat, "concat"), - "String.prototype.concat must not be mistaken for an Array builtin" - ); - // Right closure, wrong method name is also a mismatch — the predicate - // keys on the (method, closure) pair, not on "is some Array builtin". - assert!( - !crate::object::is_array_prototype_method_value(array_concat, "push"), - "Array.prototype.concat must not answer for `push`" - ); - // Non-callable / non-pointer slots are never a borrowed builtin. - assert!(!crate::object::is_array_prototype_method_value( - 1.0, "concat" - )); - assert!(!crate::object::is_array_prototype_method_value( - f64::from_bits(crate::value::TAG_UNDEFINED), - "concat" - )); - - checked = true; - break; - } - assert!( - checked, - "never obtained a GC-quiet view of Array.prototype / String.prototype" - ); -} +#[path = "tests_proto_discriminator.rs"] +mod proto_discriminator; diff --git a/crates/perry-runtime/src/array/tests_proto_discriminator.rs b/crates/perry-runtime/src/array/tests_proto_discriminator.rs new file mode 100644 index 0000000000..01b9c24ff8 --- /dev/null +++ b/crates/perry-runtime/src/array/tests_proto_discriminator.rs @@ -0,0 +1,99 @@ +//! Unit tests — Array.prototype method discrimination (split from `tests.rs` +//! for the 2,000-line file gate; `use super::*` reaches the shared helpers). + +use super::*; + +#[test] +fn array_prototype_method_discriminator_separates_foreign_builtins() { + // Reads realm intrinsics and holds raw pointers across allocating calls, + // and libtest gives each test its own thread where `GLOBAL_THIS_PTR` can be + // re-created — so resolve the whole snapshot inside one iteration and retry + // until a GC-quiet pass yields a self-consistent view. Mirrors + // `array_literal_shares_the_realm_array_prototype`'s loop above. + let mut checked = false; + for _ in 0..256 { + let global = crate::object::js_get_global_this(); + let global_ptr = + crate::value::js_nanbox_get_pointer(global) as *const crate::object::ObjectHeader; + if global_ptr.is_null() { + std::thread::yield_now(); + continue; + } + + let proto_of = |ctor_name: &[u8]| -> Option { + let ctor = + crate::object::js_object_get_field_by_name(global_ptr, string_key(ctor_name)); + if !ctor.is_pointer() { + return None; + } + let ctor_ptr = + crate::value::js_nanbox_get_pointer(f64::from_bits(ctor.bits())) as usize; + if ctor_ptr == 0 { + return None; + } + Some(crate::closure::closure_get_dynamic_prop( + ctor_ptr, + "prototype", + )) + }; + + let (Some(array_proto), Some(string_proto)) = (proto_of(b"Array"), proto_of(b"String")) + else { + std::thread::yield_now(); + continue; + }; + let array_proto_ptr = + crate::value::js_nanbox_get_pointer(array_proto) as *const crate::object::ObjectHeader; + let string_proto_ptr = + crate::value::js_nanbox_get_pointer(string_proto) as *const crate::object::ObjectHeader; + if array_proto_ptr.is_null() || string_proto_ptr.is_null() { + std::thread::yield_now(); + continue; + } + + let array_concat = + crate::object::js_object_get_field_by_name_f64(array_proto_ptr, string_key(b"concat")); + let string_concat = + crate::object::js_object_get_field_by_name_f64(string_proto_ptr, string_key(b"concat")); + if !crate::value::JSValue::from_bits(array_concat.to_bits()).is_pointer() + || !crate::value::JSValue::from_bits(string_concat.to_bits()).is_pointer() + { + std::thread::yield_now(); + continue; + } + + // The Array borrow must still be claimed — this is the behavior the + // original classification existed to protect (`obj.concat = + // Array.prototype.concat` has to run the array engine on `obj`). + assert!( + crate::object::is_array_prototype_method_value(array_concat, "concat"), + "Array.prototype.concat must be recognized as an Array builtin" + ); + // The foreign borrow must NOT be claimed. + assert!( + !crate::object::is_array_prototype_method_value(string_concat, "concat"), + "String.prototype.concat must not be mistaken for an Array builtin" + ); + // Right closure, wrong method name is also a mismatch — the predicate + // keys on the (method, closure) pair, not on "is some Array builtin". + assert!( + !crate::object::is_array_prototype_method_value(array_concat, "push"), + "Array.prototype.concat must not answer for `push`" + ); + // Non-callable / non-pointer slots are never a borrowed builtin. + assert!(!crate::object::is_array_prototype_method_value( + 1.0, "concat" + )); + assert!(!crate::object::is_array_prototype_method_value( + f64::from_bits(crate::value::TAG_UNDEFINED), + "concat" + )); + + checked = true; + break; + } + assert!( + checked, + "never obtained a GC-quiet view of Array.prototype / String.prototype" + ); +} diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs index 7e0ae08e61..fdd60bce19 100644 --- a/crates/perry-runtime/src/object/field_get_set/ic_miss.rs +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss.rs @@ -333,8 +333,7 @@ pub(crate) unsafe fn pic_prime_get(cache: *mut PicCache, token: i64, slot: i64) // would be a wild load. Keep encoded slots out of the ways entirely; a // polymorphic site rotating overflow shapes re-primes the MRU per shape, // which is exactly the pre-#7753 behaviour. - let prev_is_overflow = - (prev_slot as u64) & u64::from(crate::proxy::IC_SLOT_OVERFLOW_BIT) != 0; + let prev_is_overflow = (prev_slot as u64) & u64::from(crate::proxy::IC_SLOT_OVERFLOW_BIT) != 0; let cascade = prev_tok != 0 && prev_tok != token && !prev_is_overflow; // One pass over the ways does three things: // * evicts `token` from a way if it has one — it now lives in the MRU @@ -448,8 +447,6 @@ pub extern "C" fn js_object_get_field_ic_overflow_load( js_object_get_field_ic_miss(obj, key, cache) } - - /// Monomorphic inline cache miss handler (issue #51). /// /// Called when the codegen-emitted ShapeId check misses. @@ -787,9 +784,7 @@ pub extern "C" fn js_object_get_field_ic_miss( // accessors), and the value must be readable through // `overflow_get` right now — if it is not, priming // would cache a lie. - if !has_own_descriptors - && (i as u32) < crate::proxy::IC_SLOT_OVERFLOW_BIT - { + if !has_own_descriptors && (i as u32) < crate::proxy::IC_SLOT_OVERFLOW_BIT { if let Some(bits) = crate::object::overflow_get(obj as usize, i) { if bits != crate::value::TAG_HOLE { let stamp = crate::object::shapes::object_shape_stamp(obj); @@ -804,8 +799,7 @@ pub extern "C" fn js_object_get_field_ic_miss( pic_prime_get( cache, token, - (i as u32 | crate::proxy::IC_SLOT_OVERFLOW_BIT) - as i64, + (i as u32 | crate::proxy::IC_SLOT_OVERFLOW_BIT) as i64, ); return f64::from_bits(bits); } @@ -1744,292 +1738,4 @@ mod poly_pic_tests { } #[cfg(test)] -mod c3c_pic_tests { - /// Installing an accessor on one object must not permanently disable every - /// property-read PIC in the process. Descriptor ownership is recorded on - /// the owning object's GC header, so a different descriptor-free object's - /// own data field remains safe to cache. - #[test] - fn unrelated_accessor_does_not_poison_plain_receiver_pic() { - let _lock = crate::gc::global_side_table_test_lock(); - let scope = crate::gc::RuntimeHandleScope::new(); - let unrelated = crate::object::js_object_alloc(0, 1); - let unrelated = scope.root_raw_mut_ptr(unrelated); - crate::object::set_accessor_descriptor( - unrelated.with_mut_ptr(|o: *mut crate::object::ObjectHeader| o as usize), - "pic_unrelated_accessor".to_string(), - crate::object::AccessorDescriptor::default(), - ); - assert!( - crate::state::state().descriptors.accessors_in_use.get(), - "test premise: the process-wide accessor latch is active" - ); - - let obj = crate::object::js_object_alloc(0, 8); - let obj = scope.root_raw_mut_ptr(obj); - let key_bytes = b"pic_plain_data"; - let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); - let key = scope.root_string_ptr(key); - obj.with_mut_ptr(|o| { - key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 42.0)) - }); - - let mut cache = [0i64; super::PIC_CACHE_WORDS]; - assert_eq!( - obj.with_mut_ptr( - |o| key.with_const_ptr(|k| super::js_object_get_field_ic_miss(o, k, &mut cache)) - ), - 42.0 - ); - assert_ne!( - cache[0], 0, - "an accessor owned by an unrelated object must not prevent this \ - descriptor-free receiver from priming its read PIC" - ); - assert_eq!(cache[1], 0, "the first own data field lives in slot 0"); - } - - /// The receiver-local half of the proof: an accessor-bearing object must - /// keep taking descriptor-aware lookup and must never seed a raw-slot hit. - #[test] - fn accessor_bearing_receiver_does_not_prime_plain_data_pic() { - let _lock = crate::gc::global_side_table_test_lock(); - let scope = crate::gc::RuntimeHandleScope::new(); - let obj = crate::object::js_object_alloc(0, 8); - let obj = scope.root_raw_mut_ptr(obj); - let key_bytes = b"pic_guarded_data"; - let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); - let key = scope.root_string_ptr(key); - obj.with_mut_ptr(|o| { - key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 17.0)) - }); - crate::object::set_accessor_descriptor( - obj.with_mut_ptr(|o: *mut crate::object::ObjectHeader| o as usize), - "pic_guarded_data".to_string(), - crate::object::AccessorDescriptor::default(), - ); - - let mut cache = [0i64; super::PIC_CACHE_WORDS]; - let via_pic = obj.with_mut_ptr(|o| { - key.with_const_ptr(|k| super::js_object_get_field_ic_miss(o, k, &mut cache)) - }); - let via_ladder = obj - .with_mut_ptr(|o| key.with_const_ptr(|k| super::js_object_get_field_by_name_f64(o, k))); - assert_eq!( - via_pic.to_bits(), - via_ladder.to_bits(), - "the miss path must preserve this receiver's accessor semantics" - ); - assert_eq!( - cache[0], 0, - "an accessor-bearing receiver must not prime a raw-slot PIC" - ); - } - - /// A class instance primes the same authoritative ShapeId token that the - /// emitted guard reads from the receiver. - #[test] - fn a_class_instance_primes_an_id_token_after_rung1() { - let _lock = crate::gc::global_side_table_test_lock(); - unsafe { - let obj = crate::object::js_object_alloc(0x6080, 8); - let key = crate::string::js_string_from_bytes(b"pic6080_x".as_ptr(), 9); - crate::object::js_object_set_field_by_name(obj, key, 7.0); - let keys = crate::object::object_keys_array(obj); - assert!(!keys.is_null(), "test premise: field append built keys"); - assert_eq!((*obj).class_id, 0x6080, "test premise: a class instance"); - - let mut cache = [0i64; super::PIC_CACHE_WORDS]; - let v = super::js_object_get_field_ic_miss(obj, key, &mut cache); - assert_eq!(v, 7.0); - - let stamp = crate::object::shapes::object_shape_stamp(obj); - assert!( - stamp != 0, - "the miss handler did not stamp a class instance — rung 1 is inert" - ); - assert_eq!( - cache[0] as u64, - stamp as u64 | crate::object::shapes::PIC_ID_TOKEN_BIT, - "a stamped class instance must prime the ID token the emitted \ - PIC computes for it, not its keys pointer" - ); - assert_ne!( - cache[0], keys as i64, - "primed the keys pointer for a stamped receiver — every hit at \ - this site would miss forever" - ); - assert_eq!(cache[2], 0, "word 2 is non-identity scratch"); - } - } - - /// ★ #6759 C3 rung 1 opens a NEW correctness surface, and this is it. - /// - /// A delete-compacted class instance receives a semantic successor ShapeId, - /// so the emitted hit path can serve it without confusing it with a - /// pristine sibling. A token that failed to move across the - /// compaction would therefore be read as a pristine sibling's shape at a - /// site that has both — the one-slot shift the whole ladder is about. - /// - /// Pins: the compacted instance's primed token differs from a pristine - /// sibling's, AND the slot it primes is the post-compaction slot. - #[test] - fn a_compacted_class_instance_primes_a_token_a_pristine_sibling_cannot_match() { - // Preserve the compacted fixture under default-on tombstones. The - // tombstone lane already produces a distinct class-instance token, - // but it keeps `c` in slot 2 and therefore cannot exercise the - // shifted-slot/token pairing this regression test owns. - let _tombstones = crate::object::delete_rest::test_scope_tombstone_deletes(false); - let _lock = crate::gc::global_side_table_test_lock(); - { - let packed = b"picdel_a\0picdel_b\0picdel_c"; - let mk = || { - crate::object::js_object_alloc_class_with_keys( - 0x6081, - 0, - 3, - packed.as_ptr(), - packed.len() as u32, - ) - }; - let key = |n: &str| crate::string::js_string_from_bytes(n.as_ptr(), n.len() as u32); - let pristine = mk(); - let compacted = mk(); - for (i, v) in [1.0f64, 2.0, 3.0].iter().enumerate() { - crate::object::js_object_set_field( - pristine, - i as u32, - crate::JSValue::from_bits(v.to_bits()), - ); - crate::object::js_object_set_field( - compacted, - i as u32, - crate::JSValue::from_bits(v.to_bits()), - ); - } - assert_eq!( - crate::object::js_object_delete_field(compacted, key("picdel_a")), - 1 - ); - - let mut c_pristine = [0i64; super::PIC_CACHE_WORDS]; - let vp = super::js_object_get_field_ic_miss(pristine, key("picdel_c"), &mut c_pristine); - assert_eq!(vp, 3.0, "pristine `c` is slot 2"); - - let mut c_compacted = [0i64; super::PIC_CACHE_WORDS]; - let vc = - super::js_object_get_field_ic_miss(compacted, key("picdel_c"), &mut c_compacted); - assert_eq!( - vc, 3.0, - "compacted `c` shifted to slot 1 and must still read 3" - ); - - assert_ne!( - c_compacted[0], 0, - "the compacted instance primed nothing — rung 1's new surface is inert" - ); - assert_ne!( - c_compacted[0], c_pristine[0], - "the compacted instance primed its pristine sibling's token — an \ - id-comparing PIC would read slot {} for a receiver whose `c` is \ - at slot {}", - c_pristine[1], c_compacted[1] - ); - assert_eq!(c_pristine[1], 2, "pristine `c` slot"); - assert_eq!(c_compacted[1], 1, "compacted `c` slot"); - } - } - - /// The PIC cache token the EMITTED code computes for `obj`, transcribed - /// from `perry-codegen/src/expr/property_get/generic_dispatch.rs`: - /// - /// ```text - /// token = valid_shape_id ? (shape_id | 1<<62) : 0 - /// ``` - /// - /// The runtime never calls this; it exists so a test can compare what the - /// miss handler PRIMES against what the hit path will COMPUTE, which is - /// the only pair whose agreement decides whether a site can ever hit. - unsafe fn emitted_pic_token(obj: *const super::ObjectHeader) -> u64 { - let shape_id = crate::object::shapes::object_shape_id(obj); - shape_id as u64 | crate::object::shapes::PIC_ID_TOKEN_BIT - } - - /// ★ The invariant #6759 C3 rung 1 broke, asserted where it broke. - /// - /// A shape's population must be UNIFORMLY stamped: the token the miss - /// handler primes from one instance is only useful if a DIFFERENT, - /// freshly-allocated instance of the same class computes the same token. - /// A prior implementation stamped class instances lazily, so instance #1 - /// primed an id token while every newborn sibling computed a different - /// identity. `token_eq` then failed at every field-read site until the - /// sibling took the miss path itself. - /// - /// This is deliberately NOT "the newborn carries a stamp" — that is a - /// presence check two different states satisfy (both-stamped and - /// both-unstamped are each fine; the mixture is the bug). Comparing the - /// primed token against a fresh sibling's COMPUTED token is what fails - /// under either half of the split. - #[test] - fn a_fresh_class_instance_computes_the_token_the_miss_handler_primed() { - let _lock = crate::gc::global_side_table_test_lock(); - unsafe { - let packed = b"picbirth_x\0picbirth_y"; - let mk = || { - crate::object::js_object_alloc_class_with_keys( - 0x6082, - 0, - 2, - packed.as_ptr(), - packed.len() as u32, - ) - }; - let key = crate::string::js_string_from_bytes(b"picbirth_x".as_ptr(), 10); - - let primed_from = mk(); - crate::object::js_object_set_field( - primed_from, - 0, - crate::JSValue::from_bits(5.0f64.to_bits()), - ); - assert_eq!( - (*primed_from).class_id, - 0x6082, - "test premise: the receiver is a class instance, not a literal" - ); - - let mut cache = [0i64; super::PIC_CACHE_WORDS]; - assert_eq!( - super::js_object_get_field_ic_miss(primed_from, key, &mut cache), - 5.0, - "test premise: the miss handler resolved the field" - ); - assert_ne!( - cache[0], 0, - "test premise: the miss handler primed SOMETHING — a zero token \ - never hits, so the comparison below would be vacuous" - ); - - // The next `new C(...)`. Nothing has resolved a field on it. - let fresh = mk(); - assert_eq!( - emitted_pic_token(fresh), - cache[0] as u64, - "a freshly allocated instance of the SAME class computes a \ - different PIC token than the one primed from its sibling, so \ - every read of a newborn instance's field misses the cache and \ - takes the full miss handler — #7983's split population" - ); - - // And the same must hold once the fresh one has itself resolved: - // priming from either instance is interchangeable. - let mut cache2 = [0i64; super::PIC_CACHE_WORDS]; - super::js_object_get_field_ic_miss(fresh, key, &mut cache2); - assert_eq!( - cache2[0], cache[0], - "two instances of one class primed two different tokens — the \ - site thrashes between them" - ); - } - } -} +mod c3c_pic_tests; diff --git a/crates/perry-runtime/src/object/field_get_set/ic_miss/c3c_pic_tests.rs b/crates/perry-runtime/src/object/field_get_set/ic_miss/c3c_pic_tests.rs new file mode 100644 index 0000000000..05efbcfcbe --- /dev/null +++ b/crates/perry-runtime/src/object/field_get_set/ic_miss/c3c_pic_tests.rs @@ -0,0 +1,289 @@ +//! #7753 C3C PIC regression tests, split out of `ic_miss.rs` to keep that +//! file under the 2000-line cap. Behaviour is unchanged. + +/// Installing an accessor on one object must not permanently disable every +/// property-read PIC in the process. Descriptor ownership is recorded on +/// the owning object's GC header, so a different descriptor-free object's +/// own data field remains safe to cache. +#[test] +fn unrelated_accessor_does_not_poison_plain_receiver_pic() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let unrelated = crate::object::js_object_alloc(0, 1); + let unrelated = scope.root_raw_mut_ptr(unrelated); + crate::object::set_accessor_descriptor( + unrelated.with_mut_ptr(|o: *mut crate::object::ObjectHeader| o as usize), + "pic_unrelated_accessor".to_string(), + crate::object::AccessorDescriptor::default(), + ); + assert!( + crate::state::state().descriptors.accessors_in_use.get(), + "test premise: the process-wide accessor latch is active" + ); + + let obj = crate::object::js_object_alloc(0, 8); + let obj = scope.root_raw_mut_ptr(obj); + let key_bytes = b"pic_plain_data"; + let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); + let key = scope.root_string_ptr(key); + obj.with_mut_ptr(|o| { + key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 42.0)) + }); + + let mut cache = [0i64; super::PIC_CACHE_WORDS]; + assert_eq!( + obj.with_mut_ptr( + |o| key.with_const_ptr(|k| super::js_object_get_field_ic_miss(o, k, &mut cache)) + ), + 42.0 + ); + assert_ne!( + cache[0], 0, + "an accessor owned by an unrelated object must not prevent this \ + descriptor-free receiver from priming its read PIC" + ); + assert_eq!(cache[1], 0, "the first own data field lives in slot 0"); +} + +/// The receiver-local half of the proof: an accessor-bearing object must +/// keep taking descriptor-aware lookup and must never seed a raw-slot hit. +#[test] +fn accessor_bearing_receiver_does_not_prime_plain_data_pic() { + let _lock = crate::gc::global_side_table_test_lock(); + let scope = crate::gc::RuntimeHandleScope::new(); + let obj = crate::object::js_object_alloc(0, 8); + let obj = scope.root_raw_mut_ptr(obj); + let key_bytes = b"pic_guarded_data"; + let key = crate::string::js_string_from_bytes(key_bytes.as_ptr(), key_bytes.len() as u32); + let key = scope.root_string_ptr(key); + obj.with_mut_ptr(|o| { + key.with_const_ptr(|k| crate::object::js_object_set_field_by_name(o, k, 17.0)) + }); + crate::object::set_accessor_descriptor( + obj.with_mut_ptr(|o: *mut crate::object::ObjectHeader| o as usize), + "pic_guarded_data".to_string(), + crate::object::AccessorDescriptor::default(), + ); + + let mut cache = [0i64; super::PIC_CACHE_WORDS]; + let via_pic = obj.with_mut_ptr(|o| { + key.with_const_ptr(|k| super::js_object_get_field_ic_miss(o, k, &mut cache)) + }); + let via_ladder = + obj.with_mut_ptr(|o| key.with_const_ptr(|k| super::js_object_get_field_by_name_f64(o, k))); + assert_eq!( + via_pic.to_bits(), + via_ladder.to_bits(), + "the miss path must preserve this receiver's accessor semantics" + ); + assert_eq!( + cache[0], 0, + "an accessor-bearing receiver must not prime a raw-slot PIC" + ); +} + +/// A class instance primes the same authoritative ShapeId token that the +/// emitted guard reads from the receiver. +#[test] +fn a_class_instance_primes_an_id_token_after_rung1() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let obj = crate::object::js_object_alloc(0x6080, 8); + let key = crate::string::js_string_from_bytes(b"pic6080_x".as_ptr(), 9); + crate::object::js_object_set_field_by_name(obj, key, 7.0); + let keys = crate::object::object_keys_array(obj); + assert!(!keys.is_null(), "test premise: field append built keys"); + assert_eq!((*obj).class_id, 0x6080, "test premise: a class instance"); + + let mut cache = [0i64; super::PIC_CACHE_WORDS]; + let v = super::js_object_get_field_ic_miss(obj, key, &mut cache); + assert_eq!(v, 7.0); + + let stamp = crate::object::shapes::object_shape_stamp(obj); + assert!( + stamp != 0, + "the miss handler did not stamp a class instance — rung 1 is inert" + ); + assert_eq!( + cache[0] as u64, + stamp as u64 | crate::object::shapes::PIC_ID_TOKEN_BIT, + "a stamped class instance must prime the ID token the emitted \ + PIC computes for it, not its keys pointer" + ); + assert_ne!( + cache[0], keys as i64, + "primed the keys pointer for a stamped receiver — every hit at \ + this site would miss forever" + ); + assert_eq!(cache[2], 0, "word 2 is non-identity scratch"); + } +} + +/// ★ #6759 C3 rung 1 opens a NEW correctness surface, and this is it. +/// +/// A delete-compacted class instance receives a semantic successor ShapeId, +/// so the emitted hit path can serve it without confusing it with a +/// pristine sibling. A token that failed to move across the +/// compaction would therefore be read as a pristine sibling's shape at a +/// site that has both — the one-slot shift the whole ladder is about. +/// +/// Pins: the compacted instance's primed token differs from a pristine +/// sibling's, AND the slot it primes is the post-compaction slot. +#[test] +fn a_compacted_class_instance_primes_a_token_a_pristine_sibling_cannot_match() { + // Preserve the compacted fixture under default-on tombstones. The + // tombstone lane already produces a distinct class-instance token, + // but it keeps `c` in slot 2 and therefore cannot exercise the + // shifted-slot/token pairing this regression test owns. + let _tombstones = crate::object::delete_rest::test_scope_tombstone_deletes(false); + let _lock = crate::gc::global_side_table_test_lock(); + { + let packed = b"picdel_a\0picdel_b\0picdel_c"; + let mk = || { + crate::object::js_object_alloc_class_with_keys( + 0x6081, + 0, + 3, + packed.as_ptr(), + packed.len() as u32, + ) + }; + let key = |n: &str| crate::string::js_string_from_bytes(n.as_ptr(), n.len() as u32); + let pristine = mk(); + let compacted = mk(); + for (i, v) in [1.0f64, 2.0, 3.0].iter().enumerate() { + crate::object::js_object_set_field( + pristine, + i as u32, + crate::JSValue::from_bits(v.to_bits()), + ); + crate::object::js_object_set_field( + compacted, + i as u32, + crate::JSValue::from_bits(v.to_bits()), + ); + } + assert_eq!( + crate::object::js_object_delete_field(compacted, key("picdel_a")), + 1 + ); + + let mut c_pristine = [0i64; super::PIC_CACHE_WORDS]; + let vp = super::js_object_get_field_ic_miss(pristine, key("picdel_c"), &mut c_pristine); + assert_eq!(vp, 3.0, "pristine `c` is slot 2"); + + let mut c_compacted = [0i64; super::PIC_CACHE_WORDS]; + let vc = super::js_object_get_field_ic_miss(compacted, key("picdel_c"), &mut c_compacted); + assert_eq!( + vc, 3.0, + "compacted `c` shifted to slot 1 and must still read 3" + ); + + assert_ne!( + c_compacted[0], 0, + "the compacted instance primed nothing — rung 1's new surface is inert" + ); + assert_ne!( + c_compacted[0], c_pristine[0], + "the compacted instance primed its pristine sibling's token — an \ + id-comparing PIC would read slot {} for a receiver whose `c` is \ + at slot {}", + c_pristine[1], c_compacted[1] + ); + assert_eq!(c_pristine[1], 2, "pristine `c` slot"); + assert_eq!(c_compacted[1], 1, "compacted `c` slot"); + } +} + +/// The PIC cache token the EMITTED code computes for `obj`, transcribed +/// from `perry-codegen/src/expr/property_get/generic_dispatch.rs`: +/// +/// ```text +/// token = valid_shape_id ? (shape_id | 1<<62) : 0 +/// ``` +/// +/// The runtime never calls this; it exists so a test can compare what the +/// miss handler PRIMES against what the hit path will COMPUTE, which is +/// the only pair whose agreement decides whether a site can ever hit. +unsafe fn emitted_pic_token(obj: *const super::ObjectHeader) -> u64 { + let shape_id = crate::object::shapes::object_shape_id(obj); + shape_id as u64 | crate::object::shapes::PIC_ID_TOKEN_BIT +} + +/// ★ The invariant #6759 C3 rung 1 broke, asserted where it broke. +/// +/// A shape's population must be UNIFORMLY stamped: the token the miss +/// handler primes from one instance is only useful if a DIFFERENT, +/// freshly-allocated instance of the same class computes the same token. +/// A prior implementation stamped class instances lazily, so instance #1 +/// primed an id token while every newborn sibling computed a different +/// identity. `token_eq` then failed at every field-read site until the +/// sibling took the miss path itself. +/// +/// This is deliberately NOT "the newborn carries a stamp" — that is a +/// presence check two different states satisfy (both-stamped and +/// both-unstamped are each fine; the mixture is the bug). Comparing the +/// primed token against a fresh sibling's COMPUTED token is what fails +/// under either half of the split. +#[test] +fn a_fresh_class_instance_computes_the_token_the_miss_handler_primed() { + let _lock = crate::gc::global_side_table_test_lock(); + unsafe { + let packed = b"picbirth_x\0picbirth_y"; + let mk = || { + crate::object::js_object_alloc_class_with_keys( + 0x6082, + 0, + 2, + packed.as_ptr(), + packed.len() as u32, + ) + }; + let key = crate::string::js_string_from_bytes(b"picbirth_x".as_ptr(), 10); + + let primed_from = mk(); + crate::object::js_object_set_field( + primed_from, + 0, + crate::JSValue::from_bits(5.0f64.to_bits()), + ); + assert_eq!( + (*primed_from).class_id, + 0x6082, + "test premise: the receiver is a class instance, not a literal" + ); + + let mut cache = [0i64; super::PIC_CACHE_WORDS]; + assert_eq!( + super::js_object_get_field_ic_miss(primed_from, key, &mut cache), + 5.0, + "test premise: the miss handler resolved the field" + ); + assert_ne!( + cache[0], 0, + "test premise: the miss handler primed SOMETHING — a zero token \ + never hits, so the comparison below would be vacuous" + ); + + // The next `new C(...)`. Nothing has resolved a field on it. + let fresh = mk(); + assert_eq!( + emitted_pic_token(fresh), + cache[0] as u64, + "a freshly allocated instance of the SAME class computes a \ + different PIC token than the one primed from its sibling, so \ + every read of a newborn instance's field misses the cache and \ + takes the full miss handler — #7983's split population" + ); + + // And the same must hold once the fresh one has itself resolved: + // priming from either instance is interchangeable. + let mut cache2 = [0i64; super::PIC_CACHE_WORDS]; + super::js_object_get_field_ic_miss(fresh, key, &mut cache2); + assert_eq!( + cache2[0], cache[0], + "two instances of one class primed two different tokens — the \ + site thrashes between them" + ); + } +} diff --git a/crates/perry-runtime/src/proxy/put_value.rs b/crates/perry-runtime/src/proxy/put_value.rs index bf87ef7db1..30192fe1b7 100644 --- a/crates/perry-runtime/src/proxy/put_value.rs +++ b/crates/perry-runtime/src/proxy/put_value.rs @@ -449,9 +449,7 @@ pub extern "C" fn js_put_value_set_ic_miss( let alloc_limit = shape.live_inline_slot_count as usize; let slot_word: u32 = if (idx as usize) < alloc_limit { idx - } else if (idx as usize) < shape.logical_key_count as usize - && idx < IC_SLOT_OVERFLOW_BIT - { + } else if (idx as usize) < shape.logical_key_count as usize && idx < IC_SLOT_OVERFLOW_BIT { idx | IC_SLOT_OVERFLOW_BIT } else { return result; diff --git a/crates/perry/tests/dense_accumulator_masked_reads.rs b/crates/perry/tests/dense_accumulator_masked_reads.rs new file mode 100644 index 0000000000..a96c1e153a --- /dev/null +++ b/crates/perry/tests/dense_accumulator_masked_reads.rs @@ -0,0 +1,165 @@ +//! A float accumulator over masked reads earns the dense range clone. +//! +//! `sum = sum * x[i & 63] + x[(i * 7) & 63]` (the `17_loop_data_dependent` +//! shape) was rejected by the dense range tier while `sum = sum * x[i & 63]` +//! was admitted — the discriminator was the accumulator's static numeric +//! proof. `+` can be concatenation, so the per-statement proof demands both +//! operands numeric; a reassigned accumulator has no such proof, because its +//! own writes read the guarded array, whose element proof only exists once +//! the guard has run. Chicken-and-egg, broken previously only for `*`. +//! +//! The matcher now peels the accumulator: it retries the proof with the +//! `LocalSet` target treated as numeric BY CONTRACT, then verifies every such +//! pending local with the same collector the lowering runs. The contract is +//! enforced at run time twice over: the clone's entry emits a genuine-double +//! tag check on the accumulator (a string-seeded accumulator routes to the +//! slow copy), and the dense entry guard validates the whole masked window +//! hole-free (a string ELEMENT fails the guard the same way). +//! +//! Measured: 17_loop_data_dependent 505 ms -> 227 ms against node's 229 ms. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile(dir: &Path, source: &str) -> (PathBuf, String) { + let entry = dir.join("main.ts"); + let output = dir.join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + let compile = Command::new(perry_bin()) + .current_dir(dir) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .env("PERRY_NO_CACHE", "1") + .env("PERRY_LLVM_KEEP_IR", "1") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + ( + output, + String::from_utf8_lossy(&compile.stderr).into_owned(), + ) +} + +fn fast_copies(stderr: &str) -> usize { + let path = stderr + .lines() + .find_map(|line| line.split("kept LLVM IR: ").nth(1)) + .map(str::trim) + .map(PathBuf::from) + .unwrap_or_else(|| panic!("PERRY_LLVM_KEEP_IR did not report an IR path\n{stderr}")); + std::fs::read_to_string(path) + .expect("read kept LLVM IR") + .lines() + .filter(|l| l.starts_with("packed_f64_range") && l.trim_end().ends_with(':')) + .count() +} + +fn run(bin: &Path, dir: &Path, moving_gc: bool) -> Output { + let mut command = Command::new(bin); + command.current_dir(dir); + if moving_gc { + command + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1"); + } + command.output().expect("run compiled binary") +} + +fn assert_stdout(output: &Output, expected: &str, moving_gc: bool) { + assert!( + output.status.success(), + "binary failed with moving_gc={moving_gc}\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert_eq!(String::from_utf8_lossy(&output.stdout), expected); +} + +/// The data-dependent recurrence gets a dense fast copy, and its result is +/// bit-identical to node's across 200k iterations of float churn. +#[test] +fn a_masked_read_accumulator_earns_the_dense_clone_and_matches_node() { + let source = r#" +function run(x: number[]): number { + let sum = 1.0; + for (let i = 0; i < 200000; i++) sum = sum * x[i & 63] + x[(i * 7) & 63]; + return sum; +} +const x: number[] = []; +for (let i = 0; i < 64; i++) x.push(0.5 + i * 0.01); +console.log("r:" + run(x)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, stderr) = compile(dir.path(), source); + assert!( + fast_copies(&stderr) > 0, + "the accumulator peel must admit the dense clone; without it the whole \ + loop pays the per-access guard tier (505ms vs node's 229ms)" + ); + for moving_gc in [false, true] { + assert_stdout( + &run(&bin, dir.path(), moving_gc), + "r:44.18806624016606\n", + moving_gc, + ); + } +} + +/// The contract's first enforcement point: an accumulator seeded with a +/// STRING must take the entry tag check into the slow copy and produce node's +/// concatenation, not a raw fadd over a string box. +#[test] +fn a_string_seeded_accumulator_takes_the_slow_copy_and_concatenates() { + let source = r#" +function run(x: number[]): string { + let sum: any = "s"; + for (let i = 0; i < 4; i++) sum = sum + x[i & 63]; + return sum; +} +const x: number[] = []; +for (let i = 0; i < 64; i++) x.push(0.5 + i * 0.01); +console.log("r:" + run(x)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout( + &run(&bin, dir.path(), moving_gc), + "r:s0.50.510.520.53\n", + moving_gc, + ); + } +} + +/// The second enforcement point: a string ELEMENT makes the dense entry guard +/// fail (the window is not hole-free numeric), so the slow copy concatenates +/// exactly as node does. +#[test] +fn a_string_element_fails_the_guard_and_the_slow_copy_matches_node() { + let source = r#" +function run(x: any[]): any { + let sum: any = 0.0; + for (let i = 0; i < 4; i++) sum = sum + x[i & 63]; + return sum; +} +const y: any[] = []; +for (let i = 0; i < 64; i++) y.push(i === 2 ? "boom" : i * 1.0); +console.log("r:" + run(y)); +"#; + let dir = tempfile::tempdir().expect("tempdir"); + let (bin, _) = compile(dir.path(), source); + for moving_gc in [false, true] { + assert_stdout(&run(&bin, dir.path(), moving_gc), "r:1boom3\n", moving_gc); + } +} diff --git a/crates/perry/tests/issue_9287_overflow_slot_ic.rs b/crates/perry/tests/issue_9287_overflow_slot_ic.rs index 8d8ed450cd..3fdbe60b37 100644 --- a/crates/perry/tests/issue_9287_overflow_slot_ic.rs +++ b/crates/perry/tests/issue_9287_overflow_slot_ic.rs @@ -185,7 +185,10 @@ for (let i = 0; i < 5000; i++) {{ console.log(o["f5"] + " " + o["f0"]); "# ), - &[("PERRY_GC_HEAP_LIMIT", "8"), ("PERRY_GC_FORCE_EVACUATE", "1")], + &[ + ("PERRY_GC_HEAP_LIMIT", "8"), + ("PERRY_GC_FORCE_EVACUATE", "1"), + ], ); assert_eq!(out, "str_1 0"); }