Skip to content

Commit

Permalink
Rollup merge of rust-lang#92828 - Amanieu:unwind-abort, r=dtolnay
Browse files Browse the repository at this point in the history
Print a helpful message if unwinding aborts when it reaches a nounwind function

This is implemented by routing `TerminatorKind::Abort` back through the panic handler, but with a special flag in the `PanicInfo` which indicates that the panic handler should *not* attempt to unwind the stack and should instead abort immediately.

This is useful for the planned change in rust-lang/lang-team#97 which would make `Drop` impls `nounwind` by default.

### Code

```rust
#![feature(c_unwind)]

fn panic() {
    panic!()
}

extern "C" fn nounwind() {
    panic();
}

fn main() {
    nounwind();
}
```

### Before

```
$ ./test
thread 'main' panicked at 'explicit panic', test.rs:4:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Illegal instruction (core dumped)
```

### After

```
$ ./test
thread 'main' panicked at 'explicit panic', test.rs:4:5
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at 'panic in a function that cannot unwind', test.rs:7:1
stack backtrace:
   0:     0x556f8f86ec9b - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::hdccefe11a6ac4396
   1:     0x556f8f88ac6c - core::fmt::write::he152b28c41466ebb
   2:     0x556f8f85d6e2 - std::io::Write::write_fmt::h0c261480ab86f3d3
   3:     0x556f8f8654fa - std::panicking::default_hook::{{closure}}::h5d7346f3ff7f6c1b
   4:     0x556f8f86512b - std::panicking::default_hook::hd85803a1376cac7f
   5:     0x556f8f865a91 - std::panicking::rust_panic_with_hook::h4dc1c5a3036257ac
   6:     0x556f8f86f079 - std::panicking::begin_panic_handler::{{closure}}::hdda1d83c7a9d34d2
   7:     0x556f8f86edc4 - std::sys_common::backtrace::__rust_end_short_backtrace::h5b70ed0cce71e95f
   8:     0x556f8f865592 - rust_begin_unwind
   9:     0x556f8f85a764 - core::panicking::panic_no_unwind::h2606ab3d78c87899
  10:     0x556f8f85b910 - test::nounwind::hade6c7ee65050347
  11:     0x556f8f85b936 - test::main::hdc6e02cb36343525
  12:     0x556f8f85b7e3 - core::ops::function::FnOnce::call_once::h4d02663acfc7597f
  13:     0x556f8f85b739 - std::sys_common::backtrace::__rust_begin_short_backtrace::h071d40135adb0101
  14:     0x556f8f85c149 - std::rt::lang_start::{{closure}}::h70dbfbf38b685e93
  15:     0x556f8f85c791 - std::rt::lang_start_internal::h798f1c0268d525aa
  16:     0x556f8f85c131 - std::rt::lang_start::h476a7ee0a0bb663f
  17:     0x556f8f85b963 - main
  18:     0x7f64c0822b25 - __libc_start_main
  19:     0x556f8f85ae8e - _start
  20:                0x0 - <unknown>
thread panicked while panicking. aborting.
Aborted (core dumped)
```
  • Loading branch information
matthiaskrgr authored Jan 22, 2022
2 parents 6e5ec70 + 24588e6 commit 87b7ad4
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 17 deletions.
27 changes: 23 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,28 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
helper.do_call(self, &mut bx, fn_abi, llfn, &args, None, cleanup);
}

fn codegen_abort_terminator(
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
mut bx: Bx,
terminator: &mir::Terminator<'tcx>,
) {
let span = terminator.source_info.span;
self.set_debug_loc(&mut bx, terminator.source_info);

// Get the location information.
let location = self.get_caller_location(&mut bx, terminator.source_info).immediate();

// Obtain the panic entry point.
let def_id = common::langcall(bx.tcx(), Some(span), "", LangItem::PanicNoUnwind);
let instance = ty::Instance::mono(bx.tcx(), def_id);
let fn_abi = bx.fn_abi_of_instance(instance, ty::List::empty());
let llfn = bx.get_fn_addr(instance);

// Codegen the actual panic invoke/call.
helper.do_call(self, &mut bx, fn_abi, llfn, &[location], None, None);
}

