An exception out of a try/finally, a nested block in a try, or a throw releases the scopes it leaves (#596)

Under -mm=rc, --verify-ownership still reported real leaks of an owned local on three unwind
shapes that #595 left open, plus a plain `throw`:

- `let s = a + b; try { h(); } finally { ... } return s;` - after the finally rethrows, `s` was
  never given back. The release cannot go in the cleanup region, which runs before the finally
  and so before it may read `s`. MLIRGen now puts the walk of #595 (mlirGenReleaseOwnedOnUnwind)
  at the end of the finally, with each release marked UNWIND_ONLY_ATTR_NAME. TryOpLowering keeps
  them in the copy of the finally the landing pad runs, and erases them from the copies normal
  completion, `return` and `break` run (through each copy's mapping: the rewriter defers an
  erasure, so an op erased from the region is still cloned into later copies).
- `try { { let s = a + b; h(); } } catch {}` - the TryOp's cleanup releases its body's scope and
  no other. A block that declares a let/const inside a TryOp body (GenContext::isUnwindCleanupScope
  somewhere up the chain) is now wrapped like a `using` block, so its cleanup releases it and the
  scopes up to that body.
- `try { h(); } catch { throw 2; }` and `let s = a + b; throw 1;` - a throw statement now releases
  the scopes it leaves up to the nearest TryOp body, as a `return` does, after taking a reference
  for the thrown value (mlirGenRetainCaptured at last use) so `throw s` is not freed with `s`. The
  thrown value is never given back, so it leaks rather than dangles. A catch clause of a try with a
  finally stops that walk, since the finally's unwind copy releases those scopes.

New test 00try_unwind_outer_locals.ts (compile + JIT + TSLANG_CORPUS, so gc/rc/none) covers the
finally on throw/complete/return/break, a throw from a catch with and without a finally, a try in
a try body, nested scopes and a loop body in a try, and throws from a function body and of a local.
The compiler before this change reports 9 slots in it; the verifier sweep covers it.

Still open: a call that throws out of a catch with no finally, a typed catch that does not match
with no finally, and a throwing call with no try around it - none has a landing pad to release
from. Found on the way, all pre-existing: an empty `catch { }` does not catch; under rc a `return`
through a finally releases the frame before the finally runs; `return` inside a finally, and a
try/catch/finally nested in a finally, crash Win32ExceptionPass.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: ASDAlexander77

tslang/include/TypeScript/Defines.h

@@ -61,6 +61,15 @@
 // MLIROwnedReference.h.
 #define OWNED_RESULT_NAMED_ATTR_NAME "__owned_result_named"
 
+// Marks a release MLIRGen put at the end of a hand-written `finally` for the exceptional leg only:
+// what the scopes around the `try` owe once the exception carries on past it. The finally region
+// is one region in MLIRGen but several copies after TryOpLowering - the copy the landing pad runs
+// before resuming, and the copies normal completion, a `return` and a `break` run - and only the
+// first leaves the function's scopes behind; on every other copy they are still live, and their
+// own exits release them. TryOpLowering keeps these in the unwind copy and erases them from the
+// rest.
+#define UNWIND_ONLY_ATTR_NAME "__unwind_only"
+
 // Marks a `ts.CreateBoundFunction` whose `this` is a capture box built for it a moment earlier,
 // rather than a receiver that belongs to somebody else. Only such a closure owns its `this`, and
 // only it gets the type tag at CLOSURE_TYPE_INDEX - a bound method must not take ownership of

tslang/lib/TypeScript/LowerToAffineLoops.cpp

@@ -1618,13 +1618,44 @@ struct TryOpLowering : public TsPattern<mlir_ts::TryOp>
             tsContext->finallyBlockOf[tryOp.getOperation()] = finallyBlock;
             finallyBlockLast = continuation->getPrevNode();
 
+            // That copy is the one the landing pad runs before the exception carries on, and the
+            // only one whose way out leaves the scopes around the try: the releases MLIRGen put at
+            // the end of the finally for them stay in it and go from every later copy. Not from a
+            // try nested in the finally - its own lowering sorts its own.
+            //
+            // Erased from each later copy through its mapping, and from the region itself only once
+            // the last copy is made: the rewriter defers an erasure, so an op erased from the
+            // region still goes into every copy cloned from it afterwards.
+            SmallVector<Operation *> unwindOnlyOps;
+            tryOp.getFinally().walk<mlir::WalkOrder::PreOrder>([&](Operation *op) {
+                if (isa<mlir_ts::TryOp>(op))
+                {
+                    return WalkResult::skip();
+                }
+
+                if (op->hasAttr(UNWIND_ONLY_ATTR_NAME))
+                {
+                    unwindOnlyOps.push_back(op);
+                }
+
+                return WalkResult::advance();
+            });
+
+            auto eraseUnwindOnlyOps = [&](const mlir::IRMapping &mapping) {
+                for (auto *op : unwindOnlyOps)
+                {
+                    rewriter.eraseOp(mapping.lookup(op));
+                }
+            };
+
             // add clone for 'return'
             if (returns.size() > 0)
             {
                 mlir::IRMapping returnFinallyMapping;
                 rewriter.cloneRegionBefore(tryOp.getFinally(), *continuation->getParent(), continuation->getIterator(),
                                             returnFinallyMapping);
                 propagateTsContextEntries(returnFinallyMapping);
+                eraseUnwindOnlyOps(returnFinallyMapping);
                 auto returnFinallyBlockLast = continuation->getPrevNode();
                 rewriter.setInsertionPoint(returnFinallyBlockLast->getTerminator());
                 auto resultOpOfReturnFinallyBlock = cast<mlir_ts::ResultOp>(returnFinallyBlockLast->getTerminator());
@@ -1655,6 +1686,7 @@ struct TryOpLowering : public TsPattern<mlir_ts::TryOp>
                         rewriter.cloneRegionBefore(tryOp.getFinally(), *continuation->getParent(),
                                                     continuation->getIterator(), jumpFinallyMapping);
                         propagateTsContextEntries(jumpFinallyMapping);
+                        eraseUnwindOnlyOps(jumpFinallyMapping);
                         auto jumpFinallyBlockLast = continuation->getPrevNode();
                         rewriter.setInsertionPoint(jumpFinallyBlockLast->getTerminator());
                         auto resultOpOfJumpFinallyBlock = cast<mlir_ts::ResultOp>(jumpFinallyBlockLast->getTerminator());
@@ -1679,6 +1711,11 @@ struct TryOpLowering : public TsPattern<mlir_ts::TryOp>
                 }
             }
 
