Stream: project-ffi-unwind

Topic: moving toward an RFC


nikomatsakis (Mar 26 2020 at 21:13, on Zulip):

OK, so, we discussed this in today's lang team meeting. It seems like most folks at this juncture favor the "extern C unwind" alternative -- there are some summaries in the ffi-unwind section here.

nikomatsakis (Mar 26 2020 at 21:15, on Zulip):

We were saying that it seems like at this point it would make sense to make a revised RFC that states

nikomatsakis (Mar 26 2020 at 21:15, on Zulip):

I think that covers it

nikomatsakis (Mar 26 2020 at 21:21, on Zulip):

I have one hesitation, but I don't think it has to block the RFC. In particular, I've been pondering the case of foreign exceptions propagating across Rust frames. I think that ideally we'd be able to support this, but if we did decide not to -- to make it UB, essentially -- then that might push me to support the "one ABI" option. After all, in that case, there is no "new UB" involved in switching from panic=unwind to panic=abort -- foreign exceptions are UB no matter what. Basically this would (iiuc) just mean that we no longer annotate foreign functions as 'nounwind' unless we have panic=abort (because Rust panics might propagate).

(It would though still imply that changing the Rust unwinding mechanism to be distinct from the system mechanism is more costly; I don't personally think we should change the Rust unwinding mechanism anyway, so that doesn't bother me so much.)

However, it's also true that in that case we could just deprecate "C unwind" and make it synonymous with "C", so I don't really care. And ultimately I do think it's important for us to enable smoother C++ interop, and I think permitting exceptions might be an important part of that.

(All of that said I might like to include some bit of these trade-offs in the RFC)

bjorn3 (Mar 27 2020 at 09:38, on Zulip):

nikomatsakis said:

libstd is always compiled with panic=unwind, so a foreign unwind will always find a landing pad when the current thread is either the main thread for --crate-type bin, or the current thread is spawned using std::thread::spawn.

Amanieu (Mar 27 2020 at 10:05, on Zulip):

This is not related to foreign unwinding. We mark extern "C" functions as nounwind, so it is unsound if we let a panic out of them.

Amanieu (Mar 27 2020 at 10:06, on Zulip):

To clarify: this is talking about extern "C" functions written in Rust.

bjorn3 (Mar 27 2020 at 10:12, on Zulip):

Ok, I thought this was about extern "C" { fn foo(); }.

nikomatsakis (Mar 28 2020 at 01:18, on Zulip):

I guess the question is -- who wants to start writing the RFC :) @BatmanAoD (Kyle Strand) are you interested?

BatmanAoD (Kyle Strand) (Mar 28 2020 at 22:06, on Zulip):

I can take a shot at that this week.

nikomatsakis (Apr 02 2020 at 17:30, on Zulip):

@BatmanAoD (Kyle Strand) any update?

nikomatsakis (Apr 02 2020 at 17:30, on Zulip):

I'm probably not able to attend a meeting today

nikomatsakis (Apr 02 2020 at 17:30, on Zulip):

But I think we're at the "write RFC stage" anyhow

BatmanAoD (Kyle Strand) (Apr 02 2020 at 17:32, on Zulip):

I've started but haven't gotten very far.

BatmanAoD (Kyle Strand) (Apr 02 2020 at 17:34, on Zulip):

I copied various notes into the repo, too, with links to the originals. I made a separate PR, but I can just merge it without review if you don't mind.

centril (Apr 02 2020 at 18:26, on Zulip):

clarifies that all unwinding across a C ABI is UB except the case of: * forced unwind where no destructors are on stack

@nikomatsakis I don't think we sufficiently discussed forced unwind (option 1. vs. 2); there seemed to be cases where this could cause -C panic=unwind <=> -C panic=abort to generate UB whereas I got the impression that wasn't the case earlier in the meeting

BatmanAoD (Kyle Strand) (Apr 02 2020 at 18:33, on Zulip):

