fix(ir): release loop-carried locals whose slot widens to boxed storage after their store was lowered

docs/internals/memory-model.md

@@ -278,7 +278,7 @@ When one of these checks trips, the program exits with a fatal heap-debug error
 
 ### When memory is freed
 
-- **Variable reassignment**: when a heap-backed local/global/static slot is overwritten, codegen releases the previous owner through the appropriate runtime path (`__rt_heap_free_safe` for persisted strings, `__rt_decref_*` for refcounted arrays / hashes / objects)
+- **Variable reassignment**: when a heap-backed local/global/static slot is overwritten, codegen releases the previous owner through the appropriate runtime path (`__rt_heap_free_safe` for persisted strings, `__rt_decref_*` for refcounted arrays / hashes / objects). When a store inside a loop is lowered before a later store has widened the slot to boxed storage (e.g. an inner `for` counter re-initialized by the outer body but widened Int→Mixed by its `++` update), lowering emits a deferred `release_local_slot` and the backend decides against the slot's final widened storage type, so the previous iteration's box is still released
 - **`unset()`**: releases the current heap-backed value before nulling the slot
 - **Targeted cycle collection**: when decref reaches a container/object graph that may only be keeping itself alive, `__rt_gc_collect_cycles` counts heap-only incoming edges, marks externally reachable blocks, and deep-frees the remaining unreachable array/hash/object island
 - **Generator frame release**: Generator frames are object-kind heap blocks, but their custom Mixed slots and active `yield from` delegate are released by a Generator-specific branch in object deep-free

src/codegen/frame.rs

@@ -534,7 +534,7 @@ fn local_slot_has_store(function: &Function, slot: LocalSlotId) -> bool {
 }
 
 /// Returns PHP-visible locals whose slot is rewritten to a ref-cell pointer.
