Skip to content

Commit

Permalink
Prevent __rust_begin_short_backtrace frames from being tail-call op…
Browse files Browse the repository at this point in the history
…timised away
  • Loading branch information
eggyal committed Aug 7, 2020
1 parent 4d43423 commit 5792840
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 12 deletions.
9 changes: 7 additions & 2 deletions library/std/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,9 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! {

let loc = info.location().unwrap(); // The current implementation always returns Some
let msg = info.message().unwrap(); // The current implementation always returns Some
rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc);
crate::sys_common::backtrace::__rust_end_short_backtrace(move || {
rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc);
})
}

/// This is the entry point of panicking for the non-format-string variants of
Expand All @@ -453,7 +455,10 @@ pub fn begin_panic<M: Any + Send>(msg: M) -> ! {
intrinsics::abort()
}

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

struct PanicPayload<A> {
inner: Option<A>,
Expand Down
10 changes: 6 additions & 4 deletions library/std/src/rt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@ fn lang_start_internal(
sys::args::init(argc, argv);

// Let's run some code!
let exit_code = panic::catch_unwind(|| {
sys_common::backtrace::__rust_begin_short_backtrace(move || main())
});
let exit_code = panic::catch_unwind(main);

sys_common::cleanup();
exit_code.unwrap_or(101) as isize
Expand All @@ -64,5 +62,9 @@ fn lang_start<T: crate::process::Termination + 'static>(
argc: isize,
argv: *const *const u8,
) -> isize {
lang_start_internal(&move || main().report(), argc, argv)
lang_start_internal(
&move || crate::sys_common::backtrace::__rust_begin_short_backtrace(main).report(),
argc,
argv,
)
}
39 changes: 34 additions & 5 deletions library/std/src/sys_common/backtrace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::
bt_fmt.add_context()?;
let mut idx = 0;
let mut res = Ok(());
// Start immediately if we're not using a short backtrace.
let mut start = print_fmt != PrintFmt::Short;
backtrace_rs::trace_unsynchronized(|frame| {
if print_fmt == PrintFmt::Short && idx > MAX_NB_FRAMES {
return false;
Expand All @@ -89,16 +91,24 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::
stop = true;
return;
}
if sym.contains("__rust_end_short_backtrace") {
start = true;
return;
}
}
}

res = bt_fmt.frame().symbol(frame, symbol);
if start {
res = bt_fmt.frame().symbol(frame, symbol);
}
});
if stop {
return false;
}
if !hit {
res = bt_fmt.frame().print_raw(frame.ip(), None, None, None);
if start {
res = bt_fmt.frame().print_raw(frame.ip(), None, None, None);
}
}

idx += 1;
Expand All @@ -123,10 +133,29 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt::
pub fn __rust_begin_short_backtrace<F, T>(f: F) -> T
where
F: FnOnce() -> T,
F: Send,
T: Send,
{
f()
let result = f();

// prevent this frame from being tail-call optimised away
crate::hint::black_box(());

result
}

/// Fixed frame used to clean the backtrace with `RUST_BACKTRACE=1`. Note that
/// this is only inline(never) when backtraces in libstd are enabled, otherwise
/// it's fine to optimize away.
#[cfg_attr(feature = "backtrace", inline(never))]
pub fn __rust_end_short_backtrace<F, T>(f: F) -> T
where
F: FnOnce() -> T,
{
let result = f();

// prevent this frame from being tail-call optimised away
crate::hint::black_box(());

result
}

pub enum RustBacktrace {
Expand Down
5 changes: 4 additions & 1 deletion library/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -514,7 +514,10 @@ pub fn run_test(
/// Fixed frame used to clean the backtrace with `RUST_BACKTRACE=1`.
#[inline(never)]
fn __rust_begin_short_backtrace<F: FnOnce()>(f: F) {
f()
f();

// prevent this frame from being tail-call optimised away
black_box(());
}

fn run_test_in_process(
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/panics/issue-47429-short-backtraces.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Regression test for #47429: short backtraces were not terminating correctly

// compile-flags: -O
// run-fail
// check-run-results
// exec-env:RUST_BACKTRACE=1

// ignore-msvc see #62897 and `backtrace-debuginfo.rs` test
// ignore-android FIXME #17520
// ignore-cloudabi spawning processes is not supported
// ignore-openbsd no support for libbacktrace without filename
// ignore-wasm no panic or subprocess support
// ignore-emscripten no panic or subprocess support
// ignore-sgx no subprocess support

fn main() {
panic!()
}
5 changes: 5 additions & 0 deletions src/test/ui/panics/issue-47429-short-backtraces.run.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
thread 'main' panicked at 'explicit panic', $DIR/issue-47429-short-backtraces.rs:17:5
stack backtrace:
0: std::panicking::begin_panic
1: issue_47429_short_backtraces::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

0 comments on commit 5792840

Please sign in to comment.