From: Alex Crichton Date: Thu, 15 Nov 2018 14:17:58 +0000 (-0800) Subject: rustc: Switch `extern` functions to abort by default on panic X-Git-Url: https://git.lizzy.rs/?a=commitdiff_plain;h=1091eee65b91a9968f2a0b2f0c7d6e8e27d8d033;p=rust.git 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 ffe5e123f9b..30edc4744ec 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 @@ 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, + 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 f13eeb6692c..87185a20c50 100644 --- 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 6c90937de3f..0bd6146f5aa 100644 --- 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 c23aab409a9..2964f2e5847 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 @@ 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 59608b11939..2b82ebe0bc2 100644 --- 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 1d5bcc4ba39..9c69d7d8cf4 100644 --- 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 b0601b3bbef..ab52d9be6b8 100644 --- 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 index 00000000000..ed07cf1d6b2 --- /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 or the MIT license +// , 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 17b2ee39c88..12b5b786766 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 f6eff601c4b..12df0f32134 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)]