+            for (auto *op : unwindOnlyOps)
+            {
+                rewriter.eraseOp(op);
+            }
+
             rewriter.inlineRegionBefore(tryOp.getFinally(), continuation);
             exitBlockLast = continuation->getPrevNode();
         }

tslang/lib/TypeScript/MLIRGenImpl.h

@@ -502,6 +502,29 @@ class MLIRGenImpl
         return false;
     }
 
+    // Whether this block declares a `let` or `const` of its own - a local that may own a
+    // reference, and that the TryOp around it would not release if an exception left the block
+    // (see mlirGen(Block)). Over-approximates: the type that decides it is not known yet.
+    bool blockDeclaresLocals(ts::Block blockAST, int skipStatements = 0)
+    {
+        auto index = 0;
+        for (auto statement : blockAST->statements)
+        {
+            if (index++ < skipStatements || (SyntaxKind)statement != SyntaxKind::VariableStatement)
+            {
+                continue;
+            }
+
+            auto flags = statement.as<VariableStatement>()->declarationList->flags;
+            if ((flags & NodeFlags::Let) == NodeFlags::Let || (flags & NodeFlags::Const) == NodeFlags::Const)
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     // Whether every top-level `using` this block declares initializes directly from
     // `new SomeClass(...)`.
     //
@@ -836,7 +859,8 @@ class MLIRGenImpl
         return mlir::success();
     }
 
-    void releaseScopeOwnedVars(mlir::Location location, const SmallVector<mlir::Value> &ownedVars)
+    void releaseScopeOwnedVars(mlir::Location location, const SmallVector<mlir::Value> &ownedVars,
+                               bool unwindOnly = false)
     {
         // reverse declaration order, the order a scope is unwound in: a later local may
         // hold the only other reference to what an earlier one points at
@@ -846,22 +870,33 @@ class MLIRGenImpl
             // released as a cell whatever its contents are owned by - the block has to go
             // back regardless. A slot that is neither is here only because it might have
             // turned into a cell and did not; see trackPossibleCell.
+            mlir::Operation *releaseOp = nullptr;
             if (isCapturedVariableCell(storage))
             {
-                builder.create<mlir_ts::ReleaseCellOp>(location, storage);
+                releaseOp = builder.create<mlir_ts::ReleaseCellOp>(location, storage);
             }
             else if (isOwnedLocalSlot(storage))
             {
-                builder.create<mlir_ts::ReleaseSlotOp>(location, storage);
+                releaseOp = builder.create<mlir_ts::ReleaseSlotOp>(location, storage);
+            }
+
+            if (releaseOp && unwindOnly)
+            {
+                releaseOp->setAttr(UNWIND_ONLY_ATTR_NAME, builder.getUnitAttr());
             }
         }
     }
 
     // The rest of the unwind leg of a catch-less TryOp's cleanup region - the one wrapped around
