From c1bcf5c54833707518d1403f49030605edc39497 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 6 Sep 2021 12:21:47 +0200 Subject: [PATCH 1/5] Add -Z panic-in-drop={unwind,abort} command-line option --- compiler/rustc_interface/src/tests.rs | 1 + compiler/rustc_session/src/options.rs | 16 ++++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_interface/src/tests.rs b/compiler/rustc_interface/src/tests.rs index afab919bc3c..81433e57102 100644 --- a/compiler/rustc_interface/src/tests.rs +++ b/compiler/rustc_interface/src/tests.rs @@ -743,6 +743,7 @@ fn test_debugging_options_tracking_hash() { tracked!(no_profiler_runtime, true); tracked!(osx_rpath_install_name, true); tracked!(panic_abort_tests, true); + tracked!(panic_in_drop, PanicStrategy::Abort); tracked!(partially_uninit_const_threshold, Some(123)); tracked!(plt, Some(true)); tracked!(polonius, true); diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index 9a1be40558c..a82a2bef0b4 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -349,6 +349,7 @@ mod desc { pub const parse_threads: &str = parse_number; pub const parse_passes: &str = "a space-separated list of passes, or `all`"; pub const parse_panic_strategy: &str = "either `unwind` or `abort`"; + pub const parse_opt_panic_strategy: &str = parse_panic_strategy; pub const parse_relro_level: &str = "one of: `full`, `partial`, or `off`"; pub const parse_sanitizers: &str = "comma separated list of sanitizers: `address`, `hwaddress`, `leak`, `memory` or `thread`"; @@ -549,7 +550,7 @@ mod parse { } } - crate fn parse_panic_strategy(slot: &mut Option, v: Option<&str>) -> bool { + crate fn parse_opt_panic_strategy(slot: &mut Option, v: Option<&str>) -> bool { match v { Some("unwind") => *slot = Some(PanicStrategy::Unwind), Some("abort") => *slot = Some(PanicStrategy::Abort), @@ -558,6 +559,15 @@ mod parse { true } + crate fn parse_panic_strategy(slot: &mut PanicStrategy, v: Option<&str>) -> bool { + match v { + Some("unwind") => *slot = PanicStrategy::Unwind, + Some("abort") => *slot = PanicStrategy::Abort, + _ => return false, + } + true + } + crate fn parse_relro_level(slot: &mut Option, v: Option<&str>) -> bool { match v { Some(s) => match s.parse::() { @@ -958,7 +968,7 @@ options! { "optimization level (0-3, s, or z; default: 0)"), overflow_checks: Option = (None, parse_opt_bool, [TRACKED], "use overflow checks for integer arithmetic"), - panic: Option = (None, parse_panic_strategy, [TRACKED], + panic: Option = (None, parse_opt_panic_strategy, [TRACKED], "panic strategy to compile crate with"), passes: Vec = (Vec::new(), parse_list, [TRACKED], "a list of extra LLVM passes to run (space separated)"), @@ -1184,6 +1194,8 @@ options! { "pass `-install_name @rpath/...` to the macOS linker (default: no)"), panic_abort_tests: bool = (false, parse_bool, [TRACKED], "support compiling tests with panic=abort (default: no)"), + panic_in_drop: PanicStrategy = (PanicStrategy::Unwind, parse_panic_strategy, [TRACKED], + "panic strategy for panics in drops"), parse_only: bool = (false, parse_bool, [UNTRACKED], "parse only; do not compile, assemble, or link (default: no)"), partially_uninit_const_threshold: Option = (None, parse_opt_number, [TRACKED], From 8c7a05a23f427625f99b74ca506a0f5d8eb9bc11 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 6 Sep 2021 12:59:52 +0200 Subject: [PATCH 2/5] Treat drop_in_place as nounwind with -Z panic-in-drop=abort The AbortUnwindCalls MIR pass will eliminate any unnecessary cleanups and will prevent any unwinds from leaking out by forcing an abort. --- .../rustc_mir_transform/src/abort_unwinding_calls.rs | 10 ++++++---- compiler/rustc_typeck/src/collect.rs | 9 ++++++++- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs index 855dcbc431b..1abb64219f6 100644 --- a/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs +++ b/compiler/rustc_mir_transform/src/abort_unwinding_calls.rs @@ -5,6 +5,7 @@ use rustc_middle::mir::*; use rustc_middle::ty::layout; use rustc_middle::ty::{self, TyCtxt}; use rustc_target::spec::abi::Abi; +use rustc_target::spec::PanicStrategy; /// A pass that runs which is targeted at ensuring that codegen guarantees about /// unwinding are upheld for compilations of panic=abort programs. @@ -82,10 +83,11 @@ impl<'tcx> MirPass<'tcx> for AbortUnwindingCalls { }; layout::fn_can_unwind(tcx, flags, sig.abi()) } - TerminatorKind::Drop { .. } - | TerminatorKind::DropAndReplace { .. } - | TerminatorKind::Assert { .. } - | TerminatorKind::FalseUnwind { .. } => { + TerminatorKind::Drop { .. } | TerminatorKind::DropAndReplace { .. } => { + tcx.sess.opts.debugging_opts.panic_in_drop == PanicStrategy::Unwind + && layout::fn_can_unwind(tcx, CodegenFnAttrFlags::empty(), Abi::Rust) + } + TerminatorKind::Assert { .. } | TerminatorKind::FalseUnwind { .. } => { layout::fn_can_unwind(tcx, CodegenFnAttrFlags::empty(), Abi::Rust) } _ => continue, diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 3688fa05e03..8b1d1e450d0 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -46,7 +46,7 @@ use rustc_session::lint; use rustc_session::parse::feature_err; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; -use rustc_target::spec::{abi, SanitizerSet}; +use rustc_target::spec::{abi, PanicStrategy, SanitizerSet}; use rustc_trait_selection::traits::error_reporting::suggestions::NextTypeParamName; use std::iter; @@ -2683,6 +2683,13 @@ fn codegen_fn_attrs(tcx: TyCtxt<'_>, id: DefId) -> CodegenFnAttrs { codegen_fn_attrs.flags |= CodegenFnAttrFlags::TRACK_CALLER; } + // With -Z panic-in-drop=abort, drop_in_place never unwinds. + if tcx.sess.opts.debugging_opts.panic_in_drop == PanicStrategy::Abort { + if Some(id) == tcx.lang_items().drop_in_place_fn() { + codegen_fn_attrs.flags |= CodegenFnAttrFlags::NEVER_UNWIND; + } + } + let supported_target_features = tcx.supported_target_features(LOCAL_CRATE); let mut inline_span = None; From 007bd17a68a502aef3df4799c7fa7c07985d7747 Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Mon, 6 Sep 2021 13:10:11 +0200 Subject: [PATCH 3/5] Apply noreturn and nounwind LLVM attributes to callsites --- compiler/rustc_codegen_llvm/src/abi.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs index cd55a61cbaf..b7180fd84c3 100644 --- a/compiler/rustc_codegen_llvm/src/abi.rs +++ b/compiler/rustc_codegen_llvm/src/abi.rs @@ -511,7 +511,12 @@ impl<'tcx> FnAbiLlvmExt<'tcx> for FnAbi<'tcx, Ty<'tcx>> { } fn apply_attrs_callsite(&self, bx: &mut Builder<'a, 'll, 'tcx>, callsite: &'ll Value) { - // FIXME(wesleywiser, eddyb): We should apply `nounwind` and `noreturn` as appropriate to this callsite. + if self.ret.layout.abi.is_uninhabited() { + llvm::Attribute::NoReturn.apply_callsite(llvm::AttributePlace::Function, callsite); + } + if !self.can_unwind { + llvm::Attribute::NoUnwind.apply_callsite(llvm::AttributePlace::Function, callsite); + } let mut i = 0; let mut apply = |cx: &CodegenCx<'_, '_>, attrs: &ArgAttributes| { From a149bed3bd68bb5af407b98c0645082a2bf409ac Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Thu, 9 Sep 2021 13:31:19 +0100 Subject: [PATCH 4/5] Ensure that crates are linked with compatible panic-in-drop settings --- .../rustc_metadata/src/dependency_format.rs | 32 +++++++++++++------ .../src/rmeta/decoder/cstore_impl.rs | 1 + compiler/rustc_metadata/src/rmeta/encoder.rs | 1 + compiler/rustc_metadata/src/rmeta/mod.rs | 1 + compiler/rustc_middle/src/query/mod.rs | 4 +++ 5 files changed, 30 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_metadata/src/dependency_format.rs b/compiler/rustc_metadata/src/dependency_format.rs index 2d4deb1d8d5..b8d22560618 100644 --- a/compiler/rustc_metadata/src/dependency_format.rs +++ b/compiler/rustc_metadata/src/dependency_format.rs @@ -400,21 +400,35 @@ fn verify_ok(tcx: TyCtxt<'_>, list: &[Linkage]) { continue; } let cnum = CrateNum::new(i + 1); - let found_strategy = tcx.panic_strategy(cnum); - let is_compiler_builtins = tcx.is_compiler_builtins(cnum); - if is_compiler_builtins || desired_strategy == found_strategy { + if tcx.is_compiler_builtins(cnum) { continue; } - sess.err(&format!( - "the crate `{}` is compiled with the \ + let found_strategy = tcx.panic_strategy(cnum); + if desired_strategy != found_strategy { + sess.err(&format!( + "the crate `{}` is compiled with the \ panic strategy `{}` which is \ incompatible with this crate's \ strategy of `{}`", - tcx.crate_name(cnum), - found_strategy.desc(), - desired_strategy.desc() - )); + tcx.crate_name(cnum), + found_strategy.desc(), + desired_strategy.desc() + )); + } + + let found_drop_strategy = tcx.panic_in_drop_strategy(cnum); + if tcx.sess.opts.debugging_opts.panic_in_drop != found_drop_strategy { + sess.err(&format!( + "the crate `{}` is compiled with the \ + panic-in-drop strategy `{}` which is \ + incompatible with this crate's \ + strategy of `{}`", + tcx.crate_name(cnum), + found_drop_strategy.desc(), + tcx.sess.opts.debugging_opts.panic_in_drop.desc() + )); + } } } } diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 41839c58021..2d707d619c9 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -160,6 +160,7 @@ provide! { <'tcx> tcx, def_id, other, cdata, has_panic_handler => { cdata.root.has_panic_handler } is_profiler_runtime => { cdata.root.profiler_runtime } panic_strategy => { cdata.root.panic_strategy } + panic_in_drop_strategy => { cdata.root.panic_in_drop_strategy } extern_crate => { let r = *cdata.extern_crate.lock(); r.map(|c| &*tcx.arena.alloc(c)) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index d8b9a479976..4944bfe57fa 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -691,6 +691,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { hash: tcx.crate_hash(LOCAL_CRATE), stable_crate_id: tcx.def_path_hash(LOCAL_CRATE.as_def_id()).stable_crate_id(), panic_strategy: tcx.sess.panic_strategy(), + panic_in_drop_strategy: tcx.sess.opts.debugging_opts.panic_in_drop, edition: tcx.sess.edition(), has_global_allocator: tcx.has_global_allocator(LOCAL_CRATE), has_panic_handler: tcx.has_panic_handler(LOCAL_CRATE), diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index a487753f462..3024552aec4 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -204,6 +204,7 @@ crate struct CrateRoot<'tcx> { hash: Svh, stable_crate_id: StableCrateId, panic_strategy: PanicStrategy, + panic_in_drop_strategy: PanicStrategy, edition: Edition, has_global_allocator: bool, has_panic_handler: bool, diff --git a/compiler/rustc_middle/src/query/mod.rs b/compiler/rustc_middle/src/query/mod.rs index c93996162e3..ed64f50a6f0 100644 --- a/compiler/rustc_middle/src/query/mod.rs +++ b/compiler/rustc_middle/src/query/mod.rs @@ -1159,6 +1159,10 @@ rustc_queries! { fatal_cycle desc { "query a crate's configured panic strategy" } } + query panic_in_drop_strategy(_: CrateNum) -> PanicStrategy { + fatal_cycle + desc { "query a crate's configured panic-in-drop strategy" } + } query is_no_builtins(_: CrateNum) -> bool { fatal_cycle desc { "test whether a crate has `#![no_builtins]`" } From 5862a0004a65e32ea3a36d33c52e305cd75a69fe Mon Sep 17 00:00:00 2001 From: Amanieu d'Antras Date: Fri, 10 Sep 2021 19:42:32 +0100 Subject: [PATCH 5/5] Add test for -Z panic-in-drop=abort --- src/test/codegen/panic-in-drop-abort.rs | 54 +++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 src/test/codegen/panic-in-drop-abort.rs diff --git a/src/test/codegen/panic-in-drop-abort.rs b/src/test/codegen/panic-in-drop-abort.rs new file mode 100644 index 00000000000..62d093507dd --- /dev/null +++ b/src/test/codegen/panic-in-drop-abort.rs @@ -0,0 +1,54 @@ +// compile-flags: -Z panic-in-drop=abort -O + +// Ensure that unwinding code paths are eliminated from the output after +// optimization. + +#![crate_type = "lib"] +use std::any::Any; +use std::mem::forget; + +pub struct ExternDrop; +impl Drop for ExternDrop { + #[inline(always)] + fn drop(&mut self) { + // This call may potentially unwind. + extern "Rust" { + fn extern_drop(); + } + unsafe { + extern_drop(); + } + } +} + +struct AssertNeverDrop; +impl Drop for AssertNeverDrop { + #[inline(always)] + fn drop(&mut self) { + // This call should be optimized away as unreachable. + extern "C" { + fn should_not_appear_in_output(); + } + unsafe { + should_not_appear_in_output(); + } + } +} + +// CHECK-LABEL: normal_drop +// CHECK-NOT: should_not_appear_in_output +#[no_mangle] +pub fn normal_drop(x: ExternDrop) { + let guard = AssertNeverDrop; + drop(x); + forget(guard); +} + +// CHECK-LABEL: indirect_drop +// CHECK-NOT: should_not_appear_in_output +#[no_mangle] +pub fn indirect_drop(x: Box) { + let guard = AssertNeverDrop; + drop(x); + forget(guard); +}