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

[mirchaemanuel]

Fixes #619 for boxed mixed arguments. Based on current main 9f74f5300.

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.

[nahime0]

@greptileai please review

[greptile-apps]

Greptile Summary

Fixes issue #619: when a callee only conditionally returns a parameter (if ($c) return $x; return 7;), the ReturnArgAlias::Parameters summary is a MAY fact, so the caller was suppressing the argument's release on every code path — correct on the branch that hands the box back, but leaking one block per call on the branches that don't. The fix introduces a new EIR instruction ReleaseUnlessAliases that compares the argument's box pointer to the returned value's box pointer at runtime and releases the argument only when they differ.

• ReleaseUnlessAliases(arg, result) is lowered for both AArch64 and x86_64, reusing the existing pointer-comparison shape from ABI-side cleanup slots; the comparison is restricted to directly-comparable boxed Mixed/Union types to avoid false "not aliased" reads for container arguments wrapped in a box.
• Bare container arguments retain the previous unconditional suppression (still leaking on the non-alias path), documented with a pinning test that will turn red when that shape is covered.
• Three regression tests are added: non-alias path releases the arg, mixed alias/non-alias iterations stay balanced, and the container restriction is pinned.

Confidence Score: 5/5

Safe to merge. The change is narrowly scoped to Mixed/Union boxed arguments, both ABI targets are covered, the existing #604 guard still passes, and the deliberate container-arg restriction is pinned by a test.

The new instruction is correctly validated (2-operand count check), dispatched, and lowered on both AArch64 and x86_64 using the same pointer-comparison shape already proven for ABI cleanup slots. The comparable guard prevents the double-free regression for container args. Three new tests exercise the non-alias path, the mixed-paths balance, and the documented leak-pinning restriction. No issues found.

Files Needing Attention: No files require special attention.

Important Files Changed

Filename	Overview
src/codegen/lower_inst/ownership.rs	Adds lower_release_unless_aliases: loads arg and result box pointers, compares them, and releases only when they differ. Both AArch64 and x86_64 are handled. lower_release retains its docblock at line 114.
src/ir/instr.rs	Adds ReleaseUnlessAliases variant with correct effects (REFCOUNT_OP
src/ir/validator.rs	Adds operand-count validation (exactly 2) for ReleaseUnlessAliases; correctly isolated from the existing 1-operand ownership ops group.
src/ir_lower/expr/mod.rs	Emits ReleaseUnlessAliases instead of an unconditional skip when both arg and result have a directly comparable Mixed/Union boxed representation; the continue still fires for non-comparable container args, preserving the documented deliberate restriction.
src/codegen/lower_inst.rs	Adds dispatch arm for Op::ReleaseUnlessAliases in lower_instruction, delegating to the new ownership handler.
tests/codegen/runtime_gc/regressions.rs	Three new regression tests: non-alias path releases the arg, container arg still leaks (pins the deliberate restriction), and mixed alias/non-alias iterations stay balanced. Expected sums verified arithmetically correct.
CHANGELOG.md	Adds one [Unreleased] entry describing the fix, the scope (boxed mixed args), and the deliberate container restriction.

Sequence Diagram

sequenceDiagram
    participant EIR as ir_lower/expr
    participant IR as EIR (ReleaseUnlessAliases)
    participant CG as codegen/lower_inst

    Note over EIR: result_reuses_arg = true, arg and result are Mixed/Union
    EIR->>IR: emit ReleaseUnlessAliases(arg, result)

    Note over CG: lower_release_unless_aliases()
    CG->>CG: load_value_to_result(arg) → value_reg
    CG->>CG: load_value_to_reg(result, scratch_reg)
    CG->>CG: cmp value_reg, scratch_reg

    alt pointers equal (aliased — ownership moved into result)
        CG->>CG: b.eq / je → skip_label (no release)
    else pointers differ (not aliased — callee dropped arg)
        CG->>CG: emit_decref_if_refcounted(Mixed)
    end

    CG->>CG: skip_label:

Loading

_{Reviews (2): Last reviewed commit: "docs(codegen): restore lower_release's d..." | 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]

The boxed mixed implementation looks technically sound, and the two earlier docblock findings are fixed. I am requesting changes for the remaining merge-readiness items:

1. Preserve tracking for the known container leak. The PR description starts with Fixes #619, so merging it will auto-close #619, but the PR deliberately leaves the bare-container reproducer leaking and includes a test that expects that leak. Please either change the closing keyword so #619 stays open, or open a dedicated follow-up issue and reference it clearly from the PR and changelog before allowing #619 to close.

2. Refresh the branch against current main. The PR head is 28d6daf, current main is d89856f, the branch is 22 commits behind, and GitHub reports CONFLICTING / DIRTY. This should be handled with a contributor-safe maintainer integration and conflict resolution.

3. Obtain terminal CI on the exact refreshed head. The current head only has the labels and Greptile checks. The earlier implementation run had an infrastructure archive failure rather than a test assertion failure, but it is still not a complete green exact-head signal across the supported matrix.

Once issue tracking, current-main integration, and exact-head CI are resolved, I do not see another code blocker within the deliberately limited boxed-mixed scope.