-    // a block or a switch for its `using`. Once the region has given back its own scope, the
-    // exception keeps unwinding, and the scopes around it that have no cleanup region of their
-    // own get no other chance: a function's body holding `let s = a + b` and, further in,
-    // `{ using r = new R(); ... }` leaked `s` on every exception through the inner block.
+    // a block or a switch for its `using`, or around a block with locals inside a try body. Once
+    // the region has given back its own scope, the exception keeps unwinding, and the scopes
+    // around it that have no cleanup region of their own get no other chance: a function's body
+    // holding `let s = a + b` and, further in, `{ using r = new R(); ... }` leaked `s` on every
+    // exception through the inner block.
+    //
+    // The same walk serves the two other places an exception leaves a scope: a `throw` statement,
+    // which raises it there, and the copy of a hand-written `finally` that runs on the unwind leg
+    // and then resumes it (`unwindOnly`, see UNWIND_ONLY_ATTR_NAME).
     //
     // Walks outwards from the scope around the wrapped one, releasing as it goes, and stops at
     // the first scope that is itself a TryOp's body (isUnwindCleanupScope) without touching its
@@ -876,17 +911,32 @@ class MLIRGenImpl
     // locals declared ahead of the wrapped scope are in the lists yet, so every slot released
     // here has had its store - or, hoisted in front of a switch whose case did not run, is
     // null, which a release does nothing with (VariableOpLowering).
-    void mlirGenReleaseOwnedOnUnwind(mlir::Location location, const GenContext *genContext)
+    void mlirGenReleaseOwnedOnUnwind(mlir::Location location, const GenContext *genContext, bool unwindOnly = false)
     {
         for (; genContext != nullptr && !genContext->isUnwindCleanupScope; genContext = genContext->parentBlockContext)
         {
             if (genContext->ownedVars != nullptr)
             {
-                releaseScopeOwnedVars(location, *genContext->ownedVars);
+                releaseScopeOwnedVars(location, *genContext->ownedVars, unwindOnly);
             }
         }
     }
 
+    // Whether an exception out of a scope opened in this context lands on a TryOp's landing pad
+    // in this function: some scope between here and the function is a TryOp's body.
+    static bool isInsideUnwindCleanupScope(const GenContext &genContext)
+    {
+        for (auto *context = &genContext; context != nullptr; context = context->parentBlockContext)
+        {
+            if (context->isUnwindCleanupScope)
+            {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
     // Does this reference address a captured variable's cell - a heap block the frame shares
     // with the closures that captured it - rather than ordinary frame storage?
     //

tslang/lib/TypeScript/MLIRGenStatements.cpp

@@ -157,6 +157,12 @@ namespace mlirgen
         // that is an exit the scope's owned locals have to be released before - see
         // blockHasTypedCatch. Same wrapping, same cleanup region, different reason to want it.
         //
+        // So are a block's own locals, once the block sits inside a TryOp's body: the TryOp's
+        // cleanup releases its body's scope and no other, so an exception out of a nested block
+        // - `try { { let s = a + b; h(); } } catch {}` - left `s` behind. The wrap's cleanup
+        // gives back the block's scope and the scopes between it and that body
+        // (mlirGenReleaseOwnedOnUnwind). Outside any TryOp nothing would run it.
+        //
         // The two remaining conditions each guard against a real, still-open bug the wrapping
         // would otherwise hit (see their comments). blockUsingInitializersAreAllNewExpr is
         // about disposing a `using`, so it only has a say when there is one - but it keeps its
@@ -167,7 +173,8 @@ namespace mlirgen
         auto declaresUsing = blockDeclaresUsing(blockAST, skipStatements);
         auto usingIsDisposable = !declaresUsing || blockUsingInitializersAreAllNewExpr(blockAST, skipStatements);
         auto rethrowNeedsCleanup = !compileOptions.isWindows && blockHasTypedCatch(blockAST, skipStatements);
-        if (usingIsDisposable && (declaresUsing || rethrowNeedsCleanup) && !blockIsInsideCatchOrFinally())
+        auto localsNeedCleanup = isInsideUnwindCleanupScope(genContext) && blockDeclaresLocals(blockAST, skipStatements);
+        if (usingIsDisposable && (declaresUsing || rethrowNeedsCleanup || localsNeedCleanup) && !blockIsInsideCatchOrFinally())
         {
             return mlirGenBlockWithUnwindCleanup(blockAST, genContext, skipStatements);
         }
@@ -1395,6 +1402,16 @@ namespace mlirgen
         EXIT_IF_FAILED_OR_NO_VALUE(result)
         auto exception = V(result);
 
+        // The exception leaves every scope from here out to the nearest TryOp body, and those
+        // scopes have no landing pad of their own to release their locals from - the function's
+        // body, a `catch` clause's enclosing scopes. So the throw gives them back itself, as a
+        // `return` would, and first takes a reference for the exception, as a `return` does for
+        // its caller: `throw s` must not be freed by the release of `s`. Nothing gives that
+        // reference back - a caught value is not owned (takeOwnershipOfLocal) - so a thrown heap
+        // value leaks rather than dangles.
+        mlirGenRetainCaptured(location, mlir::ValueRange{exception}, /*atLastUse=*/true);
+        mlirGenReleaseOwnedOnUnwind(location, &genContext);
+
         auto throwOp = builder.create<mlir_ts::ThrowOp>(location, exception);
 
         if (!genContext.allowPartialResolve)
@@ -1527,7 +1544,16 @@ namespace mlirgen
                 }
             }
 
-            auto result = mlirGen(tryStatementAST->catchClause->block, tryGenContext);
+            // With a finally, an exception out of the catch clause unwinds into the finally's
+            // unwind copy, which releases the scopes around the `try` (below) - so a `throw` in
+            // the clause must stop short of them, as it does at a TryOp's body
+            GenContext catchGenContext(tryGenContext);
+            if (tryStatementAST->finallyBlock)
+            {
+                catchGenContext.isUnwindCleanupScope = true;
+            }
+
+            auto result = mlirGen(tryStatementAST->catchClause->block, catchGenContext);
             EXIT_IF_FAILED(result)
         }
 
