fix nounwind attribute logic

Author: RalfJung

src/librustc_codegen_llvm/attributes.rs

@@ -268,22 +268,18 @@ pub fn from_fn_attrs(
         // optimize based on this!
         false
     } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
-        // If a specific #[unwind] attribute is present, use that
+        // If a specific #[unwind] attribute is present, use that.
+        // FIXME: We currently assume it can unwind even with `#[unwind(aborts)]`.
         true
     } else if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_ALLOCATOR_NOUNWIND) {
         // 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
+        if cx.tcx.is_foreign_item(id) && sig.abi != Abi::Rust && sig.abi != Abi::RustCall {
+            // Foreign non-Rust 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.
-            false
         } else {
             // Anything else defined in Rust is assumed that it can possibly
             // unwind

src/test/codegen/extern-functions.rs

@@ -6,14 +6,36 @@
 extern {
 // CHECK: Function Attrs: nounwind
 // CHECK-NEXT: declare void @extern_fn
-    fn extern_fn();
-// CHECK-NOT: Function Attrs: nounwind
+    fn extern_fn(); // assumed not to unwind
+// CHECK-NOT: nounwind
 // CHECK: declare void @unwinding_extern_fn
     #[unwind(allowed)]
     fn unwinding_extern_fn();
+// CHECK-NOT: nounwind
+// CHECK: declare void @aborting_extern_fn
+    #[unwind(aborts)]
+    fn aborting_extern_fn(); // FIXME: we don't have the attribute here
+}
+
+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-NOT: nounwind
+// CHECK: declare void @rust_aborting_extern_fn
+    #[unwind(aborts)]
+    fn rust_aborting_extern_fn(); // FIXME: we don't have the attribute here
 }
 
 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() {}