Stream: project-ffi-unwind

Topic: Foreign exceptions should be UB under -C panic=abort


view this post on Zulip Amanieu (Jan 03 2020 at 07:15):

In @nikomatsakis's draft he says:

Changing the behavior from -Cpanic=unwind to -Cpanic=abort should not cause Undefined Behavior.

view this post on Zulip Amanieu (Jan 03 2020 at 07:16):

I don't think that it is possible to guarantee this in practice.

view this post on Zulip Amanieu (Jan 03 2020 at 07:17):

The main issue is the granularity at which we can catch exceptions. We basically have 2 options:

view this post on Zulip Amanieu (Jan 03 2020 at 07:18):

The only way to make foreign exceptions non-UB under -C panic=abort would be to catch all types of foreign exceptions around calls to potentially unwinding FFI functions and then abort.

view this post on Zulip Amanieu (Jan 03 2020 at 07:19):

However this means that we would abort on functions like pthread_exit and even longjmp (on Windows).

view this post on Zulip Amanieu (Jan 03 2020 at 07:21):

The current consensus is that unwinding across Rust code is allowed if there is nothing to drop in the unwound frames (i.e. unwinding is equivalent to longjmp). However if we made foreign exceptions abort under -C panic=abort then these cases would no longer work.

view this post on Zulip Amanieu (Jan 03 2020 at 07:22):

Or to rephrase the question: is unwinding allowed under -C panic=abort if there are no destructors in the unwound frames?

view this post on Zulip Amanieu (Jan 03 2020 at 07:23):

My position would be to just say that unwinding into code compiled with -C panic=abort is UB if the unwind would have run any destructors. This matches the semantics of C++ code compiled with -fno-exceptions.

view this post on Zulip Amanieu (Jan 03 2020 at 07:30):

One option that would work is to keep emitting drop code but just replacing the contents with a call to abort. This allows foreign unwinding to work fine through frames with no destructors, but aborts if any destructors are found. However you then lose the code size benefits.

view this post on Zulip Amanieu (Jan 03 2020 at 07:36):

In summary, the 3 options for -C panic=abort are:

  1. Foreign exceptions are UB if they would have caused a destructor to run.
  2. Wrap FFI calls and abort if an exception leaks out of them. This breaks longjmp and pthread_exit.
  3. Keep emitting destructors but have them simply call abort. This will increase code size.

view this post on Zulip simulacrum (Jan 03 2020 at 10:33):

For option 3, we might be able to make up the code size in the common case of "few unknown calls" by only emitting aborting destructors once for known call stacks (e.g., if you know everything will hit main without getting caught, then you can have the abort on unwind in main, right?)

view this post on Zulip simulacrum (Jan 03 2020 at 10:34):

you'd need to be a bit more careful -- it's not just call stack, I guess, but more so "destructor containing stack"

view this post on Zulip nikomatsakis (Jan 03 2020 at 13:43):

One option that would work is to keep emitting drop code but just replacing the contents with a call to abort. This allows foreign unwinding to work fine through frames with no destructors, but aborts if any destructors are found. However you then lose the code size benefits.

Well, you keep some, right?

view this post on Zulip nikomatsakis (Jan 03 2020 at 13:43):

I think it's not only longjmp but even segfaults that trigger unwinding in Windows (@Alex Crichton said something like that to me at one point) -- you are correct that we need to consider that.

view this post on Zulip nikomatsakis (Jan 03 2020 at 13:45):

Is it possible to identify those cases (e.g., by checking the type of the exception) and allow them to continue unwinding, while intercepting other things?

view this post on Zulip nikomatsakis (Jan 03 2020 at 13:47):

I sort of remember assuming something like this at some point but forgetting about it.

view this post on Zulip Amanieu (Jan 03 2020 at 14:08):

We don't really have control over that since we are using _CxxFrameHandler3 as our personality function.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:14):

Control over what exceptions we intercept?

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:18):

Presumably we could also test in the shim, though obviously that would make it more complex?

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:20):

I'm pondering how much this wrinkle changes things.

In particular, one of the advantages of the "C unwind" variant is that it identifies the "possible unwinding" cases more narrowly (though just how narrow such cases are is less than clear to me, given this annoying wrinkle of pthreads triggering unwinding from functions like read).

If we decide that foreign exceptions executing dtors if just UB, then this doesn't matter one way or the other.

If we do insert shims, it might matter, but only if we can put the shim at the call site. If, as you point out, we have to potentially catch the unwinding at any point, that's not helpful at all.

view this post on Zulip Amanieu (Jan 03 2020 at 14:24):

I had a look through LLVM and it seems that it sets a flag on the generated function to make it ignore all SEH exceptions except longjmp.

view this post on Zulip Amanieu (Jan 03 2020 at 14:25):

So longjmp and C++ exceptions will run destructors, but SEH exceptions from segfaults won't.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:27):

