Windows: a try's cleanup hands the exception straight to its catch (#457, #456)

tslang/lib/TypeScriptExceptionPass/Win32ExceptionPass.cpp

@@ -593,10 +593,18 @@ struct Win32ExceptionPassCode
                     CurrentBB = emptyBlockBefore->getPrevNode();
                 }
 
-                CleanupReturnInst::Create(catchRegion.cleanupPad, CSIBlock, CurrentBB);
+                // A cleanup with a handler to go on to (a `using` scope, or an owning local's
+                // release, inside a try with a catch) hands the exception straight to it, as clang
+                // does: `cleanupret unwind label <handler>`. Through the catch-all funclet below,
+                // which rethrows, the exception was lost whenever the function also allocated on
+                // the stack dynamically (a two-argument `print`): the handler did not catch the
+                // rethrow, and the program terminated (#457). The funclet is still built, for a
+                // cleanup with no handler here, and is left unreachable otherwise.
+                auto *II = catchRegion.unwindInfoOp;
+                auto *handlerPad = II ? II->getUnwindDest() : nullptr;
+                CleanupReturnInst::Create(catchRegion.cleanupPad, handlerPad ? handlerPad : CSIBlock, CurrentBB);
 
                 // add rethrow code
-                auto *II = catchRegion.unwindInfoOp;
 
                 // beside the cleanup pad, in the same parent: cleanupret unwinds to it
                 auto *CSI = CatchSwitchInst::Create(parentPadOf(catchRegionsWorkSet, catchRegion),
@@ -669,12 +677,13 @@ struct Win32ExceptionPassCode
                         if (auto *II = cast<InvokeInst>(callBase))
                         {
                             // an invoke into a pad nested in this cleanup stays as it is
-                            if (II->getUnwindDest() != CSIBlock &&
+                            auto *unwindTo = handlerPad ? handlerPad : CSIBlock;
+                            if (II->getUnwindDest() != unwindTo &&
                                 !isDescendantPad(catchRegionsWorkSet, catchRegion, II->getUnwindDest()))
                             {
                                 LLVM_DEBUG(llvm::dbgs() << "\n!! FIX INVOKE(cleanup): " << *callBase << "\n");
 
-                                auto newCallBase = ToInvoke(callBase, CSIBlock, opBundleCleanup);
+                                auto newCallBase = ToInvoke(callBase, unwindTo, opBundleCleanup);
                                 callBase->replaceAllUsesWith(newCallBase);
                                 callBase->eraseFromParent();
                             }

tslang/test/tester/CMakeLists.txt

@@ -475,6 +475,7 @@ tslang_add_test(NAME test-compile-00-nullable-to-any COMMAND test-runner "${PROJ
 tslang_add_test(NAME test-compile-00-ternary-new-or-null COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts")
 tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 tslang_add_test(NAME test-compile-00-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
+tslang_add_test(NAME test-compile-00-try-cleanup-then-catch COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_cleanup_then_catch.ts")
 tslang_add_test(NAME test-compile-00-global-union-from-call COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_union_from_call.ts")
 tslang_add_test(NAME test-compile-00-capture-in-new-arguments COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts")
 tslang_add_test(NAME test-compile-01-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/01class_new.ts")
@@ -963,6 +964,7 @@ tslang_add_test(NAME test-jit-00-nullable-to-any COMMAND test-runner -jit "${PRO
 tslang_add_test(NAME test-jit-00-ternary-new-or-null COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts")
 tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 tslang_add_test(NAME test-jit-00-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
+tslang_add_test(NAME test-jit-00-try-cleanup-then-catch COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_cleanup_then_catch.ts")
 tslang_add_test(NAME test-jit-00-global-union-from-call COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_union_from_call.ts")
 tslang_add_test(NAME test-jit-00-capture-in-new-arguments COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts")
 tslang_add_test(NAME test-jit-01-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01class_new.ts")
@@ -1773,6 +1775,7 @@ set(TSLANG_CORPUS
     00if_return.ts
     00in_method_names.ts
     00in.ts
+    00try_cleanup_then_catch.ts
     00global_object_literal.ts
     00infer.ts
     00global_union_from_call.ts

tslang/test/tester/tests/00try_cleanup_then_catch.ts

@@ -0,0 +1,109 @@
+// A try whose scope cleans up on the way out (an owning `let`, a `using`) hands an exception thrown
+// in it to its catch (#457, #456). On Windows the cleanup went through a catch-all that rethrew,
+// and the catch never saw the rethrow when the function also allocated on the stack dynamically (a
+// two-argument `print`): the program terminated.
+class Node {
+    v = 0;
+}
+
+class Res {
+    [Symbol.dispose]() {
+        print("disposed");
+    }
+}
+
+function fail(): Node {
+    throw "x";
+}
+
+function letThenThrow(s: Node) {
+    let caught = false;
+    try {
+        let t = s;
+        print("in try", t.v);
+        throw 1;
+    } catch {
+        caught = true;
+    }
+
+    return caught;
+}
+
+function printInCatch() {
+    let caught = false;
+    try {
+        let t = new Node();
+        throw 1;
+    } catch {
+        print("caught", 5);
+        caught = true;
+    }
+
+    return caught;
+}
+
+function usingThenThrow() {
+    let caught = false;
+    try {
+        using r = new Res();
+        throw 1;
+    } catch {
+        print("caught", 6);
+        caught = true;
+    }
+
+    return caught;
+}
+
+function initializerThrows() {
+    let caught = false;
+    try {
+        let back = fail();
+        print("not reached", back.v);
+    } catch {
+        caught = true;
+    }
+
+    return caught;
+}
+
+function castThrows() {
+    const n: any = 5;
+    let caught = false;
+    try {
+        let back = <Node>n;
+        print("not reached", back.v);
+    } catch {
+        caught = true;
+    }
+
+    return caught;
+}
+
+function constControl(s: Node) {
+    let caught = false;
+    try {
+        const t = s;
+        print("in try", t.v);
+        throw 1;
+    } catch {
+        caught = true;
+    }
+
+    return caught;
+}
+
+function main() {
+    assert(letThenThrow(new Node()), "an owning let, a two-argument print, then a throw");
+    assert(printInCatch(), "an owning let, then a two-argument print in the catch");
+    assert(usingThenThrow(), "a using, then a two-argument print in the catch");
+    assert(initializerThrows(), "a let whose initializer throws (#456)");
+    assert(castThrows(), "a let whose cast throws (#456)");
+    assert(constControl(new Node()), "a const needs no cleanup");
+
+    for (let i = 0; i < 100; i++) {
+        assert(letThenThrow(new Node()));
+    }
+
+    print("done.");
+}