Stream: project-ffi-unwind

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


Amanieu (Jan 03 2020 at 07:15, on Zulip):

In @nikomatsakis's draft he says:

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

Amanieu (Jan 03 2020 at 07:16, on Zulip):

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

Amanieu (Jan 03 2020 at 07:17, on Zulip):

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

Amanieu (Jan 03 2020 at 07:18, on Zulip):

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.

Amanieu (Jan 03 2020 at 07:19, on Zulip):

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

Amanieu (Jan 03 2020 at 07:21, on Zulip):

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.

Amanieu (Jan 03 2020 at 07:22, on Zulip):

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

Amanieu (Jan 03 2020 at 07:23, on Zulip):

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.

Amanieu (Jan 03 2020 at 07:30, on Zulip):

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.

Amanieu (Jan 03 2020 at 07:36, on Zulip):

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.
simulacrum (Jan 03 2020 at 10:33, on Zulip):

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

simulacrum (Jan 03 2020 at 10:34, on Zulip):

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

nikomatsakis (Jan 03 2020 at 13:43, on Zulip):

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?

nikomatsakis (Jan 03 2020 at 13:43, on Zulip):

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.

nikomatsakis (Jan 03 2020 at 13:45, on Zulip):

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?

nikomatsakis (Jan 03 2020 at 13:47, on Zulip):

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

Amanieu (Jan 03 2020 at 14:08, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:14, on Zulip):

Control over what exceptions we intercept?

nikomatsakis (Jan 03 2020 at 14:18, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:20, on Zulip):

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.

Amanieu (Jan 03 2020 at 14:24, on Zulip):

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.

Amanieu (Jan 03 2020 at 14:25, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:27, on Zulip):

LLVM does this? Or our LLVM backend does this?

nikomatsakis (Jan 03 2020 at 14:28, on Zulip):

Do C++ exceptions not count as SEH exceptions?

nikomatsakis (Jan 03 2020 at 14:28, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:30, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:30, on Zulip):

whereas in release mode they are simply compiled out

nikomatsakis (Jan 03 2020 at 14:31, on Zulip):

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

Amanieu (Jan 03 2020 at 14:34, on Zulip):

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.

nikomatsakis (Jan 03 2020 at 14:35, on Zulip):

I see.

nikomatsakis (Jan 03 2020 at 14:36, on Zulip):

Thanks

Amanieu (Jan 03 2020 at 14:37, on Zulip):

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.

Amanieu (Jan 03 2020 at 14:38, on Zulip):

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.

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

Yes, I think that is correct.

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

and checks if it originates from longjmp

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

If so, it is "re-raised"

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

and otherwise, we abort

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

it makes the shim more complex of course

nikomatsakis (Jan 03 2020 at 14:41, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:42, on Zulip):

Yes, I think that is correct.

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

nikomatsakis (Jan 03 2020 at 14:42, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:42, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:42, on Zulip):

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

Amanieu (Jan 03 2020 at 14:43, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:43, on Zulip):

right

Amanieu (Jan 03 2020 at 14:43, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:43, on Zulip):

on windows

Amanieu (Jan 03 2020 at 14:43, on Zulip):

pthread_exit is on Linux

nikomatsakis (Jan 03 2020 at 14:44, on Zulip):

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

Amanieu (Jan 03 2020 at 14:44, on Zulip):

Yes

nikomatsakis (Jan 03 2020 at 14:44, on Zulip):

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

Amanieu (Jan 03 2020 at 14:44, on Zulip):

Also true

nikomatsakis (Jan 03 2020 at 14:44, on Zulip):

pthread_exit is kind of different

nikomatsakis (Jan 03 2020 at 14:45, on Zulip):

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

Amanieu (Jan 03 2020 at 14:45, on Zulip):

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

nikomatsakis (Jan 03 2020 at 14:45, on Zulip):

Yes, this rule must be guaranteed for sure.

Amanieu (Jan 03 2020 at 14:46, on Zulip):

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.

nikomatsakis (Jan 03 2020 at 14:47, on Zulip):

Presumably also true in C++

Amanieu (Jan 03 2020 at 14:49, on Zulip):

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.

nikomatsakis (Jan 03 2020 at 14:50, on Zulip):

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

mostly I imagine this same rule applies in C++

Amanieu (Jan 03 2020 at 14:52, on Zulip):

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

nikomatsakis (Jan 03 2020 at 15:02, on Zulip):

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

Amanieu (Jan 03 2020 at 15:12, on Zulip):

A few typos: "land pads" "cod"

Amanieu (Jan 03 2020 at 15:12, on Zulip):

Mostly correct

nikomatsakis (Jan 03 2020 at 15:56, on Zulip):

thanks

nikomatsakis (Jan 03 2020 at 15:56, on Zulip):

my bluetooth keyboard is dying a bit I think :)

nikomatsakis (Jan 03 2020 at 15:56, on Zulip):

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

nikomatsakis (Jan 03 2020 at 15:57, on Zulip):

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

nikomatsakis (Jan 03 2020 at 15:58, on Zulip):

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

nikomatsakis (Jan 03 2020 at 15:58, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:00, on Zulip):

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.

gnzlbg (Jan 03 2020 at 16:01, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:02, on Zulip):
struct Foo;
impl Drop for Foo { fn drop(&mut self) { } }

fn foo() {
    let f = Foo;
    unsafe { longjmp() }
}
gnzlbg (Jan 03 2020 at 16:02, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:03, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:03, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:05, on Zulip):

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 though depending on the destructor, it might be a safe operation to do.

Amanieu (Jan 03 2020 at 16:06, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:07, on Zulip):

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

gnzlbg (Jan 03 2020 at 16:07, on Zulip):

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.

gnzlbg (Jan 03 2020 at 16:12, on Zulip):

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

Kyle Strand (Jan 05 2020 at 22:47, on Zulip):

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?

Amanieu (Jan 05 2020 at 22:54, on Zulip):

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

Amanieu (Jan 05 2020 at 22:55, on Zulip):

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

Amanieu (Jan 05 2020 at 22:55, on Zulip):

(under -C panic=abort)

Amanieu (Jan 05 2020 at 22:56, on Zulip):

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.

Kyle Strand (Jan 05 2020 at 23:10, on Zulip):

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?

Kyle Strand (Jan 05 2020 at 23:10, on Zulip):

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

Kyle Strand (Jan 05 2020 at 23:35, on Zulip):

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

Kyle Strand (Jan 05 2020 at 23:36, on Zulip):

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"

Amanieu (Jan 06 2020 at 09:18, on Zulip):

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.

gnzlbg (Jan 07 2020 at 12:34, on Zulip):

@Amanieu why are we using that particular personality function ?

gnzlbg (Jan 07 2020 at 12:35, on Zulip):

There is, e.g., _CxxFrameHandler4

Amanieu (Jan 07 2020 at 12:48, on Zulip):

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

gnzlbg (Jan 07 2020 at 13:23, on Zulip):

:/

nikomatsakis (Jan 07 2020 at 22:53, on Zulip):

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 update: May 27 2020 at 23:10UTC