Undefined Behavior in safe code that unwinds out of extern "C" function

[gnzlbg]

UPDATE: this optimization is perfectly correct. It only breaks code that already has undefined behavior. This is however an issue since until #52652 is fixed we want to keep such code working.

ORIGINAL TITLE: mis-compilation of noreturn extern "C" definitions that unwind on stable and nightly

---

Originally reported here: #63909 (comment) , which might contain a fix, but that has not been verified yet.

The default behavior of the Rust language on nightly Rust was to abort when a panic tries to escape from functions using certain ABIs intended for FFI, like "C". #62603 changed this behavior on nightly Rust to match the stable Rust behavior, which let the function unwind, while still applying the nounwind attribute to these functions. When these functions return Never, they are also noreturn, and this results in mis-compilations on stable and nightly Rust. MWE:

extern "C" fn bar() -> ! { panic!("nounwind noreturn fn unwinds") }
extern "C" fn baz() -> i32 {
    if let Ok(_) = std::panic::catch_unwind(|| bar() ) {
        unsafe { std::hint::unreachable_unchecked() }  // makes IR nicer
    }
    42
}
fn main() { std::process::exit((baz() != 42) as i32); }

cargo run --release returns success, but RUSTFLAGS="-C lto=fat" cargo run --release returns failure.

I think that since #63909 removes the nounwind attribute, it should end up fixing this bug, but we should probably add a test for this somewhere.

---

The problem #62603 intended to solve is to allow Rust->C->Rust FFI where a Rust callback called from C can unwind through C back into Rust. This example can be adapted to this application, where the miscompilation persists:

// foo.cpp
using fn_type = void(*)();
[[ noreturn ]] extern "C" void foo(fn_type x);
[[ noreturn ]] extern "C" void foo(fn_type x)  { x(); /* unreachable: */ throw 0; }

// main.rs
extern "C" { fn foo(x: extern "C" fn() -> !) -> !; }
extern "C" fn bar() -> ! { panic!("nounwind noreturn fn unwinds") }
extern "C" fn baz() -> i32 {
    if let Ok(_) = std::panic::catch_unwind(|| unsafe { foo(bar) }) {
        unsafe { std::hint::unreachable_unchecked() }
    }
    42
}
fn main() {
   std::process::exit((baz() != 42) as i32);
}

AFAICT this miscompilation has always existed for Rust->C-Rust FFI.

[nikomatsakis]

Check-in from the @rust-lang/compiler meeting: I'm changing the nomination to nominate this issue for @rust-lang/lang. In particular, @joshtriplett, it'd be great if you can catch up on the comments here and in #63909 and give some kind of summary, since you've been spearheading this work.

[joshtriplett]

I'm not sure why this was filed in addition to #63909; I think #63909 completely addresses this, assuming we either merge it or stabilize #[unwind] before the next beta. I'm going to close this issue as a duplicate of #63909. If you feel there's a distinct issue here that #63909 wouldn't address, please clarify that and we can always reopen it.

[gnzlbg]

This issue was filled because #62603 re-introduced this soundness bug into nightly. Is this a duplicate of some other issue? #63909 is a PR.

[RalfJung]

I agree with @gnzlbg, the existence of an unmerged PR does not usually suffice to close issues.

[joshtriplett]

@RalfJung I tend to treat PRs and issues as synonymous (a PR is just an issue with commits attached) and thus potentially redundant, but your call.

It would be nice, however, if we could avoid further splitting discussion and comments across what seems like an ever-growing number of issues and PRs.

[RalfJung]

Can this be closed due to #76570? Cc @cratelyn

[cratelyn]

Yes, that sounds correct to me @RalfJung!

[RalfJung]

Awesome, I love closing soundness issues. :)