From 779d45b4eff2f206244acbcfc46ae9da27eabfc4 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Tue, 25 Aug 2026 01:30:36 +0200 Subject: [PATCH 1/2] perf(codegen): specialize short packed spread calls --- changelog.d/8772-short-packed-spread.md | 3 + crates/perry-codegen/src/expr/call_spread.rs | 5 + .../src/expr/call_spread_short.rs | 352 ++++++++++++++++++ .../src/expr/call_spread_short_tests.rs | 187 ++++++++++ crates/perry-codegen/src/expr/mod.rs | 3 + .../perry-codegen/src/runtime_decls/arrays.rs | 7 + crates/perry-runtime/src/array/flat_clone.rs | 51 ++- crates/perry-runtime/src/array/iter_object.rs | 23 +- crates/perry-runtime/src/array/iterator.rs | 31 +- crates/perry-runtime/src/array/mod.rs | 2 +- .../src/array/spread_dense_tests.rs | 35 ++ crates/perry-runtime/src/array/tests.rs | 31 ++ crates/perry-runtime/src/native_abi.rs | 11 + .../src/object/native_call_method.rs | 59 +++ crates/perry-runtime/src/symbol/iterator.rs | 24 +- .../tests/issue_8772_short_packed_spread.rs | 281 ++++++++++++++ .../issue_8772_short_packed_spread/main.ts | 33 ++ .../semantics.ts | 77 ++++ .../throwing.ts | 16 + 19 files changed, 1196 insertions(+), 35 deletions(-) create mode 100644 changelog.d/8772-short-packed-spread.md create mode 100644 crates/perry-codegen/src/expr/call_spread_short.rs create mode 100644 crates/perry-codegen/src/expr/call_spread_short_tests.rs create mode 100644 crates/perry/tests/issue_8772_short_packed_spread.rs create mode 100644 test-files/fixtures/issue_8772_short_packed_spread/main.ts create mode 100644 test-files/fixtures/issue_8772_short_packed_spread/semantics.ts create mode 100644 test-files/fixtures/issue_8772_short_packed_spread/throwing.ts diff --git a/changelog.d/8772-short-packed-spread.md b/changelog.d/8772-short-packed-spread.md new file mode 100644 index 0000000000..ac8ee89c84 --- /dev/null +++ b/changelog.d/8772-short-packed-spread.md @@ -0,0 +1,3 @@ +## perf(codegen): direct-call stable methods with short packed spread tails + +`receiver.method(fixed, ...args)` now emits guarded direct-call arms when the final spread is an exact ordinary packed Array with zero through four present elements. The guard rejects holes, iterator/prototype overrides, proxies, Array subclasses, descriptors, and oversized tails; method class/shape/invalidation guards select the concrete body, and every miss retains the full iterator-aware apply dispatcher. This removes argument-array materialization and dynamic method lookup from the common empty/one-element ECS dispatch path while preserving source-order evaluation and moving-GC roots. diff --git a/crates/perry-codegen/src/expr/call_spread.rs b/crates/perry-codegen/src/expr/call_spread.rs index 46e10c4588..83ae3ac027 100644 --- a/crates/perry-codegen/src/expr/call_spread.rs +++ b/crates/perry-codegen/src/expr/call_spread.rs @@ -330,6 +330,11 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result { } } if !skip { + if let Some(result) = + super::call_spread_short::try_lower(ctx, object, property, args)? + { + return Ok(result); + } let recv_box = lower_expr(ctx, object)?; // Build a single JS array containing every arg in order. // diff --git a/crates/perry-codegen/src/expr/call_spread_short.rs b/crates/perry-codegen/src/expr/call_spread_short.rs new file mode 100644 index 0000000000..6e04e54a5b --- /dev/null +++ b/crates/perry-codegen/src/expr/call_spread_short.rs @@ -0,0 +1,352 @@ +//! Guarded direct calls for `receiver.method(fixed..., ...shortArray)` (#8772). +//! +//! The spread expression is still evaluated exactly once and in source order. +//! A non-allocating runtime proof then admits only an exact ordinary packed +//! Array with 0..=4 present elements. The method side is independently guarded +//! by the same `(class id, ShapeId, method invalidation slot)` proof used by +//! ordinary shape-directed method calls. Either miss joins the existing apply +//! path, which drives the full iterator protocol. + +use anyhow::Result; +use perry_hir::{CallArg, Expr}; + +use crate::nanbox::double_literal; +use crate::native_value::LoweredValue; +use crate::types::{DOUBLE, I1, I32, I64, PTR}; + +use super::FnCtx; + +const MAX_SPREAD_ARITY: usize = 4; +const MAX_METHOD_ARMS: usize = 8; + +#[derive(Clone)] +struct DirectCandidate { + class_id: u32, + class_name: String, + target: String, + declared_count: usize, +} + +/// Collect concrete class implementations in deterministic class-id order. +/// +/// Rest/`arguments` bodies are intentionally left to the generic dispatcher: +/// their direct ABI allocates one or two argument arrays and would erase the +/// small-tail win. An omitted class is harmless because a guard miss always +/// reaches apply. +fn direct_candidates(ctx: &FnCtx<'_>, property: &str) -> Vec { + let mut roots: Vec<(&String, u32)> = + ctx.class_ids.iter().map(|(name, &id)| (name, id)).collect(); + roots.sort_unstable_by(|a, b| a.1.cmp(&b.1).then_with(|| a.0.cmp(b.0))); + + let mut out = Vec::new(); + let mut seen = std::collections::HashSet::new(); + for (class_name, class_id) in roots { + let Some(_keys_global) = ctx.class_keys_globals.get(class_name) else { + continue; + }; + let mut current = Some(class_name.clone()); + while let Some(owner) = current { + let key = (owner.clone(), property.to_string()); + if let Some(public_target) = ctx.methods.get(&key) { + let unsupported_abi = public_target.starts_with("perry_static_") + || matches!(ctx.method_has_rest.get(&key), Some(true)) + || matches!(ctx.method_has_synthetic_arguments.get(&key), Some(true)); + if !unsupported_abi && seen.insert((class_id, public_target.clone())) { + let target = if owner == *class_name + && ctx + .pshape_methods + .contains_key(&(owner.clone(), property.to_string())) + { + crate::collectors::pshape_method_name(public_target) + } else { + public_target.clone() + }; + out.push(DirectCandidate { + class_id, + class_name: class_name.clone(), + target, + declared_count: ctx.method_param_counts.get(&key).copied().unwrap_or(0), + }); + if out.len() == MAX_METHOD_ARMS { + return out; + } + } + break; + } + current = ctx + .classes + .get(&owner) + .and_then(|class| class.extends_name.clone()); + } + } + out +} + +fn first_element_ptr(ctx: &mut FnCtx<'_>, alloca: &str, count: usize) -> String { + let ptr = ctx.block().next_reg(); + ctx.block().emit_raw(format!( + "{ptr} = getelementptr [{count} x double], ptr {alloca}, i64 0, i64 0" + )); + ptr +} + +/// Try the #8772 lowering. `None` means the caller must retain its existing +/// source-ordered argument bundling path. +pub(crate) fn try_lower<'f, 'e>( + ctx: &mut FnCtx<'f>, + object: &'e Expr, + property: &str, + args: &'e [CallArg], +) -> Result> { + let Some(CallArg::Spread(spread_expr)) = args.last() else { + return Ok(None); + }; + if args[..args.len() - 1] + .iter() + .any(|arg| !matches!(arg, CallArg::Expr(_))) + { + return Ok(None); + } + let candidates = direct_candidates(ctx, property); + if candidates.is_empty() { + return Ok(None); + } + + // Receiver, fixed arguments, then the final spread expression: exactly the + // ECMAScript evaluation order and exactly once each. One open group keeps + // every pointer-bearing value current through both CFG diamonds and the + // allocating generic fallback. + let mut roots = crate::rooting::open_rooted_group(args.len() + 1); + let recv_root = roots.lower(ctx, object, true)?; + let mut fixed_roots = Vec::with_capacity(args.len().saturating_sub(1)); + for arg in &args[..args.len() - 1] { + let CallArg::Expr(expr) = arg else { + unreachable!() + }; + fixed_roots.push(roots.lower(ctx, expr, true)?); + } + let spread_root = roots.lower(ctx, spread_expr, true)?; + + // Re-read once below all operand evaluation. The two guards from here to a + // direct call are non-allocating; fallback re-reads again after its + // materializer allocates. + let fast_recv = roots.reread(ctx, recv_root)?; + let fast_fixed: Vec = fixed_roots + .iter() + .map(|&root| roots.reread(ctx, root)) + .collect::>()?; + let fast_spread = roots.reread(ctx, spread_root)?; + + let values_alloca = ctx.func.alloca_entry_array(DOUBLE, MAX_SPREAD_ARITY); + let values_ptr = first_element_ptr(ctx, &values_alloca, MAX_SPREAD_ARITY); + let arity = ctx.block().call( + I32, + "js_short_packed_spread_values", + &[(DOUBLE, &fast_spread), (PTR, &values_ptr)], + ); + + let method_probe_idx = ctx.new_block("short_spread.method_probe"); + let fallback_idx = ctx.new_block("short_spread.fallback"); + let merge_idx = ctx.new_block("short_spread.merge"); + let method_probe_label = ctx.block_label(method_probe_idx); + let fallback_label = ctx.block_label(fallback_idx); + let merge_label = ctx.block_label(merge_idx); + let packed = ctx.block().icmp_sge(I32, &arity, "0"); + ctx.block() + .cond_br(&packed, &method_probe_label, &fallback_label); + + // Load compiler-published ShapeIds once. Entry-init slots dominate this + // whole diamond; these ordinary loads do not allocate. + ctx.current_block = method_probe_idx; + let expected_shapes: Vec = candidates + .iter() + .map(|candidate| { + let keys = ctx + .class_keys_globals + .get(&candidate.class_name) + .expect("candidate required a keys global") + .clone(); + crate::typed_shape::load_class_shape_id(ctx, &candidate.class_name, &keys) + }) + .collect(); + let key_idx = ctx.strings.intern(property); + let entry = ctx.strings.entry(key_idx); + let method_guard_slot = (entry.dispatch_hash & 0xffff).to_string(); + let shape_out = ctx.func.alloca_entry(I32); + let live_class = ctx.block().call( + I32, + "js_method_direct_shape_class", + &[ + (DOUBLE, &fast_recv), + (PTR, &shape_out), + (I32, &method_guard_slot), + ], + ); + let live_shape = ctx.block().load(I32, &shape_out); + + let candidate_test_idxs: Vec = (0..candidates.len()) + .map(|index| ctx.new_block(&format!("short_spread.target_test{index}"))) + .collect(); + let candidate_select_idxs: Vec = (0..candidates.len()) + .map(|index| ctx.new_block(&format!("short_spread.target{index}"))) + .collect(); + let first_candidate_label = ctx.block_label(candidate_test_idxs[0]); + ctx.block().br(&first_candidate_label); + + let mut phi_inputs: Vec<(String, String)> = Vec::new(); + let undefined = double_literal(f64::from_bits(crate::nanbox::TAG_UNDEFINED)); + for (candidate_no, candidate) in candidates.iter().enumerate() { + ctx.current_block = candidate_test_idxs[candidate_no]; + let target_label = ctx.block_label(candidate_select_idxs[candidate_no]); + let miss_label = candidate_test_idxs + .get(candidate_no + 1) + .map(|&index| ctx.block_label(index)) + .unwrap_or_else(|| fallback_label.clone()); + let cid_ok = ctx + .block() + .icmp_eq(I32, &live_class, &candidate.class_id.to_string()); + let shape_ok = ctx + .block() + .icmp_eq(I32, &live_shape, &expected_shapes[candidate_no]); + let target_ok = ctx.block().and(I1, &cid_ok, &shape_ok); + ctx.block().cond_br(&target_ok, &target_label, &miss_label); + + ctx.current_block = candidate_select_idxs[candidate_no]; + let arity_blocks: Vec = (0..=MAX_SPREAD_ARITY) + .map(|spread_arity| { + ctx.new_block(&format!( + "short_spread.target{candidate_no}.arity{spread_arity}" + )) + }) + .collect(); + let arity_tests: Vec = (1..=MAX_SPREAD_ARITY) + .map(|spread_arity| { + ctx.new_block(&format!( + "short_spread.target{candidate_no}.arity_test{spread_arity}" + )) + }) + .collect(); + for spread_arity in 0..=MAX_SPREAD_ARITY { + if spread_arity > 0 { + ctx.current_block = arity_tests[spread_arity - 1]; + } + let hit = ctx.block_label(arity_blocks[spread_arity]); + let miss = arity_tests + .get(spread_arity) + .map(|&index| ctx.block_label(index)) + .unwrap_or_else(|| fallback_label.clone()); + let matches = ctx.block().icmp_eq(I32, &arity, &spread_arity.to_string()); + ctx.block().cond_br(&matches, &hit, &miss); + } + + for (spread_arity, &block_idx) in arity_blocks.iter().enumerate() { + ctx.current_block = block_idx; + let mut user_args = fast_fixed.clone(); + for index in 0..spread_arity { + let slot = ctx + .block() + .gep(DOUBLE, &values_alloca, &[(I64, &index.to_string())]); + user_args.push(ctx.block().load(DOUBLE, &slot)); + } + let mut direct_args = Vec::with_capacity(candidate.declared_count + 1); + direct_args.push(fast_recv.clone()); + direct_args.extend(user_args.into_iter().take(candidate.declared_count)); + while direct_args.len() < candidate.declared_count + 1 { + direct_args.push(undefined.clone()); + } + let direct_slices: Vec<(crate::types::LlvmType, &str)> = direct_args + .iter() + .map(|value| (DOUBLE, value.as_str())) + .collect(); + let value = ctx.block().call(DOUBLE, &candidate.target, &direct_slices); + let after = ctx.block().label.clone(); + ctx.block().br(&merge_label); + phi_inputs.push((value, after)); + } + } + + // Every rejection shares the original apply dispatch. The helper only + // constructs its source-ordered argument array; method lookup remains in + // js_native_call_method_apply_by_id so overrides and wrong receivers retain + // the generic semantics. + ctx.current_block = fallback_idx; + let (fixed_ptr, fixed_len) = if fixed_roots.is_empty() { + ("null".to_string(), "0".to_string()) + } else { + let fixed_alloca = ctx.func.alloca_entry_array(DOUBLE, fixed_roots.len()); + for (index, &root) in fixed_roots.iter().enumerate() { + let value = roots.reread(ctx, root)?; + let slot = ctx + .block() + .gep(DOUBLE, &fixed_alloca, &[(I64, &index.to_string())]); + ctx.block().store(DOUBLE, &value, &slot); + } + ( + first_element_ptr(ctx, &fixed_alloca, fixed_roots.len()), + fixed_roots.len().to_string(), + ) + }; + let fallback_spread = roots.reread(ctx, spread_root)?; + let args_array = ctx.block().call( + I64, + "js_spread_tail_fallback_args", + &[ + (PTR, &fixed_ptr), + (I64, &fixed_len), + (DOUBLE, &fallback_spread), + ], + ); + let fallback_recv = roots.reread(ctx, recv_root)?; + let dispatch_global = ctx.strings.static_dispatch_global(key_idx); + let method_id = crate::strings::emit_static_dispatch_id(ctx.block(), &dispatch_global); + let fallback_value = ctx.block().call( + DOUBLE, + "js_native_call_method_apply_by_id", + &[ + (DOUBLE, &fallback_recv), + (I64, &method_id), + (I64, &args_array), + ], + ); + let fallback_after = ctx.block().label.clone(); + ctx.block().br(&merge_label); + phi_inputs.push((fallback_value, fallback_after)); + + ctx.current_block = merge_idx; + let incoming: Vec<(&str, &str)> = phi_inputs + .iter() + .map(|(value, label)| (value.as_str(), label.as_str())) + .collect(); + let result = ctx.block().phi(DOUBLE, &incoming); + roots.release(ctx); + + let targets = candidates + .iter() + .map(|candidate| candidate.target.as_str()) + .collect::>() + .join(","); + ctx.record_lowered_value( + "MethodSpreadCall", + None, + "short_packed_spread_direct_call", + &LoweredValue::js_value(result.clone()), + None, + None, + None, + false, + false, + vec![ + "packed_spread_arities=0,1,2,3,4".to_string(), + format!("method={property}"), + format!("direct_targets={targets}"), + "spread_guard=exact_ordinary_packed_array,no_holes,max_length_4".to_string(), + "iterator_guard=builtin_array_iterator,no_own_iterator,no_custom_prototype" + .to_string(), + "method_identity_guard=js_method_direct_shape_class(class_id,shape_id,invalidation_slot)" + .to_string(), + "generic_fallback=js_spread_tail_fallback_args+js_native_call_method_apply_by_id" + .to_string(), + ], + ); + Ok(Some(result)) +} diff --git a/crates/perry-codegen/src/expr/call_spread_short_tests.rs b/crates/perry-codegen/src/expr/call_spread_short_tests.rs new file mode 100644 index 0000000000..699c03f601 --- /dev/null +++ b/crates/perry-codegen/src/expr/call_spread_short_tests.rs @@ -0,0 +1,187 @@ +//! IR ratchets for guarded short packed-spread calls (#8772). + +use perry_hir::types::Type; +use perry_hir::{CallArg, Class, Expr, Function, Module, ModuleInitKind, Param, Stmt}; + +fn param(id: u32, name: &str, ty: Type, default: Option) -> Param { + Param { + id, + name: name.to_string(), + ty, + default, + decorators: Vec::new(), + is_rest: false, + arguments_object: None, + } +} + +fn reset(id: u32, default: f64) -> Function { + Function { + id, + name: "reset".to_string(), + type_params: Vec::new(), + params: vec![ + param(id + 1, "entity", Type::Any, None), + param(id + 2, "delta", Type::Number, Some(Expr::Number(default))), + ], + return_type: Type::Number, + body: vec![Stmt::Return(Some(Expr::LocalGet(id + 2)))], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + } +} + +fn class(id: u32, name: &str, default: f64) -> Class { + Class { + id, + name: name.to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: Vec::new(), + constructor: None, + methods: vec![reset(id + 10, default)], + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + computed_members: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + specialized_from: None, + } +} + +fn fixture() -> Module { + let mut module = Module::new("issue_8772_short_spread.ts"); + module.classes = vec![class(100, "Position", 1.0), class(200, "Velocity", 2.0)]; + module.functions = vec![Function { + id: 1, + name: "invoke".to_string(), + type_params: Vec::new(), + params: vec![ + param(2, "instance", Type::Any, None), + param(3, "entity", Type::Any, None), + param(4, "args", Type::Array(Box::new(Type::Any)), None), + ], + return_type: Type::Number, + body: vec![Stmt::Return(Some(Expr::CallSpread { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::LocalGet(2)), + property: "reset".to_string(), + byte_offset: 0, + }), + args: vec![ + CallArg::Expr(Expr::LocalGet(3)), + CallArg::Spread(Expr::LocalGet(4)), + ], + type_args: Vec::new(), + }))], + is_async: false, + is_generator: false, + is_strict: false, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }]; + module.init_kind = ModuleInitKind::Eager; + module +} + +fn emit() -> String { + let opts = crate::CompileOptions { + emit_ir_only: true, + ..Default::default() + }; + String::from_utf8(crate::compile_module(&fixture(), opts).expect("fixture compiles")) + .expect("LLVM IR is UTF-8") +} + +fn function_body<'a>(ir: &'a str, fragment: &str) -> &'a str { + let name = ir + .find(fragment) + .unwrap_or_else(|| panic!("missing function {fragment:?}\n{ir}")); + let start = ir[..name] + .rfind("\ndefine ") + .unwrap_or_else(|| panic!("missing definition for {fragment:?}")); + let tail = &ir[start + 1..]; + let end = tail.find("\n}\n").expect("terminated function definition"); + &tail[..end + 2] +} + +fn named_block<'a>(function: &'a str, label: &str) -> &'a str { + let needle = format!("\n{label}"); + let start = function + .find(&needle) + .map(|offset| offset + 1) + .unwrap_or_else(|| panic!("missing block {label:?}\n{function}")); + let tail = &function[start..]; + let end = tail[1..] + .find("\nshort_spread.") + .map(|offset| offset + 1) + .unwrap_or(tail.len()); + &tail[..end] +} + +#[test] +fn empty_and_one_element_arms_call_reset_directly_without_apply() { + let ir = emit(); + let invoke = function_body(&ir, "__invoke("); + assert!(invoke.contains("call i32 @js_short_packed_spread_values(")); + assert!(invoke.contains("call i32 @js_method_direct_shape_class(")); + + for candidate in 0..2 { + for arity in 0..=1 { + let block = named_block( + invoke, + &format!("short_spread.target{candidate}.arity{arity}"), + ); + assert!( + block.contains("call double @perry_method_") && block.contains("__reset("), + "packed arity {arity} must call a selected reset body directly\n{block}" + ); + assert!( + !block.contains("js_native_call_method_apply") + && !block.contains("js_spread_tail_fallback_args"), + "packed arity {arity} must contain no apply machinery\n{block}" + ); + } + } +} + +#[test] +fn guard_misses_retain_one_full_iterator_apply_fallback() { + let ir = emit(); + let invoke = function_body(&ir, "__invoke("); + let fallback = named_block(invoke, "short_spread.fallback"); + assert_eq!( + fallback + .matches("call i64 @js_spread_tail_fallback_args(") + .count(), + 1, + "fallback must materialize fixed+spread exactly once\n{fallback}" + ); + assert_eq!( + fallback + .matches("call double @js_native_call_method_apply_by_id(") + .count(), + 1, + "fallback must retain dynamic method apply\n{fallback}" + ); +} diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 54eb58b38b..8f06398e1d 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -161,6 +161,9 @@ mod write_pic_barrier_tests; // and it now lives outside `crate::expr`. #[cfg(test)] mod call_spread_rooting_tests; +mod call_spread_short; +#[cfg(test)] +mod call_spread_short_tests; #[cfg(test)] mod issue7628_rooting_tests; pub(crate) mod shadow_slot; diff --git a/crates/perry-codegen/src/runtime_decls/arrays.rs b/crates/perry-codegen/src/runtime_decls/arrays.rs index 2a4e81251e..5449b0d3c0 100644 --- a/crates/perry-codegen/src/runtime_decls/arrays.rs +++ b/crates/perry-codegen/src/runtime_decls/arrays.rs @@ -213,6 +213,13 @@ pub fn declare_phase_b_arrays(module: &mut LlModule) { module.declare_function("js_array_set_length_strict", VOID, &[I64, DOUBLE]); // Array.from() — js_array_clone handles arrays, Sets, and Maps. module.declare_function("js_array_clone", I64, &[I64]); + // #8772: non-allocating exact packed-array guard for a final spread tail. + // Writes at most four values to caller-owned stack storage and returns + // arity 0..4, or -1 for the generic iterator path. + module.declare_function("js_short_packed_spread_values", I32, &[DOUBLE, PTR]); + // Generic `fixed..., ...spread` materializer used after the short-array or + // guarded-method proof fails. It drives the complete iterator protocol. + module.declare_function("js_spread_tail_fallback_args", I64, &[PTR, I64, DOUBLE]); // #2773: Array.from(source) — throws TypeError for nullish sources, keeps // number/boolean/symbol -> [], otherwise materializes via js_array_clone. // Takes the raw NaN-boxed value so the tag bits survive. diff --git a/crates/perry-runtime/src/array/flat_clone.rs b/crates/perry-runtime/src/array/flat_clone.rs index 2ebb9f8818..a07d953272 100644 --- a/crates/perry-runtime/src/array/flat_clone.rs +++ b/crates/perry-runtime/src/array/flat_clone.rs @@ -53,6 +53,8 @@ unsafe fn receiver_gc_type(ptr: *const ArrayHeader) -> u8 { /// - `array_proto_iterator_modified`: user code replaced or deleted /// `Array.prototype[Symbol.iterator]`, so the builtin walk is no longer what /// a spread must run. +/// - `object_static_prototype`: `Object.setPrototypeOf(array, custom)` can +/// replace the inherited iterator without touching Array.prototype. /// - `has_own_symbol_property`: the instance carries its OWN `[Symbol.iterator]`, /// which shadows the prototype's. Existence is probed WITHOUT invoking an /// accessor, so falling through to the slow path calls a user getter exactly @@ -86,17 +88,56 @@ pub(crate) fn dense_spread_source(value: f64) -> Option<*const ArrayHeader> { if crate::array::array_proto_iterator_modified() { return None; } - let iter_sym = crate::symbol::well_known_symbol("iterator"); - if iter_sym.is_null() { + if crate::object::prototype_chain::object_static_prototype(arr as usize).is_some() { return None; } - let sym_value = f64::from_bits(crate::value::JSValue::pointer(iter_sym as *const u8).bits()); - if unsafe { crate::symbol::has_own_symbol_property(value, sym_value) } { - return None; + // Do not materialize Symbol.iterator from a guard. If it is not cached, + // user code cannot have installed it as an own key; if it is cached, the + // side-table existence probe below is non-allocating and never invokes an + // accessor. This keeps dense_spread_source usable in call-site guards that + // hold evaluated operands in SSA registers. + let iter_sym = crate::symbol::well_known_symbol_if_cached("iterator"); + if !iter_sym.is_null() { + let sym_value = + f64::from_bits(crate::value::JSValue::pointer(iter_sym as *const u8).bits()); + if unsafe { crate::symbol::has_own_symbol_property(value, sym_value) } { + return None; + } } Some(arr) } +/// Copy a short, exact packed-array spread tail into caller-owned storage. +/// +/// Returns the element count (`0..=4`) on success and `-1` when spread must use +/// the generic iterator path. In addition to [`dense_spread_source`]'s exact +/// ordinary-array proof, this rejects holes: the general dense-copy path may +/// normalize a hole to `undefined`, while a direct-call arm promises that each +/// value came from a present packed slot. +/// +/// This helper is deliberately non-allocating. Generated code evaluates and +/// roots `receiver`, fixed arguments, and the spread expression before calling +/// it, then uses the copied values only when the returned arity is nonnegative. +#[no_mangle] +pub unsafe extern "C" fn js_short_packed_spread_values(value: f64, out: *mut f64) -> i32 { + let Some(arr) = dense_spread_source(value) else { + return -1; + }; + let len = (*arr).length as usize; + if len > 4 || (len != 0 && out.is_null()) { + return -1; + } + let elements = (arr as *const u8).add(std::mem::size_of::()) as *const u64; + for index in 0..len { + let bits = std::ptr::read(elements.add(index)); + if bits == crate::value::TAG_HOLE { + return -1; + } + std::ptr::write(out.add(index), f64::from_bits(bits)); + } + len as i32 +} + /// Element-copy an array [`dense_spread_source`] has already proven ordinary. /// /// `value` is the NaN-boxed receiver rather than a raw pointer because diff --git a/crates/perry-runtime/src/array/iter_object.rs b/crates/perry-runtime/src/array/iter_object.rs index 201063f0c6..aabd8f796e 100644 --- a/crates/perry-runtime/src/array/iter_object.rs +++ b/crates/perry-runtime/src/array/iter_object.rs @@ -25,8 +25,8 @@ //! Buffer iterator one. use super::*; -use crate::object::{js_object_alloc, js_object_get_field, js_object_set_field, ObjectHeader}; -use crate::value::{js_nanbox_get_pointer, js_nanbox_pointer, JSValue, TAG_UNDEFINED}; +use crate::object::{ObjectHeader, js_object_alloc, js_object_get_field, js_object_set_field}; +use crate::value::{JSValue, TAG_UNDEFINED, js_nanbox_get_pointer, js_nanbox_pointer}; /// Class id reserved for array iterators. Sits adjacent to the Buffer /// iterator id (0xFFFF0005) in the 0xFFFF prefix reserved for @@ -46,6 +46,11 @@ const KIND_VALUES_NULL_DONE: i32 = 3; /// iterator this reads `length` and each indexed property from the Arguments /// object on every step, so mutations made before exhaustion are observable. const KIND_ARGUMENTS_VALUES: i32 = 4; +/// Values iterator over an Array Proxy. The backing field stores the proxy's +/// NaN-boxed registry id rather than an `ArrayHeader` pointer; `.next()` uses +/// live `LengthOfArrayLike` / `Get` operations so proxy traps and mutations are +/// observed with the same timing as `%ArrayIteratorPrototype%.next`. +const KIND_PROXY_VALUES: i32 = 5; /// Clean a NaN-boxed array pointer to a raw `*mut ArrayHeader`, or null. fn unbox_array_ptr(value: f64) -> *mut ArrayHeader { @@ -88,6 +93,9 @@ unsafe fn alloc_iterator(arr_ptr: *mut ArrayHeader, kind: i32) -> f64 { /// `arr.values()` iterator — yields each element value. pub fn array_values_iter(arr_f64: f64) -> f64 { + if crate::proxy::js_proxy_is_proxy(arr_f64) != 0 { + return unsafe { alloc_iterator_backing(arr_f64, KIND_PROXY_VALUES) }; + } let arr_ptr = unbox_array_ptr(arr_f64); if arr_ptr.is_null() { return f64::from_bits(TAG_UNDEFINED); @@ -673,6 +681,8 @@ pub unsafe fn dispatch_array_iterator_method( let len = if kind == KIND_ARGUMENTS_VALUES { crate::object::arguments_object_length(backing_ptr as *const ObjectHeader) + } else if kind == KIND_PROXY_VALUES { + super::generic::al_length(backing_f64).clamp(0, u32::MAX as i64) as u32 } else if backing_ptr == 0 { 0 } else { @@ -698,11 +708,12 @@ pub unsafe fn dispatch_array_iterator_method( // field 0, which the collector DOES rewrite, instead of reusing // the pre-store copy. `iter_obj()` re-reads the iterator's own // address from its root for the same reason. - let backing_ptr = - js_nanbox_get_pointer(f64::from_bits(js_object_get_field(iter_obj(), 0).bits())) - as usize; + let backing_f64 = f64::from_bits(js_object_get_field(iter_obj(), 0).bits()); + let backing_ptr = js_nanbox_get_pointer(backing_f64) as usize; let elem = if kind == KIND_ARGUMENTS_VALUES { crate::object::arguments_object_index_value(backing_ptr as *const ObjectHeader, idx) + } else if kind == KIND_PROXY_VALUES { + super::generic::al_get(backing_f64, idx as i64) } else if backing_ptr == 0 { f64::from_bits(TAG_UNDEFINED) } else { @@ -710,7 +721,7 @@ pub unsafe fn dispatch_array_iterator_method( }; let value = match kind { - KIND_VALUES | KIND_VALUES_NULL_DONE | KIND_ARGUMENTS_VALUES => { + KIND_VALUES | KIND_VALUES_NULL_DONE | KIND_ARGUMENTS_VALUES | KIND_PROXY_VALUES => { JSValue::from_bits(elem.to_bits()) } KIND_KEYS => JSValue::number(idx as f64), diff --git a/crates/perry-runtime/src/array/iterator.rs b/crates/perry-runtime/src/array/iterator.rs index 296e9d883c..0d263a2a6f 100644 --- a/crates/perry-runtime/src/array/iterator.rs +++ b/crates/perry-runtime/src/array/iterator.rs @@ -34,9 +34,9 @@ use crate::value::nanbox_string_key; #[no_mangle] pub extern "C" fn js_for_of_to_array(val_f64: f64) -> f64 { use crate::gc::{ - GcHeader, GC_HEADER_SIZE, GC_TYPE_ARRAY, GC_TYPE_LAZY_ARRAY, GC_TYPE_MAP, GC_TYPE_SET, + GC_HEADER_SIZE, GC_TYPE_ARRAY, GC_TYPE_LAZY_ARRAY, GC_TYPE_MAP, GC_TYPE_SET, GcHeader, }; - use crate::value::{js_nanbox_pointer, JSValue}; + use crate::value::{JSValue, js_nanbox_pointer}; let jsv = JSValue::from_bits(val_f64.to_bits()); if let Some(entries) = entries_array_for_small_handle_value(val_f64) { @@ -192,9 +192,9 @@ fn is_callable_value(value: f64) -> bool { } fn named_field(value: f64, name: &[u8]) -> f64 { - use crate::object::{js_object_get_field_by_name, ObjectHeader}; + use crate::object::{ObjectHeader, js_object_get_field_by_name}; use crate::string::js_string_from_bytes; - use crate::value::{js_nanbox_get_pointer, TAG_UNDEFINED}; + use crate::value::{TAG_UNDEFINED, js_nanbox_get_pointer}; let ptr = js_nanbox_get_pointer(value); if ptr == 0 { @@ -729,7 +729,7 @@ fn array_has_own_iterator(value: f64) -> bool { } pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { - use crate::value::{js_nanbox_get_pointer, js_nanbox_pointer, JSValue, POINTER_MASK}; + use crate::value::{JSValue, POINTER_MASK, js_nanbox_get_pointer, js_nanbox_pointer}; // #7498: the spread receiver is a GC-managed value, and this function // carries it across a dozen classification probes AND the whole @@ -784,6 +784,15 @@ pub(crate) fn array_from_spread_value(value: f64) -> *mut ArrayHeader { throw_not_iterable(value()); } + // An Array Proxy is a small registry id, not an `ArrayHeader`. Route it + // through the full GetIterator path before any raw-address classifiers. + // `array_values_iter` stores the proxy value as a dedicated live backing, + // so the default Array iterator performs `Get(length)` / `Get(index)` + // through traps instead of dereferencing the id or unwrapping the target. + if crate::proxy::js_proxy_is_proxy(value()) != 0 { + return js_iterator_to_array(crate::symbol::js_get_iterator(value())); + } + // #7533: the overwhelmingly common spread — an ordinary dense array — is a // straight element copy that nobody can observe as anything else. Take it // before the classification probes and long before the `@@iterator` walk @@ -1160,9 +1169,9 @@ pub(crate) fn has_iterator_next(value: f64) -> bool { pub(crate) fn sync_iterator_to_array_if_not_async(iter_f64: f64) -> Option<*mut ArrayHeader> { use crate::closure; - use crate::object::{js_object_get_field_by_name, ObjectHeader}; + use crate::object::{ObjectHeader, js_object_get_field_by_name}; use crate::string::js_string_from_bytes; - use crate::value::{js_nanbox_get_pointer, TAG_UNDEFINED}; + use crate::value::{TAG_UNDEFINED, js_nanbox_get_pointer}; let arr = js_array_alloc(8); let iter_ptr = js_nanbox_get_pointer(iter_f64); @@ -1279,9 +1288,9 @@ fn settled_promise_value(value: f64) -> Option { #[no_mangle] pub extern "C" fn js_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader { use crate::closure; - use crate::object::{js_object_get_field_by_name, ObjectHeader}; + use crate::object::{ObjectHeader, js_object_get_field_by_name}; use crate::string::js_string_from_bytes; - use crate::value::{js_nanbox_get_pointer, TAG_UNDEFINED}; + use crate::value::{TAG_UNDEFINED, js_nanbox_get_pointer}; // #7475: EVERY value this loop carries across a `.next()` call is a // GC-managed object, and `.next()` allocates the `{ value, done }` result @@ -1456,9 +1465,9 @@ pub extern "C" fn js_iterator_rest_to_array(iter_f64: f64, done_f64: f64) -> f64 fn js_async_iterator_to_array(iter_f64: f64) -> *mut ArrayHeader { use crate::closure; - use crate::object::{js_object_get_field_by_name, ObjectHeader}; + use crate::object::{ObjectHeader, js_object_get_field_by_name}; use crate::string::js_string_from_bytes; - use crate::value::{js_nanbox_get_pointer, TAG_TRUE, TAG_UNDEFINED}; + use crate::value::{TAG_TRUE, TAG_UNDEFINED, js_nanbox_get_pointer}; let arr = js_array_alloc(8); let iter_ptr = js_nanbox_get_pointer(iter_f64); diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index 636b9fdc20..bbd80beca2 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -69,7 +69,7 @@ pub use self::element_shape::{ pub(crate) use self::element_shape::{test_element_shape_record_exists, test_serialize}; pub use self::flat_clone::{ js_array_clone, js_array_clone_for_spread, js_array_entries, js_array_flat, - js_array_flat_depth, js_array_keys, js_array_values, + js_array_flat_depth, js_array_keys, js_array_values, js_short_packed_spread_values, }; pub use self::from_concat::{ array_from_full, array_of_full, js_array_concat_variadic, js_array_from_mapped, diff --git a/crates/perry-runtime/src/array/spread_dense_tests.rs b/crates/perry-runtime/src/array/spread_dense_tests.rs index 124b06e19c..235c1682cb 100644 --- a/crates/perry-runtime/src/array/spread_dense_tests.rs +++ b/crates/perry-runtime/src/array/spread_dense_tests.rs @@ -234,3 +234,38 @@ fn a_sparse_array_whose_length_outruns_its_storage_is_not_eligible() { unsafe { (*src).length = (*src).capacity + 1 }; assert!(dense_spread_source(boxed(src)).is_none()); } + +#[test] +fn short_packed_call_guard_copies_empty_and_one_element_arrays() { + let mut out = [f64::NAN; 4]; + let empty = js_array_alloc(0); + assert_eq!( + unsafe { js_short_packed_spread_values(boxed(empty), out.as_mut_ptr()) }, + 0 + ); + + let one = dense(&[37.0]); + assert_eq!( + unsafe { js_short_packed_spread_values(boxed(one), out.as_mut_ptr()) }, + 1 + ); + assert_eq!(out[0], 37.0); +} + +#[test] +fn short_packed_call_guard_rejects_holes_and_oversized_arrays() { + let mut out = [0.0; 4]; + let holey = js_array_push_hole(js_array_push_f64(js_array_alloc(2), 1.0)); + assert_eq!( + unsafe { js_short_packed_spread_values(boxed(holey), out.as_mut_ptr()) }, + -1, + "a hole must take the iterator/apply fallback" + ); + + let oversized = dense(&[1.0, 2.0, 3.0, 4.0, 5.0]); + assert_eq!( + unsafe { js_short_packed_spread_values(boxed(oversized), out.as_mut_ptr()) }, + -1, + "only arities 0 through 4 are specialized" + ); +} diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index 151e794cbf..56409601e2 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -74,6 +74,37 @@ fn flattenable_array_ptr_accepts_only_arrays_and_array_proxies() { assert_eq!(flattenable_array_ptr(nested_proxy), array); } +#[test] +fn array_proxy_values_iterator_uses_live_trapped_reads() { + let array = js_array_alloc(4); + js_array_push_f64(array, 7.0); + js_array_push_f64(array, 8.0); + let array_value = boxed_pointer(array as *mut u8); + let handler = crate::object::js_object_alloc(0, 0); + let proxy = crate::proxy::js_proxy_new(array_value, boxed_pointer(handler as *mut u8)); + + let scope = crate::gc::RuntimeHandleScope::new(); + let iter_h = scope.root_nanbox_f64(array_values_iter(proxy)); + let next = || unsafe { + let iter = crate::value::js_nanbox_get_pointer(iter_h.get_nanbox_f64()) + as *mut crate::object::ObjectHeader; + let result = dispatch_array_iterator_method(iter, "next"); + let result = crate::value::js_nanbox_get_pointer(result) + as *const crate::object::ObjectHeader; + ( + f64::from_bits(crate::object::js_object_get_field(result, 0).bits()), + crate::object::js_object_get_field(result, 1).bits() == crate::value::TAG_TRUE, + ) + }; + + assert_eq!(next(), (7.0, false)); + js_array_set_f64(array, 1, 9.0); + assert_eq!(next(), (9.0, false), "indexed Get must stay live"); + js_array_push_f64(array, 10.0); + assert_eq!(next(), (10.0, false), "length Get must stay live"); + assert!(next().1); +} + fn array_keys_contain(keys: *mut ArrayHeader, name: &[u8]) -> bool { let key = string_key(name); for i in 0..js_array_length(keys) { diff --git a/crates/perry-runtime/src/native_abi.rs b/crates/perry-runtime/src/native_abi.rs index 7141671220..e57b2b6b2c 100644 --- a/crates/perry-runtime/src/native_abi.rs +++ b/crates/perry-runtime/src/native_abi.rs @@ -255,6 +255,17 @@ static KEEP_JS_NATIVE_CALL_METHOD_BY_ID: unsafe extern "C-unwind" fn( #[used] static KEEP_JS_NATIVE_CALL_METHOD_APPLY_BY_ID: unsafe extern "C-unwind" fn(f64, i64, i64) -> f64 = crate::object::js_native_call_method_apply_by_id; +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_SPREAD_TAIL_FALLBACK_ARGS: unsafe extern "C-unwind" fn( + *const f64, + usize, + f64, +) -> i64 = crate::object::js_spread_tail_fallback_args; +#[cfg(feature = "keepalive-anchors")] +#[used] +static KEEP_JS_SHORT_PACKED_SPREAD_VALUES: unsafe extern "C" fn(f64, *mut f64) -> i32 = + crate::array::js_short_packed_spread_values; /// Validate and lower a manifest `f32` parameter. #[no_mangle] diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index bc6303a099..5989e5c956 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -536,6 +536,65 @@ pub unsafe extern "C-unwind" fn js_native_call_method_apply_by_id( ) } +/// Materialize `fixed..., ...spread` for the generic branch of a short packed +/// spread callsite. The fast branch has already evaluated all operands; doing +/// the fallback assembly here preserves that source order without re-running +/// an expression, and [`js_array_clone_for_spread`] preserves every observable +/// iterator case (own/prototype overrides, proxies, accessors, and throws). +/// +/// The returned array is consumed immediately by +/// [`js_native_call_method_apply_by_id`]. Every input and both arrays are held +/// in mutable runtime handles because iterator materialization and array pushes +/// can evacuate the nursery. +#[no_mangle] +pub unsafe extern "C-unwind" fn js_spread_tail_fallback_args( + fixed_ptr: *const f64, + fixed_len: usize, + spread: f64, +) -> i64 { + let fixed = if fixed_ptr.is_null() || fixed_len == 0 { + &[][..] + } else { + std::slice::from_raw_parts(fixed_ptr, fixed_len) + }; + let scope = crate::gc::RuntimeHandleScope::new(); + let fixed_handles = scope.root_nanbox_f64_slice(fixed); + let spread_handle = scope.root_nanbox_f64(spread); + + let spread_array = crate::array::js_array_clone_for_spread(spread_handle.get_nanbox_f64()); + let spread_array_handle = scope.root_raw_mut_ptr(spread_array); + let spread_len = spread_array_handle.with_const_ptr(|arr: *const crate::array::ArrayHeader| { + if arr.is_null() { + 0 + } else { + crate::array::js_array_length(arr) as usize + } + }); + let capacity = fixed_len.saturating_add(spread_len).min(u32::MAX as usize) as u32; + let result_handle = scope.root_raw_mut_ptr(crate::array::js_array_alloc(capacity)); + + for value in &fixed_handles { + let next = crate::array::js_array_push_f64( + result_handle.get_raw_mut_ptr(), + value.get_nanbox_f64(), + ); + result_handle.set_raw_mut_ptr(next); + } + for index in 0..spread_len { + let value = spread_array_handle + .with_const_ptr(|arr| crate::array::js_array_get_f64(arr, index as u32)); + // The push can collect while `value` is otherwise only a Rust local. + let value_scope = crate::gc::RuntimeHandleScope::new(); + let value_handle = value_scope.root_nanbox_f64(value); + let next = crate::array::js_array_push_f64( + result_handle.get_raw_mut_ptr(), + value_handle.get_nanbox_f64(), + ); + result_handle.set_raw_mut_ptr(next); + } + result_handle.get_raw_mut_ptr::() as i64 +} + /// The numeric property key of an `obj[key](...)` call, as the raw `f64` index /// `js_object_get_index_polymorphic` consumes, or `None` when `key` is not a /// number. Both representations a numeric key can arrive in are accepted: a diff --git a/crates/perry-runtime/src/symbol/iterator.rs b/crates/perry-runtime/src/symbol/iterator.rs index 6d73692e0b..9a22c757ed 100644 --- a/crates/perry-runtime/src/symbol/iterator.rs +++ b/crates/perry-runtime/src/symbol/iterator.rs @@ -3,7 +3,7 @@ //! `ToPrimitive` (`[Symbol.toPrimitive]`) dispatch. use super::*; -use crate::string::{js_string_from_bytes, StringHeader}; +use crate::string::{StringHeader, js_string_from_bytes}; /// `Object.getOwnPropertySymbols(obj)` — returns an array of symbol keys on /// the object. Looks up the side table populated by @@ -37,11 +37,7 @@ pub unsafe extern "C" fn js_object_get_own_property_symbols(obj_f64: f64) -> i64 } keys.sort_by_key(|sym_key| { let ptr = *sym_key as *const SymbolHeader; - if ptr.is_null() { - u64::MAX - } else { - (*ptr).id - } + if ptr.is_null() { u64::MAX } else { (*ptr).id } }); keys }; @@ -107,11 +103,7 @@ pub unsafe extern "C" fn js_object_get_own_property_symbols(obj_f64: f64) -> i64 } entries[data_len..].sort_by_key(|(sym_ptr_usize, _)| { let ptr = *sym_ptr_usize as *const SymbolHeader; - if ptr.is_null() { - u64::MAX - } else { - (*ptr).id - } + if ptr.is_null() { u64::MAX } else { (*ptr).id } }); let mut arr = crate::array::js_array_alloc(entries.len() as u32); for (sym_ptr_usize, _val_bits) in entries.iter() { @@ -204,6 +196,12 @@ pub extern "C" fn js_iterator_result_validate(result: f64) -> f64 { /// array-memcpy / index-loop arms) so they don't reach this helper. #[no_mangle] pub extern "C" fn js_get_iterator(val_f64: f64) -> f64 { + // Array proxies satisfy IsArray but are small registry ids rather than + // dense `ArrayHeader` pointers. They must reach the ordinary symbol lookup + // below (so a get trap / custom @@iterator remains observable); the + // forwarded builtin iterator now uses KIND_PROXY_VALUES for live trapped + // length/index reads. + let is_proxy = crate::proxy::js_proxy_is_proxy(val_f64) != 0; // `class X extends Array` — the instance is object-backed (a plain // `ObjectHeader` with indexed fields + `length`), but `array_values_iter` // reads a dense `ArrayHeader`. `js_array_is_array` now reports true for such @@ -217,7 +215,9 @@ pub extern "C" fn js_get_iterator(val_f64: f64) -> f64 { let snapshot = crate::array::array_subclass_dense_snapshot(val_f64); return crate::array::array_values_iter(snapshot); } - } else if crate::array::js_array_is_array(val_f64).to_bits() == crate::value::TAG_TRUE { + } else if !is_proxy + && crate::array::js_array_is_array(val_f64).to_bits() == crate::value::TAG_TRUE + { if !crate::array::array_proto_iterator_modified() { return crate::array::array_values_iter(val_f64); } diff --git a/crates/perry/tests/issue_8772_short_packed_spread.rs b/crates/perry/tests/issue_8772_short_packed_spread.rs new file mode 100644 index 0000000000..6410127f10 --- /dev/null +++ b/crates/perry/tests/issue_8772_short_packed_spread.rs @@ -0,0 +1,281 @@ +//! End-to-end coverage for #8772: guarded direct method calls with a final +//! short packed-array spread tail. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::Once; + +const GC_ENV_OVERRIDES: &[&str] = &[ + "PERRY_GEN_GC", + "PERRY_GC_SCAVENGE", + "PERRY_GC_SCAVENGE_NURSERY_MB", + "PERRY_GC_MOVING_SAFEPOINT", + "PERRY_GC_MOVING_LOOP_POLLS", + "PERRY_GC_FORCE_EVACUATE", + "PERRY_GC_VERIFY_EVACUATION", + "PERRY_CONSERVATIVE_STACK_SCAN", + "PERRY_WRITE_BARRIERS", + "PERRY_GC_INCREMENTAL", + "PERRY_GC_HEAP_LIMIT", +]; + +fn remove_gc_env_overrides(command: &mut Command) { + for key in GC_ENV_OVERRIDES { + command.env_remove(key); + } +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +fn fixture_dir() -> PathBuf { + workspace_root().join("test-files/fixtures/issue_8772_short_packed_spread") +} + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn target_debug_dir() -> PathBuf { + let target = std::env::var_os("CARGO_TARGET_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| workspace_root().join("target")); + target.join("debug") +} + +fn ensure_runtime_archive() { + static BUILD_RUNTIME: Once = Once::new(); + BUILD_RUNTIME.call_once(|| { + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let mut command = Command::new(cargo); + command + .current_dir(workspace_root()) + .arg("build") + .arg("-p") + .arg("perry-runtime-static") + .arg("-p") + .arg("perry-stdlib-static"); + let output = command.output().expect("build static runtime archives"); + assert_success("static runtime build", &output); + }); +} + +fn assert_success(label: &str, output: &Output) { + assert!( + output.status.success(), + "{label} failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn copy_fixture(dir: &Path, file: &str) { + std::fs::copy(fixture_dir().join(file), dir.join(file)) + .unwrap_or_else(|error| panic!("copy {file}: {error}")); +} + +fn compile(dir: &Path, entry: &str) -> PathBuf { + ensure_runtime_archive(); + let output = dir.join(format!("{entry}.bin")); + let mut command = Command::new(perry_bin()); + command + .current_dir(dir) + .arg("compile") + .arg(entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .arg("--trace") + .arg("llvm") + .arg("--opt-report=json") + .arg("--explain-lowering") + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_RUNTIME_DIR", target_debug_dir()); + remove_gc_env_overrides(&mut command); + let compiled = command.output().expect("compile fixture"); + assert_success("perry compile", &compiled); + output +} + +fn run(binary: &Path, dir: &Path, moving_gc: bool) -> String { + let mut command = Command::new(binary); + command.current_dir(dir); + remove_gc_env_overrides(&mut command); + if moving_gc { + command + .env("PERRY_GC_SCAVENGE", "1") + .env("PERRY_GC_SCAVENGE_NURSERY_MB", "1") + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1") + .env("PERRY_GC_INCREMENTAL", "0"); + } + let output = command.output().expect("run compiled fixture"); + assert_success("compiled fixture", &output); + String::from_utf8(output.stdout).expect("fixture stdout is UTF-8") +} + +fn run_node(dir: &Path, entry: &str) -> String { + let output = run_node_output(dir, entry); + assert_success("Node oracle", &output); + String::from_utf8(output.stdout).expect("Node stdout is UTF-8") +} + +fn run_node_output(dir: &Path, entry: &str) -> Output { + Command::new("node") + .current_dir(dir) + .arg("--experimental-strip-types") + .arg(entry) + .output() + .expect("run Node oracle") +} + +fn run_failure(binary: &Path, dir: &Path, moving_gc: bool) -> Output { + let mut command = Command::new(binary); + command.current_dir(dir); + remove_gc_env_overrides(&mut command); + if moving_gc { + command + .env("PERRY_GC_SCAVENGE", "1") + .env("PERRY_GC_SCAVENGE_NURSERY_MB", "1") + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1") + .env("PERRY_GC_INCREMENTAL", "0"); + } + let output = command.output().expect("run failing compiled fixture"); + assert!( + !output.status.success(), + "compiled fixture unexpectedly passed" + ); + output +} + +fn read_lowering_artifacts(dir: &Path) -> String { + let lowering = dir.join(".perry-trace/lowering"); + let run_dir = std::fs::read_dir(&lowering) + .expect("read lowering directory") + .filter_map(Result::ok) + .map(|entry| entry.path()) + .find(|path| path.is_dir()) + .expect("lowering run directory"); + std::fs::read_dir(run_dir) + .expect("read lowering run") + .filter_map(Result::ok) + .filter(|entry| { + entry.file_name().to_str().is_some_and(|name| { + name.starts_with("perry_native_reps_") && name.ends_with(".json") + }) + }) + .map(|entry| std::fs::read_to_string(entry.path()).expect("read lowering artifact")) + .collect::() +} + +fn function_ir<'a>(ir: &'a str, fragment: &str) -> &'a str { + let name = ir + .find(fragment) + .unwrap_or_else(|| panic!("missing function {fragment:?}")); + let start = ir[..name] + .rfind("\ndefine ") + .unwrap_or_else(|| panic!("missing definition for {fragment:?}")); + let tail = &ir[start + 1..]; + let end = tail.find("\n}\n").expect("terminated function definition"); + &tail[..end + 2] +} + +fn named_blocks(function: &str, prefixes: &[&str]) -> String { + let mut selected = false; + let mut result = String::new(); + for line in function.lines() { + if !line.starts_with([' ', '\t']) && line.contains(':') { + selected = prefixes.iter().any(|prefix| line.contains(prefix)); + } + if selected { + result.push_str(line); + result.push('\n'); + } + } + result +} + +#[test] +fn repro_has_direct_empty_and_one_arms_and_matches_node_under_moving_gc() { + let temp = tempfile::tempdir().expect("tempdir"); + copy_fixture(temp.path(), "main.ts"); + let binary = compile(temp.path(), "main.ts"); + + let node = run_node(temp.path(), "main.ts"); + assert_eq!(node, "90000900000\n"); + assert_eq!(run(&binary, temp.path(), false), node); + assert_eq!(run(&binary, temp.path(), true), node); + + let ir = std::fs::read_to_string(temp.path().join(".perry-trace/llvm/main_ts.ll")) + .expect("read main LLVM IR"); + let invoke = function_ir(&ir, "__invoke("); + assert!(invoke.contains("call i32 @js_short_packed_spread_values(")); + assert!(invoke.contains("call i32 @js_method_direct_shape_class(")); + let direct = named_blocks( + invoke, + &[ + "short_spread.target0.arity0", + "short_spread.target0.arity1", + "short_spread.target1.arity0", + "short_spread.target1.arity1", + ], + ); + assert!(direct.contains("call double @perry_method_") && direct.contains("__reset(")); + assert!( + !direct.contains("js_native_call_method_apply") + && !direct.contains("js_spread_tail_fallback_args") + ); + let fallback = named_blocks(invoke, &["short_spread.fallback"]); + assert!(fallback.contains("js_spread_tail_fallback_args")); + assert!(fallback.contains("js_native_call_method_apply_by_id")); + + let artifacts = read_lowering_artifacts(temp.path()); + for required in [ + "packed_spread_arities=0,1,2,3,4", + "method=reset", + "spread_guard=exact_ordinary_packed_array,no_holes,max_length_4", + "method_identity_guard=js_method_direct_shape_class(class_id,shape_id,invalidation_slot)", + "generic_fallback=js_spread_tail_fallback_args+js_native_call_method_apply_by_id", + ] { + assert!( + artifacts.contains(required), + "explain-lowering artifact must contain {required:?}\n{artifacts}" + ); + } +} + +#[test] +fn every_exotic_spread_and_dispatch_case_matches_node_under_moving_gc() { + let temp = tempfile::tempdir().expect("tempdir"); + copy_fixture(temp.path(), "semantics.ts"); + let binary = compile(temp.path(), "semantics.ts"); + let node = run_node(temp.path(), "semantics.ts"); + assert_eq!(run(&binary, temp.path(), false), node); + assert_eq!(run(&binary, temp.path(), true), node); +} + +#[test] +fn throwing_iterator_matches_node_under_moving_gc() { + let temp = tempfile::tempdir().expect("tempdir"); + copy_fixture(temp.path(), "throwing.ts"); + let binary = compile(temp.path(), "throwing.ts"); + + let node = run_node_output(temp.path(), "throwing.ts"); + assert!(!node.status.success(), "Node fixture unexpectedly passed"); + assert!(String::from_utf8_lossy(&node.stderr).contains("iterator-boom")); + + for moving_gc in [false, true] { + let perry = run_failure(&binary, temp.path(), moving_gc); + let stderr = String::from_utf8_lossy(&perry.stderr); + assert!( + stderr.contains("iterator-boom"), + "Perry error must preserve iterator throw, got:\n{stderr}" + ); + } +} diff --git a/test-files/fixtures/issue_8772_short_packed_spread/main.ts b/test-files/fixtures/issue_8772_short_packed_spread/main.ts new file mode 100644 index 0000000000..df05760fea --- /dev/null +++ b/test-files/fixtures/issue_8772_short_packed_spread/main.ts @@ -0,0 +1,33 @@ +class Position { + x: number; + constructor() { this.x = 0; } + reset(entity: { id: number }, delta: number = 1): void { + this.x = entity.id + delta; + } +} + +class Velocity { + dx: number; + constructor() { this.dx = 0; } + reset(entity: { id: number }, delta: number = 2): void { + this.dx = entity.id + delta; + } +} + +const position = new Position(); +const velocity = new Velocity(); +const empty: number[] = []; +const one: number[] = [3]; + +function invoke(instance: any, entity: { id: number }, args: number[]): void { + instance.reset(entity, ...args); +} + +let checksum = 0; +for (let i = 0; i < 300000; i++) { + const entity = { id: i }; + invoke(position, entity, empty); + invoke(velocity, entity, one); + checksum += position.x + velocity.dx; +} +console.log(checksum); diff --git a/test-files/fixtures/issue_8772_short_packed_spread/semantics.ts b/test-files/fixtures/issue_8772_short_packed_spread/semantics.ts new file mode 100644 index 0000000000..63af0fd0a9 --- /dev/null +++ b/test-files/fixtures/issue_8772_short_packed_spread/semantics.ts @@ -0,0 +1,77 @@ +class Receiver { + run(prefix: string, value: any = "default"): string { + return prefix + "=" + String(value); + } +} + +class WideReceiver { + run(prefix: string, a: any, b: any, c: any, d: any, e: any): string { + return prefix + "=" + [a, b, c, d, e].join(","); + } +} + +class Tail extends Array {} + +function invoke(instance: any, prefix: string, args: any[]): string { + return instance.run(prefix, ...args); +} + +function invokeWide(instance: any, prefix: string, args: any[]): string { + return instance.run(prefix, ...args); +} + +const result: Record = {}; +const receiver = new Receiver(); + +result.empty = invoke(receiver, "empty", []); +result.one = invoke(receiver, "one", [1]); + +const hole = new Array(1); +result.hole = invoke(receiver, "hole", hole); + +const accessor: any[] = ["first", "discarded"]; +Object.defineProperty(accessor, "0", { + configurable: true, + get: function() { + accessor.length = 1; + return "getter"; + } +}); +result.accessorMutation = invoke(receiver, "accessor", accessor); + +const own: any[] = ["own-iterator"]; +own[Symbol.iterator] = own.values; +result.ownIterator = invoke(receiver, "own", own); + +const proxied: any = new Proxy(["proxy"], {}); +result.proxy = invoke(receiver, "proxy", proxied); + +const subclass = new Tail(); +subclass.push("subclass"); +result.subclass = invoke(receiver, "subclass", subclass); + +const mutated = ["old", "discarded"]; +mutated[0] = "new"; +mutated.length = 1; +result.elementAndLengthMutation = invoke(receiver, "mutated", mutated); + +const replaced: any = new Receiver(); +replaced.run = function(prefix: string, value: any): string { + return "replacement=" + prefix + ":" + value; +}; +result.replacedMethod = invoke(replaced, "method", [9]); + +const plain: any = { + run: function(prefix: string, value: any): string { + return "plain=" + prefix + ":" + value; + } +}; +result.wrongReceiver = invoke(plain, "receiver", [8]); + +result.oversized = invokeWide( + new WideReceiver(), + "wide", + [1, 2, 3, 4, 5] +); + +console.log(JSON.stringify(result)); diff --git a/test-files/fixtures/issue_8772_short_packed_spread/throwing.ts b/test-files/fixtures/issue_8772_short_packed_spread/throwing.ts new file mode 100644 index 0000000000..063c80af42 --- /dev/null +++ b/test-files/fixtures/issue_8772_short_packed_spread/throwing.ts @@ -0,0 +1,16 @@ +class Receiver { + run(value: any): string { + return String(value); + } +} + +function invoke(instance: any, args: any[]): string { + return instance.run(...args); +} + +const throwing: any = {}; +throwing[Symbol.iterator] = function(): any { + throw new Error("iterator-boom"); +}; + +invoke(new Receiver(), throwing); From 2de72a4ca60fbdae7eaf159d0c1fd12e4806c729 Mon Sep 17 00:00:00 2001 From: Ralph Kuepper Date: Tue, 25 Aug 2026 07:52:14 +0200 Subject: [PATCH 2/2] fix: address short packed spread review feedback --- ...-spread.md => 8788-short-packed-spread.md} | 0 crates/perry-codegen/src/expr/call_spread.rs | 2 +- .../src/expr/call_spread_short_tests.rs | 4 +- crates/perry-codegen/src/expr/mod.rs | 20 +++--- crates/perry-runtime/src/array/flat_clone.rs | 2 + crates/perry-runtime/src/array/iter_object.rs | 30 +++++---- crates/perry-runtime/src/array/mod.rs | 62 +++++++++---------- crates/perry-runtime/src/array/tests.rs | 6 +- crates/perry-runtime/src/native_abi.rs | 2 +- .../src/object/native_call_method.rs | 19 +++--- .../tests/issue_8772_short_packed_spread.rs | 1 + 11 files changed, 78 insertions(+), 70 deletions(-) rename changelog.d/{8772-short-packed-spread.md => 8788-short-packed-spread.md} (100%) diff --git a/changelog.d/8772-short-packed-spread.md b/changelog.d/8788-short-packed-spread.md similarity index 100% rename from changelog.d/8772-short-packed-spread.md rename to changelog.d/8788-short-packed-spread.md diff --git a/crates/perry-codegen/src/expr/call_spread.rs b/crates/perry-codegen/src/expr/call_spread.rs index 83ae3ac027..37c4ccfe3b 100644 --- a/crates/perry-codegen/src/expr/call_spread.rs +++ b/crates/perry-codegen/src/expr/call_spread.rs @@ -45,7 +45,7 @@ use crate::rooting::{self, Arg, Repr}; use crate::type_analysis::receiver_class_name; use crate::types::{DOUBLE, I32, I64}; -use super::{downgrade_buffer_aliases_in_expr, lower_expr, nanbox_pointer_inline, FnCtx}; +use super::{FnCtx, downgrade_buffer_aliases_in_expr, lower_expr, nanbox_pointer_inline}; /// The expression a call argument carries, whatever its spread-ness. fn call_arg_expr(a: &CallArg) -> &Expr { diff --git a/crates/perry-codegen/src/expr/call_spread_short_tests.rs b/crates/perry-codegen/src/expr/call_spread_short_tests.rs index 699c03f601..03122baba1 100644 --- a/crates/perry-codegen/src/expr/call_spread_short_tests.rs +++ b/crates/perry-codegen/src/expr/call_spread_short_tests.rs @@ -140,14 +140,14 @@ fn named_block<'a>(function: &'a str, label: &str) -> &'a str { } #[test] -fn empty_and_one_element_arms_call_reset_directly_without_apply() { +fn every_short_packed_arity_calls_reset_directly_without_apply() { let ir = emit(); let invoke = function_body(&ir, "__invoke("); assert!(invoke.contains("call i32 @js_short_packed_spread_values(")); assert!(invoke.contains("call i32 @js_method_direct_shape_class(")); for candidate in 0..2 { - for arity in 0..=1 { + for arity in 0..=4 { let block = named_block( invoke, &format!("short_spread.target{candidate}.arity{arity}"), diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index 8f06398e1d..9834293e85 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -24,7 +24,7 @@ use crate::native_value::{ }; use crate::strings::StringPool; use crate::type_analysis::{is_bigint_expr, is_bool_expr, is_numeric_expr}; -use crate::types::{DOUBLE, F32, I1, I16, I32, I64, I8, PTR}; +use crate::types::{DOUBLE, F32, I1, I8, I16, I32, I64, PTR}; // Issue #1098: expr.rs split into expr/ submodules. These are pure // mechanical moves of self-contained helper clusters out of this file; @@ -60,10 +60,10 @@ mod write_barrier; pub(crate) use crate::native_value::{materialize_js_value, materialize_js_value_without_record}; pub(crate) use array_literal::lower_array_literal; pub(crate) use buffer_access::{ - access_facts_for_spec, can_lower_buffer_access_without_calls, + BufferAccessSpec, access_facts_for_spec, can_lower_buffer_access_without_calls, can_lower_integer_typed_array_store_value, emit_buffer_access_pointer, lower_buffer_access_proof, lower_buffer_load, lower_buffer_store, lower_typed_array_load, - lower_typed_array_store, BufferAccessSpec, + lower_typed_array_store, }; pub(crate) use buffer_views::{ alias_buffer_view_slot, attach_buffer_view_facts, attach_buffer_view_pointer_state_for_expr, @@ -108,16 +108,15 @@ pub(crate) use proven_view_access::{ try_lower_proven_view_checked_f64_load, try_lower_proven_view_checked_store, }; pub(crate) use range_facts::{ - bounds_for_buffer_access_width, effective_alias_state_for_access, + IntRange, IntRangeFact, bounds_for_buffer_access_width, effective_alias_state_for_access, guarded_buffer_indices_for_condition, int_range_expr, invalidate_local_write_facts, local_value_alias_root, record_int_facts_for_let, record_int_facts_for_local_set, record_int_facts_for_update, record_local_value_alias_for_write, while_condition_range_fact, - IntRange, IntRangeFact, }; pub(crate) use strings::emit_string_literal_global; pub(crate) use typed_feedback::{ - emit_typed_feedback_record_call, emit_typed_feedback_register_site, native_region_slug, - typed_feedback_emission_enabled, TypedFeedbackContract, TypedFeedbackKind, + TypedFeedbackContract, TypedFeedbackKind, emit_typed_feedback_record_call, + emit_typed_feedback_register_site, native_region_slug, typed_feedback_emission_enabled, }; pub(crate) use url_helpers::lower_url_string_getter; pub(crate) use v8_interop::{ @@ -175,18 +174,19 @@ mod slot_rep; // #7128: the env-knob table and the pure `gates -> context flags` derivation. // Every `FnCtx` construction site goes through `RepselContextFlags` so that a // knob cannot silently acquire a second representation's sites again. -pub(crate) use repsel_gates::{static_string_lowering_enabled, RepselContextFlags}; +pub(crate) use repsel_gates::{RepselContextFlags, static_string_lowering_enabled}; // `body_context_denial` / `report_context_denial` / `MODULE_INIT_CONTEXT` are // deliberately NOT re-exported: since #7128 the only legitimate consumer is // `repsel_gates::RepselContextFlags::derive`, and a `FnCtx` construction site // that reaches for the structural rule directly is exactly how the two gates // drifted back into one bool the last two times. pub(crate) use slot_rep::{ - canonical_i32_locals_enabled, canonical_local_i32_slot, canonical_str_locals_enabled, + CanonicalI32Denial, PTR_SHAPE_SCALAR_REPLACED, SlotRep, canonical_i32_locals_enabled, + canonical_local_i32_slot, canonical_str_locals_enabled, collect_canonical_str_ineligible_locals, collect_closure_referenced_locals, deny_canonical_context, deny_canonical_i32, load_canonical_local_boxed, local_is_canonical_str, local_rep_is_canonical_i32, note_canonical_local, ptr_shape_context_rule_text, - store_canonical_local_from_double, CanonicalI32Denial, SlotRep, PTR_SHAPE_SCALAR_REPLACED, + store_canonical_local_from_double, }; pub(crate) use dispatch::{lower_expr, lower_math_operand}; diff --git a/crates/perry-runtime/src/array/flat_clone.rs b/crates/perry-runtime/src/array/flat_clone.rs index a07d953272..f0def80c51 100644 --- a/crates/perry-runtime/src/array/flat_clone.rs +++ b/crates/perry-runtime/src/array/flat_clone.rs @@ -133,6 +133,8 @@ pub unsafe extern "C" fn js_short_packed_spread_values(value: f64, out: *mut f64 if bits == crate::value::TAG_HOLE { return -1; } + // GC_STORE_AUDIT(STACK): caller-owned generated stack storage; the + // rooted spread operand keeps copied heap values live during the guard. std::ptr::write(out.add(index), f64::from_bits(bits)); } len as i32 diff --git a/crates/perry-runtime/src/array/iter_object.rs b/crates/perry-runtime/src/array/iter_object.rs index aabd8f796e..1a441029c8 100644 --- a/crates/perry-runtime/src/array/iter_object.rs +++ b/crates/perry-runtime/src/array/iter_object.rs @@ -604,14 +604,15 @@ pub extern "C" fn js_array_entries_iter_obj(arr: *const ArrayHeader) -> i64 { use crate::iter_result::{make_iter_result, make_sqlite_iter_result}; unsafe fn make_pair_array(idx: u32, value: f64) -> f64 { - let pair = crate::array::js_array_alloc(2); - (*pair).length = 2; - let elems = (pair as *mut u8).add(std::mem::size_of::()) as *mut f64; - *elems.add(0) = idx as f64; - *elems.add(1) = value; - crate::array::note_array_slot(pair, 0, (idx as f64).to_bits()); - crate::array::note_array_slot(pair, 1, value.to_bits()); - js_nanbox_pointer(pair as i64) + let scope = crate::gc::RuntimeHandleScope::new(); + let value_h = scope.root_nanbox_f64(value); + let pair_h = scope.root_raw_mut_ptr(crate::array::js_array_alloc(2)); + pair_h.with_mut_ptr(|pair: *mut ArrayHeader| { + (*pair).length = 2; + crate::array::store_array_slot(pair, 0, (idx as f64).to_bits()); + crate::array::store_array_slot(pair, 1, value_h.get_nanbox_u64()); + }); + pair_h.with_mut_ptr(|pair: *mut ArrayHeader| js_nanbox_pointer(pair as i64)) } /// Dispatch `.next()` / `[Symbol.iterator]()` on an array iterator object. @@ -719,22 +720,27 @@ pub unsafe fn dispatch_array_iterator_method( } else { crate::array::js_array_get_f64(backing_ptr as *const ArrayHeader, idx) }; + // A Proxy get trap can return a young heap value. Root it before + // either pair or iterator-result construction allocates, then + // reload through the handle at each constructor boundary. + let elem_h = scope.root_nanbox_f64(elem); let value = match kind { KIND_VALUES | KIND_VALUES_NULL_DONE | KIND_ARGUMENTS_VALUES | KIND_PROXY_VALUES => { - JSValue::from_bits(elem.to_bits()) + JSValue::from_bits(elem_h.get_nanbox_u64()) } KIND_KEYS => JSValue::number(idx as f64), KIND_ENTRIES => { - let pair = make_pair_array(idx, elem); + let pair = make_pair_array(idx, elem_h.get_nanbox_f64()); JSValue::from_bits(pair.to_bits()) } _ => JSValue::undefined(), }; + let value_h = scope.root_nanbox_u64(value.bits()); if kind == KIND_VALUES_NULL_DONE { - make_sqlite_iter_result(value, false) + make_sqlite_iter_result(JSValue::from_bits(value_h.get_nanbox_u64()), false) } else { - make_iter_result(value, false) + make_iter_result(JSValue::from_bits(value_h.get_nanbox_u64()), false) } } // Iterators are themselves iterable — `[Symbol.iterator]()` on one diff --git a/crates/perry-runtime/src/array/mod.rs b/crates/perry-runtime/src/array/mod.rs index bbd80beca2..c9eb239377 100644 --- a/crates/perry-runtime/src/array/mod.rs +++ b/crates/perry-runtime/src/array/mod.rs @@ -95,17 +95,17 @@ pub(crate) use self::generic_object::{ object_pop as generic_object_pop, object_shift as generic_object_shift, object_sort, object_splice, }; -pub(crate) use self::header::{ - array_has_arguments_object_flag, mark_array_as_arguments_object, - prune_dead_array_named_property_owners, rebuild_array_numeric_raw_f64_allow_holes, - rebuild_array_numeric_raw_f64_dense_window, rebuild_array_numeric_raw_f64_dense_window_i32, -}; pub use self::header::{ - js_array_clear_numeric_layout, js_array_declare_all_pointer_elements, + ArrayHeader, js_array_clear_numeric_layout, js_array_declare_all_pointer_elements, js_array_is_numeric_f64_layout, js_array_mark_arguments_object, js_array_mark_numeric_f64_layout, js_array_note_numeric_write, js_tagged_template_get_or_init, js_tagged_template_register_raw, js_template_raw, scan_template_raw_roots, - scan_template_raw_roots_mut, ArrayHeader, + scan_template_raw_roots_mut, +}; +pub(crate) use self::header::{ + array_has_arguments_object_flag, mark_array_as_arguments_object, + prune_dead_array_named_property_owners, rebuild_array_numeric_raw_f64_allow_holes, + rebuild_array_numeric_raw_f64_dense_window, rebuild_array_numeric_raw_f64_dense_window_i32, }; #[cfg(test)] pub(crate) use self::header::{ @@ -119,11 +119,11 @@ pub use self::immutable::{ #[cfg(test)] pub(crate) use self::indexing::test_keys_array_slot_fallbacks; pub(crate) use self::indexing::{ - array_has_own_index, array_iteration_is_exotic, array_proto_iterator_modified, - array_prototype_has_index_flag, array_spec_get, array_spec_has_index, - invalidate_array_index_fast_path, keys_array_len_capped_to_capacity, keys_array_slot, - note_array_proto_iterator_write, note_object_prototype_index_write, - object_prototype_has_index_flag, PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED, + PERRY_ARRAY_INDEX_FAST_PATH_INVALIDATED, array_has_own_index, array_iteration_is_exotic, + array_proto_iterator_modified, array_prototype_has_index_flag, array_spec_get, + array_spec_has_index, invalidate_array_index_fast_path, keys_array_len_capped_to_capacity, + keys_array_slot, note_array_proto_iterator_write, note_object_prototype_index_write, + object_prototype_has_index_flag, }; pub use self::indexing::{ js_array_get_element, js_array_get_element_f64, js_array_get_f64, js_array_get_f64_unchecked, @@ -136,15 +136,15 @@ pub use self::indexing::{ pub use self::is_array::js_array_is_array; pub(crate) use self::iter_methods::throw_reduce_of_empty; pub use self::iter_methods::{ - js_array_at, js_array_every, js_array_filter, js_array_find, js_array_findIndex, - js_array_find_last, js_array_find_last_index, js_array_flatMap, js_array_forEach, js_array_map, + js_array_at, js_array_every, js_array_filter, js_array_find, js_array_find_last, + js_array_find_last_index, js_array_findIndex, js_array_flatMap, js_array_forEach, js_array_map, js_array_map_discard, js_array_reduce, js_array_some, js_array_to_locale_string, js_validate_array_callback, js_validate_array_map_callback, }; pub use self::iter_object::{ - arguments_values_iter, array_entries_iter, array_keys_iter, array_values_iter, - array_values_iter_null_done, dispatch_array_iterator_method, js_array_entries_iter_obj, - js_array_keys_iter_obj, js_array_values_iter_obj, ARRAY_ITERATOR_CLASS_ID, + ARRAY_ITERATOR_CLASS_ID, arguments_values_iter, array_entries_iter, array_keys_iter, + array_values_iter, array_values_iter_null_done, dispatch_array_iterator_method, + js_array_entries_iter_obj, js_array_keys_iter_obj, js_array_values_iter_obj, }; pub(crate) use self::iterator::is_builtin_iterator_class_id; pub(crate) use self::iterator::iter_bt_dump; @@ -212,20 +212,20 @@ pub(crate) use self::alloc::array_length_from_property_value_or_throw; pub(crate) use self::alloc::{js_array_from_arraylike, js_array_from_string_codepoints}; pub(crate) use self::flat_clone::{dense_spread_copy, dense_spread_source, flattenable_array_ptr}; pub(crate) use self::header::{ - array_byte_size, array_is_frozen, array_is_sealed_or_no_extend, array_named_property_delete, - array_named_property_delete_by_name, array_named_property_get, - array_named_property_get_by_name, array_named_property_has, array_named_property_names, - array_named_property_set, array_numeric_raw_f64_get, array_numeric_raw_f64_push_inbounds, - array_numeric_raw_f64_set_inbounds, array_object_flags, array_object_flags_from_tag, - array_object_flags_resolved, array_ptr_as_proxy, array_receiver_addr, array_receiver_gc_tag, - buffer_receiver_as_uint8_typed_array, canonicalize_array_numeric_store_value, - canonicalize_array_numeric_store_value_from_flags, clean_arr_ptr, clean_arr_ptr_mut, - clear_array_numeric_layout, clear_array_numeric_layout_ptr, gc_element_slot_range, - mark_array_layout_unknown, mark_array_raw_f64_holes_fresh, normalize_array_receiver, - note_array_slot, note_array_slot_layout_only, rebuild_array_layout, rebuild_array_layout_exact, - refresh_array_numeric_layout, replay_array_growth_write_barriers, set_array_numeric_layout, - store_array_slot, transfer_array_numeric_layout, typed_array_receiver, value_bits_to_number, - NumericArrayLayout, MIN_ARRAY_CAPACITY, + MIN_ARRAY_CAPACITY, NumericArrayLayout, array_byte_size, array_is_frozen, + array_is_sealed_or_no_extend, array_named_property_delete, array_named_property_delete_by_name, + array_named_property_get, array_named_property_get_by_name, array_named_property_has, + array_named_property_names, array_named_property_set, array_numeric_raw_f64_get, + array_numeric_raw_f64_push_inbounds, array_numeric_raw_f64_set_inbounds, array_object_flags, + array_object_flags_from_tag, array_object_flags_resolved, array_ptr_as_proxy, + array_receiver_addr, array_receiver_gc_tag, buffer_receiver_as_uint8_typed_array, + canonicalize_array_numeric_store_value, canonicalize_array_numeric_store_value_from_flags, + clean_arr_ptr, clean_arr_ptr_mut, clear_array_numeric_layout, clear_array_numeric_layout_ptr, + gc_element_slot_range, mark_array_layout_unknown, mark_array_raw_f64_holes_fresh, + normalize_array_receiver, note_array_slot, note_array_slot_layout_only, rebuild_array_layout, + rebuild_array_layout_exact, refresh_array_numeric_layout, replay_array_growth_write_barriers, + set_array_numeric_layout, store_array_slot, transfer_array_numeric_layout, + typed_array_receiver, value_bits_to_number, }; // Sole caller is the regex-engine-gated `regex::exec_array`, so the helper and diff --git a/crates/perry-runtime/src/array/tests.rs b/crates/perry-runtime/src/array/tests.rs index 56409601e2..b9995de502 100644 --- a/crates/perry-runtime/src/array/tests.rs +++ b/crates/perry-runtime/src/array/tests.rs @@ -89,8 +89,8 @@ fn array_proxy_values_iterator_uses_live_trapped_reads() { let iter = crate::value::js_nanbox_get_pointer(iter_h.get_nanbox_f64()) as *mut crate::object::ObjectHeader; let result = dispatch_array_iterator_method(iter, "next"); - let result = crate::value::js_nanbox_get_pointer(result) - as *const crate::object::ObjectHeader; + let result = + crate::value::js_nanbox_get_pointer(result) as *const crate::object::ObjectHeader; ( f64::from_bits(crate::object::js_object_get_field(result, 0).bits()), crate::object::js_object_get_field(result, 1).bits() == crate::value::TAG_TRUE, @@ -1384,7 +1384,7 @@ fn test_array_last_index_of() { assert_eq!(js_array_last_index_of_jsvalue(arr, 2.0, -10.0, 1), -1); // < -length assert_eq!(js_array_last_index_of_jsvalue(arr, 2.0, 100.0, 1), 3); // clamp to len-1 assert_eq!(js_array_last_index_of_jsvalue(arr, 2.0, 0.0, 1), -1); // only index 0 - // Empty array. + // Empty array. let empty = js_array_alloc(1); assert_eq!(js_array_last_index_of_jsvalue(empty, 1.0, 0.0, 0), -1); } diff --git a/crates/perry-runtime/src/native_abi.rs b/crates/perry-runtime/src/native_abi.rs index e57b2b6b2c..3c6eb7a0c8 100644 --- a/crates/perry-runtime/src/native_abi.rs +++ b/crates/perry-runtime/src/native_abi.rs @@ -4,7 +4,7 @@ //! used by the rest of the runtime. Manifest lowering calls them before handing //! raw scalars, pointers, buffer spans, strings, or promises to native code. -use crate::buffer::{is_registered_buffer, resolve_span_data_ptr, BufferHeader}; +use crate::buffer::{BufferHeader, is_registered_buffer, resolve_span_data_ptr}; use crate::object::ObjectHeader; use crate::promise::Promise; use crate::value::{JSValue, POINTER_MASK, TAG_FALSE, TAG_TRUE}; diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 5989e5c956..37bf382edd 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -561,7 +561,8 @@ pub unsafe extern "C-unwind" fn js_spread_tail_fallback_args( let fixed_handles = scope.root_nanbox_f64_slice(fixed); let spread_handle = scope.root_nanbox_f64(spread); - let spread_array = crate::array::js_array_clone_for_spread(spread_handle.get_nanbox_f64()); + let (_, rooted_spread) = spread_handle.across_nanbox(|| ()); + let spread_array = crate::array::js_array_clone_for_spread(rooted_spread); let spread_array_handle = scope.root_raw_mut_ptr(spread_array); let spread_len = spread_array_handle.with_const_ptr(|arr: *const crate::array::ArrayHeader| { if arr.is_null() { @@ -574,10 +575,9 @@ pub unsafe extern "C-unwind" fn js_spread_tail_fallback_args( let result_handle = scope.root_raw_mut_ptr(crate::array::js_array_alloc(capacity)); for value in &fixed_handles { - let next = crate::array::js_array_push_f64( - result_handle.get_raw_mut_ptr(), - value.get_nanbox_f64(), - ); + let (_, rooted_value) = value.across_nanbox(|| ()); + let next = result_handle + .with_mut_ptr(|result| crate::array::js_array_push_f64(result, rooted_value)); result_handle.set_raw_mut_ptr(next); } for index in 0..spread_len { @@ -586,13 +586,12 @@ pub unsafe extern "C-unwind" fn js_spread_tail_fallback_args( // The push can collect while `value` is otherwise only a Rust local. let value_scope = crate::gc::RuntimeHandleScope::new(); let value_handle = value_scope.root_nanbox_f64(value); - let next = crate::array::js_array_push_f64( - result_handle.get_raw_mut_ptr(), - value_handle.get_nanbox_f64(), - ); + let (_, rooted_value) = value_handle.across_nanbox(|| ()); + let next = result_handle + .with_mut_ptr(|result| crate::array::js_array_push_f64(result, rooted_value)); result_handle.set_raw_mut_ptr(next); } - result_handle.get_raw_mut_ptr::() as i64 + result_handle.with_mut_ptr(|result: *mut crate::array::ArrayHeader| result as i64) } /// The numeric property key of an `obj[key](...)` call, as the raw `f64` index diff --git a/crates/perry/tests/issue_8772_short_packed_spread.rs b/crates/perry/tests/issue_8772_short_packed_spread.rs index 6410127f10..f93c1bb279 100644 --- a/crates/perry/tests/issue_8772_short_packed_spread.rs +++ b/crates/perry/tests/issue_8772_short_packed_spread.rs @@ -7,6 +7,7 @@ use std::sync::Once; const GC_ENV_OVERRIDES: &[&str] = &[ "PERRY_GEN_GC", + "PERRY_GEN_GC_EVACUATE", "PERRY_GC_SCAVENGE", "PERRY_GC_SCAVENGE_NURSERY_MB", "PERRY_GC_MOVING_SAFEPOINT",