-fn promoted_ref_cell_local_slots(function: &Function) -> HashSet<LocalSlotId> {
+pub(super) fn promoted_ref_cell_local_slots(function: &Function) -> HashSet<LocalSlotId> {
     let mut slots = function
         .instructions
         .iter()
@@ -583,7 +583,7 @@ fn loaded_local_slot(function: &Function, value: ValueId) -> Option<LocalSlotId>
 /// .rodata, out-of-range) and frees plausible live heap blocks, so it safely handles
 /// the zero-length owned strings that `__rt_str_persist` now allocates. The previous
 /// `cbz len` guard skipped them and leaked every owned empty string at scope exit.
-fn emit_main_string_cleanup(ctx: &mut FunctionContext<'_>, offset: usize) {
+pub(super) fn emit_main_string_cleanup(ctx: &mut FunctionContext<'_>, offset: usize) {
     let (ptr_reg, _) = abi::string_result_regs(ctx.emitter);
     let result_reg = abi::int_result_reg(ctx.emitter);
     abi::load_at_offset(ctx.emitter, ptr_reg, offset);
@@ -604,7 +604,7 @@ fn emit_main_string_cleanup(ctx: &mut FunctionContext<'_>, offset: usize) {
 }
 
 /// Releases a refcounted local when the slot contains a non-null heap pointer.
-fn emit_main_refcounted_cleanup(ctx: &mut FunctionContext<'_>, offset: usize, ty: &PhpType) {
+pub(super) fn emit_main_refcounted_cleanup(ctx: &mut FunctionContext<'_>, offset: usize, ty: &PhpType) {
     let result_reg = abi::int_result_reg(ctx.emitter);
     let done = ctx.next_label("main_refcounted_cleanup_done");
     abi::load_at_offset(ctx.emitter, result_reg, offset);

src/codegen/lower_inst.rs

@@ -79,6 +79,7 @@ pub(super) fn lower_instruction(ctx: &mut FunctionContext<'_>, inst_id: InstId)
         Op::PromoteLocalRefCell => lower_promote_local_ref_cell(ctx, &inst),
         Op::AliasLocalRefCell => lower_alias_local_ref_cell(ctx, &inst),
         Op::ReleaseLocalRefCell => lower_release_local_ref_cell(ctx, &inst),
+        Op::ReleaseLocalSlot => lower_release_local_slot(ctx, &inst),
         Op::LoadGlobal => lower_load_global(ctx, &inst),
         Op::StoreGlobal => lower_store_global(ctx, &inst),
         Op::ExternGlobalLoad => lower_extern_global_load(ctx, &inst),
@@ -6361,6 +6362,43 @@ fn release_local_ref_cell_owner(
     Ok(())
 }
 
+/// Lowers a deferred `release_local_slot`: releases the refcounted value currently
+/// held in a local frame slot, typed by the slot's FINAL storage type.
+///
+/// Lowering emits this op before a retaining loop store when the slot's storage
+/// type still looked untracked at that point but a later store on a back-edge
+/// path could widen it (issue #534: an inner `for` counter widened Int→Mixed by
+/// its checked-add update leaked one Mixed box per outer iteration when the
+/// outer body re-initialized it). Only here, after widening finished, is the
+/// decision sound. The slot is either zero (prologue zero-initializes cleanup
+/// locals, and the null-guarded release helpers skip zero) or an owned value
+/// boxed by a previous retaining store, so releasing it is always balanced.
+fn lower_release_local_slot(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> {
+    let slot = expect_local_slot(inst)?;
+    // Slots rewritten to ref-cell pointers hold the cell address, not an owned
+    // value; the ref-cell owner machinery releases those instead.
+    if local_slot_stores_ref_cell_pointer(ctx, slot)
+        || super::frame::promoted_ref_cell_local_slots(ctx.function).contains(&slot)
+    {
+        return Ok(());
+    }
+    let ty = ctx.local_php_type(slot)?.codegen_repr();
+    let offset = ctx.local_offset(slot)?;
+    match ty {
+        // Owned strings are freed through the validating helper, which skips
+        // null/uninitialized slots and non-heap (.rodata) literal pointers.
+        PhpType::Str => super::frame::emit_main_string_cleanup(ctx, offset),
+        PhpType::Callable => super::frame::emit_main_refcounted_cleanup(ctx, offset, &ty),
+        other if other.is_refcounted() => {
+            super::frame::emit_main_refcounted_cleanup(ctx, offset, &other)
+        }
+        // The slot never widened to refcounted storage: nothing can be owned.
+        // Lowering normally prunes these, so this arm is only a safety net.
+        _ => {}
+    }
+    Ok(())
+}
+
 /// Lowers `unset($local)` by breaking any promoted alias and writing PHP null locally.
 fn lower_unset_local(ctx: &mut FunctionContext<'_>, inst: &Instruction) -> Result<()> {
     let slot = expect_local_slot(inst)?;

src/ir/builder.rs

@@ -106,6 +106,38 @@ impl<'f> Builder<'f> {
         self.func.locals[slot.as_raw() as usize].php_type.clone()
     }
 
+    /// Neutralizes deferred `release_local_slot` ops whose slot never widened to
+    /// lifetime-tracked storage.
+    ///
+    /// Lowering emits `release_local_slot` before a loop store when the slot's
+    /// storage type LOOKS untracked at that point but a later store on a
+    /// back-edge path may still widen it (e.g. a `for` counter widened
+    /// Int→Mixed by its checked-add update). Once the whole body is lowered the
+    /// final storage types are known, so ops guarding slots that stayed
+    /// untracked are rewritten to `nop`s. This keeps scalar slots eligible for
+    /// dead-store elimination and load forwarding, which conservatively exclude
+    /// any slot named by an unknown op.
+    pub fn prune_untracked_release_local_slot_ops(&mut self) {
+        for inst in &mut self.func.instructions {
+            if inst.op != Op::ReleaseLocalSlot {
+                continue;
+            }
+            let Some(Immediate::LocalSlot(slot)) = inst.immediate else {
+                continue;
+            };
+            let storage_type = &self.func.locals[slot.as_raw() as usize].php_type;
+            if Ownership::php_type_needs_lifetime_tracking(storage_type) {
+                continue;
+            }
+            // The slot's final storage is a plain scalar: the deferred release
+            // can never free anything, so erase it instead of shipping a no-op
+            // that pessimizes slot analyses in later passes.
+            inst.op = Op::Nop;
+            inst.immediate = None;
+            inst.effects = Op::Nop.default_effects();
+        }
+    }
+
     /// Returns the semantic role of a local slot.
     pub fn local_kind(&self, slot: LocalSlotId) -> LocalKind {
         self.func.locals[slot.as_raw() as usize].kind

src/ir/instr.rs

@@ -196,6 +196,7 @@ pub enum Op {
     PromoteLocalRefCell,
     AliasLocalRefCell,
     ReleaseLocalRefCell,
+    ReleaseLocalSlot,
     LoadGlobal,
     StoreGlobal,
     LoadStaticLocal,
@@ -438,6 +439,7 @@ impl Op {
             },
             AliasLocalRefCell => E::READS_LOCAL | E::WRITES_LOCAL,
             ReleaseLocalRefCell => E::READS_LOCAL | E::WRITES_LOCAL | E::WRITES_HEAP | E::REFCOUNT_OP,
+            ReleaseLocalSlot => E::READS_LOCAL | E::WRITES_HEAP | E::REFCOUNT_OP,
             LoadGlobal | LoadStaticProperty | ScopedConstantGet | ClassAttrNames
             | ClassAttrArgs | ClassGetAttributes | CatchCurrent => E::READS_GLOBAL,
             StoreGlobal | StoreStaticLocal | StoreStaticProperty | InitStaticLocal | IncludeOnceMark
@@ -550,6 +552,7 @@ impl Op {
             PromoteLocalRefCell => "promote_local_ref_cell",
             AliasLocalRefCell => "alias_local_ref_cell",
             ReleaseLocalRefCell => "release_local_ref_cell",
+            ReleaseLocalSlot => "release_local_slot",
             LoadGlobal => "load_global",
             StoreGlobal => "store_global",
             LoadStaticLocal => "load_static_local",

src/ir/validator.rs

@@ -278,7 +278,7 @@ fn validate_instruction_immediate(inst_id: InstId, inst: &Instruction) -> Result
             require_immediate(inst_id, inst, "data id", |imm| matches!(imm, Imm::Data(_)))
         }
         LoadLocal | StoreLocal | UnsetLocal | LoadRefCell | StoreRefCell | ReleaseLocalRefCell
-        | BindRefCellPtr
+        | ReleaseLocalSlot | BindRefCellPtr
         | LoadStaticLocal | StoreStaticLocal | InitStaticLocal | InvokerRefArg => require_immediate(inst_id, inst, "local slot", |imm| {
             matches!(imm, Imm::LocalSlot(_))
         }),
@@ -395,7 +395,8 @@ fn validate_opcode_rules(function: &Function, inst_id: InstId, inst: &Instructio
         LoadLocal | LoadRefCell | LoadGlobal | LoadStaticLocal | LoadStaticProperty | ExternGlobalLoad => {
             check_count(inst_id, inst, 0, "0")
         }
-        UnsetLocal | PromoteLocalRefCell | AliasLocalRefCell | ReleaseLocalRefCell => {
+        UnsetLocal | PromoteLocalRefCell | AliasLocalRefCell | ReleaseLocalRefCell
+        | ReleaseLocalSlot => {
             check_count(inst_id, inst, 0, "0")
         }
         StoreLocal | StoreGlobal | StoreStaticLocal | InitStaticLocal | StoreStaticProperty | ExternGlobalStore

src/ir_lower/context.rs

@@ -615,6 +615,49 @@ impl<'m, 'f> LoweringContext<'m, 'f> {
         crate::ir_lower::ownership::release_if_owned(self, previous, span);
     }
 
+    /// Releases the previous occupant of a slot that a retaining `store_local` is
+    /// about to overwrite, deciding against the FINAL storage type when needed.
+    ///
+    /// When the slot's storage type already needs lifetime tracking this emits the
+    /// eager load+release pair immediately. When it does not, the slot can STILL be
+    /// widened to refcounted storage by a store lowered later that reaches this one
+    /// through a loop back-edge (e.g. an inner `for` counter re-initialized by the
+    /// outer body but widened Int→Mixed by its checked-add update). The storage
+    /// type visible here is stale in that case, so inside loops a deferred
+    /// `release_local_slot` is emitted instead: the backend releases the occupant
+    /// using the final widened storage type, and `prune_untracked_release_local_slot_ops`
+    /// erases the op when the slot never widens (issue #534: without this, the
+    /// previous outer iteration's Mixed box leaked on every re-initialization).
+    fn release_stored_local_value_before_retaining_store(
+        &mut self,
+        name: &str,
+        slot: LocalSlotId,
+        span: Option<Span>,
+    ) {
+        let storage_type = self.builder.local_php_type(slot);
+        if Ownership::php_type_needs_lifetime_tracking(&storage_type) {
+            self.release_stored_local_value(name, slot, span);
+            return;
+        }
+        if self.loop_stack.is_empty() {
+            // Outside loops no back-edge can execute a later widening store before
+            // this one, so the untracked storage type is final for this path.
+            return;
+        }
+        // Ref-bound locals keep a cell pointer in the frame slot and are released
+        // through the ref-cell owner machinery, never through a raw slot release.
+        if self.is_ref_bound_local(name) {
+            return;
+        }
+        self.emit_void(
+            Op::ReleaseLocalSlot,
+            Vec::new(),
+            Some(Immediate::LocalSlot(slot)),
+            Op::ReleaseLocalSlot.default_effects(),
+            span,
+        );
+    }
+
     /// Emits a store to a PHP local slot, updates type facts, and returns the stored value.
     pub(crate) fn store_local(&mut self, name: &str, value: LoweredValue, php_type: PhpType, span: Option<Span>) -> LoweredValue {
         self.clear_static_callable_local(name);
@@ -661,7 +704,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> {
             && local_kind_uses_plain_store_cleanup(previous_kind)
             && previous_slot.is_some_and(|slot| self.initialized_slots.contains(&slot))
         {
-            self.release_stored_local_value(name, slot, span);
+            self.release_stored_local_value_before_retaining_store(name, slot, span);
         }
         // A loop-carried slot can exist globally without being definitely initialized
         // on this CFG path. Release the runtime occupant before overwriting it.
@@ -670,7 +713,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> {
             && previous_slot.is_some_and(|slot| !self.initialized_slots.contains(&slot))
             && !self.loop_stack.is_empty()
         {
-            self.release_stored_local_value(name, slot, span);
+            self.release_stored_local_value_before_retaining_store(name, slot, span);
         }
         // A first syntactic store inside a loop body (main or function) can still
         // overwrite a prior runtime iteration's value: the slot has no straight-line
@@ -684,7 +727,7 @@ impl<'m, 'f> LoweringContext<'m, 'f> {
             && previous_slot.is_none()
             && !self.loop_stack.is_empty()
         {
-            self.release_stored_local_value(name, slot, span);
+            self.release_stored_local_value_before_retaining_store(name, slot, span);
         }
         let value = if (uses_global || previous_kind == LocalKind::PhpLocal)
             && !transfer_callable_source_to_store

src/ir_lower/function.rs

@@ -622,6 +622,9 @@ fn lower_body_into_function(
         crate::ir_lower::stmt::lower_stmt(&mut ctx, stmt);
     }
     terminate_open_block(&mut ctx);
+    // Final storage types are now known: erase deferred loop-store releases that
+    // guard slots which never widened to lifetime-tracked storage (issue #534).
+    ctx.builder.prune_untracked_release_local_slot_ops();
     ctx.into_closures()
 }
 

src/ir_passes/peephole/load_store.rs

@@ -103,6 +103,7 @@ fn escaping_slots(function: &Function) -> HashSet<LocalSlotId> {
             Op::PromoteLocalRefCell
                 | Op::AliasLocalRefCell
                 | Op::ReleaseLocalRefCell
+                | Op::ReleaseLocalSlot
                 | Op::InvokerRefArg
         ) {
             for slot in slots_of(inst) {

tests/codegen/runtime_gc/regressions.rs

@@ -1230,3 +1230,123 @@ echo "done";
         "promoting an indexed array literal to hash storage must free the source array (issue #408)"
     );
 }
+
+/// Regression test for issue #534: a refcounted local assigned inside a NESTED
+/// inner loop must not leak one block per OUTER iteration. The inner counter's
+/// re-initialization (`$k = 0`) is lowered while the slot still looks like an
+/// Int, but the `$k++` update widens the slot to boxed Mixed storage, so the
+/// re-init store must still release the previous outer iteration's box.
+#[test]
+fn test_regression_534_nested_loop_local_does_not_leak_per_outer_iteration() {
+    let out = compile_and_run_with_heap_debug(
+        r#"<?php
+$acc = 0;
+for ($i = 0; $i < 1000; $i++) {
+    for ($k = 0; $k < 1; $k++) {
+        $s = 'x' . $i;
+        $acc = $acc + strlen($s);
+    }
+}
+echo 'acc=' . $acc;
+"#,
+    );
+    assert!(out.success, "program failed: {}", out.stderr);
+    assert_eq!(out.stdout, "acc=3890");
+    assert!(
+        out.stderr.contains("HEAP DEBUG: leak summary: clean"),
+        "expected a clean heap (issue #534: one leaked block per outer iteration), got: {}",
+        out.stderr
+    );
+}
+
+/// Regression test for issue #534 (boxed element variant): the same nested shape
+/// reading a boxed array element into a local leaked the previous outer
+/// iteration's Mixed box on every `$k = 0` re-initialization.
+#[test]
+fn test_regression_534_nested_loop_array_element_does_not_leak() {
+    let out = compile_and_run_with_heap_debug(
+        r#"<?php
+$arr = ['aa', 'bb'];
+$acc = 0;
+for ($i = 0; $i < 1000; $i++) {
+    for ($k = 0; $k < 1; $k++) {
+        $lc = $arr[$k];
+        $acc = $acc + strlen($lc);
+    }
+}
+echo 'acc=' . $acc;
+"#,
+    );
+    assert!(out.success, "program failed: {}", out.stderr);
+    assert_eq!(out.stdout, "acc=2000");
+    assert!(
+        out.stderr.contains("HEAP DEBUG: leak summary: clean"),
+        "expected a clean heap (issue #534: one leaked block per outer iteration), got: {}",
+        out.stderr
+    );
+}
+
+/// Regression test for issue #534 (int-only variant): the leak was the INNER
+/// LOOP COUNTER's Mixed box, not the refcounted body value, so a nested loop
+/// with a purely integer body leaked identically. Also proves that scalar
+/// arithmetic locals in nested loops stay leak-free after the deferred-release
+/// fix.
+#[test]
+fn test_regression_534_nested_loop_int_counter_box_released() {
+    let out = compile_and_run_with_heap_debug(
+        r#"<?php
+$acc = 0;
+for ($i = 0; $i < 1000; $i++) {
+    for ($k = 0; $k < 3; $k++) {
+        $acc = $acc + $k;
+    }
+}
+echo 'acc=' . $acc;
+"#,
+    );
+    assert!(out.success, "program failed: {}", out.stderr);
+    assert_eq!(out.stdout, "acc=3000");
+    assert!(
+        out.stderr.contains("HEAP DEBUG: leak summary: clean"),
+        "expected a clean heap (issue #534: leaked inner-counter Mixed boxes), got: {}",
+        out.stderr
+    );
+}
+
+/// Regression test for issue #534 (function scope + conditional assignment):
+/// inside a function, an inner-loop local conditionally reassigned across
+/// outer iterations must release the carried value without double-freeing on
+/// iterations that skip the assignment, and triple nesting with `while` inner
+/// loops must stay balanced too.
+#[test]
+fn test_regression_534_nested_loop_conditional_and_triple_nesting_clean() {
+    let out = compile_and_run_with_heap_debug(
+        r#"<?php
+function run(int $outer): int {
+    $acc = 0;
+    $s = 'seed';
+    $i = 0;
+    while ($i < $outer) {
+        for ($j = 0; $j < 2; $j++) {
+            for ($k = 0; $k < 2; $k++) {
+                if ($k == 0) {
+                    $s = 'x' . $i . $j;
+                }
+            }
+        }
+        $acc = $acc + strlen($s);
+        $i++;
+    }
+    return $acc;
+}
+echo 'acc=' . run(300);
+"#,
+    );
+    assert!(out.success, "program failed: {}", out.stderr);
+    assert_eq!(out.stdout, "acc=1390");
+    assert!(
+        out.stderr.contains("HEAP DEBUG: leak summary: clean"),
+        "expected a clean heap for conditional nested-loop reassignment, got: {}",
+        out.stderr
+    );
+}