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 ffe5e12..30edc47 100644
--- 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 @@
/// 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 @@
/// 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 @@
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 f13eeb6..87185a2 100644
--- a/src/librustc_codegen_llvm/callee.rs
+++ b/src/librustc_codegen_llvm/callee.rs
@@ -94,7 +94,7 @@
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 6c90937..0bd6146 100644
--- a/src/librustc_codegen_llvm/context.rs
+++ b/src/librustc_codegen_llvm/context.rs
@@ -409,7 +409,6 @@
));
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 c23aab4..2964f2e 100644
--- 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 @@
_ => {},
}
- if cx.tcx.sess.panic_strategy() != PanicStrategy::Unwind {
- attributes::unwind(llfn, false);
- }
-
attributes::non_lazy_bind(cx.sess(), llfn);
llfn
@@ -132,10 +127,6 @@
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 59608b1..2b82ebe 100644
--- a/src/librustc_codegen_llvm/intrinsic.rs
+++ b/src/librustc_codegen_llvm/intrinsic.rs
@@ -1081,7 +1081,7 @@
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 1d5bcc4..9c69d7d 100644
--- a/src/librustc_codegen_llvm/mono_item.rs
+++ b/src/librustc_codegen_llvm/mono_item.rs
@@ -82,7 +82,12 @@
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 b0601b3..ab52d9b 100644
--- a/src/librustc_mir/build/mod.rs
+++ b/src/librustc_mir/build/mod.rs
@@ -628,12 +628,7 @@
// 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
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 17b2ee3..12b5b78 100644
--- 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 f6eff60..12df0f3 100644
--- 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)]
