rustc: Switch `extern` functions to abort by default on panic

This was intended to land way back in 1.24, but it was backed out due to
breakage which has long since been fixed. An unstable `#[unwind]`
attribute can be used to tweak the behavior here, but this is currently
simply switching rustc's internal default to abort-by-default if an
`extern` function panics, making our codegen sound primarily (as
currently you can produce UB with safe code)

Closes #52652

diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs
index ffe5e123f9b8b597e697aea2974618d0a038a334..30edc4744ecd4eff0f8d0b8e344e405323394b07 100644 (file)
--- a/src/librustc_codegen_llvm/attributes.rs
+++ b/src/librustc_codegen_llvm/attributes.rs
@@ -15,7 +15,7 @@
 use rustc::hir::def_id::{DefId, LOCAL_CRATE};
 use rustc::session::Session;
 use rustc::session::config::Sanitizer;
-use rustc::ty::TyCtxt;
+use rustc::ty::{self, TyCtxt, PolyFnSig};
 use rustc::ty::layout::HasTyCtxt;
 use rustc::ty::query::Providers;
 use rustc_data_structures::sync::Lrc;
@@ -23,6 +23,7 @@
 use rustc_target::spec::PanicStrategy;
 use rustc_codegen_ssa::traits::*;
 
+use abi::Abi;
 use attributes;
 use llvm::{self, Attribute};
 use llvm::AttributePlace::Function;
