sync adding nounwind attribute with generating abort-on-panic shim

src/librustc/hir/mod.rs

@@ -2646,9 +2646,6 @@ bitflags! {
         /// `#[rustc_allocator]`: a hint to LLVM that the pointer returned from this
         /// function is never null.
         const ALLOCATOR                 = 1 << 1;
-        /// `#[unwind]`: an indicator that this function may unwind despite what
-        /// its ABI signature may otherwise imply.
-        const UNWIND                    = 1 << 2;
         /// `#[rust_allocator_nounwind]`, an indicator that an imported FFI
         /// function will never unwind. Probably obsolete by recent changes with
         /// #[unwind], but hasn't been removed/migrated yet

src/librustc/ty/context.rs

@@ -65,7 +65,7 @@ use std::ops::{Deref, Bound};
 use std::iter;
 use std::sync::mpsc;
 use std::sync::Arc;
-use rustc_target::spec::abi;
+use rustc_target::spec::{abi, PanicStrategy};
 use rustc_macros::HashStable;
 use syntax::ast;
 use syntax::attr;
@@ -1581,6 +1581,28 @@ impl<'tcx> TyCtxt<'tcx> {
     pub fn has_strict_asm_symbol_naming(&self) -> bool {
         self.gcx.sess.target.target.arch.contains("nvptx")
     }
+
+    /// Determine if this function gets a shim to catch panics and abort.
+    pub fn abort_on_panic_shim(&self, fn_def_id: DefId, _abi: abi::Abi) -> bool {
+        // Validate `#[unwind]` syntax regardless of platform-specific panic strategy
+        let attrs = &self.get_attrs(fn_def_id);
+        let unwind_attr = attr::find_unwind_attr(Some(self.sess.diagnostic()), attrs);
+
+        // We never unwind, so it's not relevant to stop an unwind
+        if self.sess.panic_strategy() != PanicStrategy::Unwind { return false; }
+
+        // We cannot add landing pads, so don't add one
+        if self.sess.no_landing_pads() { return false; }
+
+        // For the common case, check the attribute.
+        match unwind_attr {
+            Some(attr::UnwindAttr::Allowed) => false,
+            Some(attr::UnwindAttr::Aborts) => true,
+            None =>
+                // Default: don't catch panics.
+                false, // FIXME(#58794), should be `abi != Abi::Rust && abi != Abi::RustCall`
+        }
+    }
 }
 
 impl<'tcx> TyCtxt<'tcx> {

src/librustc_codegen_llvm/attributes.rs

@@ -14,7 +14,6 @@ use rustc_data_structures::fx::FxHashMap;
 use rustc_target::spec::PanicStrategy;
 use rustc_codegen_ssa::traits::*;
 
-use crate::abi::Abi;
 use crate::attributes;
 use crate::llvm::{self, Attribute};
 use crate::llvm::AttributePlace::Function;
@@ -267,31 +266,20 @@ pub fn from_fn_attrs(
         // In panic=abort mode we assume nothing can unwind anywhere, so
         // optimize based on this!
         false
-    } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
-        // If a specific #[unwind] attribute is present, use that
-        true
     } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) {
-        // Special attribute for allocator functions, which can't unwind
+        // Special attribute for allocator functions, which can't unwind.
         false
     } else if let Some(id) = id {
         let sig = cx.tcx.normalize_erasing_late_bound_regions(ty::ParamEnv::reveal_all(), &sig);
-        if cx.tcx.is_foreign_item(id) {
-            // Foreign items like `extern "C" { fn foo(); }` are assumed not to
-            // unwind
-            false
-        } else if sig.abi != Abi::Rust && sig.abi != Abi::RustCall {
-            // Any items defined in Rust that *don't* have the `extern` ABI are
-            // defined to not unwind. We insert shims to abort if an unwind
-            // happens to enforce this.
+        if cx.tcx.abort_on_panic_shim(id, sig.abi) {
+            // Since we are adding a shim to abort on panic, this cannot unwind.
             false
         } else {
-            // Anything else defined in Rust is assumed that it can possibly
-            // unwind
+            // Anything else defined in Rust and can possibly unwind.
             true
         }
     } else {
-        // assume this can possibly unwind, avoiding the application of a
-        // `nounwind` attribute below.
+        // Assume this can possibly unwind.
         true
     });
 

src/librustc_mir/build/mod.rs

@@ -11,11 +11,9 @@ use rustc::middle::region;
 use rustc::mir::*;
 use rustc::ty::{self, Ty, TyCtxt};
 use rustc::util::nodemap::HirIdMap;
-use rustc_target::spec::PanicStrategy;
 use rustc_data_structures::indexed_vec::{IndexVec, Idx};
 use std::u32;
 use rustc_target::spec::abi::Abi;
-use syntax::attr::{self, UnwindAttr};
 use syntax::symbol::kw;
 use syntax_pos::Span;
 
@@ -485,29 +483,6 @@ macro_rules! unpack {
     };
 }
 
-fn should_abort_on_panic(tcx: TyCtxt<'_>, fn_def_id: DefId, abi: Abi) -> bool {
-    // Not callable from C, so we can safely unwind through these
-    if abi == Abi::Rust || abi == Abi::RustCall { return false; }
-
-    // Validate `#[unwind]` syntax regardless of platform-specific panic strategy
-    let attrs = &tcx.get_attrs(fn_def_id);
-    let unwind_attr = attr::find_unwind_attr(Some(tcx.sess.diagnostic()), attrs);
-
-    // We never unwind, so it's not relevant to stop an unwind
-    if tcx.sess.panic_strategy() != PanicStrategy::Unwind { return false; }
-
-    // We cannot add landing pads, so don't add one
-    if tcx.sess.no_landing_pads() { return false; }
-
-    // This is a special case: some functions have a C abi but are meant to
-    // unwind anyway. Don't stop them.
-    match unwind_attr {
-        None => false, // FIXME(#58794)
-        Some(UnwindAttr::Allowed) => false,
-        Some(UnwindAttr::Aborts) => true,
-    }
-}
-
 ///////////////////////////////////////////////////////////////////////////
 /// the main entry point for building MIR for a function
 
