Stream: project-ffi-unwind

Topic: UB testcase


view this post on Zulip RalfJ (Mar 11 2021 at 08:19):

Could someone please confirm that https://github.com/rust-lang/miri/issues/1740 is indeed an example of UB under the ffi-unwind RFC, and as such should be detected by Miri?

view this post on Zulip simulacrum (Mar 11 2021 at 12:29):

Hm, can you clarify which example? Unwinding from "C" functions indeed will be UB, I believe- unwind(allowed) I wouldn't expect to change that, and would expect for it to go away soon.

view this post on Zulip Amanieu (Mar 11 2021 at 13:00):

@RalfJ #[unwind(allowed)] makes unwinding not UB. It has the same effect as extern "C-unwind" but will be phased out in favor of the latter.

view this post on Zulip simulacrum (Mar 11 2021 at 13:21):

Hm, do we store it into metadata? For some reason I didn't recall us doing so, but good to know if it does.

view this post on Zulip RalfJ (Mar 13 2021 at 10:05):

Amanieu said:

RalfJ #[unwind(allowed)] makes unwinding not UB.

I see, thanks.

view this post on Zulip RalfJ (Mar 13 2021 at 10:05):

Hm, but it still seems to be UB with -C panic=abort

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 13 2021 at 15:48):

Correct.

view this post on Zulip RalfJ (Mar 13 2021 at 16:25):

If that is intended to stay that way, we should probably update the unwinding UB in https://doc.rust-lang.org/reference/behavior-considered-undefined.html

view this post on Zulip RalfJ (Mar 13 2021 at 16:29):

looks like the ways to cause UB are

view this post on Zulip RalfJ (Mar 13 2021 at 16:29):

doesn't the latter make things like the Rust lua wrappers unsound with -C panic=abort? That seems like a footgun

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 13 2021 at 16:33):

Lua wrappers are force-unwound or longjmp'd over, I think.

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 13 2021 at 16:34):

Force-unwinding should not be UB, I think, but I need to check the RFC again to be sure that's explicit.

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 13 2021 at 16:35):

Is unwinding in panic=abort possible in pure Rust, or does it require FFI somewhere?

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 13 2021 at 16:42):

(I have been assuming the latter)

view this post on Zulip RalfJ (Mar 13 2021 at 18:20):

okay maybe lua is a bad example

view this post on Zulip RalfJ (Mar 13 2021 at 18:21):

BatmanAoD (Kyle Strand) said:

Is unwinding in panic=abort possible in pure Rust, or does it require FFI somewhere?

it requires FFI, but that's the point -- how do I write a safe Rust wrapper around FFI that can unwind?

view this post on Zulip RalfJ (Mar 13 2021 at 18:21):

looks like that is impossible since if someone uses my safe wrapper with -C panic=abort, we have UB

view this post on Zulip Amanieu (Mar 13 2021 at 19:36):

The plan is for extern "C-unwind" to catch foreign unwinds and turn them into aborts when building with panic=abort.

view this post on Zulip Amanieu (Mar 13 2021 at 19:36):

I'm not sure what the current behavior is.

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 13 2021 at 20:02):

That should indeed be the current behavior; the only problem is that the ABI is behind a feature gate.

view this post on Zulip RalfJ (Mar 14 2021 at 15:56):

Amanieu said:

The plan is for extern "C-unwind" to catch foreign unwinds and turn them into aborts when building with panic=abort.

opened a soundness issue for that https://github.com/rust-lang/rust/issues/83116

view this post on Zulip RalfJ (Mar 14 2021 at 15:56):

BatmanAoD (Kyle Strand) said:

That should indeed be the current behavior; the only problem is that the ABI is behind a feature gate.

I cannot see any trace of such an abort-to-unwind wrapper in the LLVM IR for that issue, am I looking at the wrong place?

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 14 2021 at 17:57):

That appears to be an oversight, then. @Katelyn Martin, we won't be able to stabilize "C-unwind" without that feature, but I don't know how to implement it or whether we have anything similar already in the compiler.

view this post on Zulip nikomatsakis (Mar 20 2021 at 11:32):

Wait, I'm trying to remember now -- were we going to put those wrappers all the time? Wasn't it the case that you can force-unwind over a frame with no dtors, at least?

view this post on Zulip nikomatsakis (Mar 20 2021 at 11:32):

presumably the RFC has the details though

view this post on Zulip bjorn3 (Mar 20 2021 at 11:33):

It should be possible to abort on regular unwinds, but still allow forced-unwinds I think.

view this post on Zulip RalfJ (Mar 20 2021 at 13:24):

nikomatsakis said:

Wait, I'm trying to remember now -- were we going to put those wrappers all the time? Wasn't it the case that you can force-unwind over a frame with no dtors, at least?

