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

[discussion-only] My thoughts an MVP RFC for the full stable "C unwind" feature #9

Closed
wants to merge 16 commits into from

Conversation

gnzlbg
Copy link
Contributor

@gnzlbg gnzlbg commented Oct 15, 2019

So this is what I would consider an MVP for stabilization.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems pretty close to the "Draft RFC" I was working on. At some future point I'd like to take some of this material. Certain details I think we may wish to defer (notably, the details about catching non-Rust exceptions).

rfcs/0000-simple_unwind_annotation.md Outdated Show resolved Hide resolved
rfcs/0000-simple_unwind_annotation.md Outdated Show resolved Hide resolved
whether such unwindings can always, sometimes, or never be caught with
`catch_unwind` or not is target-dependent. If some of these unwindings that do
not originate in Rust can be caught, their value is then of type
`std::panic::ForeignException` (that is, the `Result::Err(Any)` that
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we should leave "catching of foreign exceptions" for future work and not try to say anything about it.

Copy link
Contributor Author

@gnzlbg gnzlbg Oct 16, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a complex part of the "C unwind" feature so I'm not sure either if it belongs in an MVP.

I think that if we were to say that "unwinding with a foreign exception into Rust is not allowed (UB)", then most of the complexity goes out.

What brings most of the complexity is allowing foreign exceptions to unwind into Rust, e.g., to support longjmp, or pthread_cancel, because then it would be nice if we at least document what that does: whether unwinding calls destructors, whether catch_unwind aborts, or catches, or lets foreign exceptions escape, whether catch_unwind can distinguish Rust panic from foreign exceptions, etc.

If that's a complexity we want to accept for an MVP, then being able to catch foreign exceptions adds little extra complexity, since C++ has done all of the design and implementation work for us with std::exception_ptr, and clang already implements it for the same targets that we support, while at the same time adding a lot of power for multi-threaded programs. For example, without catching foreign exceptions, when a foreign exception reaches a thread boundary the program aborts or has UB. But if you can catch them, you can actually pass that exception to the calling thread and then re-raise it and continue unwinding to fail gracefully, or just log it and directly re-start the thread, etc.

I can move this feature to the future section, but I think that as we investigate what the targets can and cannot do, we should, since we are already at it, document how foreign exceptions work or can work on each target.

conforms to the native ABI. When a panic that originates in Rust, unwinds
through a `"C unwind"` function call back into Rust, the translation is
guaranteed to be lossless _if the panic was not modified by non-Rust code_, and
this then behaves as if the panic would have never left Rust.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I included similar text in my draft RFC.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm not sure is if this "goal" is achievable everywhere (it is on the Itanium-like targets, and probably on Windows).

I worded it as a guarantee because I think we should really try to guarantee this because I have the feeling that otherwise weird things might happen but I don't know what they would be.

If most targets can support this, it might be worth it to consider whether it would be acceptable for "C unwind" to just not be available on targets in which this cannot be upheld. We can always offer a different ABI string with weaker guarantees for those.

I'm going to add this as the first unresolved question.

@BatmanAoD
Copy link
Member

Superseded by #28

@BatmanAoD BatmanAoD closed this Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants