Skip to content

Commit fe66eaa

Browse files
committed
Fix backtraces with -C panic=abort on linux; emit unwind tables by default
The linux backtrace unwinder relies on unwind tables to work properly, and generating and printing a backtrace is done by for example the default panic hook. Begin emitting unwind tables by default again with `-C panic=abort` (see history below) so that backtraces work. History ======= Backtraces with `-C panic=abort` used to work in Rust 1.22 but broke in Rust 1.23, because in 1.23 we stopped emitting unwind tables with `-C panic=abort` (see 24cc38e). In 1.45 (see cda9946) a workaround in the form of `-C force-unwind-tables=yes` was added. `-C panic=abort` was added in [Rust 1.10](https://blog.rust-lang.org/2016/07/07/Rust-1.10/#what-s-in-1-10-stable) and the motivation was binary size and compile time. But given how confusing that behavior has turned out to be, it is better to make binary size optimization opt-in with `-C force-unwind-tables=no` rather than default since the current default breaks backtraces. Besides, if binary size is a primary concern, there are many other tricks that can be used that has a higher impact.
1 parent 15283f6 commit fe66eaa

File tree

6 files changed

+74
-9
lines changed

6 files changed

+74
-9
lines changed

compiler/rustc_session/src/session.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -758,7 +758,8 @@ impl Session {
758758
// ELF x86-64 abi, but it can be disabled for some compilation units.
759759
//
760760
// Typically when we're compiling with `-C panic=abort` we don't need
761-
// `uwtable` because we can't generate any exceptions!
761+
// `uwtable` because we can't generate any exceptions! But note that
762+
// some targets require unwind tables to generate backtraces.
762763
// Unwind tables are needed when compiling with `-C panic=unwind`, but
763764
// LLVM won't omit unwind tables unless the function is also marked as
764765
// `nounwind`, so users are allowed to disable `uwtable` emission.

compiler/rustc_target/src/spec/base/android.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,6 @@ pub(crate) fn opts() -> TargetOptions {
88
base.tls_model = TlsModel::Emulated;
99
base.has_thread_local = false;
1010
base.supported_sanitizers = SanitizerSet::ADDRESS;
11-
// This is for backward compatibility, see https://github.com/rust-lang/rust/issues/49867
12-
// for context. (At that time, there was no `-C force-unwind-tables`, so the only solution
13-
// was to always emit `uwtable`).
14-
base.default_uwtable = true;
1511
base.crt_static_respected = true;
1612
base
1713
}

compiler/rustc_target/src/spec/base/linux.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ pub(crate) fn opts() -> TargetOptions {
1212
relro_level: RelroLevel::Full,
1313
has_thread_local: true,
1414
crt_static_respected: true,
15+
// We want backtraces to work by default and they rely on unwind tables
16+
// (regardless of `-C panic` strategy).
17+
default_uwtable: true,
1518
supported_split_debuginfo: Cow::Borrowed(&[
1619
SplitDebuginfo::Packed,
1720
SplitDebuginfo::Unpacked,

compiler/rustc_target/src/spec/targets/arm_unknown_linux_gnueabihf.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ pub(crate) fn target() -> Target {
1919
max_atomic_width: Some(64),
2020
mcount: "\u{1}__gnu_mcount_nc".into(),
2121
llvm_mcount_intrinsic: Some("llvm.arm.gnu.eabi.mcount".into()),
22+
// The default on linux is to have `default_uwtable=true`, but on
23+
// this target we get an "`__aeabi_unwind_cpp_pr0` not defined"
24+
// linker error, so set it to `true` here.
25+
// FIXME(#146996): Remove this override once #146996 has been fixed.
26+
default_uwtable: false,
2227
..base::linux_gnu::opts()
2328
},
2429
}

tests/run-make/panic-abort-eh_frame/rmake.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
// An `.eh_frame` section in an object file is a symptom of an UnwindAction::Terminate
22
// being inserted, useful for determining whether or not unwinding is necessary.
3-
// This is useless when panics would NEVER unwind due to -C panic=abort. This section should
4-
// therefore never appear in the emit file of a -C panic=abort compilation, and this test
5-
// checks that this is respected.
6-
// See https://github.com/rust-lang/rust/pull/112403
3+
// This is useless when panics would NEVER unwind due to -C panic=abort and when we don't need
4+
// being able to generate backtraces (which depend on unwind tables on linux). This section should
5+
// therefore never appear in the emit file of a -C panic=abort compilation
6+
// with -C force-unwind-tables=no, and this test checks that this is respected.
7+
// See https://github.com/rust-lang/rust/pull/112403 and
8+
// https://github.com/rust-lang/rust/pull/143613.
79

810
//@ only-linux
911
// FIXME(Oneirical): the DW_CFA symbol appears on Windows-gnu, because uwtable
@@ -19,6 +21,7 @@ fn main() {
1921
.panic("abort")
2022
.edition("2021")
2123
.arg("-Zvalidate-mir")
24+
.arg("-Cforce-unwind-tables=no")
2225
.run();
2326
llvm_objdump().arg("--dwarf=frames").input("foo.o").run().assert_stdout_not_contains("DW_CFA");
2427
}
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//! Test that with `-C panic=abort` the backtrace is not cut off by default
2+
//! (i.e. without using `-C force-unwind-tables=yes`) by ensuring that our own
3+
//! functions are in the backtrace. If we just check one function it might be
4+
//! the last function, so make sure the backtrace can continue by checking for
5+
//! two functions. Regression test for
6+
//! <https://github.com/rust-lang/rust/issues/81902>.
7+
8+
//@ run-pass
9+
//@ needs-subprocess
10+
// We want to test if unwind tables are emitted by default. We must make sure
11+
// to disable debuginfo to test that, because enabling debuginfo also means that
12+
// unwind tables are emitted, which prevents us from testing what we want.
13+
// We also need to set opt-level=0 to avoid optimizing away our functions.
14+
//@ compile-flags: -C panic=abort -C opt-level=0 -C debuginfo=0
15+
//@ no-prefer-dynamic
16+
//@ ignore-apple
17+
//@ ignore-arm-unknown-linux-gnueabihf FIXME(#146996) Try removing this once #146996 has been fixed.
18+
//@ ignore-msvc Backtraces on Windows requires debuginfo which we can't use here
19+
20+
static FN_1: &str = "this_function_must_be_in_the_backtrace";
21+
fn this_function_must_be_in_the_backtrace() {
22+
and_this_function_too();
23+
}
24+
25+
static FN_2: &str = "and_this_function_too";
26+
fn and_this_function_too() {
27+
panic!("generate panic backtrace");
28+
}
29+
30+
fn run_test() {
31+
let output = std::process::Command::new(std::env::current_exe().unwrap())
32+
.arg("whatever")
33+
.env("RUST_BACKTRACE", "full")
34+
.output()
35+
.unwrap();
36+
let backtrace = std::str::from_utf8(&output.stderr).unwrap();
37+
38+
fn assert(function_name: &str, backtrace: &str) {
39+
assert!(
40+
backtrace.contains(function_name),
41+
"ERROR: no `{}` in stderr! actual stderr: {}",
42+
function_name,
43+
backtrace
44+
);
45+
}
46+
assert(FN_1, backtrace);
47+
assert(FN_2, backtrace);
48+
}
49+
50+
fn main() {
51+
let args: Vec<String> = std::env::args().collect();
52+
if args.len() == 1 {
53+
run_test();
54+
} else {
55+
this_function_must_be_in_the_backtrace();
56+
}
57+
}

0 commit comments

Comments
 (0)