Skip to content

A return, break or continue through a finally disposes and releases the scopes the finally reads after it runs - #600

Merged
ASDAlexander77 merged 1 commit into
mainfrom
fix-scope-exit-through-finally
Oct 17, 2026
Merged

ASDAlexander77 merged 1 commit into
mainfrom
fix-scope-exit-through-finally

Conversation

@ASDAlexander77

Copy link
Copy Markdown
Owner

Follows #598, using the same design for disposal and for break/continue.

Bugs

  1. return disposed too early. { using r = new R(); try { return 1; } finally { r.read(); } } disposed r before the finally ran, in every memory model. JS disposes it after the finally. The cause: mlirGenDisposable had no finally guard.
  2. break/continue released and disposed too early. A jump out of a try in a loop body, or in a block nested in it, disposed and released the scopes around the try up to its loop before it went through the finally copy. Under -mm=rc the finally then read freed strings.

Fix

  • scopeExitStopsAtFinallyGuard: both walks, dispose and release, now stop at finallyGuardScope for FullStack, BreakScope and LoopScope. A break or continue notes (kind, label) in GenContext::finallyEscapingJumps, which the body guard and the catch guard share.
  • mlirGenScopeExitAfterFinally replaces mlirGenReleaseOwnedOnReturn. At the end of the finally it adds one ts.If true for the return and one for each escaping jump. Each holds the dispose and release of the scopes from the try outwards, using that jump's own walk. Each is tagged RETURN_ONLY_ATTR_NAME, or JUMP_ONLY_ATTR_NAME = "break:<label>" / "continue:<label>".
  • IfOp's RemoveStaticCondition skips tagged ifs. Otherwise the canonicalizer that runs before lowering inlines them and the tag is lost.
  • TryOpLowering makes one jump copy per (target, key). An unlabelled break and a break outer can share a target and still each get their own scope exit. The group that belongs to a copy is unwrapped (inlineBlockBefore); all other groups are erased. If the wrapper were left in place it would lower to a constant cond_br, and the ownership verifier would see its other edge as a path that never releases.

Tests

00try_scope_exit_through_finally.ts is registered for compile, JIT and TSLANG_CORPUS. It covers:

  • return with a using in a block, in a catch, and through two finallys
  • break and continue from a nested block and from the loop body
  • break and continue with a using
  • a labelled continue outer
  • a break through two finallys
  • a break out of a switch case and out of a labelled block

Values are read after churn().

  • JIT under gc, rc and none: all pass. Before the fix, gc failed on the dispose order and rc read freed strings.
  • --emit=mlir-affine --verify-ownership -mm=rc: quiet on every test that contains a finally, including the new one.
  • ctest -C Release -j 8 --timeout 300: 100% passed, 0 of 4156 failed.

Not fixed (separate, existing bug)

On Windows, a break out of a catch clause crashes (0x80000003) in both JIT and AOT, under gc and none, with or without a finally:
for (...) { try { h(); } catch { if (i == 1) break; } }. The test leaves this case out.

🤖 Generated with Claude Code

…he scopes the finally reads after it runs

A `return` disposed the `using`s around a try before its finally ran, and a
`break` or `continue` out of a try disposed and released the scopes around it
up to its loop before the finally ran (under -mm=rc the finally read freed
strings). Both walks now stop at the try's finallyGuardScope; the finally ends
with one `ts.If` per place a jump goes, tagged return-only or jump-only by
kind and label, and TryOpLowering keeps each in its own copy, unwrapped.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ASDAlexander77
ASDAlexander77 merged commit 99a20b8 into main Oct 17, 2026
2 checks passed
@ASDAlexander77
ASDAlexander77 deleted the fix-scope-exit-through-finally branch October 17, 2026 20:45
ASDAlexander77 added a commit that referenced this pull request Oct 23, 2026
* rc: a temporary a finally reads is released after the finally

`const v = mv(); try { if (c) continue; } finally { s += v.get(); }`: a
folded const (a call's result named by a const) is released where its
block ends, and in front of each break, continue or return that leaves
the block early. That release ran before the finally the jump goes
through, and the finally then read a freed block. #600 fixed the same
for locals in slots; a folded const is released by
releaseDiscardedTemporaries instead.

When a finally between the jump and the temporary's block reads it, the
release now goes at the end of the outermost such finally, in a ts.If
tagged with the jump (JUMP_ONLY_ATTR_NAME, RETURN_ONLY_ATTR_NAME), so
TryOpLowering keeps it in that jump's copy of the finally alone - the
scheme MLIRGen uses for the scopes the jump leaves. Jumps with one key
share one copy and get one release. Fuzzer (#646) R6, rc.

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

* rc: a jump through a finally that jumps itself releases a temporary once

`for (..) { const v = mv(); try { try { continue; } finally { break; } }
finally {} }`: the temporary was released in front of the `continue`,
and again in front of the `break` its finally then took. The release
for a jump that passes through a finally now always goes at the end of
the outermost such finally, tagged with the jump, not only when the
finally reads the temporary: a finally that leaves by a jump of its own
never reaches it, and the jump's own release is then the only one.
Fuzzer (#646), found past the first 1000 programs.

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

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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