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:
  - servo/rust-smallvec#14
  - bluss/arrayvec#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 rust-lang/lang-team#97

Author: bors

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| {

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);

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()
+                ));
+            }
         }
     }
 }

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))

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),

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,

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]`" }

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,

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],

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;

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<dyn Any>) {
+    let guard = AssertNeverDrop;
+    drop(x);
+    forget(guard);
+}