Stream: wg-ffi-unwind

Topic: ffi-unwind – meaning of C


gnzlbg (Nov 08 2019 at 11:13, on Zulip):

@nikomatsakis thanks for writing those thoughts

gnzlbg (Nov 08 2019 at 11:15, on Zulip):

I think there is also the issue of how "fine grained" we want to be with respect to the checks.

gnzlbg (Nov 08 2019 at 11:18, on Zulip):

About FFI with C ( but the same could apply to Rust), users want to FFI with C APIs that unwind and also that do not unwind, and for both cases, they sometimes want to "check" that they behave like they expected.

gnzlbg (Nov 08 2019 at 11:19, on Zulip):

That is, it is a reasonable use case to be able to interface with a C API that does not unwind, ever, without any checks.

gnzlbg (Nov 08 2019 at 11:19, on Zulip):

But it is also reasonable to want to maybe add checks in debug-builds, even for those, just in case, for example.

gnzlbg (Nov 08 2019 at 11:20, on Zulip):

And from there we go to interfacing with APIs that can unwind, where it might be reasonable to insert checks with -C panic=abort to make sure they don't unwind, but where it might also be reasonable to not insert those checks in that case, or to even produce a compiler error when a user tries to compile a crate that uses them with -C panic=abort (why wait till run-time here? well, for some APIs, it can be that they do not unwind under certain conditions, and the user upholds those).

gnzlbg (Nov 08 2019 at 11:22, on Zulip):

