Allow foreign exceptions to unwind through Rust code and Rust panics to unwind through FFI#65646
Conversation
|
Also note that I haven't tested this with SEH yet, so the test will probably fail on windows. Emscripten seems to be using some hack for unwinding that I don't fully understand, so I except that to fail the test as well. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
c7cd231 to
2c074a4
Compare
|
This now means that foreign exceptions are not caught by the |
|
Allowing an exception to unwind to the thread boundary will cause an abort anyways at the point of the throw since it won't be able to find a catch handler. |
I think we should have a test for the error message here. Also, this is not always the case, e.g., when the exception thrown performs forced unwinding, e.g., the exception thrown by cancellation points when |
Forced unwinds work differently from normal unwinds: there is a callback that is called on every frame which allows the caller of |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
(It's unclear to me which team(s) owns what here, so I've nominated for all the relevant ones. Also, this interacts with the unwinding WG probably. cc @nikomatsakis @BatmanAoD) |
|
Since the only code that can observe this change is code that has UB I think this is strictly a T-compiler issue. |
@Amanieu in my tests, the current implementation of |
bffd426 to
40ef77e
Compare
|
I definitely think this falls squarely under the nascent "wg-ffi-unwind". |
|
I don't have a Windows machine available for testing; can anyone confirm whether this approach works for SEH? (Also, does anyone know what the current behavior of |
|
I'm going to let bors run the tests on the Windows targets since they are all tier 1. @bors try |
|
⌛ Trying commit 40ef77edc067975666ec3bcd62e80d7684512543 with merge 0eb353bac089a59e81b031c52c99a3fcef84cd8d... |
|
@Amanieu In fact, @alexcrichton fixed some logic along those lines some time back. I was concerned about your statement above that you expect this patch to fail on Windows, but I look forward to bors showing us that your pessimism was unsubstantiated 😄 |
|
☀️ Try build successful - checks-azure |
|
I know previously when we didn't abort on unwinding across the FFI barrier, C++ exceptions would call Rust drop code just fine and vice versa on |
|
@bors try |
|
⌛ Trying commit b8ccab341d0d6ed9fc3efa72aa0eb3bf1bc88793 with merge 1879eb5440b3f015d11e77e9c3b4f80a7c390384... |
|
💔 Test failed - checks-azure |
|
@bors r=nikomatsakis |
|
📌 Commit 52a3ae5ea6a99ff25f6e37087ae2b91d3f365f89 has been approved by |
|
⌛ Testing commit 52a3ae5ea6a99ff25f6e37087ae2b91d3f365f89 with merge 09033866b52e4f7927b0e35903c2e2c1bfddf4a7... |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-azure |
|
@bors r=nikomatsakis |
|
📌 Commit 003e996 has been approved by |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
💔 Test failed - checks-azure |
This allows catch_panic to ignore C++ exceptions.
|
@bors r=nikomatsakis |
|
📌 Commit f223e0d has been approved by |
|
☀️ Test successful - checks-azure |
This PR fixes interactions between Rust panics and foreign (mainly C++) exceptions.
C++ exceptions (and other FFI exceptions) can now safely unwind through Rust code:
#[unwind(allowed)]. If this is not the case then LLVM may optimize landing pads away with the assumption that they are unreachable.catch_unwindwill not catch the exception, instead the exception will silently continue unwinding past it.Rust panics can now safely unwind through C++ code:
catch (...), after which it can be either rethrown or discarded.Tests have been added to ensure all of the above works correctly.
Some notes about non-C++ exceptions:
pthread_cancelandpthread_exituse unwinding on glibc. This has the same behavior as a C++ exception: destructors are run but it cannot be caught bycatch_unwind.longjmpon Windows is implemented using unwinding. Destructors are run on MSVC, but not on MinGW. In both cases the unwind cannot be caught bycatch_unwind.#[unwind(allowed)], otherwise LLVM will optimize out the destructors since they seem unreachable.I haven't updated any of the documentation, so officially unwinding through FFI is still UB. However this is a step towards making it well-defined.
Fixes #65441
cc @gnzlbg
r? @alexcrichton