Auto merge of rust-lang#75048 - eggyal:force-no-tco-start-backtrace-frame, r=Mark-Simulacrum

Prevent `__rust_begin_short_backtrace` frames from being tail-call optimised away

I've stumbled across some situations where there (unexpectedly) was no `__rust_begin_short_backtrace` frame on the stack during unwinding.

On closer examination, it appeared that the calls to that function had been tail-call optimised away.

This PR follows [@bjorn3's suggestion on Zulip](https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Disabling.20tail.20call.20optimisation.3F/near/205699133), by adding calls to `black_box` that hint to rustc not to perform TCO.

Fixes rust-lang#47429

Author: bors

library/std/src/panicking.rs

@@ -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
@@ -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>,

library/std/src/rt.rs

@@ -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
@@ -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,
+    )
 }

library/std/src/sys_common/backtrace.rs

@@ -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;
@@ -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;
@@ -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 {

library/test/src/lib.rs

@@ -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(

src/test/ui/panics/issue-47429-short-backtraces.rs

@@ -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!()
+}

src/test/ui/panics/issue-47429-short-backtraces.run.stderr

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