Uh oh!
There was an error while loading. Please reload this page.
Allow foreign exceptions to unwind through Rust code and Rust panics to unwind through FFI - #65646
Conversation
Amanieu
commented
Oct 21, 2019
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. |
rust-highfive
commented
Oct 21, 2019
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
2c074a4Comparegnzlbg
commented
Oct 21, 2019
This now means that foreign exceptions are not caught by the |
Amanieu
commented
Oct 21, 2019
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. |
Uh oh!
There was an error while loading. Please reload this page.
gnzlbg
commented
Oct 21, 2019
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 |
Amanieu
commented
Oct 21, 2019
Forced unwinds work differently from normal unwinds: there is a callback that is called on every frame which allows the caller of |
rust-highfive
commented
Oct 21, 2019
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 |
Centril
commented
Oct 21, 2019
(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) |
gnzlbg
commented
Oct 21, 2019
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
40ef77eComparenikomatsakis
commented
Oct 21, 2019
I definitely think this falls squarely under the nascent "wg-ffi-unwind". |
BatmanAoD
commented
Oct 21, 2019
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 |
Amanieu
commented
Oct 21, 2019
I'm going to let bors run the tests on the Windows targets since they are all tier 1. @bors try |
bors
commented
Oct 21, 2019
⌛ Trying commit 40ef77edc067975666ec3bcd62e80d7684512543 with merge 0eb353bac089a59e81b031c52c99a3fcef84cd8d... |
@Amanieu In fact, @alexcrichtonfixed 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 😄 |
bors
commented
Oct 22, 2019
☀️ Try build successful - checks-azure |
retep998
commented
Oct 22, 2019
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 |
Amanieu
commented
Oct 22, 2019
@bors try |
bors
commented
Oct 22, 2019
⌛ Trying commit b8ccab341d0d6ed9fc3efa72aa0eb3bf1bc88793 with merge 1879eb5440b3f015d11e77e9c3b4f80a7c390384... |
bors
commented
Nov 2, 2019
💔 Test failed - checks-azure |
Amanieu
commented
Nov 2, 2019
@bors r=nikomatsakis |
bors
commented
Nov 2, 2019
📌 Commit 52a3ae5ea6a99ff25f6e37087ae2b91d3f365f89 has been approved by |
bors
commented
Nov 2, 2019
⌛ Testing commit 52a3ae5ea6a99ff25f6e37087ae2b91d3f365f89 with merge 09033866b52e4f7927b0e35903c2e2c1bfddf4a7... |
rust-highfive
commented
Nov 2, 2019
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 |
bors
commented
Nov 2, 2019
💔 Test failed - checks-azure |
Amanieu
commented
Nov 2, 2019
@bors r=nikomatsakis |
bors
commented
Nov 2, 2019
📌 Commit 003e996 has been approved by |
bors
commented
Nov 3, 2019
rust-highfive
commented
Nov 3, 2019
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 |
bors
commented
Nov 3, 2019
💔 Test failed - checks-azure |
This allows catch_panic to ignore C++ exceptions.
Amanieu
commented
Nov 3, 2019
@bors r=nikomatsakis |
bors
commented
Nov 3, 2019
📌 Commit f223e0d has been approved by |
bors
commented
Nov 3, 2019
bors
commented
Nov 3, 2019
☀️ 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