@@ -60,7 +61,7 @@ pub fn emit_uwtable(val: &'ll Value, emit: bool) {
 
 /// Tell LLVM whether the function can or cannot unwind.
 #[inline]
-pub fn unwind(val: &'ll Value, can_unwind: bool) {
+fn unwind(val: &'ll Value, can_unwind: bool) {
     Attribute::NoUnwind.toggle_llfn(Function, val, !can_unwind);
 }
 
@@ -150,9 +151,10 @@ pub fn non_lazy_bind(sess: &Session, llfn: &'ll Value) {
 /// Composite function which sets LLVM attributes for function depending on its AST (`#[attribute]`)
 /// attributes.
 pub fn from_fn_attrs(
-    cx: &CodegenCx<'ll, '_>,
+    cx: &CodegenCx<'ll, 'tcx>,
     llfn: &'ll Value,
     id: Option<DefId>,
+    sig: PolyFnSig<'tcx>,
 ) {
     let codegen_fn_attrs = id.map(|id| cx.tcx.codegen_fn_attrs(id))
         .unwrap_or_else(|| CodegenFnAttrs::new());
@@ -194,28 +196,37 @@ pub fn from_fn_attrs(
             llvm::AttributePlace::ReturnValue, llfn);
     }
 
-    let can_unwind = if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::UNWIND) {
-        Some(true)
+    unwind(llfn, if cx.tcx.sess.panic_strategy() != PanicStrategy::Unwind {
+        // 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) {
-        Some(false)
-
-    // Perhaps questionable, but we assume that anything defined
-    // *in Rust code* may unwind. Foreign items like `extern "C" {
-    // fn foo(); }` are assumed not to unwind **unless** they have
-    // a `#[unwind]` attribute.
-    } else if id.map(|id| !cx.tcx.is_foreign_item(id)).unwrap_or(false) {
-        Some(true)
-    } else {
-        None
-    };
-
-    match can_unwind {
-        Some(false) => attributes::unwind(llfn, false),
-        Some(true) if cx.tcx.sess.panic_strategy() == PanicStrategy::Unwind => {
-            attributes::unwind(llfn, true);
+        // 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.
+            false
+        } else {
+            // Anything else defined in Rust is assumed that it can possibly
+            // unwind
+            true
         }
-        Some(true) | None => {}
-    }
+    } else {
+        // assume this can possibly unwind, avoiding the application of a
+        // `nounwind` attribute below.
+        true
+    });
 
     // Always annotate functions with the target-cpu they are compiled for.
     // Without this, ThinLTO won't inline Rust functions into Clang generated

diff --git a/src/librustc_codegen_llvm/callee.rs b/src/librustc_codegen_llvm/callee.rs
index f13eeb6692c351de940c9127d998d99b69bcc3b9..87185a20c50913ceb3dcc895ffaa38fd5dbb043f 100644 (file)
--- a/src/librustc_codegen_llvm/callee.rs
+++ b/src/librustc_codegen_llvm/callee.rs
@@ -94,7 +94,7 @@ pub fn get_fn(
         if instance.def.is_inline(tcx) {
             attributes::inline(cx, llfn, attributes::InlineAttr::Hint);
         }
-        attributes::from_fn_attrs(cx, llfn, Some(instance.def.def_id()));
+        attributes::from_fn_attrs(cx, llfn, Some(instance.def.def_id()), sig);
 
         let instance_def_id = instance.def_id();
 

diff --git a/src/librustc_codegen_llvm/context.rs b/src/librustc_codegen_llvm/context.rs
index 6c90937de3f9baa897b9dd3d1adcd336d8474bc0..0bd6146f5aaa0e002924e625562e08c7ee1398ff 100644 (file)
--- a/src/librustc_codegen_llvm/context.rs
+++ b/src/librustc_codegen_llvm/context.rs
@@ -409,7 +409,6 @@ fn eh_unwind_resume(&self) -> &'ll Value {
         ));
 
         let llfn = self.declare_fn("rust_eh_unwind_resume", sig);
-        attributes::unwind(llfn, true);
         attributes::apply_target_cpu_attr(self, llfn);
         unwresume.set(Some(llfn));
         llfn

diff --git a/src/librustc_codegen_llvm/declare.rs b/src/librustc_codegen_llvm/declare.rs
index c23aab409a9a8d3ba906047ecf0f3abe9a4e1eb2..2964f2e58470f00497fc84320e50af16be22d852 100644 (file)
--- a/src/librustc_codegen_llvm/declare.rs
+++ b/src/librustc_codegen_llvm/declare.rs
@@ -26,8 +26,7 @@
 use rustc::ty::layout::LayoutOf;
 use rustc::session::config::Sanitizer;
 use rustc_data_structures::small_c_str::SmallCStr;
-use rustc_target::spec::PanicStrategy;
-use abi::{Abi, FnType, FnTypeExt};
+use abi::{FnType, FnTypeExt};
 use attributes;
 use context::CodegenCx;
 use type_::Type;
@@ -86,10 +85,6 @@ fn declare_raw_fn(
         _ => {},
     }
 
-    if cx.tcx.sess.panic_strategy() != PanicStrategy::Unwind {
-        attributes::unwind(llfn, false);
-    }
-
     attributes::non_lazy_bind(cx.sess(), llfn);
 
     llfn
@@ -132,10 +127,6 @@ fn declare_fn(
             llvm::Attribute::NoReturn.apply_llfn(Function, llfn);
         }
 
-        if sig.abi != Abi::Rust && sig.abi != Abi::RustCall {
-            attributes::unwind(llfn, false);
-        }
-
         fty.apply_attrs_llfn(llfn);
 
         llfn

diff --git a/src/librustc_codegen_llvm/intrinsic.rs b/src/librustc_codegen_llvm/intrinsic.rs
index 59608b119397624313a30fe7dc1228e6b83815e6..2b82ebe0bc25c5bc11afa28247d8e3aa7eed4fd0 100644 (file)
--- a/src/librustc_codegen_llvm/intrinsic.rs
+++ b/src/librustc_codegen_llvm/intrinsic.rs
@@ -1081,7 +1081,7 @@ fn gen_fn<'ll, 'tcx>(
         Abi::Rust
     ));
     let llfn = cx.define_internal_fn(name, rust_fn_sig);