@@ -599,7 +574,7 @@ where
     let source_info = builder.source_info(span);
     let call_site_s = (call_site_scope, source_info);
     unpack!(block = builder.in_scope(call_site_s, LintLevel::Inherited, |builder| {
-        if should_abort_on_panic(tcx, fn_def_id, abi) {
+        if tcx.abort_on_panic_shim(fn_def_id, abi) {
             builder.schedule_abort();
         }
 

src/librustc_typeck/collect.rs

@@ -2527,8 +2527,6 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs {
             codegen_fn_attrs.flags |= CodegenFnAttrFlags::COLD;
         } else if attr.check_name(sym::rustc_allocator) {
             codegen_fn_attrs.flags |= CodegenFnAttrFlags::ALLOCATOR;
-        } else if attr.check_name(sym::unwind) {
-            codegen_fn_attrs.flags |= CodegenFnAttrFlags::UNWIND;
         } else if attr.check_name(sym::ffi_returns_twice) {
             if tcx.is_foreign_item(id) {
                 codegen_fn_attrs.flags |= CodegenFnAttrFlags::FFI_RETURNS_TWICE;

src/test/codegen/extern-functions.rs

@@ -4,16 +4,38 @@
 #![feature(unwind_attributes)]
 
 extern {
-// CHECK: Function Attrs: nounwind
-// CHECK-NEXT: declare void @extern_fn
+// CHECK-NOT: nounwind
+// CHECK: declare void @extern_fn
     fn extern_fn();
-// CHECK-NOT: Function Attrs: nounwind
+// CHECK-NOT: nounwind
 // CHECK: declare void @unwinding_extern_fn
     #[unwind(allowed)]
     fn unwinding_extern_fn();
+// CHECK: Function Attrs: nounwind
+// CHECK-NEXT: declare void @aborting_extern_fn
+    #[unwind(aborts)]
+    fn aborting_extern_fn();
+}
+
+extern "Rust" {
+// CHECK-NOT: nounwind
+// CHECK: declare void @rust_extern_fn
+    fn rust_extern_fn();
+// CHECK-NOT: nounwind
+// CHECK: declare void @rust_unwinding_extern_fn
+    #[unwind(allowed)]
+    fn rust_unwinding_extern_fn();
+// CHECK: Function Attrs: nounwind
+// CHECK-NEXT: declare void @rust_aborting_extern_fn
+    #[unwind(aborts)]
+    fn rust_aborting_extern_fn();
 }
 
 pub unsafe fn force_declare() {
     extern_fn();
     unwinding_extern_fn();
+    aborting_extern_fn();
+    rust_extern_fn();
+    rust_unwinding_extern_fn();
+    rust_aborting_extern_fn();
 }

src/test/codegen/nounwind-extern.rs

@@ -2,5 +2,8 @@
 
 #![crate_type = "lib"]
 
+// The `nounwind` attribute does not get added by rustc; it is present here because LLVM
+// analyses determine that this function does not unwind.
+
 // CHECK: Function Attrs: norecurse nounwind
 pub extern fn foo() {}

src/test/codegen/unwind-extern.rs

@@ -0,0 +1,15 @@
+// compile-flags: -C opt-level=0
+
+#![crate_type = "lib"]
+#![feature(unwind_attributes)]
+
+// make sure these all do *not* get the attribute
+// CHECK-NOT: nounwind
+
+pub extern fn foo() {} // right now we don't abort-on-panic, so we also shouldn't have `nounwind`
+#[unwind(allowed)]
+pub extern fn foo_allowed() {}
+
+pub extern "Rust" fn bar() {}
+#[unwind(allowed)]
+pub extern "Rust" fn bar_allowed() {}

src/test/ui/abi/abort-on-c-abi.rs → src/test/ui/abi/abort-on-panic.rs

@@ -19,22 +19,43 @@ extern "C" fn panic_in_ffi() {
     panic!("Test");
 }
 
+#[unwind(aborts)]
+extern "Rust" fn panic_in_rust_abi() {
+    panic!("TestRust");
+}
+
 fn test() {
     let _ = panic::catch_unwind(|| { panic_in_ffi(); });
     // The process should have aborted by now.
     io::stdout().write(b"This should never be printed.\n");
     let _ = io::stdout().flush();
 }
 
+fn testrust() {
+    let _ = panic::catch_unwind(|| { panic_in_rust_abi(); });
+    // The process should have aborted by now.
+    io::stdout().write(b"This should never be printed.\n");
+    let _ = io::stdout().flush();
+}
+
 fn main() {
     let args: Vec<String> = env::args().collect();
     if args.len() > 1 && args[1] == "test" {
         return test();
     }
+    if args.len() > 1 && args[1] == "testrust" {
+        return testrust();
+    }
 
     let mut p = Command::new(&args[0])
                         .stdout(Stdio::piped())
                         .stdin(Stdio::piped())
                         .arg("test").spawn().unwrap();
     assert!(!p.wait().unwrap().success());
+
+    let mut p = Command::new(&args[0])
+                        .stdout(Stdio::piped())
+                        .stdin(Stdio::piped())
+                        .arg("testrust").spawn().unwrap();
+    assert!(!p.wait().unwrap().success());
 }