avoid triple-backtrace due to panic-during-cleanup

This commit is contained in:
Ralf Jung 2023-08-27 20:02:46 +02:00
parent b60f7b51a2
commit 1087e90a2e
10 changed files with 90 additions and 31 deletions

View file

@ -395,9 +395,10 @@ pub unsafe fn __rdl_oom(size: usize, _align: usize) -> ! {
if unsafe { __rust_alloc_error_handler_should_panic != 0 } {
panic!("memory allocation of {size} bytes failed")
} else {
core::panicking::panic_nounwind_fmt(format_args!(
"memory allocation of {size} bytes failed"
))
core::panicking::panic_nounwind_fmt(
format_args!("memory allocation of {size} bytes failed"),
/* force_no_backtrace */ false,
)
}
}
}

View file

@ -28,6 +28,7 @@ pub struct PanicInfo<'a> {
message: Option<&'a fmt::Arguments<'a>>,
location: &'a Location<'a>,
can_unwind: bool,
force_no_backtrace: bool,
}
impl<'a> PanicInfo<'a> {
@ -42,9 +43,10 @@ pub fn internal_constructor(
message: Option<&'a fmt::Arguments<'a>>,
location: &'a Location<'a>,
can_unwind: bool,
force_no_backtrace: bool,
) -> Self {
struct NoPayload;
PanicInfo { location, message, payload: &NoPayload, can_unwind }
PanicInfo { location, message, payload: &NoPayload, can_unwind, force_no_backtrace }
}
#[unstable(
@ -141,6 +143,17 @@ pub fn location(&self) -> Option<&Location<'_>> {
pub fn can_unwind(&self) -> bool {
self.can_unwind
}
#[unstable(
feature = "panic_internals",
reason = "internal details of the implementation of the `panic!` and related macros",
issue = "none"
)]
#[doc(hidden)]
#[inline]
pub fn force_no_backtrace(&self) -> bool {
self.force_no_backtrace
}
}
#[stable(feature = "panic_hook_display", since = "1.26.0")]

View file