LLVM does this? Or our LLVM backend does this?

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:28):

Do C++ exceptions not count as SEH exceptions?

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:28):

(I'm trying to understand what you wrote; it seems like "ignores all SEH exceptions" would also mean C++ exceptions are ignored)

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:30):

Another option, incidentally, would be to say that "unwinding that may execute dtors is UB" (as you said) but -- in debug mode -- to make those dtors abort

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:30):

whereas in release mode they are simply compiled out

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:31):

So longjmp and C++ exceptions will run destructors, but SEH exceptions from segfaults won't.

This isn't true in our code, right? Or at least, with -Cpanic=abort, we strip out all the landing pads and mark everything as "no unwind", so presumably our destructors don't run

view this post on Zulip Amanieu (Jan 03 2020 at 14:34):

Sorry, I mean that when generating EH tables, the LLVM backend sets a flag to indicate that only C++ exception and longjmp are handled. This flag is read by _CxxFrameHandler3 and it indicates that all SEH exceptions except C++ exceptions and longjmp are ignored.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:35):

I see.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:36):

Thanks

view this post on Zulip Amanieu (Jan 03 2020 at 14:37):

I'm presuming here that we want longjmp to continue working correctly (i.e. it is well-defined to unwind through frames with no destructors) on both panic=unwind and panic=abort.

view this post on Zulip Amanieu (Jan 03 2020 at 14:38):

Also, we want the same to apply to pthread_exit (on Linux): it should be able to safely unwind through Rust code as long as that code has no destructors.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

Yes, I think that is correct.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

I guess @Amanieu another option would be that the "abort shim" inspects the inspection that is thrown

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

and checks if it originates from longjmp

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

If so, it is "re-raised"

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

and otherwise, we abort

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

it makes the shim more complex of course

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:41):

but it seems "locallized" to the call site at least, rather than being infectious?

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:42):

Yes, I think that is correct.

to be clear, I think it's correct that we want longjmp to work

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:42):

pthread_exit is a really interesting wrinkle that I don't know how to think about

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:42):

I don't think it's all that useful to guarantee that it works "only if there are no dtors"

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:42):

well, I mean, I guess it's potentially useful

view this post on Zulip Amanieu (Jan 03 2020 at 14:43):

Actually what I meant is "it works under -C panic=abort only if there are no dtors".

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:43):

right

view this post on Zulip Amanieu (Jan 03 2020 at 14:43):

Under -C panic=unwind it would always work, and run destructors while unwinding.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:43):

on windows

view this post on Zulip Amanieu (Jan 03 2020 at 14:43):

pthread_exit is on Linux

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:44):

I presume longjmp over a frame with dtors would be UB, as it is in C++

view this post on Zulip Amanieu (Jan 03 2020 at 14:44):

Yes

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:44):

(but on windows it would potentially be guaranteed to unwind and execute dtors)

view this post on Zulip Amanieu (Jan 03 2020 at 14:44):

Also true

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:44):

pthread_exit is kind of different

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:45):

in that you might want to declare that it executes dtors with -Cpanic=unwind , but is UB with -Cpanic=abort unless there are no dtors

view this post on Zulip Amanieu (Jan 03 2020 at 14:45):

The basic rule is: if you unwind across a Rust frame, you must execute destructors.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:45):

Yes, this rule must be guaranteed for sure.

view this post on Zulip Amanieu (Jan 03 2020 at 14:46):

Incidentally, musl's pthread_exit which doesn't unwind is still UB to use if there are frames with destructors on the stack, since pthread_exit frees the stack which implicitly acts as unwinding the frames without running their destructors.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:47):

Presumably also true in C++

view this post on Zulip Amanieu (Jan 03 2020 at 14:49):

I don't think it is explicitly said in the C++ standard (since the C++ standard library doesn't have an equivalent of pthread_exit), but probably true.

view this post on Zulip nikomatsakis (Jan 03 2020 at 14:50):

The basic rule is: if you unwind across a Rust frame, you must execute destructors.

mostly I imagine this same rule applies in C++

view this post on Zulip Amanieu (Jan 03 2020 at 14:52):

C++ does explicitly say that skipping destructors with longjmp is UB.

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:02):

@Amanieu I updated the corresponding hackmd section, take a look

view this post on Zulip Amanieu (Jan 03 2020 at 15:12):

A few typos: "land pads" "cod"

view this post on Zulip Amanieu (Jan 03 2020 at 15:12):

Mostly correct

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:56):

thanks

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:56):

my bluetooth keyboard is dying a bit I think :)

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:56):

some keys I have to press a few times or they don't register... leads to typos like "cod" :)

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:57):

(although I'm plenty prone to typos with any keyboard...)

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:58):

I was surprised @Amanieu to see you talking about pthread_exit; I thought it was pthread_cancel that sometimes used unwinding

