Skip to content

Add -Z panic-in-drop={unwind,abort} command-line option #88759

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion compiler/rustc_codegen_llvm/src/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
32 changes: 23 additions & 9 deletions compiler/rustc_metadata/src/dependency_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
));
}
}
}
}
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_metadata/src/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]`" }
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_mir_transform/src/abort_unwinding_calls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 14 additions & 2 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`";
Expand Down Expand Up @@ -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),
Expand All @@ -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>() {
Expand Down Expand Up @@ -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)"),
Expand Down Expand Up @@ -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"),
Comment on lines +1197 to +1198
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add accepted values list and default (like in some other options), plus maybe unstable book entry?

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],
Expand Down
9 changes: 8 additions & 1 deletion compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down
54 changes: 54 additions & 0 deletions src/test/codegen/panic-in-drop-abort.rs
Original file line number Diff line number Diff line change
@@ -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);
}