/// Returns `true` if this is indeed a panic intrinsic and codegen is done.
fn codegen_panic_intrinsic(
&mut self,
Expand Down Expand Up @@ -1014,10 +1036,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::TerminatorKind::Resume => self.codegen_resume_terminator(helper, bx),

mir::TerminatorKind::Abort => {
bx.abort();
// `abort` does not terminate the block, so we still need to generate
// an `unreachable` terminator after it.
bx.unreachable();
self.codegen_abort_terminator(helper, bx, terminator);
}

mir::TerminatorKind::Goto { target } => {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_hir/src/lang_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,7 @@ language_item_table! {
PanicInfo, sym::panic_info, panic_info, Target::Struct, GenericRequirement::None;
PanicLocation, sym::panic_location, panic_location, Target::Struct, GenericRequirement::None;
PanicImpl, sym::panic_impl, panic_impl, Target::Fn, GenericRequirement::None;
PanicNoUnwind, sym::panic_no_unwind, panic_no_unwind, Target::Fn, GenericRequirement::Exact(0);
/// libstd panic entry point. Necessary for const eval to be able to catch it
BeginPanic, sym::begin_panic, begin_panic_fn, Target::Fn, GenericRequirement::None;

Expand Down
10 changes: 9 additions & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -807,10 +807,18 @@ impl<'a, 'tcx> MirVisitor<'tcx> for MirNeighborCollector<'a, 'tcx> {
self.output.push(create_fn_mono_item(tcx, instance, source));
}
}
mir::TerminatorKind::Abort { .. } => {
let instance = Instance::mono(
tcx,
tcx.require_lang_item(LangItem::PanicNoUnwind, Some(source)),
);
if should_codegen_locally(tcx, &instance) {
self.output.push(create_fn_mono_item(tcx, instance, source));
}
}
mir::TerminatorKind::Goto { .. }
| mir::TerminatorKind::SwitchInt { .. }
| mir::TerminatorKind::Resume
| mir::TerminatorKind::Abort
| mir::TerminatorKind::Return
| mir::TerminatorKind::Unreachable => {}
mir::TerminatorKind::GeneratorDrop
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -990,6 +990,7 @@ symbols! {
panic_implementation,
panic_info,
panic_location,
panic_no_unwind,
panic_runtime,
panic_str,
panic_unwind,
Expand Down
16 changes: 15 additions & 1 deletion library/core/src/panic/panic_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ pub struct PanicInfo<'a> {
payload: &'a (dyn Any + Send),
message: Option<&'a fmt::Arguments<'a>>,
location: &'a Location<'a>,
can_unwind: bool,
}

impl<'a> PanicInfo<'a> {
Expand All @@ -44,9 +45,10 @@ impl<'a> PanicInfo<'a> {
pub fn internal_constructor(
message: Option<&'a fmt::Arguments<'a>>,
location: &'a Location<'a>,
can_unwind: bool,
) -> Self {
struct NoPayload;
PanicInfo { location, message, payload: &NoPayload }
PanicInfo { location, message, payload: &NoPayload, can_unwind }
}

#[unstable(
Expand Down Expand Up @@ -127,6 +129,18 @@ impl<'a> PanicInfo<'a> {
// deal with that case in std::panicking::default_hook and core::panicking::panic_fmt.
Some(&self.location)
}

/// Returns whether the panic handler is allowed to unwind the stack from
/// the point where the panic occurred.
///
/// This is true for most kinds of panics with the exception of panics
/// caused by trying to unwind out of a `Drop` implementation or a function
/// whose ABI does not support unwinding.
#[must_use]
#[unstable(feature = "panic_can_unwind", issue = "92988")]
pub fn can_unwind(&self) -> bool {
self.can_unwind
}
}

#[stable(feature = "panic_hook_display", since = "1.26.0")]
Expand Down
27 changes: 26 additions & 1 deletion library/core/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,31 @@ fn panic_bounds_check(index: usize, len: usize) -> ! {
panic!("index out of bounds: the len is {} but the index is {}", len, index)
}

#[cfg(not(bootstrap))]
#[cold]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))]
#[track_caller]
#[lang = "panic_no_unwind"] // needed by codegen for panic in nounwind function
fn panic_no_unwind() -> ! {
if cfg!(feature = "panic_immediate_abort") {
super::intrinsics::abort()
}

// NOTE This function never crosses the FFI boundary; it's a Rust-to-Rust call
// that gets resolved to the `#[panic_handler]` function.
extern "Rust" {
#[lang = "panic_impl"]
fn panic_impl(pi: &PanicInfo<'_>) -> !;
}

// PanicInfo with the `can_unwind` flag set to false forces an abort.
let fmt = format_args!("panic in a function that cannot unwind");
let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller(), false);

// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
unsafe { panic_impl(&pi) }
}

/// The entry point for panicking with a formatted message.
///
/// This is designed to reduce the amount of code required at the call
Expand Down Expand Up @@ -104,7 +129,7 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
fn panic_impl(pi: &PanicInfo<'_>) -> !;
}

let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller());
let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller(), true);

// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
unsafe { panic_impl(&pi) }
Expand Down
1 change: 1 addition & 0 deletions library/std/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
#![feature(once_cell)]
#![feature(panic_info_message)]
#![feature(panic_internals)]
#![feature(panic_can_unwind)]
#![feature(panic_unwind)]
#![feature(pin_static_ref)]
#![feature(portable_simd)]
Expand Down
22 changes: 14 additions & 8 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,9 +576,14 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {
let msg = info.message().unwrap(); // The current implementation always returns Some
crate::sys_common::backtrace::__rust_end_short_backtrace(move || {
if let Some(msg) = msg.as_str() {
rust_panic_with_hook(&mut StrPanicPayload(msg), info.message(), loc);
rust_panic_with_hook(&mut StrPanicPayload(msg), info.message(), loc, info.can_unwind());
} else {
rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc);
rust_panic_with_hook(
&mut PanicPayload::new(msg),
info.message(),
loc,
info.can_unwind(),
);
}
})
}
Expand All @@ -602,7 +607,7 @@ pub const fn begin_panic<M: Any + Send>(msg: M) -> ! {

let loc = Location::caller();
return crate::sys_common::backtrace::__rust_end_short_backtrace(move || {
rust_panic_with_hook(&mut PanicPayload::new(msg), None, loc)
rust_panic_with_hook(&mut PanicPayload::new(msg), None, loc, true)
});

struct PanicPayload<A> {
Expand Down Expand Up @@ -647,6 +652,7 @@ fn rust_panic_with_hook(
payload: &mut dyn BoxMeUp,
message: Option<&fmt::Arguments<'_>>,
location: &Location<'_>,
can_unwind: bool,
) -> ! {
let (must_abort, panics) = panic_count::increase();

Expand All @@ -663,14 +669,14 @@ fn rust_panic_with_hook(
} else {
// Unfortunately, this does not print a backtrace, because creating
// a `Backtrace` will allocate, which we must to avoid here.
let panicinfo = PanicInfo::internal_constructor(message, location);
let panicinfo = PanicInfo::internal_constructor(message, location, can_unwind);
rtprintpanic!("{}\npanicked after panic::always_abort(), aborting.\n", panicinfo);
}
intrinsics::abort()
crate::sys::abort_internal();
}

unsafe {
let mut info = PanicInfo::internal_constructor(message, location);
let mut info = PanicInfo::internal_constructor(message, location, can_unwind);
let _guard = HOOK_LOCK.read();
match HOOK {
// Some platforms (like wasm) know that printing to stderr won't ever actually
Expand All @@ -691,13 +697,13 @@ fn rust_panic_with_hook(
};
}

if panics > 1 {
if panics > 1 || !can_unwind {
// If a thread panics while it's already unwinding then we
// have limited options. Currently our preference is to
// just abort. In the future we may consider resuming
// unwinding or otherwise exiting the thread cleanly.
rtprintpanic!("thread panicked while panicking. aborting.\n");
intrinsics::abort()
crate::sys::abort_internal();
}

rust_panic(payload)
Expand Down
7 changes: 6 additions & 1 deletion library/std/src/sys/unix/process/process_unix/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,5 +53,10 @@ fn test_command_fork_no_unwind() {
let status = got.expect("panic unexpectedly propagated");
dbg!(status);
let signal = status.signal().expect("expected child process to die of signal");
assert!(signal == libc::SIGABRT || signal == libc::SIGILL || signal == libc::SIGTRAP);
assert!(
signal == libc::SIGABRT
|| signal == libc::SIGILL
|| signal == libc::SIGTRAP
|| signal == libc::SIGSEGV
);
}
2 changes: 1 addition & 1 deletion src/test/codegen/unwind-and-panic-abort.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ extern "C-unwind" {

// CHECK: Function Attrs:{{.*}}nounwind
// CHECK-NEXT: define{{.*}}void @foo
// CHECK: call void @llvm.trap()
// CHECK: call void @_ZN4core9panicking15panic_no_unwind
#[no_mangle]
pub unsafe extern "C" fn foo() {
bar();
Expand Down

0 comments on commit 87b7ad4

Please sign in to comment.