-    attributes::from_fn_attrs(cx, llfn, None);
+    attributes::from_fn_attrs(cx, llfn, None, rust_fn_sig);
     let bx = Builder::new_block(cx, llfn, "entry-block");
     codegen(bx);
     llfn

diff --git a/src/librustc_codegen_llvm/mono_item.rs b/src/librustc_codegen_llvm/mono_item.rs
index 1d5bcc4ba3986ea97c27b5107b9f381279a3deca..9c69d7d8cf4ffa1731930a02381997a486ee0919 100644 (file)
--- a/src/librustc_codegen_llvm/mono_item.rs
+++ b/src/librustc_codegen_llvm/mono_item.rs
@@ -82,7 +82,12 @@ fn predefine_fn(&self,
         if instance.def.is_inline(self.tcx) {
             attributes::inline(self, lldecl, attributes::InlineAttr::Hint);
         }
-        attributes::from_fn_attrs(self, lldecl, Some(instance.def.def_id()));
+        attributes::from_fn_attrs(
+            self,
+            lldecl,
+            Some(instance.def.def_id()),
+            mono_sig,
+        );
 
         self.instances.borrow_mut().insert(instance, lldecl);
     }

diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs
index b0601b3bbefb172e70a98355a142c929deef6f71..ab52d9be6b81bc7255f5aeb2e0530a298886a061 100644 (file)
--- a/src/librustc_mir/build/mod.rs
+++ b/src/librustc_mir/build/mod.rs
@@ -628,12 +628,7 @@ fn should_abort_on_panic<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
     // unwind anyway. Don't stop them.
     let attrs = &tcx.get_attrs(fn_def_id);
     match attr::find_unwind_attr(Some(tcx.sess.diagnostic()), attrs) {
-        None => {
-            // FIXME(rust-lang/rust#48251) -- Had to disable
-            // abort-on-panic for backwards compatibility reasons.
-            false
-        }
-
+        None => true,
         Some(UnwindAttr::Allowed) => false,
         Some(UnwindAttr::Aborts) => true,
     }

diff --git a/src/test/codegen/nounwind-extern.rs b/src/test/codegen/nounwind-extern.rs
new file mode 100644 (file)
index 0000000..ed07cf1
--- /dev/null
+++ b/src/test/codegen/nounwind-extern.rs
@@ -0,0 +1,16 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-flags: -O
+
+#![crate_type = "lib"]
+
+// CHECK: Function Attrs: norecurse nounwind
+pub extern fn foo() {}

diff --git a/src/test/run-pass/abort-on-c-abi.rs b/src/test/run-pass/abort-on-c-abi.rs
index 17b2ee39c884b727326d2f392cb952aaeea740b1..12b5b786766d67e2e7515e12c14eff41e2498ad2 100644 (file)
--- a/src/test/run-pass/abort-on-c-abi.rs
+++ b/src/test/run-pass/abort-on-c-abi.rs
@@ -15,14 +15,11 @@
 // ignore-cloudabi no env and process
 // ignore-emscripten no processes
 
-#![feature(unwind_attributes)]
-
 use std::{env, panic};
 use std::io::prelude::*;
 use std::io;
 use std::process::{Command, Stdio};
 
-#[unwind(aborts)]
 extern "C" fn panic_in_ffi() {
     panic!("Test");
 }

diff --git a/src/test/run-pass/extern/extern-call-deep.rs b/src/test/run-pass/extern/extern-call-deep.rs
index f6eff601c4b7a9deb91f1eebeb5bbd559785350d..12df0f32134a2348620ef55cb4297613f9e46c07 100644 (file)
--- a/src/test/run-pass/extern/extern-call-deep.rs
+++ b/src/test/run-pass/extern/extern-call-deep.rs
@@ -10,6 +10,7 @@
 
 // run-pass
 // ignore-wasm32-bare no libc to test ffi with
+// ignore-emscripten blows the JS stack
 
 #![feature(rustc_private)]