I thought @Amanieu had reassured me on that issue (where Proposal 1 avoids UB that's present in Proposal 2) during our meeting on 2 March, but I don't see that recorded in our notes, unfortunately. I thought I remembered a comment to the effect that it's essentially a wash for some reason...

centril (Apr 02 2020 at 18:34, on Zulip):

T-Lang pre-triage now; I'll try to look later

BatmanAoD (Kyle Strand) (Apr 02 2020 at 19:10, on Zulip):

@Amanieu do you think the initial RFC, introducing "C unwind", should specify what happens when a frame with catch_unwind is unwound by a foreign exception? Or should that be UB for now?

Amanieu (Apr 02 2020 at 19:11, on Zulip):

I personally feel that we should just specify it, but the lang team seems to be tending towards making it UB for now until a proper use case for it comes up.

BatmanAoD (Kyle Strand) (Apr 02 2020 at 19:14, on Zulip):

Is there any reason not to specify it as abort for now?

BatmanAoD (Kyle Strand) (Apr 02 2020 at 19:14, on Zulip):

I guess changing that would be _technically_ breaking...and I think that was mentioned in the PR discussion...

Amanieu (Apr 02 2020 at 19:17, on Zulip):

We only check the type of the exception in catch_unwind, so we can't guarantee that we will always abort.

BatmanAoD (Kyle Strand) (Apr 02 2020 at 19:23, on Zulip):

I'm not sure I understand - isn't that sufficient to guarantee that foreign exceptions will abort in any case where they're guaranteed to reach the catch_unwind in the first place?

Amanieu (Apr 02 2020 at 20:00, on Zulip):

Not if you have C++ calling Rust which calls C++ again, all without a catch_unwind, and the inner C++ throws an exception.

BatmanAoD (Kyle Strand) (Apr 02 2020 at 20:37, on Zulip):

Oh, I'm only asking about catch_unwind.

BatmanAoD (Kyle Strand) (Apr 02 2020 at 20:42, on Zulip):

If you want to review the spec as I've explained it in my draft RFC, here it is: https://github.com/BatmanAoD/project-ffi-unwind/blob/c-unwind-rfc/rfcs/0000-c-unwind-abi.md

BatmanAoD (Kyle Strand) (Apr 02 2020 at 21:03, on Zulip):

@nikomatsakis I forgot until just now that your above comment says the RFC should leave behavior for foreign exceptions "unspecified." Is there a reason for doing that instead of introducing the specification based on the table in the blog post?

BatmanAoD (Kyle Strand) (Apr 02 2020 at 21:23, on Zulip):

BatmanAoD (Kyle Strand) said:

I thought Amanieu had reassured me on that issue (where Proposal 1 avoids UB that's present in Proposal 2) during our meeting on 2 March, but I don't see that recorded in our notes, unfortunately. I thought I remembered a comment to the effect that it's essentially a wash for some reason...

Oh, I think I remember the reason: in the case of a forced unwind, having destructors in scope is almost certainly UB anyway. For instance, with longjmp, that is definitely true in the general case.

BatmanAoD (Kyle Strand) (Apr 03 2020 at 03:30, on Zulip):

Okay, I've got a rough draft of an RFC. Thanks to Amanieu for reviewing it already!

BatmanAoD (Kyle Strand) (Apr 03 2020 at 16:51, on Zulip):

@Amanieu @nikomatsakis Why do we need to leave the behavior of foreign exceptions entering through extern "C" { fn entrypoint(); } undefined for now? Was this discussed in the lang team meeting on 3/26? All I see in the minutes is:

Amanieu (Apr 03 2020 at 16:53, on Zulip):

Because nobody has proposed a use case for it.

Amanieu (Apr 03 2020 at 16:54, on Zulip):

So far the only people asking about FFI unwinding are the ones who want to throw Rust panics over foreign code.

BatmanAoD (Kyle Strand) (Apr 03 2020 at 16:55, on Zulip):

Hm.... I am pretty confident that Lucet uses it already

Amanieu (Apr 03 2020 at 16:59, on Zulip):

I think it's fine if we just go ahead and make foreign exceptions well-defined. We'll leave it for the RFC discussion.

BatmanAoD (Kyle Strand) (Apr 03 2020 at 16:59, on Zulip):

Okay. Should I mark those two comments in your review resolved?

Amanieu (Apr 03 2020 at 17:01, on Zulip):

Sure.

BatmanAoD (Kyle Strand) (Apr 03 2020 at 17:07, on Zulip):

:thumbs_up:

BatmanAoD (Kyle Strand) (Apr 03 2020 at 17:37, on Zulip):

@Amanieu Okay, I've changed the sections discussing forced unwind; I _think_ they're correct now. I also called out the fact that the one case where changing panic=unwind to panic=abortintroduces UB is actually already UB (even with panic=unwind) most platforms. (In practice, on all but one platform)

nikomatsakis (Apr 03 2020 at 19:18, on Zulip):

My inclination was to leave that as "not yet specified" -- it seems to me that we don't yet have a satisfactory answer about the interaction with "catch panic", for one thing. I wasn't super happy with "catch but convert to Rust panic", which seems unlikely to be what anyone wants. Abort is "ok" but ungreat, it'd be nice to at least offer an alternative that permits one to rethrow with full fidelity.

nikomatsakis (Apr 03 2020 at 19:19, on Zulip):

I guess I just don't know that we've even written out the full range of options in a way to discuss

nikomatsakis (Apr 03 2020 at 19:19, on Zulip):

I'm not entirely convinced we even want foreign exceptions to ever be able to propagate across Rust code, although I also (somewhat contradictorily) think we want to enable "fluent" C++ interaction, and it seems like that would require us to support foreign exceptions more fully

BatmanAoD (Kyle Strand) (Apr 03 2020 at 19:56, on Zulip):

I think most use cases probably wouldn't involve catch_panic, just like most uses of panic across C++ frames would use catch_panic rather than C++ catch to stop the panic unwind

BatmanAoD (Kyle Strand) (Apr 03 2020 at 19:58, on Zulip):

I think one reason I want to support C++ exceptions is just that it seems... arrogant, somehow, to make panic-unwinds the only type of exception that we permit to cross the C++/Rust boundary.

BatmanAoD (Kyle Strand) (Apr 03 2020 at 19:58, on Zulip):

"Rust's FFI doesn't prevent you from using your other languages' features" is another of those language-values that I have in my head that may not be shared universally.

BatmanAoD (Kyle Strand) (Apr 03 2020 at 19:59, on Zulip):

Also, UB in this case seems strictly worse than "it just does what you expect."

nikomatsakis (Apr 06 2020 at 21:53, on Zulip):

BatmanAoD (Kyle Strand) said:

"Rust's FFI doesn't prevent you from using your other languages' features" is another of those language-values that I have in my head that may not be shared universally.

I would call it "Rust gives seamless interop with other systems language, esp. C and C++"

nikomatsakis (Apr 06 2020 at 21:53, on Zulip):

though we're not there yet :)

nikomatsakis (Apr 06 2020 at 21:53, on Zulip):

it's true-ish for C, I think it should be true for C++

nikomatsakis (Apr 06 2020 at 21:53, on Zulip):

otoh, we do require "opt-in" for that interop elsewhere

nikomatsakis (Apr 06 2020 at 21:54, on Zulip):

but it's definitely an argument for permitting us to interop with C++ exceptions

nikomatsakis (Apr 06 2020 at 21:54, on Zulip):

(and one I find convincing)

Last update: Jun 05 2020 at 23:15UTC