Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permitting "foreign" languages to dispose of Rust panics #130369

Open
BatmanAoD opened this issue Sep 14, 2024 · 9 comments
Open

Permitting "foreign" languages to dispose of Rust panics #130369

BatmanAoD opened this issue Sep 14, 2024 · 9 comments
Labels
A-panic Area: Panicking machinery C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@BatmanAoD
Copy link
Member

BatmanAoD commented Sep 14, 2024

It is currently undefined behavior for a "foreign" language (such as C++) to catch a Rust panic, regardless of what the "foreign" code then does with the exception. That is, either disposing of or rethrowing the panic are both unconditional undefined behavior.

Permitting other languages to rethrow the panic, and permitting them to dispose of the panic, are separate "features" Rust could provide. This issue is only for safe disposal.

Note that Rust can document a function that a different runtime must call in order to safely dispose of a Rust panic. Per @chorman0773, Itanium specifies this cross-language mechanism this way:

[the runtime catching the exception is] supposed to call the unwind_cleanup function stored in the _Unwind_Exception struct with _URC_FOREIGN_EXCEPTION_CAUGHT.

Currently, the main thing that the Rust runtime would need to do when notified that a panic has been disposed of by a foreign runtime is to decrement the panic count. @bjorn3, @nbdd0121, @workingjubilee, and @Amanieu may have additional context on what would be required for foreign languages to safely dispose of Rust panics.

Original discussion: rust-lang/reference#1226 (comment)

CC @rust-lang/libs-api ; CC @rust-lang/lang

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 14, 2024
@jieyouxu jieyouxu added T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. A-panic Area: Panicking machinery labels Sep 15, 2024
@RalfJung
Copy link
Member

Cc @rust-lang/opsem

Itanium specifies this cross-language mechanism this way:

(It's exception_cleanup, not unwind_cleanup.)

We currently set exception_cleanup to this function

extern "C" fn __rust_drop_panic() -> ! {
rtabort!("Rust panics must be rethrown");
}

So, we actually do something well-defined, though the error message one gets is probably a bit confusing.

@RalfJung
Copy link
Member

In fact we even have a comment saying that this message is confusing 😂

// A foreign Rust exception, treat it slightly differently from other
// foreign exceptions, because call into `_Unwind_DeleteException` will
// call into `__rust_drop_panic` which produces a confusing
// "Rust panic must be rethrown" message.

So we except "foreign Rust runtimes" from the confusion, but if the unwind gets caught by a foreign non-Rust runtime we continue to show the confusing error.

@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 15, 2024
@BatmanAoD
Copy link
Member Author

Okay, so we should document that we expect foreign runtimes to call this if they catch a panic, and that the effect is currently to abort but may change in the future?

Any suggestions for a less confusing panic message?

@PatchMixolydic
Copy link
Contributor

Probably not great, but better than nothing: Rust panic caught by foreign runtime; cannot continue. aborting.

The message could also benefit from a short explanation of why the program cannot continue, even if it's just cannot continue safely/cannot continue soundly.

@nbdd0121
Copy link
Contributor

I think it's possible to implement it properly so a foreign runtime to catch Rust exception and resume normal execution, so I'd avoid "safely" or "soundly". It's just that it's not yet implemented.

@RalfJung
Copy link
Member

"caught" is the wrong term here I think; this is called when a Rust panic is discarded by a foreign runtime.

discarding Rust panic in a foreign runtime is currently not supported; aborting

@nbdd0121
Copy link
Contributor

"Caught" is used in (Itanium ABI](https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html), e.g. _URC_FOREIGN_EXCEPTION_CAUGHT is the reason code given to cleanup function.

"Discarding" is also confusing IMO because it's not immediate clear how Rust panic is discarded when somebody writes try { ... } catch(...) {}.

Would Rust panics caught by a foreign runtime and not rethrown. This is not currently supported, aborting. be a more accurate way of describing? After all, only legal operations for a caught foreign exception is (1) rethrow (2) cleanup and (0) diverge.

@RalfJung
Copy link
Member

"Caught" is used in (Itanium ABI](https://itanium-cxx-abi.github.io/cxx-abi/abi-eh.html), e.g. _URC_FOREIGN_EXCEPTION_CAUGHT is the reason code given to cleanup function.

Ah okay, I thought that term only applied in the "rethrow" case. Never mind then.

@Amanieu
Copy link
Member

Amanieu commented Sep 19, 2024

IMO we should just allow Rust panics to be caught and discarded by foreign code. The only thing we need to do is decrement the panic count in the cleanup function and everything should just work. Note that the panic count specifically only counts the number of active Rust panics (for the current Rust runtime, in case there are multiple in the address space).

Nothing needs to be done for foreign code re-throwing a Rust panic since that just acts like the catch never happened in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-panic Area: Panicking machinery C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants