Auto merge of rust-lang#143613 - Enselic:panic-abort-uwtables, r=petrochenkov

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.

Closes rust-lang#81902 which is **regression-from-stable-to-stable**
Closes rust-lang#94815

### 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 rust-lang#45031 and rust-lang#81902 (comment)).

In 1.45 a workaround in the form of `-C force-unwind-tables=yes` was added (see rust-lang#69984).

`-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.

# Release Note Entry Draft:

## Compatibility Notes

* [Fix backtraces with `-C panic=abort` on Linux by generating unwind tables by default](rust-lang#143613). Build with `-C force-unwind-tables=no` to keep omitting unwind tables.

try-job: aarch64-apple
try-job: armhf-gnu
try-job: aarch64-msvc-1

Author: bors

compiler/rustc_session/src/session.rs

@@ -758,7 +758,8 @@ impl Session {
         //     ELF x86-64 abi, but it can be disabled for some compilation units.
         //
         // Typically when we're compiling with `-C panic=abort` we don't need
-        // `uwtable` because we can't generate any exceptions!
+        // `uwtable` because we can't generate any exceptions! But note that
+        // some targets require unwind tables to generate backtraces.
         // Unwind tables are needed when compiling with `-C panic=unwind`, but
         // LLVM won't omit unwind tables unless the function is also marked as
         // `nounwind`, so users are allowed to disable `uwtable` emission.

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

@@ -8,10 +8,6 @@ pub(crate) fn opts() -> TargetOptions {
     base.tls_model = TlsModel::Emulated;
     base.has_thread_local = false;
     base.supported_sanitizers = SanitizerSet::ADDRESS;
-    // This is for backward compatibility, see https://github.com/rust-lang/rust/issues/49867
-    // for context. (At that time, there was no `-C force-unwind-tables`, so the only solution
-    // was to always emit `uwtable`).
-    base.default_uwtable = true;
     base.crt_static_respected = true;
     base
 }

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

@@ -12,6 +12,9 @@ pub(crate) fn opts() -> TargetOptions {
         relro_level: RelroLevel::Full,
         has_thread_local: true,
         crt_static_respected: true,
+        // We want backtraces to work by default and they rely on unwind tables
+        // (regardless of `-C panic` strategy).
+        default_uwtable: true,
         supported_split_debuginfo: Cow::Borrowed(&[
             SplitDebuginfo::Packed,
             SplitDebuginfo::Unpacked,

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

@@ -19,6 +19,11 @@ pub(crate) fn target() -> Target {
             max_atomic_width: Some(64),
             mcount: "\u{1}__gnu_mcount_nc".into(),
             llvm_mcount_intrinsic: Some("llvm.arm.gnu.eabi.mcount".into()),
+            // The default on linux is to have `default_uwtable=true`, but on
+            // this target we get an "`__aeabi_unwind_cpp_pr0` not defined"
+            // linker error, so set it to `true` here.
+            // FIXME(#146996): Remove this override once #146996 has been fixed.
+            default_uwtable: false,
             ..base::linux_gnu::opts()
         },
     }

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

@@ -1,9 +1,11 @@
 // An `.eh_frame` section in an object file is a symptom of an UnwindAction::Terminate
 // being inserted, useful for determining whether or not unwinding is necessary.
-// This is useless when panics would NEVER unwind due to -C panic=abort. This section should
-// therefore never appear in the emit file of a -C panic=abort compilation, and this test
-// checks that this is respected.
-// See https://github.com/rust-lang/rust/pull/112403
+// This is useless when panics would NEVER unwind due to -C panic=abort and when we don't need
+// being able to generate backtraces (which depend on unwind tables on linux). This section should
+// therefore never appear in the emit file of a -C panic=abort compilation
+// with -C force-unwind-tables=no, and this test checks that this is respected.
+// See https://github.com/rust-lang/rust/pull/112403 and
+// https://github.com/rust-lang/rust/pull/143613.
 
 //@ only-linux
 // FIXME(Oneirical): the DW_CFA symbol appears on Windows-gnu, because uwtable
@@ -19,6 +21,7 @@ fn main() {
         .panic("abort")
         .edition("2021")
         .arg("-Zvalidate-mir")
+        .arg("-Cforce-unwind-tables=no")
         .run();
     llvm_objdump().arg("--dwarf=frames").input("foo.o").run().assert_stdout_not_contains("DW_CFA");
 }

tests/ui/panics/panic-abort-backtrace-without-debuginfo.rs

@@ -0,0 +1,57 @@
+//! Test that with `-C panic=abort` the backtrace is not cut off by default
+//! (i.e. without using `-C force-unwind-tables=yes`) by ensuring that our own
+//! functions are in the backtrace. If we just check one function it might be
+//! the last function, so make sure the backtrace can continue by checking for
+//! two functions. Regression test for
+//! <https://github.com/rust-lang/rust/issues/81902>.
+
+//@ run-pass
+//@ needs-subprocess
+// We want to test if unwind tables are emitted by default. We must make sure
+// to disable debuginfo to test that, because enabling debuginfo also means that
+// unwind tables are emitted, which prevents us from testing what we want.
+// We also need to set opt-level=0 to avoid optimizing away our functions.
+//@ compile-flags: -C panic=abort -C opt-level=0 -C debuginfo=0
+//@ no-prefer-dynamic
+//@ ignore-apple
+//@ ignore-arm-unknown-linux-gnueabihf FIXME(#146996) Try removing this once #146996 has been fixed.
+//@ ignore-msvc Backtraces on Windows requires debuginfo which we can't use here
+
+static FN_1: &str = "this_function_must_be_in_the_backtrace";
+fn this_function_must_be_in_the_backtrace() {
+    and_this_function_too();
+}
+
+static FN_2: &str = "and_this_function_too";
+fn and_this_function_too() {
+    panic!("generate panic backtrace");
+}
+
+fn run_test() {
+    let output = std::process::Command::new(std::env::current_exe().unwrap())
+        .arg("whatever")
+        .env("RUST_BACKTRACE", "full")
+        .output()
+        .unwrap();
+    let backtrace = std::str::from_utf8(&output.stderr).unwrap();
+
+    fn assert(function_name: &str, backtrace: &str) {
+        assert!(
+            backtrace.contains(function_name),
+            "ERROR: no `{}` in stderr! actual stderr: {}",
+            function_name,
+            backtrace
+        );
+    }
+    assert(FN_1, backtrace);
+    assert(FN_2, backtrace);
+}
+
+fn main() {
+    let args: Vec<String> = std::env::args().collect();
+    if args.len() == 1 {
+        run_test();
+    } else {
+        this_function_must_be_in_the_backtrace();
+    }
+}