What are the LLVM rules for force-unwinding out of a function call marked nounwind?
If LLVM says this is UB, then we have the same UB in Rust, since with panic=abort we add nounwind basically everywhere.

view this post on Zulip Amanieu (Mar 20 2021 at 14:43):

C code longjmps over nounwind frames all the time. So I assume it is allowed.

view this post on Zulip Connor Horman (Mar 20 2021 at 14:43):

force-unwind is longjmp, right?

view this post on Zulip Connor Horman (Mar 20 2021 at 14:43):

Yeah, I believe C++ noexcept gets turned into nouwind as well, so same thing there.

view this post on Zulip Connor Horman (Mar 20 2021 at 14:49):

https://godbolt.org/z/597Psh
According to this CE build, it does. And in fact, longjmpitself is nouwind, so I think its safe to assume that longjmping out of a nounwind function is not UB in llvm.

view this post on Zulip bjorn3 (Mar 20 2021 at 15:45):

longjmp is only implemented as force-unwind on Windows. On all other platforms it simply restores all registers and the signal handler state. Force-unwinding uses the DWARF/SEH unwinding tables to unwind the stack and run destructors like with regular unwinding, except that it can't be catched. nounwind would cause the compiler to omit the destructors, but it will still emit the unwinding tables necessary for force-unwinding I think.

view this post on Zulip bjorn3 (Mar 20 2021 at 15:46):

https://stackoverflow.com/questions/14268080/cancelling-a-thread-that-has-a-mutex-locked-does-not-unlock-the-mutex

On GNU/Linux pthread_cancel is implemented with a special exception of type __cxxabi::__forced_unwind, so when a thread is cancelled an exception is thrown and the stack is unwound. If a mutex is locked by an RAII type then its destructor will be guaranteed to run if the stack is unwound by a __forced_unwind exception.

view this post on Zulip hyd-dev (Mar 20 2021 at 16:30):

bjorn3 said:

nounwind would cause the compiler to omit the destructors, but it will still emit the unwinding tables necessary for force-unwinding I think.

cc #69984

view this post on Zulip hyd-dev (Mar 20 2021 at 17:03):

https://bugzilla.mozilla.org/show_bug.cgi?id=1302078, the link mentioned in the comment around must_emit_unwind_tables in #69984, looks also relevant.

view this post on Zulip hyd-dev (Mar 20 2021 at 17:40):

(deleted)

view this post on Zulip RalfJ (Mar 20 2021 at 17:55):

bjorn3 said:

https://stackoverflow.com/questions/14268080/cancelling-a-thread-that-has-a-mutex-locked-does-not-unlock-the-mutex

On GNU/Linux pthread_cancel is implemented with a special exception of type __cxxabi::__forced_unwind, so when a thread is cancelled an exception is thrown and the stack is unwound. If a mutex is locked by an RAII type then its destructor will be guaranteed to run if the stack is unwound by a __forced_unwind exception.

if RAII destructors like mutex are run on this kind of unwinding, then it seems to be different from what was called "forced unwinding" in the Rust FFI WG?

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 20 2021 at 18:08):

Rust's interaction with forced unwinding is not specified; that's what the longjmp/cancelable discussion is about.

Per the "C-unwind" RFC, forced unwinding over non-POFs will never be well defined, so we don't need to worry about that case. We only need to ensure that forced unwinding over POFs is safe (and in the future, we may restrict that to POFs with a specific annotation).

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 20 2021 at 18:12):

nikomatsakis said:

Wait, I'm trying to remember now -- were we going to put those wrappers all the time? Wasn't it the case that you can force-unwind over a frame with no dtors, at least?

The wrappers should explicitly not trap on forced-unwinding.

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 20 2021 at 18:13):

But yes, when compiling with panic=abort, all _invocations_ of "C-unwind" functions would need to use such a wrapper.

view this post on Zulip bjorn3 (Mar 20 2021 at 18:31):

In case of rust force unwinding on arm ignores cleanup: https://github.com/rust-lang/rust/blob/41b315a470d583f6446599984ff9ad3bd61012b2/library/panic_unwind/src/gcc.rs#L148-L150
On other platforms I do think it runs destructors when the unwind table generated by LLVM contains an entry for the current call: https://github.com/rust-lang/rust/blob/41b315a470d583f6446599984ff9ad3bd61012b2/library/panic_unwind/src/gcc.rs#L226-L260

view this post on Zulip BatmanAoD (Kyle Strand) (Mar 20 2021 at 18:35):

@bjorn3 I don't think it matters. As I said, Rust's behavior is explicitly not specified for force unwinding over frames with cleanup, and we have no plans to specify it in the future.


Last updated: Jan 26 2022 at 07:32 UTC