I think that all those use cases are valid, and that some of them can be kind of addressed with the nounwind keyword, or the unwind(...) attribute (e.g. #[cfg_attr(test, unwinds(abort))]).

gnzlbg (Nov 08 2019 at 11:24, on Zulip):

The problem of using attributes for customizing this behavior is that then the types of your API (e.g. extern "C" vs extern "C unwind") might depend on whether you are doing a debug or release build.

gnzlbg (Nov 08 2019 at 11:24, on Zulip):

So maybe using a flag, e.g., like -C debug-assertions to control the behavior without changing types might be better, but then we add another "knob" to the compiler.

gnzlbg (Nov 08 2019 at 11:25, on Zulip):

Although a reasonable behavior might be to, for extern "C unwind" declarations in -C panic=abort crates to insert abort-on-unwind shims only if -C debug-assertions is enabled for the crate, reusing that knob for that.

nikomatsakis (Nov 08 2019 at 14:43, on Zulip):

@gnzlbg -- I just amended the doc a lot, but I didn't have time to read these comments of yours yet -- also, I deleted your hackmd comments from the doc since I think they might be addressed, and I don't know another way to "resolve" comments in hackmd (it's comment system kinda' sucks)

gnzlbg (Nov 08 2019 at 14:44, on Zulip):

thanks, i'll take a look

nikomatsakis (Nov 08 2019 at 14:44, on Zulip):

But it is also reasonable to want to maybe add checks in debug-builds, even for those, just in case, for example.

on this note, I totally agree, that is a worthwhile variation to consider, rather like how we handle overflow (except that we don't consider it UB in release builds)

nikomatsakis (Nov 08 2019 at 14:45, on Zulip):

well, that said, I'm not sure if I like it. I think the key concern is that -Cpanic=abort is something imposed on a libary from the outside -- if the library is only compatible with one mode without UB (because it invokes foreign functions that unwind), we probably want to consider ways for it to declare that, versus just having it silently trigger UB. It feels.. different to me. Have to think about it.

gnzlbg (Nov 08 2019 at 14:58, on Zulip):

Yeah, me too.

gnzlbg (Nov 08 2019 at 14:58, on Zulip):

That's also why I'm not sure whether delaying an error in that case until run-time is the best solution either.

gnzlbg (Nov 08 2019 at 14:59, on Zulip):

If you compile a crate with -C panic=abort and then call a extern "C unwind" { fn foo(); } then, maybe you want a compilation error ?

gnzlbg (Nov 08 2019 at 15:00, on Zulip):

A compilation error wouldn't be great either, because people might end up having to work around that by doing:

#[cfg(panic = "abort")] extern "C" { .. }
#[cfg(panic = "unwind")] extern "C unwind" { .. }

which seems unnecessary.

gnzlbg (Nov 08 2019 at 15:01, on Zulip):

Yet there are different pros/cons of the compilation-error, run-time error, or just UB cases, and for certain uses I imagine wanting some of them at one point or another, so I don't see a clear winner.

gnzlbg (Nov 08 2019 at 15:03, on Zulip):

if the library is only compatible with one mode without UB (because it invokes foreign functions that unwind), we probably want to consider ways for it to declare that, versus just having it silently trigger UB. It feels.. different to me.

I don't remember in which context this was discussed (maybe @Kyle Strand does?), but I recall that it might be worth it to allow specifying that a crate only works with -C panic=unwind, for example, at the crate level.

gnzlbg (Nov 08 2019 at 15:03, on Zulip):

So that if you try to compile it with -C panic=abort, compilation fails.

gnzlbg (Nov 08 2019 at 15:04, on Zulip):

A different strategy might also be to solve this at the library level.

gnzlbg (Nov 08 2019 at 15:04, on Zulip):

We can say that unwinding from a "C unwind" function with -C panic=abort "unwinds"

gnzlbg (Nov 08 2019 at 15:04, on Zulip):

and that this unwinding is UB if it deallocates Rust types without calling their destructors

gnzlbg (Nov 08 2019 at 15:05, on Zulip):

(otherwise, there shouldn't really be an issue with it just unwinding - a longjmp with -C panic=abort does just that)

gnzlbg (Nov 08 2019 at 15:06, on Zulip):

We could then expose an panic::abort_on_unwind<F>(F) API that can be used to call F and abort if it unwinds.

gnzlbg (Nov 08 2019 at 15:06, on Zulip):

And that can be implemented in a library, without any language support, by just using catch_unwind.

gnzlbg (Nov 08 2019 at 15:07, on Zulip):

(or a similar API that allows catching foreign exceptions)

gnzlbg (Nov 08 2019 at 15:08, on Zulip):

People can then build their own "abort_on_unwind<F>(F)"-like helpers that do what they want (e.g. abort only in debug builds).

nikomatsakis (Nov 08 2019 at 16:06, on Zulip):

The problem of using attributes for customizing this behavior is that then the types of your API (e.g. extern "C" vs extern "C unwind") might depend on whether you are doing a debug or release build.

I think if we are going with attributes, that it will not be reflected in the ABI. One or the other.

nikomatsakis (Nov 08 2019 at 16:07, on Zulip):

That's also why I'm not sure whether delaying an error in that case until run-time is the best solution either.

Yes, I agree, and this is an argument in favor of making people opt-in to unwinding

nikomatsakis (Nov 08 2019 at 16:08, on Zulip):

but, as you say, the unwinding may be something that only occurs under certain runtime conditions --

nikomatsakis (Nov 08 2019 at 16:08, on Zulip):

and moreover, aborting may be the right response!

nikomatsakis (Nov 08 2019 at 16:08, on Zulip):

i.e., the whole point of -Cpanic=abort isn't that panics and fatal errors never happen, it's the we abort when they do

nikomatsakis (Nov 08 2019 at 16:09, on Zulip):

I don't really see the advantage of "push this to the library level"

nikomatsakis (Nov 08 2019 at 16:09, on Zulip):

That just seems like the existing "C" approach to me

nikomatsakis (Nov 08 2019 at 16:09, on Zulip):

Or, put another way, that seems like it has precisely the problem that -Cpanic=abort kind of pushes UB onto libraries, and that people may fail to prepare properly for that.

nikomatsakis (Nov 08 2019 at 16:10, on Zulip):

Thinking more about the "debugmode aborts", I think that would be a good thing to do for calls to functions where it is UB for them to unwind

nikomatsakis (Nov 08 2019 at 16:10, on Zulip):

(but not in optimized builds)

nikomatsakis (Nov 08 2019 at 16:10, on Zulip):

but I don't think it's answer to the question of -Cpanic=abort inducing UB "from the outside". I think an answer there has to be one where it is hard to do it wrong.

Kyle Strand (Nov 09 2019 at 00:02, on Zulip):

I don't remember in which context this was discussed (maybe Kyle Strand does?), but I recall that it might be worth it to allow specifying that a crate only works with -C panic=unwind, for example, at the crate level.

The only discussion I remember along these lines was in RFC #2699.

Last update: Nov 15 2019 at 10:40UTC