fix(ir,codegen): release conditionally-aliased call arguments at runtime (#619)

[mirchaemanuel]

Fixes #619 for boxed mixed arguments. Follow-up #665 tracks the bare-container path deliberately left out of scope.

Integrated current main e015218f5 in maintainer merge c1a3fd23e.

Root cause

ReturnArgAlias::Parameters is a MAY summary — a union over branches — so proven_aliases_parameter also holds for a callee that returns the parameter only conditionally (if ($c) return $x; return 7;). The suppression site (src/ir_lower/expr/mod.rs) therefore skipped the argument release on every path. That is required on the branch that hands the box back, or it is freed twice (#604), and it leaks one block per call on the branches that do not.

The lowering has to decide once, statically, for a fact that is only known per call. So the decision moves to runtime.

Fix

A new EIR instruction, ReleaseUnlessAliases(arg, result): after the call, compare the argument payload against the value the callee returned and release the argument only when they differ. Same pointer means ownership moved into the result and the caller must keep its hands off; different pointer means the callee dropped it and the caller still owns it.

Both supported ABIs lower it, reusing the comparison shape emit_branch_if_cleanup_temp_aliases_result already uses for ABI-side cleanup slots. On a callee that genuinely always returns the parameter the comparison always matches, so #604's behaviour is unchanged.

Deliberate restriction, and why

The comparison is emitted only when both sides are boxed mixed, i.e. directly comparable as single pointers.

I initially emitted it unconditionally. That regressed four tests with heap debug detected bad refcount: a mixed result that wraps a bare container holds a different pointer than the container itself, so the comparison reads "not aliased" for a value the result does own, and releases it twice. Bare container arguments therefore keep the previous suppression and their pre-existing leak — the one that predates #618 — and test_conditional_return_callee_container_arg_still_leaks_on_non_alias_path pins that leak so the restriction stays deliberate and the test turns red the day the container path is covered. Covering it means reaching through the box to its payload field, which I'd rather do as its own change than smuggle in here.

Tests

In tests/codegen/runtime_gc/regressions.rs, next to the #604 family:

• non-alias path releases the argument — the issue's arithmetic-box repro; leaked 20 blocks / 800 bytes before
• mixed paths in one program (maybe($i + 1, $i % 2)) — releasing on the aliasing iterations would double-free, skipping it on the others leaks; leaked exactly 10 blocks before, one per non-aliasing iteration
• container argument still leaks — the documented restriction above
• the existing test_conditional_return_callee_alias_path_stays_balanced is the Direct owned boxed-mixed call argument with a consumed return value corrupts refcounts (heap debug: bad refcount) #604 guard and still passes

Verification on 35dc9e18b, macOS ARM64

the 4 conditional-return tests	4 passed, 0 failed
same, ELEPHC_IR_OPT=off	4 passed, 0 failed
runtime_gc	256 passed, 0 failed
callables	534 passed, 0 failed
error_tests	1139 passed, 0 failed
ir_backend_smoke_test	255 passed, 0 failed
cargo test -p elephc --lib	919 passed, 0 failed
cargo build / git diff --check	clean, zero warnings

Not run locally: the Linux targets — no Docker on this machine. The x86_64 lowering is the mirror of the AArch64 one and reuses the existing comparison helper's shape, but CI owns the real signal.

Maintainer integration (2026-08-03)

• Opened Bare container call arguments leak on non-aliasing conditional-return paths #665 to track representation-aware runtime alias disambiguation for bare container arguments.
• Merged current main e015218f51a5f321a8f8b7ddbced38f177ae1f53 without rewriting contributor commits.
• Resolved the changelog conflict by preserving every main entry and linking the residual container limitation to Bare container call arguments leak on non-aliasing conditional-return paths #665.
• Revalidated cargo build, the four conditional_return_callee regressions with EIR optimization enabled and disabled, assembly-comment alignment, and git diff --check.
• CI on merge head c1a3fd23eca9b97ed23538738b8b770de4569f59 is the authoritative supported-target signal.

[nahime0]

@greptileai please review

[greptile-apps]

Greptile Summary

This PR introduces a new EIR instruction ReleaseUnlessAliases(arg, result) to fix a memory leak (issue #619) where call arguments with a conditional-return callee were suppressed from release on every code path. The root cause was that ReturnArgAlias::Parameters is a MAY summary, so proven_aliases_parameter also fires for callees that only conditionally return the parameter; the caller therefore skipped the release on all paths.

• Adds Op::ReleaseUnlessAliases to the EIR with REFCOUNT_OP | WRITES_HEAP | READS_HEAP effects, a validator operand count check, and a display name; lowers it on both AArch64 and x86_64 by comparing the argument payload against the returned value and releasing only when they differ.
• Restricts the conditional release to cases where both the argument and result codegen_repr() are PhpType::Mixed | PhpType::Union(_), deliberately leaving bare-container arguments at the existing (leaked) suppression until a separate comparison-through-box mechanism is implemented (tracked in Bare container call arguments leak on non-aliasing conditional-return paths #665); pins the remaining leak with test_conditional_return_callee_container_arg_still_leaks_on_non_alias_path.
• Adds three regression tests covering the non-alias release path, the deliberate container limitation, and the mixed alias/non-alias alternating case, all verified with heap-debug output.

Confidence Score: 5/5

Safe to merge; the fix is well-scoped, both ABI targets are covered, and the heap-debug test suite validates alias and non-alias paths end-to-end.

The new instruction, its effects mask, the validator guard, and the two-target lowering are all internally consistent. The deliberate container-argument limitation is documented, pinned with a failing-intent test, and tracked in #665. The only observation is a load-order fragility in the new lowering helper — result is captured after int_result_reg is overwritten rather than before, reversing the convention established by the existing comparison helper — but current tests pass because the REFCOUNT_OP effects cause the register allocator to move the call result out of the int-result register before the instruction fires.

Files Needing Attention: src/codegen/lower_inst/ownership.rs — the load ordering of value vs result in lower_release_unless_aliases.

Important Files Changed

Filename	Overview
src/codegen/lower_inst/ownership.rs	Adds lower_release_unless_aliases; load-order of value vs result diverges from the established emit_branch_if_cleanup_temp_aliases_result pattern in a way that is fragile if result happens to be allocated to int_result_reg.
src/ir_lower/expr/mod.rs	Emits ReleaseUnlessAliases for Mixed/Union-typed owning temporaries when result_reuses_arg is true and both codegen_repr() values are boxed-mixed; falls through to the existing continue for non-comparable cases preserving prior behaviour.
src/ir/instr.rs	Adds ReleaseUnlessAliases opcode variant with correct effects (REFCOUNT_OP
src/ir/validator.rs	Adds check_count(2) for ReleaseUnlessAliases; operand-count guard is correct, no type-check validation at IR level but emission site enforces Mixed/Union restriction.
src/codegen/lower_inst.rs	Adds dispatch arm for ReleaseUnlessAliases; straightforward single-line addition.
tests/codegen/runtime_gc/regressions.rs	Adds three well-structured regression tests: non-alias-path leak fix, deliberate container limitation, and mixed alias/non-alias paths; arithmetic and expected outputs are correct.
CHANGELOG.md	New changelog entry accurately describes the fix, the boxed-mixed restriction, and the bare-container follow-up in #665.

Sequence Diagram

sequenceDiagram
    participant EIR as EIR Lowering
    participant IR as EIR Instruction
    participant CG as Codegen
    participant OWN as Ownership Lowerer

    Note over EIR: release_owned_call_arg_temporaries_with_signature()
    EIR->>EIR: value_is_owning_temporary(arg)?
    EIR->>EIR: "result_reuses_arg = proven/may alias?"
    alt !independently_boxed and result_reuses_arg
        EIR->>EIR: "arg_repr = codegen_repr(arg), result_repr = codegen_repr(result)"
        alt both are Mixed or Union
            EIR->>IR: emit Op::ReleaseUnlessAliases(arg, result)
        else bare container or non-comparable
            Note over EIR: continue — no release, existing leak tracked in #665
        end
    else no alias
        EIR->>IR: emit Op::Release(arg)
    end
    Note over CG: At codegen time
    CG->>OWN: lower_release_unless_aliases(inst)
    OWN->>OWN: load value into int_result_reg
    OWN->>OWN: load result into symbol_scratch_reg
    OWN->>OWN: cmp int_result_reg, symbol_scratch_reg
    alt pointers equal — alias path
        OWN->>OWN: b.eq / je skip_label — ownership in result, do not release
    else pointers differ — non-alias path
        OWN->>OWN: emit_decref_if_refcounted — callee dropped arg, release it
    end
    OWN->>OWN: skip_label:

Loading

_{Reviews (4): Last reviewed commit: "Merge origin/main into fix/619-runtime-a..." | Re-trigger Greptile}

[mirchaemanuel]

CI note — the two failures are infrastructure, not test failures.

Build & Test died in the checkout step:

fatal: unable to access 'https://github.com/illegalstudio/elephc/':
  Failed to connect to github.com port 443 after 75003 ms: Couldn't connect to server
The process '/opt/homebrew/bin/git' failed with exit code 128

and the Codegen Tests (macos-aarch64 9/16) shard failed unpacking its test archive:

failed to unpack `target/debug/deps/error_tests-3b0afadc715685d4` into .../nextest-archive-7W9Vyf/...

Neither reached an assertion — no test reported a failure. The other 111 checks are green. A re-run should clear both; the branch is also CONFLICTING against current main since it is based on 9f74f5300, so a rebase would re-trigger CI anyway. Leaving that to you.

[mirchaemanuel]

Good catch from the bot — both findings were valid and are fixed in 28d6daf9b.

Inserting lower_release_unless_aliases directly above lower_release moved that function's docblock onto the new one. The result was exactly as described: lower_release left undocumented, and the new function carrying a stale opening line that described the old one. Restored the original line to lower_release and dropped it from the new doc.

Verified after the change: cargo build clean with zero warnings, conditional_return_callee 4/4, git diff --check clean.

The CI failures on this PR remain unrelated to the code — the checkout step timed out (Failed to connect to github.com port 443) and the codegen shard failed unpacking its nextest archive; no test reported a failure.

[nahime0]

Maintainer follow-up: I handled the requested PR maintenance directly.

• I opened #665 to track the bare-container leak that remains outside this boxed-mixed fix.
• I merged current main into the contributor branch without rewriting contributor history; the integration commit is c1a3fd2.
• I resolved the CHANGELOG conflict while preserving the entries from main and linking the follow-up issue.
• I revalidated the build, the four conditional-return regressions with EIR optimization both on and off, assembly-comment alignment, and git diff --check.
• Exact-head CI restarted on c1a3fd2 in run 30826743063.

There were no requested code changes within the boxed-mixed scope. Tracking and current-main integration have now been completed by me as maintainer, and CI is running on the exact updated head, so this request-changes review can be dismissed.

Addressed directly by the maintainer: follow-up issue opened, current main merged without rewriting contributor history, focused validation completed, and exact-head CI restarted.

[nahime0]

@Guikingone if you give a run of review on this PR I'll proceed to merge it.