From 3fda7086cc810ce3003b91f74eb1c17e9f016319 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Wed, 28 Jul 2021 11:40:53 -0400 Subject: [PATCH 1/2] Add regression test --- .../panic-short-backtrace-windows-x86_64.rs | 44 +++++++++++++++++++ ...-short-backtrace-windows-x86_64.run.stderr | 3 ++ 2 files changed, 47 insertions(+) create mode 100644 src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs create mode 100644 src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr diff --git a/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs new file mode 100644 index 0000000000000..eae3f861519a0 --- /dev/null +++ b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs @@ -0,0 +1,44 @@ +// Regression test for #87481: short backtrace formatting cut off the entire stack trace. + +// Codegen-units is specified here so that we can replicate a typical rustc invocation which +// is not normally limited to 1 CGU. This is important so that the `__rust_begin_short_backtrace` +// and `__rust_end_short_backtrace` symbols are not marked internal to the CGU and thus will be +// named in the symbol table. +// compile-flags: -O -Ccodegen-units=8 + +// run-fail +// check-run-results +// exec-env:RUST_BACKTRACE=1 + +// Backtraces are pretty broken in general on i686-pc-windows-msvc (#62897). +// only-x86_64-pc-windows-msvc + +fn main() { + a(); +} + +// Make these no_mangle so dbghelp.dll can figure out the symbol names. + +#[no_mangle] +#[inline(never)] +fn a() { + b(); +} + +#[no_mangle] +#[inline(never)] +fn b() { + c(); +} + +#[no_mangle] +#[inline(never)] +fn c() { + d(); +} + +#[no_mangle] +#[inline(never)] +fn d() { + panic!("d was called"); +} diff --git a/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr new file mode 100644 index 0000000000000..f855346532fce --- /dev/null +++ b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr @@ -0,0 +1,3 @@ +thread 'main' panicked at 'd was called', $DIR/panic-short-backtrace-windows-x86_64.rs:43:5 +stack backtrace: +note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. From 286cdc81a85e8490cdc8ff42ea5085ced0df09f4 Mon Sep 17 00:00:00 2001 From: Wesley Wiser Date: Thu, 29 Jul 2021 13:06:37 -0400 Subject: [PATCH 2/2] [backtraces]: look for the `begin` symbol only after seeing `end` On `x86_64-pc-windows-msvc`, we often get backtraces which look like this: ``` 10: 0x7ff77e0e9be5 - std::panicking::rust_panic_with_hook 11: 0x7ff77e0e11b4 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5769736bdb11136c 12: 0x7ff77e0e116f - std::sys_common::backtrace::__rust_end_short_backtrace::h61c7ecb1b55338ae 13: 0x7ff77e0f89dd - std::panicking::begin_panic::h8e60ef9f82a41805 14: 0x7ff77e0e108c - d 15: 0x7ff77e0e1069 - c 16: 0x7ff77e0e1059 - b 17: 0x7ff77e0e1049 - a 18: 0x7ff77e0e1039 - core::ptr::drop_in_place::{{closure}}>::h1bfcd14d5e15ba81 19: 0x7ff77e0e1186 - std::sys_common::backtrace::__rust_begin_short_backtrace::h5769736bdb11136c 20: 0x7ff77e0e100c - std::rt::lang_start::{{closure}}::ha054184bbf9921e3 ``` Notice that `__rust_begin_short_backtrace` appears on frame 11 before `__rust_end_short_backtrace` on frame 12. This is because in typical release binaries without debug symbols, dbghelp.dll, which we use to walk and symbolize the stack, does not know where CGU internal functions start or end and so the closure invoked by `__rust_end_short_backtrace` is incorrectly described as `__rust_begin_short_backtrace` because it happens to be near that symbol. While that can obviously change, this has been happening quite consistently since #75048. Since this is a very small change to the std and the change makes sense by itself, I think this is worth doing. This doesn't completely resolve the situation for release binaries on Windows, since without debug symbols, the stack printed can still show incorrect symbol names (this is why the test uses `#[no_mangle]`) but it does slightly improve the situation in that you see the same backtrace you would see with `RUST_BACKTRACE=full` or in a debugger (without the uninteresting bits at the top and bottom). --- library/std/src/sys_common/backtrace.rs | 2 +- .../ui/panics/panic-short-backtrace-windows-x86_64.rs | 5 +++++ .../panic-short-backtrace-windows-x86_64.run.stderr | 8 +++++++- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys_common/backtrace.rs b/library/std/src/sys_common/backtrace.rs index a549770d8b378..e6a099f0e81a0 100644 --- a/library/std/src/sys_common/backtrace.rs +++ b/library/std/src/sys_common/backtrace.rs @@ -75,7 +75,7 @@ unsafe fn _print_fmt(fmt: &mut fmt::Formatter<'_>, print_fmt: PrintFmt) -> fmt:: hit = true; if print_fmt == PrintFmt::Short { if let Some(sym) = symbol.name().and_then(|s| s.as_str()) { - if sym.contains("__rust_begin_short_backtrace") { + if start && sym.contains("__rust_begin_short_backtrace") { stop = true; return; } diff --git a/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs index eae3f861519a0..fd01337296fb7 100644 --- a/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs +++ b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.rs @@ -10,6 +10,11 @@ // check-run-results // exec-env:RUST_BACKTRACE=1 +// We need to normalize out frame 5 because without debug info, dbghelp.dll doesn't know where CGU +// internal functions like `main` start or end and so it will return whatever symbol happens +// to be located near the address. +// normalize-stderr-test: "5: .*" -> "5: some Rust fn" + // Backtraces are pretty broken in general on i686-pc-windows-msvc (#62897). // only-x86_64-pc-windows-msvc diff --git a/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr index f855346532fce..799a8b30e997b 100644 --- a/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr +++ b/src/test/ui/panics/panic-short-backtrace-windows-x86_64.run.stderr @@ -1,3 +1,9 @@ -thread 'main' panicked at 'd was called', $DIR/panic-short-backtrace-windows-x86_64.rs:43:5 +thread 'main' panicked at 'd was called', $DIR/panic-short-backtrace-windows-x86_64.rs:48:5 stack backtrace: + 0: std::panicking::begin_panic + 1: d + 2: c + 3: b + 4: a + 5: some Rust fn note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.