view this post on Zulip nikomatsakis (Jan 03 2020 at 15:58):

I don't see much discussion of this in the man page

view this post on Zulip gnzlbg (Jan 03 2020 at 16:00):

The current consensus is that unwinding across Rust code is allowed if there is nothing to drop in the unwound frames (i.e. unwinding is equivalent to longjmp)

There is consensus about this.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:01):

However, there is also consensus that there are some destructors that it is not incorrect not to call.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:02):

Example:

struct Foo;
impl Drop for Foo { fn drop(&mut self) { } }

fn foo() {
    let f = Foo;
    unsafe { longjmp() }
}

view this post on Zulip gnzlbg (Jan 03 2020 at 16:02):

In fact, there are actually very few destructors that must be called for safety.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:03):

Some examples would be Pin, crossbeam::scope, etc.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:03):

For example, not calling the destructor of a Vec leaks the vector, but that's something that we already say its safe.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:05):

This is orthogonal to whether we define not calling those constructors as UB or not. We can artificially say that deallocating a stack variable without calling its destructor is UB, even if, depending on the actual destructor, the operation is actually safe.

view this post on Zulip Amanieu (Jan 03 2020 at 16:06):

@nikomatsakis pthread_exit calls __do_cancel which calls __pthread_unwind which calls _Unwind_ForcedUnwind.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:07):

I'm not sure this UB buys us any optimizations, but this would maybe buy us a more simpler specification.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:07):

I say maybe because it is, for example, possible to construct a Pin on the heap, and deallocate it without running its destructor using unsafe code. So Pin already documents that running its destructor before the Pin is deallocated is required for safety.

view this post on Zulip gnzlbg (Jan 03 2020 at 16:12):

And crossbeam scope should probably do the same if its API allows that to happen.

view this post on Zulip BatmanAoD (Kyle Strand) (Jan 05 2020 at 22:47):

We don't really have control over that since we are using _CxxFrameHandler3 as our personality function.

@Amanieu

Is writing our own personality function possible/feasible? Would that simplify the problem at all?

view this post on Zulip Amanieu (Jan 05 2020 at 22:54):

Eh, maybe, but difficult. A lot of the SEH stuff is undocumented APIs that LLVM had to reverse engineer.

view this post on Zulip Amanieu (Jan 05 2020 at 22:55):

On the other hand, we need to decide whether a C++ exception is allowed to unwind across Rust frames that have no destructors.

view this post on Zulip Amanieu (Jan 05 2020 at 22:55):

(under -C panic=abort)

view this post on Zulip Amanieu (Jan 05 2020 at 22:56):

We've got some consensus that we should allow this for longjmp and pthread_exit (forced unwind) at least, since those work with C code.

view this post on Zulip BatmanAoD (Kyle Strand) (Jan 05 2020 at 23:10):

You've mentioned "forced unwind" before (and Alex Crichton mentioned it in the PR preventing -Cpanic=abort from aborting on longjmp). Is that standard ABI nomenclature?

view this post on Zulip BatmanAoD (Kyle Strand) (Jan 05 2020 at 23:10):

My impression (from that PR) was that forced unwinding is reasonably simple to detect

view this post on Zulip BatmanAoD (Kyle Strand) (Jan 05 2020 at 23:35):

@nikomatsakis @Amanieu Come to think of it, I have a more fundamental question. Is the -Cpanic=abort issue actually related to the question of whether or not to introduce "C unwind"? Isn't it something we'll need to address either way?

view this post on Zulip BatmanAoD (Kyle Strand) (Jan 05 2020 at 23:36):

If the way we handle -Cpanic=abort isn't directly related to the "C unwind" question, then I don't think that discussion belongs in the blog post, which should remain focused on "C/unwind"

view this post on Zulip Amanieu (Jan 06 2020 at 09:18):

It is tangentially related since one of the arguments for "C unwind" was that it with panic=abort the compiler would only have to add catch blocks to "C unwind" functions instead of all "C" functions.

view this post on Zulip gnzlbg (Jan 07 2020 at 12:34):

@Amanieu why are we using that particular personality function ?

view this post on Zulip gnzlbg (Jan 07 2020 at 12:35):

There is, e.g., _CxxFrameHandler4

view this post on Zulip Amanieu (Jan 07 2020 at 12:48):

Because that's the only one that LLVM currently supports.

view this post on Zulip gnzlbg (Jan 07 2020 at 13:23):

:/

view this post on Zulip nikomatsakis (Jan 07 2020 at 22:53):

Come to think of it, I have a more fundamental question. Is the -Cpanic=abort issue actually related to the question of whether or not to introduce "C unwind"? Isn't it something we'll need to address either way?

@Kyle Strand Yes, I believe it is something we have to address either way, and the blog post tries to emphasize that


Last updated: Jan 26 2022 at 07:32 UTC