Stream: project-ffi-unwind

Topic: next steps


view this post on Zulip BatmanAoD (Kyle Strand) (Mar 04 2020 at 20:42):

@nikomatsakis Sorry about the confusion regarding the meeting on Monday. How do you suggest we take our next steps? Should we attempt to schedule another meeting with the lang team?

view this post on Zulip nikomatsakis (Mar 05 2020 at 15:02):

@BatmanAoD (Kyle Strand) my bad, too

view this post on Zulip nikomatsakis (Mar 05 2020 at 15:02):

yeah, we ought to reschedule

view this post on Zulip nikomatsakis (Mar 05 2020 at 15:02):

I am behind on scheduling meetings this month, it's been "a week"

view this post on Zulip nikomatsakis (Mar 05 2020 at 15:02):

What were the weeks that worked well for unwind folks?

view this post on Zulip Amanieu (Mar 05 2020 at 16:17):

I'm away next week

view this post on Zulip Amanieu (Mar 05 2020 at 16:18):

@nikomatsakis We do have some notes from the meeting though: https://hackmd.io/rG_5ksyCTuKsjks5cHONZQ?view

view this post on Zulip Amanieu (Mar 05 2020 at 16:19):

Basically we decided that proposal 2 was strictly superior to proposal 1 (1&2 are very similar).

view this post on Zulip Amanieu (Mar 05 2020 at 16:19):

So there's only 2 & 3 left now.

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 16:20):

@nikomatsakis Thanks. I think we can just look back at the Doodle poll again: https://doodle.com/poll/d9xevh43spf6rx8n

Both the 16th and the 23rd would appear to work. @centril @acfoltzer has your availability changed since you filled in the poll?

view this post on Zulip acfoltzer (Mar 05 2020 at 16:21):

Still good for me!

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 17:52):

@nikomatsakis The lang team already has that time reserved, correct? If so I think we can assume that Centril can make it on the 16th and just go ahead and schedule that as the new time.

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 17:52):

Also, should we publish another blog post, providing the new date and removing the proposal-1 entries from the table?

view this post on Zulip Amanieu (Mar 05 2020 at 17:54):

I think we'll probably have a more productive meeting if we keep it small.

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:38):

the 16th is good I think

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:38):

let's do it

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:38):

I'll review the notes, @Amanieu

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:38):

I don't remember what is 1, 2, and 3

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:38):

We won't have @Josh Triplett I think

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:38):

but that's inevitable

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 18:38):

On the 16th?

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:39):

since they're away for 4 weeks or something on vacation

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 18:39):

Ah

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 18:39):

Okay, I was slightly worried when you said "inevitable"!

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 18:40):

Niko, since you created the Zulip poll, can you "finalize" it?

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:43):

Yes

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 18:55):

Thanks.

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:57):

I'll post an announcement too

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:57):

I'm just trying to decide if we should have any other lang-team meetings

view this post on Zulip nikomatsakis (Mar 05 2020 at 18:57):

full disclosure, I'm feeling a bit tired this month and lacking in enthusiasm :)

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 19:03):

Sorry to hear it; hope the rest of the month goes better for you! Or at least that April goes better...

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 05 2020 at 19:06):

nikomatsakis said:

I'll post an announcement too

In #t-lang?

view this post on Zulip Alex Crichton (Jun 04 2021 at 18:31):

Hey all, I've been "drafted" by Till to help finish out implementing what's necessary for C-unwind. I talked with @katelyn martin a bit, and I wanted to confirm what the final pieces are to implement in rustc. Hoping y'all can help me fill in gaps in my knowledge and let me know if anything is missing!

My impression of what's left is:

Is that right? If so that's hopefully not too much to tackle :)

One question I would have though is what the expected stabilization story for this feature is (or maybe that's still a TODO item). It looks like #![feature(c_unwind)] changes the behavior of the compiler, so if we were to stabilize it today it means that the same code would behave differently on nightly and on stable. Is this expected to be an issue? Or is it generally expected that we'll just stabilize the change in behavior and it's subtle enough that anyone affected can switch to nightly from the current stable?

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:00):

Hey @Alex Crichton !

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:00):

I've not been following the bugs super closely, I imagine @BatmanAoD (Kyle Strand) or @Amanieu can confirm

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:00):

but that sounds right

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:01):

Regarding the stabilization story, we haven't talked too much about that. I think a nice thing would be if there is a pattern people can write that is correct on both nightly and stable

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:01):

I'm not sure if that's true though!

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:01):

but really the number of affected crates here is--afaik---fairly small

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:01):

so I would guess we can work closely with them

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:01):

post some blog posts

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:01):

and make it through

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:02):

actually @BatmanAoD (Kyle Strand) do we have a canonical list? we should probably prep one :)

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:02):

or write a PSA blog post to try and get people's attention

view this post on Zulip nikomatsakis (Jun 04 2021 at 19:02):

and have them contact us

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:03):

nikomatsakis said:

actually BatmanAoD (Kyle Strand) do we have a canonical list? we should probably prep one :smile:

Hm... can we simply introduce a new GitHub issues tag for this?

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:09):

@Alex Crichton Welcome to the project; thanks for stepping in! (Even if you were "voluntold"...)

For bug 2, the real complication, in my mind, is truly external functions. We want to guarantee that even if an exception enters the Rust runtime from C++ (or equivalent), the runtime is aborted. I have been assuming that this needs to be implemented via some sort of shim that would be linked in every callee instead of linking the actual external function.

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:10):

oh I thought it would be as simple as just having an aborting landing pad for all function calls with an ABI that can unwind?

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:11):

Oh, maybe! Yes, I think that would have the desired effect.

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:12):

