diff --git a/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs b/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs index 27fbf9ff31..59c561daec 100644 --- a/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs +++ b/crates/perry-codegen/src/codegen/argument_shape_clone_tests.rs @@ -197,9 +197,13 @@ fn guarded_call_routes_to_shadow_rooted_direct_field_clone() { assert!( clone.contains("@js_shadow_slot_bind(") && clone.find("@js_shadow_slot_bind(") - < clone - .find("getelementptr double") - .or_else(|| clone.find("inttoptr i64")), + < [ + clone.find("getelementptr double"), + clone.find("inttoptr i64"), + ] + .into_iter() + .flatten() + .min(), "the tagged parameter slot must be bound before fixed-offset access:\n{clone}" ); assert!( diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index f5ad672265..de5993cd4a 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -108,6 +108,8 @@ pub(super) fn compile_method( debug_assert!(!pshape_arg_clone || pshape_arg_plan.is_some()); debug_assert!(!pshape_arg_clone || !is_index_clone); debug_assert!(!pshape_arg_clone || !ptr_array_cache_clone); + debug_assert!(!pshape_arg_clone || typed_public_trampoline.is_none()); + debug_assert!(!pshape_arg_clone || !force_generic_body); let family_name = if pshape_arg_clone { crate::collectors::pshape_args_method_name(&public_llvm_name) } else if ptr_array_cache_clone { diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index 0f21557b32..2f480ac6b0 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -174,20 +174,28 @@ impl Drop for CompileProgress { } } +#[cfg(test)] +mod argument_shape_clone_tests; pub(crate) mod arguments; mod artifact_context; mod artifacts; mod boxed_locals; +#[cfg(test)] +mod clone_suffix_tests; mod closure; mod closure_collect; mod ctor_arity; #[cfg(test)] +mod declared_string_add_tests; +#[cfg(test)] mod emission_order_tests; mod entry; pub mod entry_outline; mod func_registry; mod function; #[cfg(test)] +mod guarded_undefined_method_tests; +#[cfg(test)] mod hoisted_callback_method_tests; #[cfg(test)] mod index_method_clone_tests; @@ -195,14 +203,6 @@ mod indexed_method_artifacts; mod ordinary_method_artifacts; // `pub(crate)` so `crate::linker` can read the inline-hot-small policy // (`inline_hot_small_enabled` / `inline_hot_small_hint_threshold`). -#[cfg(test)] -mod argument_shape_clone_tests; -#[cfg(test)] -mod clone_suffix_tests; -#[cfg(test)] -mod declared_string_add_tests; -#[cfg(test)] -mod guarded_undefined_method_tests; pub(crate) mod helpers; mod method; mod method_registry; diff --git a/crates/perry-codegen/src/collectors/proven_args.rs b/crates/perry-codegen/src/collectors/proven_args.rs index e45598205e..c89a50e52c 100644 --- a/crates/perry-codegen/src/collectors/proven_args.rs +++ b/crates/perry-codegen/src/collectors/proven_args.rs @@ -144,10 +144,12 @@ pub(super) fn route_preserves_argument_containment( module_dispatch: &ModuleDispatchFacts, candidates: &HashMap, roots: &HashMap, + receiver_root: u32, owner_class: &str, method: &str, param_index: usize, arg: &Expr, + call_args: &[Expr], ) -> bool { let Expr::LocalGet(id) = arg else { return false; @@ -155,6 +157,21 @@ pub(super) fn route_preserves_argument_containment( let Some(root) = roots.get(id) else { return false; }; + // The clone assumes that nothing reachable through `this` or another + // formal can reshape a selected argument between its entry guard and a + // fixed-offset read. Preserve containment only when this tracked object is + // unique across every value supplied to the call. + if *root == receiver_root + || call_args.iter().enumerate().any(|(other_index, other)| { + other_index != param_index + && matches!( + other, + Expr::LocalGet(other_id) if roots.get(other_id) == Some(root) + ) + }) + { + return false; + } let Some(expected) = module_dispatch.argument_shape_class(owner_class, method, param_index) else { return false; diff --git a/crates/perry-codegen/src/collectors/ptr_shape.rs b/crates/perry-codegen/src/collectors/ptr_shape.rs index c3e96ff574..dc482bc1f5 100644 --- a/crates/perry-codegen/src/collectors/ptr_shape.rs +++ b/crates/perry-codegen/src/collectors/ptr_shape.rs @@ -1217,17 +1217,21 @@ impl<'a> UseWalk<'a> { .push(args.as_slice()); } for (param_index, a) in args.iter().enumerate() { - // `o.m(o)` escapes unless the audited exact-class - // argument clone makes this position contained. + // An audited exact-class argument clone may + // preserve this position only when the tracked + // argument is distinct from the receiver and + // every other source argument. if resolvable && super::proven_args::route_preserves_argument_containment( self.module_dispatch, self.candidates, self.roots, + root, class_name, property, param_index, a, + args, ) { continue; diff --git a/crates/perry-codegen/src/expr/mod.rs b/crates/perry-codegen/src/expr/mod.rs index af87f6da97..56bffb0710 100644 --- a/crates/perry-codegen/src/expr/mod.rs +++ b/crates/perry-codegen/src/expr/mod.rs @@ -2099,15 +2099,41 @@ impl<'a> FnCtx<'a> { return None; } match e { - perry_hir::Expr::LocalGet(id) => self - .proven_shape_params - .get(id) - .or_else(|| self.native_facts.shape_proven_ptr_local(*id)), + perry_hir::Expr::LocalGet(id) => self.ptr_shape_local_fact(*id), perry_hir::Expr::This => self.proven_this.as_ref(), _ => None, } } + /// Shared exact-shape lookup for a local, with clone-parameter overlays + /// taking precedence over ordinary native facts. + fn ptr_shape_local_fact(&self, id: u32) -> Option<&crate::collectors::PtrShapeLocal> { + self.proven_shape_params + .get(&id) + .or_else(|| self.native_facts.shape_proven_ptr_local(id)) + } + + /// Caller-side containment proof used to admit a `$pshape_args` route. + /// + /// This deliberately ignores the raw-pointer representation context gate: + /// the caller keeps a tagged value, the route rechecks its live class and + /// shape, and the clone binds its own tagged shadow slot. Only the proof + /// that no external alias can reshape the argument is consumed here. + pub(crate) fn ptr_shape_argument_route_fact( + &self, + e: &perry_hir::Expr, + ) -> Option<&crate::collectors::PtrShapeLocal> { + match e { + // Ordinary native facts are containment proofs. A selected clone + // parameter inherits the same contract from the only routes that + // can call that clone. + perry_hir::Expr::LocalGet(id) => self.ptr_shape_local_fact(*id), + // `proven_this` may come from a runtime receiver guard rather than + // containment, so it cannot justify an argument clone route. + _ => None, + } + } + /// The `Ptr` fact for `e` ignoring the context gate — the proof the /// analysis actually produced, as opposed to the proof codegen is allowed /// to act on. Report-only. @@ -2116,10 +2142,7 @@ impl<'a> FnCtx<'a> { e: &perry_hir::Expr, ) -> Option<&crate::collectors::PtrShapeLocal> { match e { - perry_hir::Expr::LocalGet(id) => self - .proven_shape_params - .get(id) - .or_else(|| self.native_facts.shape_proven_ptr_local(*id)), + perry_hir::Expr::LocalGet(id) => self.ptr_shape_local_fact(*id), perry_hir::Expr::This => self.proven_this.as_ref(), _ => None, } diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index c6276cb405..930746ab05 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -132,6 +132,7 @@ pub(crate) fn emit_inline_direct_method_shape_guard( fn emit_inline_exact_argument_shape_guard( ctx: &mut FnCtx<'_>, value: &str, + non_alias_values: &[String], expected_class_id: u32, expected_shape_id: &str, fast_label: &str, @@ -153,7 +154,12 @@ fn emit_inline_exact_argument_shape_guard( let above_floor = blk.icmp_uge(I64, &handle, &heap_floor); let below_ceiling = blk.icmp_ult(I64, &handle, &heap_ceiling); let in_heap = blk.and(I1, &above_floor, &below_ceiling); - let safe_to_deref = blk.and(I1, &tagged, &in_heap); + let mut safe_to_deref = blk.and(I1, &tagged, &in_heap); + for other in non_alias_values { + let other_bits = blk.bitcast_double_to_i64(other); + let distinct = blk.icmp_ne(I64, &bits, &other_bits); + safe_to_deref = blk.and(I1, &safe_to_deref, &distinct); + } blk.cond_br(&safe_to_deref, &deref_label, fallback_label); } @@ -191,6 +197,7 @@ pub(super) fn emit_pshape_argument_dispatch( direct_fn: &str, generic_fn: &str, direct_arg_slices: &[(crate::types::LlvmType, &str)], + source_args: &[perry_hir::Expr], ) -> Option { let key = (receiver_class_name.to_string(), property.to_string()); let plan = ctx.pshape_arg_methods.get(&key)?.clone(); @@ -198,12 +205,24 @@ pub(super) fn emit_pshape_argument_dispatch( let mut guarded = Vec::with_capacity(plan.args.len()); for arg in &plan.args { - let value = direct_arg_slices.get(arg.param_index + 1)?.1.to_string(); + let direct_index = arg.param_index + 1; + let source_arg = source_args.get(arg.param_index)?; + let caller_fact = ctx.ptr_shape_argument_route_fact(source_arg)?; + if caller_fact.class_name != arg.fact.class_name { + return None; + } + let value = direct_arg_slices.get(direct_index)?.1.to_string(); + let non_alias_values: Vec = direct_arg_slices + .iter() + .enumerate() + .filter(|(index, _)| *index != direct_index) + .map(|(_, (_, other))| (*other).to_string()) + .collect(); let class_id = *ctx.class_ids.get(&arg.fact.class_name)?; let keys_global = ctx.class_keys_globals.get(&arg.fact.class_name)?.clone(); let shape_id = crate::typed_shape::load_class_shape_id(ctx, &arg.fact.class_name, &keys_global); - guarded.push((arg.clone(), value, class_id, shape_id)); + guarded.push((arg.clone(), value, non_alias_values, class_id, shape_id)); } if guarded.is_empty() { return None; @@ -219,7 +238,7 @@ pub(super) fn emit_pshape_argument_dispatch( let fallback_label = ctx.block_label(fallback_idx); let merge_label = ctx.block_label(merge_idx); - for (index, (_, value, class_id, shape_id)) in guarded.iter().enumerate() { + for (index, (_, value, non_alias_values, class_id, shape_id)) in guarded.iter().enumerate() { let pass_label = intermediate_idxs .get(index) .map(|block| ctx.block_label(*block)) @@ -227,6 +246,7 @@ pub(super) fn emit_pshape_argument_dispatch( emit_inline_exact_argument_shape_guard( ctx, value, + non_alias_values, *class_id, shape_id, &pass_label, @@ -263,11 +283,12 @@ pub(super) fn emit_pshape_argument_dispatch( "argument_abi=tagged_js_value_shadow_rooted".to_string(), "guard_failure_fallback=generic_method".to_string(), ]; - for (arg, _, _, _) in &guarded { + for (arg, _, _, _, _) in &guarded { notes.push(format!("argument_index={}", arg.param_index)); notes.push(format!("argument_class={}", arg.fact.class_name)); notes.push("argument_guard=exact_class_and_shape".to_string()); - notes.push("argument_provenance=runtime_guarded_declared_candidate".to_string()); + notes.push("argument_alias_guard=receiver_and_formals_distinct".to_string()); + notes.push("argument_provenance=caller_containment_plus_runtime_guard".to_string()); } ctx.record_lowered_value( "MethodCall", @@ -511,6 +532,7 @@ pub(super) fn emit_guarded_direct_method_call( property: &str, direct_fn: &str, direct_arg_slices: &[(crate::types::LlvmType, &str)], + source_args: &[perry_hir::Expr], fallback_user_args: &[String], nonnegative_index_direct_fn: Option<&str>, typed_direct_fn: Option<(&str, Vec)>, @@ -1210,6 +1232,7 @@ pub(super) fn emit_guarded_direct_method_call( direct_fn, pshape_arg_fallback, direct_arg_slices, + source_args, ) { argument_specialized } else { diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index fdd441aec7..903d5ca189 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs @@ -1469,6 +1469,7 @@ pub(crate) fn try_lower_instance_method_call( &fallback_fn, generic_target, &arg_slices, + args, ) { return Ok(Some(argument_specialized)); } @@ -1551,6 +1552,7 @@ pub(crate) fn try_lower_instance_method_call( property, &fallback_fn, &arg_slices, + args, &fallback_user_args, nonnegative_index_direct_name.as_deref(), typed_direct, diff --git a/crates/perry/tests/issue_8774_argument_shape_clones.rs b/crates/perry/tests/issue_8774_argument_shape_clones.rs index fc5fa382f9..96abd20e4e 100644 --- a/crates/perry/tests/issue_8774_argument_shape_clones.rs +++ b/crates/perry/tests/issue_8774_argument_shape_clones.rs @@ -115,13 +115,13 @@ fn compile(dir: &Path, entry: &str, explain: bool) -> (PathBuf, Output) { // The clone contract under test is the portable tagged shadow slot; // this also avoids Windows' unsupported RS4GC + funclet-EH pairing in // the exception fixture. - .env("PERRY_RS4GC", "0") - // Compile-time half of the precise-root moving-loop-poll route. - .env("PERRY_GC_MOVING_LOOP_POLLS", "1"); + .env("PERRY_RS4GC", "0"); if explain { command.arg("--opt-report=json").arg("--explain-lowering"); } remove_gc_env_overrides(&mut command); + // Compile-time half of the precise-root moving-loop-poll route. Set after + // the override scrub so it survives. command.env("PERRY_GC_MOVING_LOOP_POLLS", "1"); let result = command.output().expect("run perry compile"); assert_success("perry compile", &result); @@ -347,5 +347,11 @@ fn guard_failures_match_node_and_unsafe_parameters_stay_generic() { !ir.contains("ForeignReader__read$pshape_args"), "an imported argument class must stay on the generic route:\n{ir}" ); + let alias_clone = "perry_method_main_ts__AliasReader__read$pshape_args"; + let _alias_clone_body = function_body(&ir, &format!("@{alias_clone}(")); + assert!( + !ir.contains(&format!("call double @{alias_clone}(")), + "a receiver/argument alias must never enter the argument clone:\n{ir}" + ); assert!(ir.contains("pshape_arg.fallback")); } diff --git a/test-files/fixtures/issue_8774_argument_shapes/barrel.ts b/test-files/fixtures/issue_8774_argument_shapes/barrel.ts index 86a527d5b9..969ba60d53 100644 --- a/test-files/fixtures/issue_8774_argument_shapes/barrel.ts +++ b/test-files/fixtures/issue_8774_argument_shapes/barrel.ts @@ -1 +1,7 @@ -export { Foreign, installIdAccessor, makeProxy, reshape } from "./foreign.ts"; +export { + Foreign, + installIdAccessor, + makeProxy, + reshape, + reshapeAliasedId, +} from "./foreign.ts"; diff --git a/test-files/fixtures/issue_8774_argument_shapes/foreign.ts b/test-files/fixtures/issue_8774_argument_shapes/foreign.ts index 4bd3b251f5..e74fbe0f1b 100644 --- a/test-files/fixtures/issue_8774_argument_shapes/foreign.ts +++ b/test-files/fixtures/issue_8774_argument_shapes/foreign.ts @@ -17,6 +17,15 @@ export function reshape(value: any): void { value.id = id + 1; } +// If `value` aliases an argument being read by an exact-shape clone, the +// inserted field changes its offset after the clone's entry guard. +export function reshapeAliasedId(value: any): void { + const id = value.id; + delete value.id; + value.aliasPadding = 40; + value.id = id + 1; +} + export function installIdAccessor(value: any): void { const id = value.id; Object.defineProperty(value, "id", { diff --git a/test-files/fixtures/issue_8774_argument_shapes/main.ts b/test-files/fixtures/issue_8774_argument_shapes/main.ts index d1a99538ee..bd9c5edd9b 100644 --- a/test-files/fixtures/issue_8774_argument_shapes/main.ts +++ b/test-files/fixtures/issue_8774_argument_shapes/main.ts @@ -3,6 +3,7 @@ import { installIdAccessor, makeProxy, reshape, + reshapeAliasedId, } from "./barrel.ts"; class Entity { @@ -59,6 +60,21 @@ class ForeignReader { } } +class AliasReader { + id: number; + components: number[]; + + constructor(id: number) { + this.id = id; + this.components = [1]; + } + + read(other: AliasReader): number { + reshapeAliasedId(this); + return other.id + other.components.length; + } +} + const registry = new Registry(); const results: any[] = []; @@ -93,11 +109,16 @@ results.push(registry.alias(new Entity(9))); results.push(registry.reassign(new Entity(10))); results.push(new ForeignReader().read(new Foreign(11))); -const reshaped = new Entity(12); +// The selected argument aliases `this`; the imported call changes its shape +// before the declared-field read. This call must stay out of `$pshape_args`. +const aliased = new AliasReader(12); +results.push(aliased.read(aliased)); + +const reshaped = new Entity(13); reshape(reshaped); results.push(registry.read(reshaped)); -const descriptor = new Entity(14); +const descriptor = new Entity(15); installIdAccessor(descriptor); results.push(registry.read(descriptor)); @@ -105,7 +126,7 @@ results.push(registry.read(descriptor)); (registry as any).read = function (entity: any): number { return entity.id * 10; }; -results.push(registry.read(new Entity(16))); +results.push(registry.read(new Entity(17))); console.log( JSON.stringify({ results, accessorHits, proxyHits: proxyCounter.hits }),