Skip to content

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

Open
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/534-nested-loop-carry-release
Open

fix(ir): release loop-carried locals whose slot widens to boxed storage after their store was lowered#535
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/534-nested-loop-carry-release

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Fixes #534 — with the issue's mechanism hypothesis refuted and the real one found at the IR level.

Root cause (not what the issue guessed)

The leaked value was never the assigned local: diffing --emit-ir of the leaky vs clean programs shows the nested case does emit release-old for $s. The real culprit is the inner loop counter: $k++ lowers through the checked-add path whose result is boxed Mixed (overflow-to-float), which widens the counter's frame slot storage Int→Mixed. But the outer body's $k = 0 re-initialization was lowered earlier, while the slot storage still read Int, so release_stored_local_value() early-returned on the stale type and emitted no release. At runtime every store into the (final) Mixed slot boxes — so each outer iteration's re-init orphaned the previous iteration's box: exactly N_outer − 1 leaks, independent of the inner trip count and of the loop body entirely. Proof: an int-only nested body leaks the identical 999 × 40 B on baseline.

Flags isolation confirmed core lowering (leak unchanged under --ir-opt=off, --regalloc=stack, and both).

The fix: defer the release decision until the slot's final storage type exists

The correct decision needs the slot's final storage type, which only exists after the whole body is lowered. So:

  • New deferred EIR op ReleaseLocalSlot (src/ir/instr.rs, validator entry): void, LocalSlot immediate, READS_LOCAL | WRITES_HEAP | REFCOUNT_OP.
  • src/ir_lower/context.rsrelease_stored_local_value_before_retaining_store(): when the slot storage is already lifetime-tracked, the eager load+release is emitted exactly as before (IR unchanged for every previously-correct case). When it is not, and the store is inside a loop (only there can a later store widen it through a back-edge), the deferred op is emitted instead. Ref-bound locals excluded (they release through the ref-cell owner machinery). All three release-old paths in store_local route through it.
  • src/ir/builder.rs + src/ir_lower/function.rs — post-lowering prune rewrites the op to Nop when the slot never widened, so scalar slots stay eligible for DSE/load-forwarding (verified in IR: a never-widened slot's op becomes nop; the widened counter's op survives).
  • src/codegen/lower_inst.rslower_release_local_slot(): skips ref-cell-pointer/promoted slots (defense in depth with the lowering-side exclusion), then releases the occupant typed by the final storage via the existing null-guarded epilogue cleanup emitters (emit_main_string_cleanup/emit_main_refcounted_cleanup, made pub(super)) — target-aware for ARM64 and x86_64, null-safe on the zero-initialized first iteration.
  • src/ir_passes/peephole/load_store.rs: op added to the escaping-slots set (slot memory is read directly).
  • One sentence added to docs/internals/memory-model.md.

Balance argument: the deferred op makes the same decision the eager path makes for tracked slots — release the previous occupant of a slot a retaining store is about to overwrite — just at the point where the storage type is finally known. The slot is either zero (prologue-initialized; the helpers skip null) or holds the previous retaining store's owned value.

Evidence (base cf47ee101, macOS arm64; every output equal to PHP 8.4.23 pre/post)

Case baseline fixed
nested concat, 1000 outer 999 blocks clean
single-loop control clean clean
nested array-element 999 clean
int-only nested body (the smoking gun) 999 clean
5000 outer (N−1 law) 4,999 clean
edge a: conditional assign in inner loop 999 clean
edge b: value used after inner loop / after both 999 clean
edge c: break/continue inner+outer 950 clean
edge d: init before the loops 999 clean
edge e: inner while + triple nesting 898 clean
edge f: non-refcounted ints 999 (the counter box!) clean
edge g: by-ref param local 499 clean

Post-fix --ir-opt=off and --regalloc=stack also clean.

Real-world gates (the #516 app):

Tests

4 new regression tests in tests/codegen/runtime_gc/regressions.rs (concat, array element, int-counter, conditional+triple-nesting-in-function): 0/4 pass on a stashed-src baseline build, 4/4 post-fix. Focused suites, 0 failures: loops 37, while 20, foreach 103, runtime_gc 126, nested 124, strings 225, arrays 337, for 446 — 1,418 passed. Zero warnings in both build profiles; debug-profile IR validator clean on all repros and the harness; asm-comment checker and git diff --check clean.

Notes for the maintainer

…heir store was lowered (illegalstudio#534)

A local stored inside a loop while its slot still looked like a plain
scalar skipped the release-old path, but a later store on a back-edge
path (e.g. the inner for counter's checked-add update) could widen the
slot to boxed Mixed storage. Each outer iteration's re-initialization
then orphaned the previous iteration's box, leaking exactly N_outer-1
blocks.

Lowering now emits a deferred release_local_slot before such stores;
the backend releases the occupant using the slot's FINAL widened
storage type (null-guarded, so the zero-initialized first iteration is
safe), and lowering prunes the op to a nop when the slot never widens
so scalar slots stay eligible for dead-store elimination.

Claude-Session: https://claude.ai/code/session_01Jfn7uqEH5Dog1nBzTM5DCW
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refcounted local assigned inside a nested inner loop leaks one reference per outer-loop iteration (N−1 law)

1 participant