that may not catch everything since the personality may still filter stuff, but we could in theory adjust personalities at some point in the future to be a catch-all for C++ exceptions or something reasonable like that

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:12):

So, in abort=panic, any function that _calls_ an extern "C" function would include a landing pad.

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:12):

oh? I thought it was only calling extern "C-unwind"?

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:12):

(in that extern "C" is an unsafe assertion that the function doesn't throw)

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:12):

Er, right, sorry.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:13):

Sorry, I'm multitasking and should just wait to give you info until I have taken the time to restore my focus here!

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:13):

lol no worries

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:14):

if the transition story is still a bit up in the air I don't mind helping out with that as well, and the known use cases relying on unwinding would be good to review for that as well (mostly for my own edification)

view this post on Zulip nagisa (Jun 04 2021 at 19:19):

We do have a feature where we abort when there's an unwind across extern "C" happening, though, don't we?

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:21):

yeah if #![feature(c_unwind)] is specified that's the main behavior change from stable

view this post on Zulip Alex Crichton (Jun 04 2021 at 19:22):

e.g. the codegen for "bug 1" above changes if you add that feature gate (and this is what I figured was also a question about the transition story)

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 04 2021 at 19:25):

Yes, panic! will cause an abort if it would otherwise escape from an extern "C" function.

view this post on Zulip Alex Crichton (Jun 08 2021 at 23:13):

ok I have created https://github.com/rust-lang/rust/pull/86155 with what I believe are the next steps

view this post on Zulip Alex Crichton (Jun 08 2021 at 23:14):

I also think that a reasonable stabilization story, given that PR, would be to stabilize C-unwind before we change the behavior of C, then after awhile once that's stable we change C's behavior. That would allow projects to span all three channels if necessary. After that PR C behaves like C-unwind will in the future (more or less modulo some minor details)

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2021 at 00:12):

I'll try to look at that PR soon, but from your description it sounds like exactly what's needed; thank you!

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2021 at 00:12):

(And yes, I agree that treating C like C-unwind for now is the correct path.)

view this post on Zulip Alex Crichton (Jun 09 2021 at 00:27):

I haven't implemented this, but one thought I had is that we could treat extern "C" { ... } functions as true nounwind things, but extern "C" fn foo() {} and function pointer would all behave equivalently to C-unwind. That's an option where we may not lose as much perf relative today, but tbh the perf loss is pretty amorphous as "well LLVM may have to keep more landing pads" rather than "I saw this get NN% slower", so I'm not sure whether this would be worth it as a purely transitionary thing

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2021 at 00:55):

Wouldn't keeping the declarations nounwind preserve the bug that all cross-language unwinding is LLVM UB?

view this post on Zulip Alex Crichton (Jun 09 2021 at 01:31):

indeed, yeah, so it'd be a balance between enabling that now vs preventing a hypothetical regression. I'd be wary to "enable" that though since the goal is to enable it with C-unwind rather than C, but that's just my 2c

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 09 2021 at 01:50):

I would prefer to remove the known LLVM UB without fanfare or even an announcement, and continue to have it be "officially" UB. That way we may potentially save some end-users from bad behavior.

view this post on Zulip Alex Crichton (Jun 22 2021 at 21:38):

hm ok I think that I am in a mire and I don't know what to do any more. While my PR has had discussion I don't think it's been actually reviewed yet? I listed a few concrete questions of where I think there are actually more blockers for C-unwind stabilization. Some of the MIR level concerns mostly just need someone to read it and say "nah that's fine" because I suspect it's benign, but there's two major issues which will require follow-ups to address:

Overall I don't really know how to proceed. I feel like there's just endless discussion happening without any actual reviews or anyone weighing in on what next steps are. I'm not super keen myself on just continuing this debate, I would ideally like to proceed down a selected path.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 22 2021 at 22:11):

Yeah, I understand what you mean. The feature does change the panic=abort semantics in a rather subtle way, so I guess I'm not surprised by the issues cropping up, but I don't know who's qualified to do the MIR review.

view this post on Zulip hyd-dev (Jun 23 2021 at 09:39):

Alex Crichton said:

I believe the intention of the intrinsic is that it catches only Rust exceptions but other C++-style exceptions can unwind through try.

According to https://rust-lang.github.io/rfcs/2945-c-unwind-abi.html#unresolved-questions, "the behavior of catch_unwind when a foreign exception encounters it is currently left undefined", and it might "let the exception pass through uncaught or catch some or all foreign exceptions" in the future.

view this post on Zulip Alex Crichton (Jun 23 2021 at 15:49):

Oh bjorn3 actually clarified this point. The part I was worried about was try in panic=abort mode having the wrong codegen, but actually if a C++ exception enters Rust code it'll either be UB earlier (since something was marked as "C" when it should have been "C-unwind" or it would have been caught-to-abort by Rust's ABI anyway. I think that means the try issue is no longer an issue.

That still leaves, though, the -Clto issue and someone to sign off on the ramifications on the other MIR passes.

view this post on Zulip BatmanAoD (Kyle Strand) (Jun 23 2021 at 16:59):

Do you know who would be qualified to sign off on the MIR stuff, or would you like me to look into that?

view this post on Zulip Alex Crichton (Jun 23 2021 at 18:53):

I do not know unfortunately who would be able to sign-off on oit

view this post on Zulip Alex Crichton (Jun 23 2021 at 18:54):

My own personal reading is that the other MIR bits that look at panic=abort are not consequential and can be ignored, but ideally need to be updated in the future.

view this post on Zulip nikomatsakis (Jun 25 2021 at 08:47):

@Alex Crichton you can put the PR r? me if you want

view this post on Zulip nikomatsakis (Jun 25 2021 at 08:47):

I'll try to take a look today and decide what I think about the MIR changes


Last updated: Jan 26 2022 at 07:32 UTC