Repository navigation
LLVM 17 regression: tests/ui/backtrace.rs can't double-panic anymore #109672
Description
Activity
- added a commit that references this issue
on Mar 27, 2023 so it seems like maybe LLVM is incorrect about this double-panic not being reachable or something.
this seems like a bug in LLVM itself? why did you file it against rust-lang/rust?
Hm, the IR looks fine to me -- double() still has the check for double panics. Not sure why it doesn't work.
Looks like error code 3 is _URC_FATAL_PHASE1_ERROR.
rust_eh_personality returns _URC_CONTINUE_UNWIND for the
backtrace::doubleframe and _URC_FATAL_PHASE1_ERROR for thebracktrace::mainframe.I think the problem here is that LLVM now realizes that
double()can't unwind because it will always end up aborting due to double panic. This means that the invoke todouble()inmain()will be converted into a call and won't have an LSD entry. This then causes rust_eh_personality to signal an error.This seems correct from an LLVM perspective, but not sure who/what is actually at fault here. Maybe @Amanieu knows.
_URC_FATAL_PHASE1_ERRORmeans that either:- Exception handling metadata is invalid, and libgcc (which provides the unwind implementation on Linux) is complaining.
- The personality routine returned an error (anything except _URC_CONTINUE_UNWIND or _URC_HANDLER_FOUND).
Based on my reading of the personality function (
library/std/src/peronality/gcc.rs), an error only occurs if exception handling metadata is invalid, or if there is an exception from a call site that doesn't have a corresponding entry in the LSDA call site table. This might happen if we are throwing an exception from a call site that LLVM assumes isnoexcept.Note that this is not something that LLVM should assume on its own: throwing an exception from a landing pad is well-defined and should propagate that exception up to the caller. If we (or LLVM) are something incorrectly marking a call as
noexceptthen this should be fixed.Does this still reproduce if the test case is reduced to just the
double()function? If so, could you post the IR and generated assembly code so I can have a look?Hrm, just reducing it to
double()makes it stop reproducing. Here's the smallest I was able to produce:use std::env; use std::process::Command; #[inline(never)] fn double() { struct Double; impl Drop for Double { fn drop(&mut self) { panic!("twice") } } let _d = Double; panic!("once"); } fn main() { let args: Vec<String> = env::args().collect(); if args.len() >= 2 && args[1] == "fail" { } else if args.len() >= 2 && args[1] == "double-fail" { double(); } else { Command::new(&args[0]); } }
The IR/asm for that file looks like this: https://gist.github.com/nikic/a3a4322442c8f63b507cbe5a4a32b0db
Based on my reading of the personality function (library/std/src/peronality/gcc.rs), an error only occurs if exception handling metadata is invalid, or if there is an exception from a call site that doesn't have a corresponding entry in the LSDA call site table. This might happen if we are throwing an exception from a call site that LLVM assumes is noexcept.
As I understand the problem, this is what happens. The call site is noexcept because the function is known to ultimately abort due to double panic.
Cleanup handlers get skipped during the phase 1 walk. Normally, we will always hit a catch handler eventually (in lang_start_internal if nothing else). In this case this doesn't happen because it is known that no exception can unwind that far up (once cleanup handlers have been executed).
Clang uses a
catchhandler fornoexceptfunctions (https://godbolt.org/z/aYKGGdanq) which avoids this problem.I remember we had a good reason for using
cleanupinstead ofcatchforTerminatorKind::Abort, but I can't remember what it was.cc @nbdd0121
We just don't have a reliable way to tell whether we'll be aborting or not, because
TerminatorKind::Abortis a terminator that is part of cleanup blocks. I do plan to move away fromcleanup(#104070, but tofilterinstead ofcatch, so our personality function can tell it apart from the try intrinsic), but that PR is currently blocked on having the MIR unwind refactor #102906 merged (which will make aborting action explicit).I don't think that changing how we handle the double panic abort will fully fix, because we get the same behavior if we just explicitly abort inside the cleanup:
#[inline(never)] fn double() { struct Double; impl Drop for Double { fn drop(&mut self) { std::process::abort(); } } let _d = Double; panic!("once"); }
This wouldn't be affected by the proposed changes, right?
Hah, a race condition there :)
Reacted by Nikita PopovI think we should just return
_URC_HANDLER_FOUNDfor acleanup, instead of_URC_CONTINUE_UNWINDEDIT: Actually this isn't quite right because we won't be able to resume if we returned
_URC_HANDLER_FOUNDOne possible approach is to return
_URC_HANDLER_FOUNDwhen we can't find a LSDA entry in phase 1. This should work if the caller frame is a Rust frame. However, this can be problematic if the function being transformed is aextern "C"function that's called from FFI, since the personality function will be out of our control.Hm, it looks like this also affects C++. For the following snippet:
#include <cstdlib> #include <iostream> struct Abort { ~Abort() { abort(); } }; __attribute__((noinline)) static void abort_in_dtor() { Abort abort; throw "test"; } int main() { try { abort_in_dtor(); } catch (...) { std::cout << "caught" << std::endl; } }
I get:
terminate called after throwing an instance of 'char const*' Aborted (core dumped)Rather than just an abort.
So this does also affect the
__gxx_personality_v0personality function as well.I've filed llvm/llvm-project#61945 on the LLVM side with the Clang reproducer.
Reacted by Elichai TurkelWould we still have issue for the abort case (the double-panic case should be fixed)?
test.rs:
#[no_mangle] extern "C" fn d() { struct Double; impl Drop for Double { fn drop(&mut self) { std::process::abort(); } } let _d = Double; panic!("once"); }
test.c:
void d(); int main(void) { d(); }
compile with:
rustc -O test.rs --crate-type staticlib gcc test.c libtest.a ./a.out
and I get:
thread '<unnamed>' panicked at 'once', test.rs:13:5 note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace fatal runtime error: failed to initiate panic, error 5 Aborted@nbdd0121 Is that with LLVM 17? The abort case should be fixed as well. We consider all cleanup landingpads as unwinding now, including those that just abort.
I tested this on LLVM 15 and 16 and it could be reproduced in both cases. I'll file a new issue.
- added a commit that references this issue
on May 24, 2025
LLVM change 0d4a709 (https://reviews.llvm.org/D145210) caused LLVM to have enough smarts to figure out that double panics "can't" happen - we already adjusted some tests for this (eg e4a4064 where we fixed tests/codegen/vec-shrink-panik.rs) but this seems to be a bit of an actual regression, as the code asserts the double-panic happens, but instead the double panic fails like:
so it seems like maybe LLVM is incorrect about this double-panic not being reachable or something.
Filing this bug and disabling the test in our Rust/LLVM HEADs CI for now so we see other (more urgent) breakage reliably.