Panic backtrace doesn't traverse ffi code with panic=abort

[glandium]

I tried this code:

Cargo.toml:

[package]
name = "testcase"
version = "0.1.0"
edition = "2018"

[profile.release]
panic="abort"

[profile.dev]
panic="abort"

[build-dependencies]
cc = "1"

build.rs:

fn main() {
    cc::Build::new().file("foo.c").compile("foo");
}

foo.c:

#include <stdio.h>

extern void do_rust_panic(const char *msg);

void foo() {
	do_rust_panic("foo");
        // prevent tail-call optimization so that the frame for foo is always there
	printf("foo\n");
}

src/main.rs:

use std::ffi::CStr;
use std::os::raw::c_char;

#[no_mangle]
pub unsafe extern "C" fn do_rust_panic(message: *const c_char) {
    panic!("{}", CStr::from_ptr(message).to_string_lossy());
}

extern "C" {
    fn foo();
}

fn main() {
    unsafe {
        foo();
    }
    // prevent tail-call optimization so that the frame for main is always there
    println!("Hello, world!");
}

I expected to see this happen: the panic backtrace would traverse the ffi code, like it does without panic=abort.

Instead, this happened: the panic backtrace stops at do_rust_panic:

$ RUST_BACKTRACE=1 cargo run --release
    Finished release [optimized] target(s) in 0.00s
     Running `target/release/testcase`
Hello, world!
thread 'main' panicked at 'foo', src/main.rs:6:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5
   1: std::panicking::begin_panic_fmt
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:457:5
   2: do_rust_panic
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Without panic=abort:

$ RUST_BACKTRACE=1 cargo run --release
    Finished release [optimized] target(s) in 0.00s
     Running `target/release/testcase`
thread 'main' panicked at 'foo', src/main.rs:6:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5
   1: std::panicking::begin_panic_fmt
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:457:5
   2: do_rust_panic
   3: foo
   4: testcase::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Meta

rustc --version --verbose:

rustc 1.54.0 (a178d0322 2021-07-26)
binary: rustc
commit-hash: a178d0322ce20e33eac124758e837cbd80a6f633
commit-date: 2021-07-26
host: x86_64-unknown-linux-gnu
release: 1.54.0
LLVM version: 12.0.1

Also reproducible with nightly:

rustc 1.56.0-nightly (af140757b 2021-08-22)
binary: rustc
commit-hash: af140757b4cb1a60d107c690720311ba8e06e7de
commit-date: 2021-08-22
host: x86_64-unknown-linux-gnu
release: 1.56.0-nightly
LLVM version: 13.0.0

I tried tweaking the FFI build flags, adding combinations of -fno-omit-frame-pointer, -funwind-tables and -g, and none of them changed anything.

[glandium]

The cause is given in the backtrace crate documentation:

Rust code may be compiled without unwinding information for some functions. This can also happen for Rust code compiled with -Cpanic=abort. You can remedy this, however, with -Cforce-unwind-tables as a compiler option.

$ RUSTFLAGS=-Cforce-unwind-tables RUST_BACKTRACE=1 cargo run --release 
    Finished release [optimized] target(s) in 0.00s
     Running `target/release/testcase`
thread 'main' panicked at 'foo', src/main.rs:6:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:515:5
   1: std::panicking::begin_panic_fmt
             at /rustc/a178d0322ce20e33eac124758e837cbd80a6f633/library/std/src/panicking.rs:457:5
   2: do_rust_panic
   3: foo
   4: testcase::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

[Enselic]

Triage: Since the backtrace crate was not directly used, it seems like this can be clarified/warned about somewhere. Maybe here? Not sure. But it seems like documentation can be made more discoverable.

@glandium Do you happen to remember where you looked for documentation regarding this? That place is a good candidate for improving the docs.

[glandium]

I'm not sure where I looked back then. I was probably just expecting things to work and didn't try to find the information (my bad, I guess). But if I were, I would probably look at std::panic (BTW, the std::panic module doc should probably say that plenty of information is in the doc for the macro ; also the std::panic macro doc doesn't mention panic=abort). I guess the book's chapter about panics would be a good place too (https://doc.rust-lang.org/book/ch09-01-unrecoverable-errors-with-panic.html)

[hkBst]

I'm confused. Isn't the entire point of panic=abort to not unwind and thus not produce a proper backtrace (in exchange for a smaller binary)?

[hanna-kruppe]

Capturing a stack trace is different from unwinding the stack, calling drop glue in every frame, and possibly stopping at a catch_unwind. Capturing a stack trace requires significantly less cooperation from the program, does not restrict program optimizations in the same way (because there’s no extra control flow edges the program could take via unwinding), and requires much smaller side-tables (if any - if frame pointers are preserved those are usually sufficient on their own) than unwinding. It’s also obviously very useful for diagnosing why a program aborted, not to mention debugging or profiling a running program.

As for the original issue, yeah it would be good to document the importance of -C force-unwind-tables somewhere in the std docs, such as the std::panic module or the panic macro.

[hkBst]

@hanna-kruppe thanks for the clear and detailed explanation.

Issue #94815 suggests just forcing that "-C force-unwind-tables" flag, which seems like a good idea, in which case the docs would not need to talk about this...

[hanna-kruppe]

I also think it would be good to change the default on some targets at least, but it would still be nice to explain the implications of not having the Tables somewhere. Note that some targets already default to or require such tables (eg, 64-bit windows) but I wouldn’t want to assume that all targets will necessarily default to them. Especially once build-std is stabilized and users can modify which flags the sysroot crates are compiled with.