Repository navigation
Windows: a try's cleanup hands the exception straight to its catch (#457, #456) - #472
Merged
Merged
Conversation
, #456) A try whose scope cleans up on the way out - an owning `let` (its release), or a `using` - lost its exception on Windows when the function also allocated on the stack dynamically, as a two-argument `print` does: `let t = s; print("in try", t.v); throw 1;` terminated (0x80000003) instead of running the catch, in every model, JIT and AOT. So did a `let` whose initializer throws (#456): it is the same cleanup. Win32ExceptionPass turned the cleanup's exit into `cleanupret` to a new catch-all funclet that rethrew (`_CxxThrowException(null, null)`), and that funclet unwound to the real handler. With a dynamic alloca in the function the rethrow was never caught by the handler. Hand-editing the IR to `cleanupret unwind label <handler>` - clang's shape - fixed it; keeping the rethrow and making it an invoke did not. Now a cleanup with a handler to go on to unwinds straight to it, and the invokes inside the cleanup unwind there too. The catch-all funclet is still built, for a cleanup with no handler in the function (which unwinds to the caller), and is left unreachable otherwise. Linux is untouched. Test: 00try_cleanup_then_catch (compile and JIT, and in the rc/none corpus): an owning let and a two-argument print in the try, a two-argument print in the catch, a using with a two-argument print in the catch, a let whose initializer throws and one whose cast throws (#456), with a const as the control. It terminated before the change. Fixes #457. Fixes #456. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #457. Fixes #456.
On Windows, a
trywhose scope cleans up on the way out lost its exception whenever the function also allocated on the stack dynamically, as a two-argumentprintdoes. The cleanup comes from an owninglet(its release) or ausing. For example,let t = s; print("in try", t.v); throw 1;terminated with 0x80000003 instead of running thecatch. This happened in every model, under JIT and AOT. Aletwhose initializer throws (#456) is the same cleanup and failed the same way.Cause
Win32ExceptionPassturned the cleanup's exit into acleanupretto a new catch-all funclet that rethrew (_CxxThrowException(null, null)), and that funclet unwound to the real handler. With a dynamic alloca in the function, the handler never caught the rethrow. Two hand edits of the LLVM IR settled it:cleanupret unwind label <handler>, clang's shape: fixed it.invoketo the handler: did not fix it.Fix
A cleanup with a handler to go on to now unwinds straight to it, and the invokes inside the cleanup unwind there too. The catch-all funclet is still built for a cleanup with no handler in the function, which unwinds to the caller; otherwise it is left unreachable. Linux is untouched.
Tests
00try_cleanup_then_catch.ts, registered as compile and JIT tests and in the rc/none corpus. It terminated before the change. It covers:letand a two-argumentprintin thetry;printin thecatch;usingwith a two-argumentprintin thecatch;letwhose initializer throws, and one whose cast throws (A let in try whose initializer throws is not caught (stack overflow / uncaught exception) #456);constas the control.test-compile-gc-defaultlib-collector, the known local LNK2005 against the stale installed default library, which fails on main the same way. The existing EH tests all pass:00try_*, nested catch,usingin a catch.🤖 Generated with Claude Code