Skip to content

A return through a finally releases the scopes the finally reads after it runs; nested finallys all run - #598

Merged
ASDAlexander77 merged 2 commits into
mainfrom
fix-return-through-finally-release
Oct 17, 2026
Merged

ASDAlexander77 merged 2 commits into
mainfrom
fix-return-through-finally-release

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

What was wrong

Under -mm=rc, a return from a try body or catch clause released the whole frame (mlirGenScopeExit(FullStack)) and only then ran the copy of the finally that TryOpLowering routes it through. That copy can still read every scope around the try:

function f(pre: string) { let s = pre + "4"; try { return s + "r"; } finally { churn(); trail += s + ";"; } }

read a freed s (p4r ###########; instead of p4r p4;).

Separately, in every memory model, a return through two nested finallys ran only the inner one: the outer TryOp lowers first and points the returns at its finally copy, then the inner TryOp re-points them at its own, whose closing ReturnOp went straight to the function's return block.

Fix

  • MLIRGen: a try with a finally gives its body and catch clause a guard context (GenContext::finallyGuardScope, a self-pointer so copies never inherit it). A return releases up to there and stops. The finally ends with RETURN_ONLY_ATTR_NAME releases for the scopes from there out to the next guard (mlirGenReleaseOwnedOnReturn), the counterpart of the unwind-only releases from An exception out of a try/finally, a nested block in a try, or a throw releases the scopes it leaves #596.
  • TryOpLowering: keeps the return-only releases in the return copy only, and erases them from the landing-pad copy, the break/continue copies, and the normal-completion copy. The return copy's closing ReturnOp takes over the cleanup an outer TryOp had set on the original returns, so a return chains through every enclosing finally. The outer guard then releases the outer scopes, once.

Not changed: using disposal on the return path (mlirGenDisposable(FullStack)) still runs before the finally, and break/continue through a finally still release the loop-body scope first. These are the same kind of bug and are left as follow-ups.

Tests

  • New 00try_return_through_finally.ts (compile + JIT + TSLANG_CORPUS): return from body, bare return, from a catch, from a nested block, through two finallys, from a try in a catch, from a loop, from the finally itself, plus normal completion, an exception, and an empty inner finally. Each case reads its values again after churn().
  • 00try_unwind_outer_locals.ts's returnThroughFinally now reads s in its finally, which it avoided because of this bug.
  • gc/rc/none/own all print the expected output. --emit=mlir-affine --verify-ownership is quiet under rc and own.
  • Full ctest -C Release -j 8 --timeout 300: 100% passed (4150).

🤖 Generated with Claude Code

ASDAlexander77 and others added 2 commits October 17, 2026 20:18
…r it runs

A `return` from a `try` body or catch clause released the whole frame and
then ran the finally's return copy, which could still read those scopes:
under -mm=rc `let s = ...; try { return x; } finally { log(s); }` read a
freed string. The return now stops at the scope around the `try`
(GenContext::finallyGuardScope), and the finally ends with return-only
releases (RETURN_ONLY_ATTR_NAME) for the rest, which TryOpLowering keeps
in the return copy alone.

A return through two nested finallys also skipped the outer one in every
memory model: the inner return copy's ReturnOp now takes over the cleanup
the outer TryOp gave the original returns.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… inner finally

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 merged commit c9d81b7 into main Oct 17, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the fix-return-through-finally-release branch October 17, 2026 20:13
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.

1 participant