@@ -1540,6 +1566,14 @@ namespace mlirgen
         {
             auto result = mlirGen(tryStatementAST->finallyBlock, tryGenContext);
             EXIT_IF_FAILED(result)
+
+            // The copy of the finally that runs on the unwind leg resumes the exception past the
+            // `try`, so it is the last chance the scopes around it get, up to the nearest TryOp
+            // body, as in a catch-less TryOp's cleanup. It cannot be the cleanup region itself:
+            // that runs before the finally, which may still read them. Marked unwind-only, so
+            // TryOpLowering drops them from the copies normal completion, `return` and `break`
+            // run, where those scopes are still live.
+            mlirGenReleaseOwnedOnUnwind(location, &tryGenContext, /*unwindOnly=*/true);
         }
 
         // terminator

tslang/test/tester/CMakeLists.txt

@@ -337,6 +337,7 @@ tslang_add_test(NAME test-compile-00-labelled-scope-exit COMMAND test-runner "${
 tslang_add_test(NAME test-compile-00-switch-case-scope-exit COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00switch_case_scope_exit.ts")
 tslang_add_test(NAME test-compile-00-using-dispose-order COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_dispose_order.ts")
 tslang_add_test(NAME test-compile-00-using-unwind-outer-locals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_unwind_outer_locals.ts")
+tslang_add_test(NAME test-compile-00-try-unwind-outer-locals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_unwind_outer_locals.ts")
 tslang_add_test(NAME test-compile-00-vars COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00vars.ts")
 tslang_add_test(NAME test-compile-00-var-bindings COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00var_bindings.ts")
 tslang_add_test(NAME test-compile-00-globals COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00globals.ts")
@@ -880,6 +881,7 @@ tslang_add_test(NAME test-jit-00-labelled-scope-exit COMMAND test-runner -jit "$
 tslang_add_test(NAME test-jit-00-switch-case-scope-exit COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00switch_case_scope_exit.ts")
 tslang_add_test(NAME test-jit-00-using-dispose-order COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_dispose_order.ts")
 tslang_add_test(NAME test-jit-00-using-unwind-outer-locals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00using_unwind_outer_locals.ts")
+tslang_add_test(NAME test-jit-00-try-unwind-outer-locals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_unwind_outer_locals.ts")
 tslang_add_test(NAME test-jit-00-vars COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00vars.ts")
 tslang_add_test(NAME test-jit-00-var-bindings COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00var_bindings.ts")
 tslang_add_test(NAME test-jit-00-globals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00globals.ts")
@@ -1834,6 +1836,7 @@ set(TSLANG_CORPUS
     00switch_case_scope_exit.ts
     00using_dispose_order.ts
     00using_unwind_outer_locals.ts
+    00try_unwind_outer_locals.ts
     00capture_in_new_arguments.ts
     00cast_to_f32.ts
     00catch_untyped_value.ts