diff --git a/src/codegen_ir/lower_inst/iterators.rs b/src/codegen_ir/lower_inst/iterators.rs index c348eba714..d75da55def 100644 --- a/src/codegen_ir/lower_inst/iterators.rs +++ b/src/codegen_ir/lower_inst/iterators.rs @@ -183,6 +183,7 @@ pub(super) fn lower_iter_current_value( let iterator = expect_operand(inst, 0)?; let offset = ctx.value_frame_offset(iterator)?; let result_ty = iter_current_result_type(ctx, inst)?; + let mut produced_mixed_box = false; match iterator_source_kind(ctx, iterator, inst)? { IteratorSourceKind::Indexed { elem } => { match ctx.emitter.target.arch { @@ -190,13 +191,24 @@ pub(super) fn lower_iter_current_value( Arch::X86_64 => load_current_array_value_x86_64(ctx, offset, &elem)?, } box_current_indexed_value_if_needed(ctx, &elem, &result_ty)?; + // The indexed iterator loads a borrowed string slot directly from the + // source array. When the EIR result type is Str (not boxed Mixed), the + // value is treated as an owning temporary by the lowering, so persist + // it into an owned heap copy before the caller releases it. + if elem.codegen_repr() == PhpType::Str && result_ty.codegen_repr() == PhpType::Str { + abi::emit_call_label(ctx.emitter, "__rt_str_persist"); + } + } + IteratorSourceKind::Hash => { + match ctx.emitter.target.arch { + Arch::AArch64 => load_current_hash_value_as_mixed_aarch64(ctx, offset), + Arch::X86_64 => load_current_hash_value_as_mixed_x86_64(ctx, offset), + } + produced_mixed_box = true; } - IteratorSourceKind::Hash => match ctx.emitter.target.arch { - Arch::AArch64 => load_current_hash_value_as_mixed_aarch64(ctx, offset), - Arch::X86_64 => load_current_hash_value_as_mixed_x86_64(ctx, offset), - }, IteratorSourceKind::DynamicIterable | IteratorSourceKind::DynamicMixed => { lower_dynamic_iter_current_value(ctx, inst, offset)?; + produced_mixed_box = true; } IteratorSourceKind::Object { class_name, .. } => { let return_ty = emit_object_iterator_method_call(ctx, offset, &class_name, "current")?; @@ -207,6 +219,9 @@ pub(super) fn lower_iter_current_value( box_iterator_method_result_if_needed(ctx, inst, &return_ty)?; } } + if produced_mixed_box { + unbox_dynamic_iter_current_value_if_needed(ctx, &result_ty)?; + } store_if_result(ctx, inst) } @@ -218,6 +233,71 @@ fn iter_current_result_type(ctx: &FunctionContext<'_>, inst: &Instruction) -> Re Ok(ctx.value_php_type(result)?.codegen_repr()) } +/// Unboxes a runtime-produced Mixed cell into the concrete EIR result type when the +/// iterator value has a known concrete type (e.g. `string` from `array`) +/// but the runtime lowering path always produces a boxed Mixed pointer. +fn unbox_dynamic_iter_current_value_if_needed( + ctx: &mut FunctionContext<'_>, + result_ty: &PhpType, +) -> Result<()> { + if matches!(result_ty.codegen_repr(), PhpType::Mixed | PhpType::Union(_) | PhpType::Void | PhpType::Never) { + return Ok(()); + } + let result_reg = abi::int_result_reg(ctx.emitter); + match ctx.emitter.target.arch { + Arch::AArch64 => { + ctx.emitter.instruction("mov x9, x0"); // save the Mixed box pointer across the unbox call + ctx.emitter.instruction("mov x0, x9"); // pass the Mixed box pointer to the unbox helper + abi::emit_call_label(ctx.emitter, "__rt_mixed_unbox"); + // x0 = tag, x1 = lo, x2 = hi + // AArch64 string result: x1 = ptr, x2 = len — already in place + match result_ty.codegen_repr() { + PhpType::Str => { + // x1 and x2 already hold the string pointer and length + } + PhpType::Int | PhpType::Bool | PhpType::Callable => { + if result_reg != "x1" { + ctx.emitter.instruction(&format!("mov {}, x1", result_reg)); // move the unboxed scalar payload into the integer result register + } + } + PhpType::Float => { + ctx.emitter.instruction("fmov d0, x1"); // move the unboxed float bits into the float result register + } + other if other.is_refcounted() => { + if result_reg != "x1" { + ctx.emitter.instruction(&format!("mov {}, x1", result_reg)); // move the unboxed refcounted pointer into the integer result register + } + } + _ => {} + } + } + Arch::X86_64 => { + ctx.emitter.instruction("mov r10, rax"); // save the Mixed box pointer across the unbox call + ctx.emitter.instruction("mov rax, r10"); // pass the Mixed box pointer to the unbox helper + abi::emit_call_label(ctx.emitter, "__rt_mixed_unbox"); + // rax = tag, rdi = lo, rdx = hi (x86_64 __rt_mixed_unbox output convention) + match result_ty.codegen_repr() { + PhpType::Str => { + // x86_64 string result: rax = ptr, rdx = len + ctx.emitter.instruction("mov rax, rdi"); // move the unboxed string pointer into the string result register + // rdx already holds the length from __rt_mixed_unbox + } + PhpType::Int | PhpType::Bool | PhpType::Callable => { + ctx.emitter.instruction("mov rax, rdi"); // move the unboxed scalar payload into the integer result register + } + PhpType::Float => { + ctx.emitter.instruction("movq xmm0, rdi"); // move the unboxed float bits into the float result register + } + other if other.is_refcounted() => { + ctx.emitter.instruction("mov rax, rdi"); // move the unboxed refcounted pointer into the integer result register + } + _ => {} + } + } + } + Ok(()) +} + /// Boxes an indexed iterator element only when the EIR result expects `Mixed`. fn box_current_indexed_value_if_needed( ctx: &mut FunctionContext<'_>, diff --git a/src/ir_lower/stmt/mod.rs b/src/ir_lower/stmt/mod.rs index 8af509c06c..f4d61183ca 100644 --- a/src/ir_lower/stmt/mod.rs +++ b/src/ir_lower/stmt/mod.rs @@ -1081,8 +1081,7 @@ fn lower_foreach( if value_ty == PhpType::Mixed { initialize_foreach_mixed_local_if_needed(ctx, value_var, value_needs_null_init, array.span); } else if value_needs_null_init { - ctx.declare_local(value_var, value_ty.clone()); - ctx.set_local_type(value_var, value_ty); + initialize_foreach_concrete_local_if_needed(ctx, value_var, value_ty.clone(), array.span); } } let header = ctx.builder.create_named_block("foreach.next", Vec::new()); @@ -1170,7 +1169,8 @@ fn lower_foreach( /// Returns the by-value foreach local type when Phase 04 can keep a concrete element. fn foreach_value_type(source_ty: &PhpType) -> PhpType { match source_ty.codegen_repr() { - PhpType::Array(elem) if elem.codegen_repr() == PhpType::Callable => PhpType::Callable, + PhpType::Array(elem) => foreach_element_value_type(elem.codegen_repr()), + PhpType::AssocArray { value, .. } => foreach_element_value_type(value.codegen_repr()), PhpType::Object(class_name) if class_name == "Phar" || class_name == "PharData" => { PhpType::Object("PharFileInfo".to_string()) } @@ -1178,6 +1178,15 @@ fn foreach_value_type(source_ty: &PhpType) -> PhpType { } } +/// Returns the foreach value type for a concrete array element type, widening +/// non-scalar element types to `Mixed` so the runtime always boxes them. +fn foreach_element_value_type(elem_ty: PhpType) -> PhpType { + match elem_ty { + PhpType::Str | PhpType::Int | PhpType::Bool | PhpType::Float | PhpType::Callable => elem_ty, + _ => PhpType::Mixed, + } +} + /// Returns the local value type used when a foreach binds the value by reference. fn foreach_ref_value_type(source_ty: &PhpType) -> PhpType { match source_ty.codegen_repr() { @@ -1214,6 +1223,61 @@ fn initialize_foreach_mixed_local_if_needed( ctx.store_local(name, boxed, PhpType::Mixed, Some(span)); } +/// Initializes a fresh foreach loop variable to a safe empty value before the first +/// iteration when the foreach value has a concrete type (e.g. `string` from +/// `array`). Without this, the first `release` of the previous slot value +/// would read uninitialized stack memory. +fn initialize_foreach_concrete_local_if_needed( + ctx: &mut LoweringContext<'_, '_>, + name: &str, + value_ty: PhpType, + span: Span, +) { + ctx.declare_local(name, value_ty.clone()); + ctx.set_local_type(name, value_ty.clone()); + match value_ty.codegen_repr() { + PhpType::Str => { + let empty_str_id = ctx.intern_string(""); + let empty = ctx.emit_value( + Op::ConstStr, + Vec::new(), + Some(Immediate::Data(empty_str_id)), + PhpType::Str, + Op::ConstStr.default_effects(), + Some(span), + ); + ctx.store_local(name, empty, PhpType::Str, Some(span)); + } + PhpType::Int | PhpType::Bool => { + let zero = ctx.emit_value( + Op::ConstI64, + Vec::new(), + Some(Immediate::I64(0)), + value_ty.clone(), + Op::ConstI64.default_effects(), + Some(span), + ); + ctx.store_local(name, zero, value_ty, Some(span)); + } + PhpType::Float => { + let zero = ctx.emit_value( + Op::ConstF64, + Vec::new(), + Some(Immediate::F64(0.0)), + value_ty.clone(), + Op::ConstF64.default_effects(), + Some(span), + ); + ctx.store_local(name, zero, value_ty, Some(span)); + } + _ => { + // For other refcounted or complex types, fall back to null. + let null = emit_null_value(ctx, Some(span)); + ctx.store_local(name, null, value_ty, Some(span)); + } + } +} + /// Lowers a `switch` with source-ordered pattern evaluation and PHP fallthrough. fn lower_switch( ctx: &mut LoweringContext<'_, '_>, diff --git a/tests/codegen/types/iterable/foreach.rs b/tests/codegen/types/iterable/foreach.rs index 034c6282fe..de64b5274b 100644 --- a/tests/codegen/types/iterable/foreach.rs +++ b/tests/codegen/types/iterable/foreach.rs @@ -609,3 +609,105 @@ fn test_iterable_variadic_arg_stays_boxed_in_runtime_array() { ); assert_eq!(out, "[[1,2]]"); } + +/// Regression test for issue #405: appending a foreach value into a new array and +/// returning it from a function must not corrupt the array. The foreach value is a +/// borrowed string slot from the source array; without an owned copy the appended +/// elements dangle after the source temporary is freed. +#[test] +fn test_foreach_value_append_returned_from_function() { + let out = compile_and_run( + "