Stream: project-ffi-unwind

Topic: ffi-unwind – meaning of C


view this post on Zulip gnzlbg (Nov 08 2019 at 11:13):

@nikomatsakis thanks for writing those thoughts

view this post on Zulip gnzlbg (Nov 08 2019 at 11:15):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 11:18):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 11:19):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 11:19):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 11:20):

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).

view this post on Zulip gnzlbg (Nov 08 2019 at 11:22):

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))]).

view this post on Zulip gnzlbg (Nov 08 2019 at 11:24):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 11:24):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 11:25):

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.

view this post on Zulip nikomatsakis (Nov 08 2019 at 14:43):

@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)

view this post on Zulip gnzlbg (Nov 08 2019 at 14:44):

thanks, i'll take a look

view this post on Zulip nikomatsakis (Nov 08 2019 at 14:44):

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)

view this post on Zulip nikomatsakis (Nov 08 2019 at 14:45):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 14:58):

Yeah, me too.

view this post on Zulip gnzlbg (Nov 08 2019 at 14:58):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 14:59):

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 ?

view this post on Zulip gnzlbg (Nov 08 2019 at 15:00):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 15:01):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 15:03):

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.

view this post on Zulip gnzlbg (Nov 08 2019 at 15:03):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:04):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:04):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:04):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:05):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:06):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:06):

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

view this post on Zulip gnzlbg (Nov 08 2019 at 15:07):

(or a similar API that allows catching foreign exceptions)

view this post on Zulip gnzlbg (Nov 08 2019 at 15:08):

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).

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:06):

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.

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:07):

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

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:08):

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

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:08):

and moreover, aborting may be the right response!

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:08):

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

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:09):

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

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:09):

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

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:09):

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.

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:10):

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

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:10):

(but not in optimized builds)

view this post on Zulip nikomatsakis (Nov 08 2019 at 16:10):

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.

view this post on Zulip BatmanAoD (Kyle Strand) (Nov 09 2019 at 00:02):

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 updated: Jan 26 2022 at 08:21 UTC