@ -61,7 +61,12 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
fn panic_impl(pi: &PanicInfo<'_>) -> !;
}
let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller(), true);
let pi = PanicInfo::internal_constructor(
Some(&fmt),
Location::caller(),
/* can_unwind */ true,
/* force_no_backtrace */ false,
);
// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
unsafe { panic_impl(&pi) }
@ -77,7 +82,7 @@ pub const fn panic_fmt(fmt: fmt::Arguments<'_>) -> ! {
// and unwinds anyway, we will hit the "unwinding out of nounwind function" guard,
// which causes a "panic in a function that cannot unwind".
#[rustc_nounwind]
pub fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>) -> ! {
pub fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: bool) -> ! {
if cfg!(feature = "panic_immediate_abort") {
super::intrinsics::abort()
}
@ -90,7 +95,12 @@ pub fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>) -> ! {
}
// PanicInfo with the `can_unwind` flag set to false forces an abort.
let pi = PanicInfo::internal_constructor(Some(&fmt), Location::caller(), false);
let pi = PanicInfo::internal_constructor(
Some(&fmt),
Location::caller(),
/* can_unwind */ false,
force_no_backtrace,
);
// SAFETY: `panic_impl` is defined in safe Rust code and thus is safe to call.
unsafe { panic_impl(&pi) }
@ -123,7 +133,15 @@ pub const fn panic(expr: &'static str) -> ! {
#[lang = "panic_nounwind"] // needed by codegen for non-unwinding panics
#[rustc_nounwind]
pub fn panic_nounwind(expr: &'static str) -> ! {
panic_nounwind_fmt(fmt::Arguments::new_const(&[expr]));
panic_nounwind_fmt(fmt::Arguments::new_const(&[expr]), /* force_no_backtrace */ false);
}
/// Like `panic_nounwind`, but also inhibits showing a backtrace.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[rustc_nounwind]
pub fn panic_nounwind_nobacktrace(expr: &'static str) -> ! {
panic_nounwind_fmt(fmt::Arguments::new_const(&[expr]), /* force_no_backtrace */ true);
}
#[inline]
@ -172,9 +190,12 @@ fn panic_misaligned_pointer_dereference(required: usize, found: usize) -> ! {
super::intrinsics::abort()
}
panic_nounwind_fmt(format_args!(
"misaligned pointer dereference: address must be a multiple of {required:#x} but is {found:#x}"
))
panic_nounwind_fmt(
format_args!(
"misaligned pointer dereference: address must be a multiple of {required:#x} but is {found:#x}"
),
/* force_no_backtrace */ false,
)
}
/// Panic because we cannot unwind out of a function.
@ -205,7 +226,7 @@ fn panic_cannot_unwind() -> ! {
#[rustc_nounwind]
fn panic_in_cleanup() -> ! {
// Keep the text in sync with `UnwindTerminateReason::as_str` in `rustc_middle`.
panic_nounwind("panic in a destructor during cleanup")
panic_nounwind_nobacktrace("panic in a destructor during cleanup")
}
/// This function is used instead of panic_fmt in const eval.

View file

@ -246,7 +246,9 @@ fn default_hook(info: &PanicInfo<'_>) {
pub fn panic_hook_with_disk_dump(info: &PanicInfo<'_>, path: Option<&crate::path::Path>) {
// If this is a double panic, make sure that we print a backtrace
// for this panic. Otherwise only print it if logging is enabled.
let backtrace = if panic_count::get_count() >= 2 {
let backtrace = if info.force_no_backtrace() {
None
} else if panic_count::get_count() >= 2 {
BacktraceStyle::full()
} else {
crate::panic::get_backtrace_style()
@ -294,7 +296,7 @@ pub fn panic_hook_with_disk_dump(info: &PanicInfo<'_>, path: Option<&crate::path
}
}
}
// If backtraces aren't supported, do nothing.
// If backtraces aren't supported or are forced-off, do nothing.
None => {}
}
};
@ -615,14 +617,23 @@ fn get(&mut self) -> &(dyn Any + Send) {
let loc = info.location().unwrap(); // The current implementation always returns Some
let msg = info.message().unwrap(); // The current implementation always returns Some
crate::sys_common::backtrace::__rust_end_short_backtrace(move || {
// FIXME: can we just pass `info` along rather than taking it apart here, only to have
// `rust_panic_with_hook` construct a new `PanicInfo`?
if let Some(msg) = msg.as_str() {
rust_panic_with_hook(&mut StrPanicPayload(msg), info.message(), loc, info.can_unwind());
rust_panic_with_hook(
&mut StrPanicPayload(msg),
info.message(),
loc,
info.can_unwind(),
info.force_no_backtrace(),
);
} else {
rust_panic_with_hook(
&mut PanicPayload::new(msg),
info.message(),
loc,
info.can_unwind(),
info.force_no_backtrace(),
);
}
})
@ -647,7 +658,13 @@ 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, true)
rust_panic_with_hook(
&mut PanicPayload::new(msg),
None,
loc,
/* can_unwind */ true,
/* force_no_backtrace */ false,
)
});
struct PanicPayload<A> {
@ -693,6 +710,7 @@ fn rust_panic_with_hook(
message: Option<&fmt::Arguments<'_>>,
location: &Location<'_>,
can_unwind: bool,
force_no_backtrace: bool,
) -> ! {
let must_abort = panic_count::increase(true);
@ -707,14 +725,20 @@ fn rust_panic_with_hook(
panic_count::MustAbort::AlwaysAbort => {
// 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, can_unwind);
let panicinfo = PanicInfo::internal_constructor(
message,
location,
can_unwind,
force_no_backtrace,
);
rtprintpanic!("{panicinfo}\npanicked after panic::always_abort(), aborting.\n");
}
}
crate::sys::abort_internal();
}
let mut info = PanicInfo::internal_constructor(message, location, can_unwind);
let mut info =
PanicInfo::internal_constructor(message, location, can_unwind, force_no_backtrace);
let hook = HOOK.read().unwrap_or_else(PoisonError::into_inner);
match *hook {
// Some platforms (like wasm) know that printing to stderr won't ever actually

View file

@ -6,7 +6,6 @@ second
stack backtrace:
thread 'main' panicked at RUSTLIB/core/src/panicking.rs:LL:CC:
panic in a destructor during cleanup
stack backtrace:
thread caused non-unwinding panic. aborting.
error: abnormal termination: the program aborted execution
--> RUSTLIB/std/src/sys/PLATFORM/mod.rs:LL:CC
@ -19,7 +18,7 @@ LL | ABORT();
= note: inside closure at RUSTLIB/std/src/panicking.rs:LL:CC
= note: inside `std::sys_common::backtrace::__rust_end_short_backtrace::<[closure@std::panicking::begin_panic_handler::{closure#0}], !>` at RUSTLIB/std/src/sys_common/backtrace.rs:LL:CC
= note: inside `std::panicking::begin_panic_handler` at RUSTLIB/std/src/panicking.rs:LL:CC
= note: inside `core::panicking::panic_nounwind` at RUSTLIB/core/src/panicking.rs:LL:CC
= note: inside `core::panicking::panic_nounwind_nobacktrace` at RUSTLIB/core/src/panicking.rs:LL:CC
= note: inside `core::panicking::panic_in_cleanup` at RUSTLIB/core/src/panicking.rs:LL:CC
note: inside `main`
--> $DIR/double_panic.rs:LL:CC

View file

@ -94,7 +94,7 @@ fn runtest(me: &str) {
#[cfg(not(panic = "abort"))]
{
// Make sure a stack trace is printed
let p = template(me).arg("double-fail").spawn().unwrap();
let p = template(me).arg("double-fail").env("RUST_BACKTRACE","0").spawn().unwrap();
let out = p.wait_with_output().unwrap();
assert!(!out.status.success());
let s = str::from_utf8(&out.stderr).unwrap();
@ -106,18 +106,18 @@ fn runtest(me: &str) {
contains_verbose_expected(s, "double"),
"bad output3: {}", s
);
// Make sure it's only one stack trace.
assert_eq!(s.split("stack backtrace").count(), 2);
// Make sure a stack trace isn't printed too many times
//
// Currently it is printed 3 times ("once", "twice" and "panic in a destructor during
// cleanup") but in the future the last one may be removed.
// even with RUST_BACKTRACE=1. It should be printed twice.
let p = template(me).arg("double-fail")
.env("RUST_BACKTRACE", "1").spawn().unwrap();
let out = p.wait_with_output().unwrap();
assert!(!out.status.success());
let s = str::from_utf8(&out.stderr).unwrap();
let mut i = 0;
for _ in 0..3 {
for _ in 0..2 {
i += s[i + 10..].find("stack backtrace").unwrap() + 10;
}
assert!(s[i + 10..].find("stack backtrace").is_none(),

View file

@ -4,6 +4,7 @@
// error-pattern: panic in a destructor during cleanup
// normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> ""
// normalize-stderr-test: "\n +at [^\n]+" -> ""
// normalize-stderr-test: "(core/src/panicking\.rs):[0-9]+:[0-9]+" -> "$1:$$LINE:$$COL"
// needs-unwind
// ignore-emscripten "RuntimeError" junk in output
// ignore-msvc SEH doesn't do panic-during-cleanup the same way as everyone else

View file

@ -1,10 +1,9 @@
thread 'main' panicked at $DIR/panic-in-cleanup.rs:21:5:
thread 'main' panicked at $DIR/panic-in-cleanup.rs:22:5:
explicit panic
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at $DIR/panic-in-cleanup.rs:15:9:
thread 'main' panicked at $DIR/panic-in-cleanup.rs:16:9:
BOOM
stack backtrace:
thread 'main' panicked at library/core/src/panicking.rs:126:5:
thread 'main' panicked at library/core/src/panicking.rs:$LINE:$COL:
panic in a destructor during cleanup
stack backtrace:
thread caused non-unwinding panic. aborting.

View file

@ -4,6 +4,7 @@
// error-pattern: panic in a function that cannot unwind
// normalize-stderr-test: "\n +[0-9]+:[^\n]+" -> ""
// normalize-stderr-test: "\n +at [^\n]+" -> ""
// normalize-stderr-test: "(core/src/panicking\.rs):[0-9]+:[0-9]+" -> "$1:$$LINE:$$COL"
// needs-unwind
// ignore-emscripten "RuntimeError" junk in output
#![feature(c_unwind)]

View file

@ -1,7 +1,7 @@
thread 'main' panicked at $DIR/panic-in-ffi.rs:12:5:
thread 'main' panicked at $DIR/panic-in-ffi.rs:13:5:
Test
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
thread 'main' panicked at library/core/src/panicking.rs:126:5:
thread 'main' panicked at library/core/src/panicking.rs:$LINE:$COL:
panic in a function that cannot unwind
stack backtrace:
thread caused non-unwinding panic. aborting.