about summary refs log tree commit diff
path: root/compiler
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-09-12 20:48:09 +0000
committerbors <bors@rust-lang.org>2021-09-12 20:48:09 +0000
commit51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8 (patch)
tree5cc0a4eaf3c88e826459f84b2a623026feed2195 /compiler
parentd2dfb0eb8e30d188fb1731e540bc1b418bcd046d (diff)
parent5862a0004a65e32ea3a36d33c52e305cd75a69fe (diff)
downloadrust-51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8.tar.gz
rust-51e514c0fb4f9afcaae3b02dd9ccb93e15b30ef8.zip
Auto merge of #88759 - Amanieu:panic_in_drop, r=nagisa,eddyb
Add -Z panic-in-drop={unwind,abort} command-line option

This PR changes `Drop` to abort if an unwinding panic attempts to escape it, making the process abort instead. This has several benefits:
- The current behavior when unwinding out of `Drop` is very unintuitive and easy to miss: unwinding continues, but the remaining drops in scope are simply leaked.
- A lot of unsafe code doesn't expect drops to unwind, which can lead to unsoundness:
  - https://github.com/servo/rust-smallvec/issues/14
  - https://github.com/bluss/arrayvec/issues/3
- There is a code size and compilation time cost to this: LLVM needs to generate extra landing pads out of all calls in a drop implementation. This can compound when functions are inlined since unwinding will then continue on to process drops in the callee, which can itself unwind, etc.
  - Initial measurements show a 3% size reduction and up to 10% compilation time reduction on some crates (`syn`).

One thing to note about `-Z panic-in-drop=abort` is that *all* crates must be built with this option for it to be sound since it makes the compiler assume that dropping `Box<dyn Any>` will never unwind.

cc https://github.com/rust-lang/lang-team/issues/97
Diffstat (limited to 'compiler')
-rw-r--r--compiler/rustc_codegen_llvm/src/abi.rs7
-rw-r--r--compiler/rustc_interface/src/tests.rs1
-rw-r--r--compiler/rustc_metadata/src/dependency_format.rs32
-rw-r--r--compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs1
-rw-r--r--compiler/rustc_metadata/src/rmeta/encoder.rs1
-rw-r--r--compiler/rustc_metadata/src/rmeta/mod.rs1
-rw-r--r--compiler/rustc_middle/src/query/mod.rs4
-rw-r--r--compiler/rustc_mir_transform/src/abort_unwinding_calls.rs10
-rw-r--r--compiler/rustc_session/src/options.rs16
-rw-r--r--compiler/rustc_typeck/src/collect.rs9
10 files changed, 65 insertions, 17 deletions
diff --git a/compiler/rustc_codegen_llvm/src/abi.rs b/compiler/rustc_codegen_llvm/src/abi.rs
index 824bcd0383c..1a0a3a0c340 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| {
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_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 50074803bbe..a01eaf68f01 100644
--- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
+++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
@@ -159,6 +159,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 1b24e5eae98..b0d22037f21 100644
--- a/compiler/rustc_metadata/src/rmeta/encoder.rs
+++ b/compiler/rustc_metadata/src/rmeta/encoder.rs
@@ -692,6 +692,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 575ab04ab24..1f307f3fdee 100644
--- a/compiler/rustc_metadata/src/rmeta/mod.rs
+++ b/compiler/rustc_metadata/src/rmeta/mod.rs
@@ -205,6 +205,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 5748e5319e0..985d35ff9a9 100644
--- a/compiler/rustc_middle/src/query/mod.rs
+++ b/compiler/rustc_middle/src/query/mod.rs
@@ -1167,6 +1167,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]`" }
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_session/src/options.rs b/compiler/rustc_session/src/options.rs
index 447be84b5a7..bb29a87035e 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<PanicStrategy>, v: Option<&str>) -> bool {
+    crate fn parse_opt_panic_strategy(slot: &mut Option<PanicStrategy>, 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<RelroLevel>, v: Option<&str>) -> bool {
         match v {
             Some(s) => match s.parse::<RelroLevel>() {
@@ -958,7 +968,7 @@ options! {
         "optimization level (0-3, s, or z; default: 0)"),
     overflow_checks: Option<bool> = (None, parse_opt_bool, [TRACKED],
         "use overflow checks for integer arithmetic"),
-    panic: Option<PanicStrategy> = (None, parse_panic_strategy, [TRACKED],
+    panic: Option<PanicStrategy> = (None, parse_opt_panic_strategy, [TRACKED],
         "panic strategy to compile crate with"),
     passes: Vec<String> = (Vec::new(), parse_list, [TRACKED],
         "a list of extra LLVM passes to run (space separated)"),
@@ -1186,6 +1196,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<usize> = (None, parse_opt_number, [TRACKED],
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;