diff --git a/CHANGELOG.md b/CHANGELOG.md index b59a413aa..eefc0a00e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to elephc, a PHP-to-native compiler written in Rust. Releases are listed newest first. ## [Unreleased] +- Fixed heap corruption when arrays grown inside loops are promoted to mixed-element storage (issue #452). Loop analysis now covers `$a[] =`, `$a[$i] =`, and `array_push($a, …)` sites and uses inferred call and assignment value types before lowering, so every write uses consistent boxed storage without widening homogeneous typed rebuilds such as `MultipleIterator::detachIterator`. This also fixes the silent value corruption and per-conversion leak in the string/float form of the same pattern. - Fixed `match` and ternary expressions with heterogeneous result types (issues #488 and #494): object/array/string/int/float/`null` mixes now merge to a boxed `mixed` result and keep each value-producing branch's runtime type instead of coercing every branch into one scalar-biased unified type (fatal object-to-string casts, silent string/array coercions, or compile errors). Checker inference joins all branches — including assign→inferred-return and abbreviated `?:` shapes — and agrees with EIR lowering; nullable inferred returns preserve `null`, and `gettype()` on nullable-int merge temps no longer segfaults by unboxing an inline tagged scalar as a boxed Mixed cell. Same-representation merges (`int`/`bool`, indexed arrays with different element types) remain a documented PHP divergence. - Fixed a transposed x86_64 heap magic stamped by several throwable emitters (issue #482): objects created through the runtime-raised `ValueError`/`TypeError`/`LogicException`/`Error` paths, JSON throw errors, and some SPL/hash/static-property paths carried a header magic the refcount helpers do not recognize, silently opting them out of reference counting on linux-x86_64. Every x86_64 heap kind-word stamp and magic check now goes through the shared `sentinels` helpers (`x86_64_heap_kind_word` / `X86_64_HEAP_MAGIC_HI32`), and repository lint tests keep both the transposed typo and hand-typed canonical immediates from coming back. - Fixed PHP type-hint resolution for `object` and `Closure`: namespaced `object`/`Object` hints now keep the generic object contract and reject non-object values instead of being rewritten as namespace-local classes, while global `Closure`, fully qualified `\Closure`, and imported `use Closure` hints resolve to callable storage across parameters, returns, and properties. An unimported `Closure` inside a namespace remains namespace-relative, and PHP's ban on `callable` properties still applies to plain, nullable, and union forms while `Closure`, `?Closure`, and `Closure|null` remain valid. diff --git a/src/ir_lower/expr/mod.rs b/src/ir_lower/expr/mod.rs index dc1595040..3a5667091 100644 --- a/src/ir_lower/expr/mod.rs +++ b/src/ir_lower/expr/mod.rs @@ -7142,7 +7142,7 @@ fn expand_static_indexed_spread_args(args: &[Expr]) -> Vec { } /// Returns the best available return type for a function-like call. -fn call_return_type( +pub(super) fn call_return_type( ctx: &LoweringContext<'_, '_>, name: &str, operands: &[crate::ir::ValueId], @@ -8471,7 +8471,7 @@ fn scoped_constant_value_type_for_ir( } /// Returns the element/value type for an array-access expression used inside a literal. -fn array_access_expr_value_type_for_ir( +pub(super) fn array_access_expr_value_type_for_ir( ctx: &LoweringContext<'_, '_>, array: &Expr, ) -> Option { @@ -8499,7 +8499,7 @@ fn array_access_expr_value_type_for_ir( } /// Returns the declared type for an object property expression used inside a literal. -fn property_access_expr_type_for_ir( +pub(super) fn property_access_expr_type_for_ir( ctx: &LoweringContext<'_, '_>, object: &Expr, property: &str, @@ -8520,6 +8520,18 @@ fn property_access_expr_type_for_ir( .map(|(_, ty)| normalize_value_php_type(ty.codegen_repr())) } +/// Returns the declared result type for an instance method call before its receiver is lowered. +pub(super) fn method_call_expr_type_for_ir( + ctx: &LoweringContext<'_, '_>, + object: &Expr, + method: &str, +) -> Option { + let class_name = instance_callable_object_class(ctx, object)?; + let method_key = php_symbol_key(method); + class_method_signature(ctx, &class_name, &method_key) + .map(|signature| normalize_value_php_type(signature.return_type.codegen_repr())) +} + /// Merges associative-array value types for EIR storage metadata. fn merge_ir_assoc_value_type(left: PhpType, right: PhpType) -> PhpType { if left == right { @@ -14467,6 +14479,17 @@ fn static_method_implementation_signature<'a>( .and_then(|class_info| class_info.static_methods.get(&key)) } +/// Returns the declared result type for a static method call before its arguments are lowered. +pub(super) fn static_method_call_expr_type_for_ir( + ctx: &LoweringContext<'_, '_>, + receiver: &StaticReceiver, + method: &str, +) -> Option { + static_method_implementation_signature(ctx, receiver, method) + .or_else(|| lexical_instance_static_call_signature(ctx, receiver, method)) + .map(|signature| normalize_value_php_type(signature.return_type.codegen_repr())) +} + /// Returns the instance-method signature used by `self::method()` or `parent::method()`. fn lexical_instance_static_call_signature<'a>( ctx: &'a LoweringContext<'_, '_>, diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index 5ed43fc4b..d0146cfdf 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -20,12 +20,15 @@ use crate::ir_lower::context::{ }; use crate::ir_lower::effects_lookup; use crate::ir_lower::expr::{ - array_access_element_result_type, coerce_to_int_at_span, lower_callable_array_for_assignment, + array_access_element_result_type, array_access_expr_value_type_for_ir, call_return_type, + coerce_to_int_at_span, lower_callable_array_for_assignment, lower_array_literal_with_expected_type, lower_closure_for_assignment, lower_expr, + method_call_expr_type_for_ir, property_access_expr_type_for_ir, reflection_arg_array_binding_for_expr, reflection_class_binding_for_expr, reflection_function_binding_for_expr, reflection_method_binding_for_expr, reflection_property_binding_for_expr, static_callable_binding_for_expr, - string_op_uses_scratch_storage, type_satisfies_array_access_for_ir, + static_method_call_expr_type_for_ir, string_op_uses_scratch_storage, + type_satisfies_array_access_for_ir, }; use crate::names::{php_symbol_key, property_hook_set_method}; use crate::parser::ast::{ @@ -529,8 +532,125 @@ fn lower_ifdef( ctx.clear_static_callable_locals(); } +/// Widens locals whose indexed-array element type joins to `mixed` across the loop body's +/// push sites (issue #452) and materializes the promotion once before the loop. Loop bodies +/// are lowered in a single pass, so without this an early `$a[] = ` site is emitted +/// as a raw push against the pre-promotion element type even though the back edge brings the +/// promoted `array` around, writing an unboxed scalar into boxed-cell storage on +/// iterations >= 2. Converting the array up front (in place, same pointer) and fixing its +/// type to `array` makes every push site box its value. `overrides` supplies types +/// for names bound by the loop itself (the `foreach` value/key variables) so a push of the +/// loop variable joins with its real element type. Locals without a materialized slot yet +/// (first assigned inside the body, hence fresh every iteration) are left untouched. +fn widen_loop_grown_arrays( + ctx: &mut LoweringContext<'_, '_>, + body: &[Stmt], + update: Option<&Stmt>, + overrides: &[(&str, PhpType)], + span: Option, +) { + let names = { + let lookup = |name: &str| -> Option { + if let Some((_, ty)) = overrides.iter().find(|(n, _)| *n == name) { + return Some(ty.clone()); + } + // A name with no declared slot yet (first assigned inside the loop body) is + // genuinely unknown at loop entry: report it as such instead of the Mixed + // fallback `local_type` would return, so the prescan does not take Mixed as + // widening evidence for it (mirrors the checker's `env.get` lookup). + if !ctx.local_slots.contains_key(name) { + return None; + } + Some(ctx.local_type(name)) + }; + crate::types::checker::loop_grown_mixed_array_pushes( + body, + update, + &lookup, + &mut |expr| infer_loop_growth_value_type(ctx, expr), + ) + }; + for name in names { + if !ctx.local_slots.contains_key(&name) { + continue; + } + let array_value = ctx.load_local(&name, span); + if array_value.ir_type != IrType::Heap(crate::ir::IrHeapKind::Array) { + continue; + } + let mixed_array_ty = PhpType::Array(Box::new(PhpType::Mixed)); + let converted = ctx.emit_value( + Op::ArrayToMixed, + vec![array_value.value], + None, + mixed_array_ty.clone(), + Op::ArrayToMixed.default_effects(), + span, + ); + ctx.store_mutated_local(&name, converted, mixed_array_ty, span); + } +} + +/// Returns the value type that EIR lowering already knows before it emits a loop body. +fn infer_loop_growth_value_type( + ctx: &LoweringContext<'_, '_>, + expr: &Expr, +) -> Option { + match &expr.kind { + ExprKind::Variable(name) if ctx.local_slots.contains_key(name) => { + Some(ctx.local_type(name)) + } + ExprKind::FunctionCall { name, .. } => { + Some(call_return_type(ctx, name.as_str(), &[])) + } + ExprKind::MethodCall { object, method, .. } => { + method_call_expr_type_for_ir(ctx, object, method) + } + ExprKind::StaticMethodCall { + receiver, method, .. + } => static_method_call_expr_type_for_ir(ctx, receiver, method), + ExprKind::PropertyAccess { object, property } => { + property_access_expr_type_for_ir(ctx, object, property) + } + ExprKind::ArrayAccess { array, .. } => { + array_access_expr_value_type_for_ir(ctx, array) + } + ExprKind::This => ctx.current_class.clone().map(PhpType::Object), + ExprKind::NewObject { class_name, .. } => { + Some(PhpType::Object(class_name.as_str().to_string())) + } + ExprKind::ErrorSuppress(inner) => infer_loop_growth_value_type(ctx, inner), + ExprKind::Ternary { + then_expr, + else_expr, + .. + } => { + let then_ty = infer_loop_growth_value_type(ctx, then_expr)?; + let else_ty = infer_loop_growth_value_type(ctx, else_expr)?; + if then_ty == else_ty { + Some(then_ty) + } else { + Some(PhpType::Mixed) + } + } + _ => None, + } +} + +/// Mirrors the checker's `foreach` value binding for the loop-widening prescan, preserving +/// concrete element types from locals, literals, and function-like source expressions. +fn foreach_prescan_value_type(ctx: &LoweringContext<'_, '_>, array: &Expr) -> PhpType { + let source_ty = match &array.kind { + ExprKind::Variable(name) => ctx.local_type(name), + _ => infer_loop_growth_value_type(ctx, array) + .unwrap_or_else(|| crate::types::checker::infer_expr_type_syntactic(array)), + }; + foreach_value_type(&source_ty) +} + /// Lowers a `while` loop. fn lower_while(ctx: &mut LoweringContext<'_, '_>, condition: &Expr, body: &[Stmt]) { + widen_loop_grown_arrays(ctx, body, None, &[], Some(condition.span)); let header = ctx.builder.create_named_block("while.cond", Vec::new()); let body_block = ctx.builder.create_named_block("while.body", Vec::new()); let exit = ctx.builder.create_named_block("while.exit", Vec::new()); @@ -563,6 +683,7 @@ fn lower_while(ctx: &mut LoweringContext<'_, '_>, condition: &Expr, body: &[Stmt /// Lowers a `do while` loop. fn lower_do_while(ctx: &mut LoweringContext<'_, '_>, body: &[Stmt], condition: &Expr) { + widen_loop_grown_arrays(ctx, body, None, &[], Some(condition.span)); let body_block = ctx.builder.create_named_block("do.body", Vec::new()); let cond_block = ctx.builder.create_named_block("do.cond", Vec::new()); let exit = ctx.builder.create_named_block("do.exit", Vec::new()); @@ -607,6 +728,10 @@ fn lower_for( if ctx.builder.insertion_block_is_terminated() { return; } + let widen_span = condition + .map(|c| c.span) + .or_else(|| body.first().map(|s| s.span)); + widen_loop_grown_arrays(ctx, body, update, &[], widen_span); let header = ctx.builder.create_named_block("for.cond", Vec::new()); let body_block = ctx.builder.create_named_block("for.body", Vec::new()); @@ -1219,6 +1344,15 @@ fn lower_foreach( value_by_ref: bool, body: &[Stmt], ) { + // Widen before the source is lowered so an iterated-and-pushed array is loaded with + // its fixed-point element type, and bind the loop variables' prescan types so a push + // of the foreach value joins with its real element type. + let prescan_value_ty = foreach_prescan_value_type(ctx, array); + let mut overrides: Vec<(&str, PhpType)> = vec![(value_var, prescan_value_ty)]; + if let Some(key_var) = key_var { + overrides.push((key_var, PhpType::Mixed)); + } + widen_loop_grown_arrays(ctx, body, None, &overrides, Some(array.span)); let source = lower_expr(ctx, array); let source_php_ty = ctx.builder.value_php_type(source.value); let source_ty = source_php_ty.codegen_repr(); diff --git a/src/types/checker/loop_widening.rs b/src/types/checker/loop_widening.rs new file mode 100644 index 000000000..5f61b253e --- /dev/null +++ b/src/types/checker/loop_widening.rs @@ -0,0 +1,483 @@ +//! Purpose: +//! Pre-scans a loop body for local-array growth/write sites whose element types widen an +//! indexed array to `mixed` across the loop back-edge (issue #452), so both the type +//! checker and EIR lowering can fix the array's element type to `mixed` *before* +//! processing the body once. +//! +//! Called from: +//! - `crate::types::checker::stmt_check::control_flow` (loop arms widen the `TypeEnv`). +//! - `crate::ir_lower::stmt` (loop lowering widens `local_types` and materializes the +//! promotion before emitting the body). +//! +//! Key details: +//! - Both passes are single-pass over loop bodies; without this scan an early write site +//! is typed/lowered against the pre-promotion element type and writes an unboxed +//! scalar into mixed-element storage on iterations >= 2, corrupting the heap. +//! - Sites covered: `$name[] =`, `$name[$i] =`, and `array_push($name, ...)`. +//! - Only the widening-to-`mixed` transition is reported: it is the one that changes the +//! element representation (raw scalar slots vs boxed cells). Same-type growth and +//! `never -> T` keep their current lowering. +//! - Concrete evidence comes from the caller's semantic expression inference, with a +//! literal/cast fallback for lowering-only contexts. Opaque sources do *not* force +//! `mixed` alone — that would spuriously widen same-typed rebuild loops such as +//! `MultipleIterator::detachIterator`. Opaque evidence alongside at least one concrete +//! sibling type on the same array does force `mixed`, because the back edge can promote +//! storage while the opaque site still emits a raw write. + +use crate::parser::ast::{CastType, Expr, ExprKind, Stmt, StmtKind}; +use crate::types::PhpType; + +/// Evidence a growth/write site contributes to the element-type join. +enum PushEvidence { + /// A self-evident concrete element type. + Known(PhpType), + /// A value whose static type is not safe to treat as concrete join evidence. + Opaque, +} + +/// Source recorded for a local assignment that can feed a later array write. +enum AssignedValue<'a> { + /// An ordinary assignment whose RHS can be inferred by the caller. + Expr(&'a Expr), + /// A binding without an RHS expression in this AST, such as a `foreach` variable. + Opaque, +} + +/// Returns the names of locals that currently hold a non-`mixed` indexed array and whose +/// element type joins to `mixed` across the local-array growth/write sites found in the +/// loop body (and the optional `for` update statement). `lookup` supplies the current type +/// of a local at loop entry; names it does not know are skipped as targets. `infer_value` +/// supplies the caller's best semantic type for pushed values and assignment RHSs. +pub fn loop_grown_mixed_array_pushes( + body: &[Stmt], + update: Option<&Stmt>, + lookup: &dyn Fn(&str) -> Option, + infer_value: &mut dyn FnMut(&Expr) -> Option, +) -> Vec { + let mut pushes: Vec<(&str, &Expr)> = Vec::new(); + collect_array_pushes(body, &mut pushes); + if let Some(stmt) = update { + collect_array_push_stmt(stmt, &mut pushes); + } + let mut assignments: Vec<(&str, AssignedValue<'_>)> = Vec::new(); + collect_value_assignments(body, &mut assignments); + if let Some(stmt) = update { + collect_value_assignment_stmt(stmt, &mut assignments); + } + let mut names: Vec = Vec::new(); + for (name, _) in &pushes { + if names.iter().any(|n| n == name) { + continue; + } + let Some(PhpType::Array(elem)) = lookup(name) else { + continue; + }; + if *elem == PhpType::Mixed { + continue; + } + let mut joined = (*elem).clone(); + let mut saw_known = false; + let mut saw_opaque = false; + for (_, value) in pushes.iter().filter(|(n, _)| n == name) { + match resolve_pushed_value_evidence(value, lookup, &assignments, infer_value) { + Some(PushEvidence::Known(pushed)) => { + saw_known = true; + joined = join_pushed_element_type(joined, pushed); + } + Some(PushEvidence::Opaque) => { + saw_opaque = true; + } + None => {} + } + } + // Opaque sibling next to concrete evidence: the opaque site may still emit a raw + // write after a concrete site has promoted storage across the back edge. + if saw_opaque && saw_known { + joined = PhpType::Mixed; + } + if joined == PhpType::Mixed { + names.push(name.to_string()); + } + } + names +} + +/// Resolves the evidence a pushed/written value contributes to the element join. +fn resolve_pushed_value_evidence( + value: &Expr, + lookup: &dyn Fn(&str) -> Option, + assignments: &[(&str, AssignedValue<'_>)], + infer_value: &mut dyn FnMut(&Expr) -> Option, +) -> Option { + match &value.kind { + ExprKind::Variable(name) => { + let entry = lookup(name); + let body = assigned_value_evidence(name, assignments, infer_value); + match (entry, body) { + (_, Some(PushEvidence::Opaque)) => Some(PushEvidence::Opaque), + (Some(a), Some(PushEvidence::Known(b))) => { + Some(PushEvidence::Known(join_pushed_element_type(a, b))) + } + (Some(a), None) => Some(PushEvidence::Known(a)), + (None, Some(PushEvidence::Known(b))) => Some(PushEvidence::Known(b)), + (None, None) => None, + } + } + _ => match infer_value(value).or_else(|| precise_scalar_expr_type(value)) { + Some(ty) => Some(PushEvidence::Known(ty)), + None => Some(PushEvidence::Opaque), + }, + } +} + +/// Joins the recorded in-loop assignment types for `name`, preserving opaque assignments +/// as poison so stale loop-entry evidence cannot overrule an unknown reassignment. +fn assigned_value_evidence( + name: &str, + assignments: &[(&str, AssignedValue<'_>)], + infer_value: &mut dyn FnMut(&Expr) -> Option, +) -> Option { + let mut joined: Option = None; + for (candidate, source) in assignments { + if *candidate != name { + continue; + } + let ty = match source { + AssignedValue::Expr(expr) => { + infer_value(expr).or_else(|| precise_scalar_expr_type(expr)) + } + AssignedValue::Opaque => None, + }; + let Some(ty) = ty else { + return Some(PushEvidence::Opaque); + }; + joined = Some(match joined { + Some(acc) => join_pushed_element_type(acc, ty), + None => ty, + }); + } + joined.map(PushEvidence::Known) +} + +/// Returns a self-evident concrete scalar type, or `None` when the expression must be +/// treated as opaque join evidence (the shared syntactic helper defaults too many unknown +/// constructs to `Int` to be safe here). +fn precise_scalar_expr_type(value: &Expr) -> Option { + match &value.kind { + ExprKind::IntLiteral(_) => Some(PhpType::Int), + ExprKind::FloatLiteral(_) => Some(PhpType::Float), + ExprKind::StringLiteral(_) => Some(PhpType::Str), + ExprKind::BoolLiteral(_) => Some(PhpType::Bool), + ExprKind::Null => Some(PhpType::Void), + ExprKind::Cast { target, .. } => match target { + CastType::Int => Some(PhpType::Int), + CastType::Float => Some(PhpType::Float), + CastType::String => Some(PhpType::Str), + CastType::Bool => Some(PhpType::Bool), + _ => None, + }, + ExprKind::Ternary { + then_expr, + else_expr, + .. + } => { + let then_ty = precise_scalar_expr_type(then_expr)?; + let else_ty = precise_scalar_expr_type(else_expr)?; + Some(join_pushed_element_type(then_ty, else_ty)) + } + ExprKind::ErrorSuppress(inner) => precise_scalar_expr_type(inner), + _ => None, + } +} + +/// Collects local assignments from every statement in `stmts`, recursively. +fn collect_value_assignments<'a>( + stmts: &'a [Stmt], + out: &mut Vec<(&'a str, AssignedValue<'a>)>, +) { + for stmt in stmts { + collect_value_assignment_stmt(stmt, out); + } +} + +/// Collects local assignments from one statement, mirroring the nested-statement recursion +/// of `collect_array_push_stmt` for bodies that execute within the loop iteration. +fn collect_value_assignment_stmt<'a>( + stmt: &'a Stmt, + out: &mut Vec<(&'a str, AssignedValue<'a>)>, +) { + match &stmt.kind { + StmtKind::Assign { name, value } | StmtKind::TypedAssign { name, value, .. } => { + out.push((name.as_str(), AssignedValue::Expr(value))); + } + StmtKind::If { + then_body, + elseif_clauses, + else_body, + .. + } => { + collect_value_assignments(then_body, out); + for (_, clause_body) in elseif_clauses { + collect_value_assignments(clause_body, out); + } + if let Some(else_body) = else_body { + collect_value_assignments(else_body, out); + } + } + StmtKind::IfDef { + then_body, + else_body, + .. + } => { + collect_value_assignments(then_body, out); + if let Some(else_body) = else_body { + collect_value_assignments(else_body, out); + } + } + StmtKind::While { body, .. } + | StmtKind::DoWhile { body, .. } + | StmtKind::IncludeOnceGuard { body, .. } => collect_value_assignments(body, out), + StmtKind::Foreach { + key_var, + value_var, + body, + .. + } => { + // The foreach bindings assign non-literal values each iteration: poison them. + if let Some(key_var) = key_var { + out.push((key_var.as_str(), AssignedValue::Opaque)); + } + out.push((value_var.as_str(), AssignedValue::Opaque)); + collect_value_assignments(body, out); + } + StmtKind::For { + init, + update, + body, + .. + } => { + if let Some(init) = init { + collect_value_assignment_stmt(init, out); + } + if let Some(update) = update { + collect_value_assignment_stmt(update, out); + } + collect_value_assignments(body, out); + } + StmtKind::Switch { cases, default, .. } => { + for (_, case_body) in cases { + collect_value_assignments(case_body, out); + } + if let Some(default) = default { + collect_value_assignments(default, out); + } + } + StmtKind::Try { + try_body, + catches, + finally_body, + } => { + collect_value_assignments(try_body, out); + for catch in catches { + collect_value_assignments(&catch.body, out); + } + if let Some(finally_body) = finally_body { + collect_value_assignments(finally_body, out); + } + } + StmtKind::Synthetic(stmts) => collect_value_assignments(stmts, out), + _ => {} + } +} + +/// Joins two indexed-array element types on the widening lattice used by growth sites: +/// equal types stay, `never` adopts the other side, and any other combination widens to +/// `mixed` (the representation-changing transition this scan exists to detect). +fn join_pushed_element_type(a: PhpType, b: PhpType) -> PhpType { + if a == b { + a + } else if a == PhpType::Never { + b + } else if b == PhpType::Never { + a + } else { + PhpType::Mixed + } +} + +/// Collects local-array growth/write sites from every statement in `stmts`, recursively. +fn collect_array_pushes<'a>(stmts: &'a [Stmt], out: &mut Vec<(&'a str, &'a Expr)>) { + for stmt in stmts { + collect_array_push_stmt(stmt, out); + } +} + +/// Collects growth/write sites from one statement, recursing into every nested statement +/// body that executes as part of the enclosing loop iteration. Declaration bodies +/// (functions, classes) do not execute in the loop and closures capture by value by +/// default, so neither is descended into. +fn collect_array_push_stmt<'a>(stmt: &'a Stmt, out: &mut Vec<(&'a str, &'a Expr)>) { + match &stmt.kind { + StmtKind::ArrayPush { array, value } => out.push((array.as_str(), value)), + StmtKind::ArrayAssign { array, value, .. } => out.push((array.as_str(), value)), + StmtKind::ExprStmt(expr) => collect_growth_calls_from_expr(expr, out), + StmtKind::Assign { value, .. } | StmtKind::TypedAssign { value, .. } => { + collect_growth_calls_from_expr(value, out); + } + StmtKind::Echo(expr) => collect_growth_calls_from_expr(expr, out), + StmtKind::If { + then_body, + elseif_clauses, + else_body, + .. + } => { + collect_array_pushes(then_body, out); + for (_, clause_body) in elseif_clauses { + collect_array_pushes(clause_body, out); + } + if let Some(else_body) = else_body { + collect_array_pushes(else_body, out); + } + } + StmtKind::IfDef { + then_body, + else_body, + .. + } => { + collect_array_pushes(then_body, out); + if let Some(else_body) = else_body { + collect_array_pushes(else_body, out); + } + } + StmtKind::While { body, .. } + | StmtKind::DoWhile { body, .. } + | StmtKind::Foreach { body, .. } + | StmtKind::IncludeOnceGuard { body, .. } => collect_array_pushes(body, out), + StmtKind::For { + init, + update, + body, + .. + } => { + if let Some(init) = init { + collect_array_push_stmt(init, out); + } + if let Some(update) = update { + collect_array_push_stmt(update, out); + } + collect_array_pushes(body, out); + } + StmtKind::Switch { cases, default, .. } => { + for (_, case_body) in cases { + collect_array_pushes(case_body, out); + } + if let Some(default) = default { + collect_array_pushes(default, out); + } + } + StmtKind::Try { + try_body, + catches, + finally_body, + } => { + collect_array_pushes(try_body, out); + for catch in catches { + collect_array_pushes(&catch.body, out); + } + if let Some(finally_body) = finally_body { + collect_array_pushes(finally_body, out); + } + } + StmtKind::Synthetic(stmts) => collect_array_pushes(stmts, out), + _ => {} + } +} + +/// Collects `array_push($name, ...)` growth sites from an expression tree. +fn collect_growth_calls_from_expr<'a>(expr: &'a Expr, out: &mut Vec<(&'a str, &'a Expr)>) { + match &expr.kind { + ExprKind::FunctionCall { name, args } if name.as_str().eq_ignore_ascii_case("array_push") => { + if let Some(array_name) = array_push_target_name(args) { + for arg in args.iter().skip(1) { + out.push((array_name, call_arg_value(arg))); + } + } + for arg in args { + collect_growth_calls_from_expr(call_arg_value(arg), out); + } + } + ExprKind::FunctionCall { args, .. } + | ExprKind::MethodCall { args, .. } + | ExprKind::StaticMethodCall { args, .. } + | ExprKind::NullsafeMethodCall { args, .. } + | ExprKind::ExprCall { args, .. } + | ExprKind::ClosureCall { args, .. } + | ExprKind::NewObject { args, .. } + | ExprKind::NewDynamic { args, .. } + | ExprKind::NewDynamicObject { args, .. } => { + for arg in args { + collect_growth_calls_from_expr(call_arg_value(arg), out); + } + } + ExprKind::NamedArg { value, .. } + | ExprKind::Spread(value) + | ExprKind::ErrorSuppress(value) + | ExprKind::Print(value) + | ExprKind::Negate(value) + | ExprKind::BitNot(value) + | ExprKind::Not(value) + | ExprKind::Clone(value) + | ExprKind::Throw(value) => collect_growth_calls_from_expr(value, out), + ExprKind::Cast { expr: inner, .. } => collect_growth_calls_from_expr(inner, out), + ExprKind::BinaryOp { left, right, .. } + | ExprKind::NullCoalesce { + value: left, + default: right, + } + | ExprKind::ShortTernary { + value: left, + default: right, + } => { + collect_growth_calls_from_expr(left, out); + collect_growth_calls_from_expr(right, out); + } + ExprKind::Ternary { + condition, + then_expr, + else_expr, + } => { + collect_growth_calls_from_expr(condition, out); + collect_growth_calls_from_expr(then_expr, out); + collect_growth_calls_from_expr(else_expr, out); + } + ExprKind::ArrayLiteral(elems) => { + for elem in elems { + collect_growth_calls_from_expr(elem, out); + } + } + ExprKind::ArrayLiteralAssoc(entries) => { + for (key, value) in entries { + collect_growth_calls_from_expr(key, out); + collect_growth_calls_from_expr(value, out); + } + } + _ => {} + } +} + +/// Returns the local array name targeted by `array_push`'s first argument, if any. +fn array_push_target_name<'a>(args: &'a [Expr]) -> Option<&'a str> { + let first = args.first()?; + match &call_arg_value(first).kind { + ExprKind::Variable(name) => Some(name.as_str()), + _ => None, + } +} + +/// Unwraps a possible named argument to its value expression. +fn call_arg_value(arg: &Expr) -> &Expr { + match &arg.kind { + ExprKind::NamedArg { value, .. } => value, + _ => arg, + } +} diff --git a/src/types/checker/mod.rs b/src/types/checker/mod.rs index 39eb05897..f2ebf9c7d 100644 --- a/src/types/checker/mod.rs +++ b/src/types/checker/mod.rs @@ -24,6 +24,7 @@ mod driver; mod extern_decl; mod functions; mod inference; +mod loop_widening; mod method_pass; mod schema; mod stmt_check; @@ -46,6 +47,7 @@ use crate::types::{ }; pub use inference::{infer_expr_type_syntactic, infer_return_type_syntactic}; +pub(crate) use loop_widening::loop_grown_mixed_array_pushes; pub(crate) use inference::closure_body_uses_this; pub(crate) use builtin_types::InterfaceDeclInfo; use builtin_types::validate_magic_method_contracts; diff --git a/src/types/checker/stmt_check/control_flow.rs b/src/types/checker/stmt_check/control_flow.rs index 92d4929d8..ab43d38c8 100644 --- a/src/types/checker/stmt_check/control_flow.rs +++ b/src/types/checker/stmt_check/control_flow.rs @@ -19,6 +19,29 @@ const FS_CURRENT_AS_PATHNAME: i64 = 32; const FS_CURRENT_MODE_MASK: i64 = 240; const FS_SKIP_DOTS: i64 = 4096; +/// Widens locals whose indexed-array element type joins to `mixed` across the loop body's +/// push sites (issue #452). Loop bodies are checked in a single pass, so without this the +/// entry environment types an early push site against the pre-promotion element type even +/// though the back edge brings the promoted array around; fixing the element type to +/// `mixed` up front makes every push site see the fixed-point type. +fn widen_loop_grown_array_pushes( + checker: &mut Checker, + body: &[Stmt], + update: Option<&Stmt>, + env: &mut TypeEnv, +) { + let snapshot = env.clone(); + let names = crate::types::checker::loop_grown_mixed_array_pushes( + body, + update, + &|name| snapshot.get(name).cloned(), + &mut |expr| checker.infer_type(expr, &snapshot).ok(), + ); + for name in names { + env.insert(name, PhpType::Array(Box::new(PhpType::Mixed))); + } +} + /// Restores a narrowed variable in the environment to its previously saved type after a guarded /// branch, removing it when it had no prior type. Used to keep `if`/`else` type narrowing scoped /// to its branch. @@ -131,6 +154,9 @@ impl Checker { "by-reference foreach over Iterator/IteratorAggregate objects or iterable-typed values is not supported; use an array source or remove &", )); } + // Widen after the key/value bindings are in the environment so a push of + // the foreach value variable joins with its real element type. + widen_loop_grown_array_pushes(self, body, None, env); let errors = self.check_break_continue_target_body(body, env); if errors.is_empty() { Ok(()) @@ -257,6 +283,7 @@ impl Checker { } } StmtKind::DoWhile { body, condition } => { + widen_loop_grown_array_pushes(self, body, None, env); let errors = self.check_break_continue_target_body(body, env); self.infer_type_with_assignment_effects(condition, env)?; if errors.is_empty() { @@ -266,6 +293,7 @@ impl Checker { } } StmtKind::While { condition, body } => { + widen_loop_grown_array_pushes(self, body, None, env); self.infer_type_with_assignment_effects(condition, env)?; let errors = self.check_break_continue_target_body(body, env); if errors.is_empty() { @@ -283,6 +311,7 @@ impl Checker { if let Some(s) = init { self.check_stmt(s, env)?; } + widen_loop_grown_array_pushes(self, body, update.as_deref(), env); if let Some(c) = condition { self.infer_type_with_assignment_effects(c, env)?; } diff --git a/tests/codegen/array_basics.rs b/tests/codegen/array_basics.rs index 5978c4a26..ea5b83c43 100644 --- a/tests/codegen/array_basics.rs +++ b/tests/codegen/array_basics.rs @@ -240,6 +240,117 @@ echo $a[0]->n . "|" . $a[5]->n . "|" . count($a); assert_eq!(out, "p0|p5|6"); } +/// Regression for #452: pushing heterogeneous scalars (int then float) into an empty array +/// inside a loop promotes the array to mixed-element storage on iteration 1, but the earlier +/// push site was lowered against the stale pre-promotion type and wrote an unboxed scalar +/// into the mixed array on iteration 2. Reading that element then dereferenced the raw +/// scalar as a boxed cell pointer and crashed. The loop body must see the fixed-point +/// (back-edge) element type so every push site boxes correctly. +#[test] +fn test_loop_grown_mixed_array_element_read() { + let out = compile_and_run( + r#"` through such a +/// variable; a spurious widen made its typed-property storeback fail to compile. +#[test] +fn test_loop_rebuild_of_typed_array_not_spuriously_widened() { + let out = compile_and_run( + r#"attachIterator($it); +$multi->detachIterator($it); +echo $multi->countIterators(); +"#, + ); + assert_eq!(out, "0"); +} + +/// Regression companion for #452: heterogeneous scalars carried through loop-defined +/// variables (literal assignments inside the body) must still widen the pushed array — +/// the literal-assignment scan supplies the evidence the loop-entry lookup lacks. +#[test] +fn test_loop_grown_mixed_array_via_local_literals() { + let out = compile_and_run( + r#"getInt(); + $methods[] = $source->getFloat(); + $statics[] = Values::staticInt(); + $statics[] = Values::staticFloat(); + $properties[] = $source->intValue; + $properties[] = $source->floatValue; + $elements[] = $ints[0]; + $elements[] = $floats[0]; +} +echo $methods[2], $statics[2], $properties[2], $elements[2]; +"#, + ); + assert_eq!(out, "1111"); +} + +/// Regression for #452: an in-loop typed reassignment must override stale loop-entry +/// evidence when the assigned variable is appended to an array that later becomes mixed. +#[test] +fn test_loop_grown_mixed_array_via_reassigned_entry_local() { + let out = compile_and_run( + r#"