Add a bunch of rustc_nounwind for c_unwind

[DianQK]

This PR fixes encountered dependency core issue in rust-lang/rust#116088.

After enabling c_unwind, consider the following code.

#![feature(c_unwind)]

#[no_mangle]
extern "C" fn foo() {
    unsafe {
        bar();
    }
}

extern "Rust" {
    fn bar();
}

Due to the lack of inlining or rustc_nounwind for bar, the core::panicking::panic_cannot_unwind function is called.

foo:
  push rax
  call qword ptr [rip + bar@GOTPCREL]
  pop rax
  ret
  call qword ptr [rip + core::panicking::panic_cannot_unwind@GOTPCREL]

Godbolt: https://rust.godbolt.org/z/Ea4rhfWcr.

In this PR, I'm checking if there are any undefined symbols belonging to core in the release build. I believe checking the debug build is meaningless. First, we won't distribute the debug build of compiler-builtins, and second, it has a lot of dependencies on core.

Since we're already checking for undefined symbols in core, I think checking the intrinsics example might not be meaningful anymore.

r? @Amanieu

[bjorn3]

Is mir inlining enough here or does it depend on LLVM's inliner?

[DianQK]

Is mir inlining enough here or does it depend on LLVM's inliner?

I think any one of them should work.

[DianQK]

Is mir inlining enough here or does it depend on LLVM's inliner?

Or are you referring to using -Zinline-mir-threshold=n? Or perhaps I missed some configuration information?

[bjorn3]

My question is if the inlining done by the MIR inliner is enough to fix this issue or if it depends on the codegen backend inlining too. AFAIK it is possible for the MIR inliner bail out on #[inline(always)] in some cases even when LLVM's inliner would still be able to inline.

[DianQK]

My question is if the inlining done by the MIR inliner is enough to fix this issue or if it depends on the codegen backend inlining too. AFAIK it is possible for the MIR inliner bail out on #[inline(always)] in some cases even when LLVM's inliner would still be able to inline.

I might understand now. We want it to work on Cranelift and GCC as well. Let me see if there is a way to experiment with it.

[RalfJung]

In case we do not actually want to inline some of these functions, marking them #[rustc_nounwind] should also help.

[DianQK]

My question is if the inlining done by the MIR inliner is enough to fix this issue or if it depends on the codegen backend inlining too. AFAIK it is possible for the MIR inliner bail out on #[inline(always)] in some cases even when LLVM's inliner would still be able to inline.

Godbolt: https://rust.godbolt.org/z/foe7s6ao1.

Currently, I'm not sure besides modifying LLVM code what other validation methods exist.

[DianQK]

In case we do not actually want to inline some of these functions, marking them #[rustc_nounwind] should also help.

Thanks. That's what I'm looking for. I thought it was written as extern "rustc-nounwind" fn foo.

[Amanieu]

If I understand correctly, this attribute needs to be applied to all fn defined in compiler-builtins? At this point wouldn't it be easier to do this in the compiler by checking self.tcx.is_compiler_builtins(LOCAL_CRATE)? This would solve the problem in a single place instead of having attributes peppered around the compiler_builtins code.

[Amanieu]

This should not have --release: this test specifically checks that debug builds (-Copt-level=0) do not reference any symbols in core. This is needed for people using -Z build-std without --release.

[DianQK]

So I understand that we might expect that only compiler-builtins is optimized.

[RalfJung]

If I understand correctly, this attribute needs to be applied to all fn defined in compiler-builtins? At this point wouldn't it be easier to do this in the compiler by checking self.tcx.is_compiler_builtins(LOCAL_CRATE)? This would solve the problem in a single place instead of having attributes peppered around the compiler_builtins code.

I was thinking of a crate-level version of #[rustc_nounwind]. But I also didn't know this crate already was so magic that tcx knows about it.

[nbdd0121]

If I understand correctly, this attribute needs to be applied to all fn defined in compiler-builtins? At this point wouldn't it be easier to do this in the compiler by checking self.tcx.is_compiler_builtins(LOCAL_CRATE)? This would solve the problem in a single place instead of having attributes peppered around the compiler_builtins code.

Good point. I'll have a try with this approach.

[RalfJung]

Sounds good! Make sure to only apply it to definitions though, not declarations.

[DianQK]

If I understand correctly, this attribute needs to be applied to all fn defined in compiler-builtins? At this point wouldn't it be easier to do this in the compiler by checking self.tcx.is_compiler_builtins(LOCAL_CRATE)? This would solve the problem in a single place instead of having attributes peppered around the compiler_builtins code.

I was thinking of a crate-level version of #[rustc_nounwind]. But I also didn't know this crate already was so magic that tcx knows about it.

I've tried this and it doesn't seem to work.

[DianQK]

Sounds good! Make sure to only apply it to definitions though, not declarations.

I have found that if I add to trait it causes problems.

[RalfJung]

I was thinking of a crate-level version of #[rustc_nounwind]. But I also didn't know this crate already was so magic that tcx knows about it.

I've tried this and it doesn't seem to work.

Yes, it's not implemented in the compiler (yet). I was proposing to implement it. (Sorry if that was not clear).

But @nbdd0121 is on the case already: rust-lang/rust#121605.

[DianQK]

I was thinking of a crate-level version of #[rustc_nounwind]. But I also didn't know this crate already was so magic that tcx knows about it.

I've tried this and it doesn't seem to work.

Yes, it's not implemented in the compiler (yet). I was proposing to implement it. (Sorry if that was not clear).

But @nbdd0121 is on the case already: rust-lang/rust#121605.

We can consider implementing it when we encounter it again.

[DianQK]

Prefer rust-lang/rust#121605.