Tracking issue for unwind allowed/abort

[Mark-Simulacrum]

See #52652 for the original discussion. There's also some additional discussion here: #48251.

The default was changed to abort-by-default in extern functions in this PR.

This is tracking the stabilization of the #[unwind(allowed)] (and #[unwind(abort)]) attributes.

[Mark-Simulacrum]

I've nominated this for the language team because I think we may want to consider stabilizing this "immediately." These attributes have existed for a long time and I think we're fairly confident we want something like this.

[comex]

Note that this is effectively saying that unwinding through FFI will no longer be undefined behavior in some cases; after all, there would be no point in allowing functions to be marked #[unwind(allowed)] if actually unwinding across them was necessarily undefined behavior.

The question is, which cases?

My understanding of the current implementation is that Rust-triggered unwinding behaves correctly when unwinding across C code, or across C++ code compiled with exceptions disabled, but may have issues when unwinding across C++ code that does use exceptions. That is, on platforms that support unwinding at all; WebAssembly doesn't support it unless you pass a flag to have LLVM manually emulate it, but that's just WebAssembly being broken as usual. :p I have no objection to stabilizing this subset of functionality "immediately", so 👍 on this in general.

However, I’d also like to know how hard it would be to support full interoperability between Rust and C++ unwinding – that is, allowing Rust panics to unwind across C++ code, and allowing C++ exceptions to unwind across Rust code. @alexcrichton, you seem to be the author of most of the unwinding code; can you comment on how difficult this seems from your perspective, or what the biggest obstacle might be?

One issue I know of is on Windows, where instead of using a separate personality function, Rust borrows the C++ one from msvcrt. From libpanic_unwind/seh.rs:

//! 1. The `panic` function calls the standard Windows function
//!    `_CxxThrowException` to throw a C++-like exception, triggering the
//!    unwinding process.
//! 2. All landing pads generated by the compiler use the personality function
//!    `__CxxFrameHandler3`, a function in the CRT, and the unwinding code in
//!    Windows will use this personality function to execute all cleanup code on
//!    the stack.
[..]
//! * Rust has no custom personality function, it is instead *always*
//!   `__CxxFrameHandler3`. Additionally, no extra filtering is performed, so we
//!   end up catching any C++ exceptions that happen to look like the kind we're
//!   throwing. Note that throwing an exception into Rust is undefined behavior
//!   anyway, so this should be fine.

One option would be to use a custom personality function instead of __CxxFrameHandler3, but I guess this is difficult because the implementation has some fairly complex functionality to call the right funclet; it's no more complex than what's already implemented for DWARF, but it is different and not currently implemented. Right?

Another option is to just continue treating Rust panics like C++ exceptions and fix any incompatibilities.

Looking into the implementation more…

With regard to "end up catching any C++ exceptions that happen to look like the kind we're throwing", I believe the relevant comparison is this one, which treat two _TypeDescriptor pointers as equal if (1) they are the same pointer or (2) their names are equal according to strcmp. Currently, Rust uses TypeDescriptors, one named ".PEA_K" and one named ".PA_K" of Rust panics is ".PA_K", strings which were apparently copied from the assembly output of this code because, to quote seh.rs, "I'm not actually sure what they do":

void foo() {
    uint64_t a[2] = {0, 1};
    throw a;
}

Well, .PA_K is just the mangling of unsigned __int64 *, and .PEA_K is the mangling of unsigned __int64 * __ptr64. But this could be any arbitrary string instead, which could be picked to avoid colliding with anything C++ is likely to use – though it should probably be a valid mangling in case some debugging tool tries to demangle it. For example, .?AUFooBar@@ would be the mangled name for a class FooBar; picking some suitably Rust-specific class name should be good enough.

If this is fixed:

• Unwinding C++ exceptions across Rust should work; Rust will never try to catch them as panics.
• Similarly, unwinding Rust panics across C++ code that tries to catch a specific type should work, as the type will never match.
• Unwinding Rust panics across C++ code that uses catch(...) to catch anything would result in C++ catching the panic. It doesn't seem that there's any way to prevent that while still using the C++ personality. This may be undesirable, and would differ from the behavior on other platforms, but catch(...) is a bad idea in general so it's probably not a big deal. However, the panic will not be dropped properly because the _ThrowInfo used by seh.rs does not initialize pnfnUnwind (actually pmfnUnwind, i.e. "pointer to member function"), which is supposed to point to the destructor. This can also be fixed.

[alexcrichton]

@alexcrichton, you seem to be the author of most of the unwinding code; can you comment on how difficult this seems from your perspective, or what the biggest obstacle might be?

From a technical perspective this is pretty feasible, but from a stabilization perspective is historically something we've never wanted to provide. We want the technical freedom to tweak unwinding as we see fit, which means it's not guaranteed to match what C++ does across every single platform.

[koute]

👍

I have an implementation of very fast local stack unwinding (which we use in our internal LD_PRELOAD-based memory profiler) which uses a shadow stack and trampolines to effectively cache previously unwound stack frames, and I need to clear all of the previously set trampolines when an exception gets thrown or all hell breaks loose. This needs the ability to be able to unwind through an FFI boundary. Since the default behavior was switched to abort-by-default I had to switch to nightly to get the previous behavior; it'd be nice to get this stable again.

[Mark-Simulacrum]

I am denominating this in favor of #58794.

[gnzlbg]

The question is, which cases?

IMO the only case in which we can guarantee this to work properly is if the code on the other side of the FFI is Rust compiled with the exact same toolchain.

allowing C++ exceptions to unwind across Rust code.

I'd prefer if doing that required extern "c++" { ... }.

Currently we assume that the code at the other side of extern { } and extern "C" { } is C code. This code cannot unwind and we could add LLVM nounwind attribute to it by default. The same would not be true for extern "c++" { } code, and if the Rust panic ABI differs from the C++ ABI on the platform (which probably won't be the case, but who knows), we'll need to convert panics from/to C++ when interfacing with that code unless the C++ code is nounwind (noexcept) or the extern "c++" fn Rust functions are nounwind.

[BatmanAoD]

Somehow I didn't actually know about this issue despite authoring a closely related RFC (rust-lang/rfcs#2699). Is there an actual RFC for [unwind(allow)]? I was planning on rewriting my RFC to suggest [unwind(allow)] instead of [unwind(Rust)], but if there's already such an RFC, I can work with its author instead.

[acfoltzer]

What are the best ways for interested outsiders to help move this along? The lack of stabilization here is currently making us choose between UB and using nightly for Lucet, so I'm definitely willing to get my hands dirty to help.

[BatmanAoD]

@acfoltzer

@joshtriplett just made a proposal for specification here: #63909 (comment)

So I think the discussion in that PR is the place to go if you want to either monitor progress on this or add to the discussion. (Though I don't know that "adding to the discussion" would speed things up.)

[BatmanAoD]

I think this issue should be closed now that the FFI-unwind project group exists and is working on an RFC to replace these attributes.

@koute ^ The above links will be useful to you if you are still working on the project you described and have not already heard about the project group!

[nagisa]

The attributes have been removed, closing.