diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 983048188527f..2066f14c6d796 100644 --- a/src/librustc/hir/mod.rs +++ b/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 diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index e72efdb057ab1..3c4f4cf98e2aa 100644 --- a/src/librustc/ty/context.rs +++ b/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> { diff --git a/src/librustc_codegen_llvm/attributes.rs b/src/librustc_codegen_llvm/attributes.rs index 33b50401b22f1..c2e625fca8e46 100644 --- a/src/librustc_codegen_llvm/attributes.rs +++ b/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 }); diff --git a/src/librustc_mir/build/mod.rs b/src/librustc_mir/build/mod.rs index 6a3bb8b8b86a2..7cb85c177df81 100644 --- a/src/librustc_mir/build/mod.rs +++ b/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(); } diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index 312a598af02bf..541cfd1c5aabb 100644 --- a/src/librustc_typeck/collect.rs +++ b/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; diff --git a/src/test/codegen/extern-functions.rs b/src/test/codegen/extern-functions.rs index a935d88652267..45a2c6bfc81e8 100644 --- a/src/test/codegen/extern-functions.rs +++ b/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(); } diff --git a/src/test/codegen/nounwind-extern.rs b/src/test/codegen/nounwind-extern.rs index 54d6a8d2794ba..7f634a861eeb9 100644 --- a/src/test/codegen/nounwind-extern.rs +++ b/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() {} diff --git a/src/test/codegen/unwind-extern.rs b/src/test/codegen/unwind-extern.rs new file mode 100644 index 0000000000000..3de3e7e0a584d --- /dev/null +++ b/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() {} diff --git a/src/test/ui/abi/abort-on-c-abi.rs b/src/test/ui/abi/abort-on-panic.rs similarity index 62% rename from src/test/ui/abi/abort-on-c-abi.rs rename to src/test/ui/abi/abort-on-panic.rs index 2f08730ec6132..de9046fb0978d 100644 --- a/src/test/ui/abi/abort-on-c-abi.rs +++ b/src/test/ui/abi/abort-on-panic.rs @@ -19,6 +19,11 @@ 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. @@ -26,15 +31,31 @@ fn test() { 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 = 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()); }