Ensure the personality does not panic #148105
Open
+75
−17
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use |
|
I'll open an LLD issue for this, since this doesn't seem quite right from all I found, but I think it's still useful to ensure this here. |
This comment has been minimized.
This comment has been minimized.
|
Missing an import, perhaps? |
Noratrieb
force-pushed
the
personality-panicking
branch
from
00d223b to
031bea3
Compare
|
No, the current module layout of If you are from the future and suffer from this and don't know how to debug it: I personally used build-std with |
Move `dwarf` into `gcc`, since it's only needed there and will need to make GCC-specific assumptions in the next commit.
Noratrieb
force-pushed
the
personality-panicking
branch
4 times, most recently
from
4f9d6a3 to
82ad235
Compare
|
@bors try jobs=x86_64-msvc-1,i686-msvc-1,x86_64-mingw-1,test-various,armhf-gnu,aarch64-apple |
rust-bors bot
added a commit
that referenced
this pull request
Oct 26, 2025
Ensure the personality does not panic try-job: x86_64-msvc-1 try-job: i686-msvc-1 try-job: x86_64-mingw-1 try-job: test-various try-job: armhf-gnu try-job: aarch64-apple
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
💔 Test for 6a3005f failed: CI. Failed jobs:
|
In a cdylib that uses std and is free from panics in the code, the panic machinery will *still* be pulled in because of the personality function when using LLD. The personality function is used by unwinding to figure out what to do when unwinding through a function. Each function that participates in unwind has an associated FDE (frame descriptor entries) in `.eh_frame`. This FDE points to a CIE (common information entry), which can reference a language-specific personality function, like `rust_eh_personality` in our case. As long as there is a CIE that references the personality, the function cannot be removed. If all references to a CIE get removed (because the functions and their associated FDEs have been removed), LLD will not remove the CIE, likely due to the ordering of its passes). Binutils ld will remove it. In the case where the CIE is still around (despite not being used), it will still reference the personality function, so that will still be around. This is not great since it's a bunch of code, but also not _that_ much. But this is where panicking comes in. Before this change, the personality function internally made use of `dyn Fn`. This caused an indirect call that LLVM was not able to analyze as guaranteed free of unwinding, even during fat LTO. This meant that an `invoke` was used, with a landing pad. In an `extern "C"` function, which the personality function is, all landing pads call `panic_cannot_unwind`, which is a `panic_nounwind`, which is, obviously, a panic. And as a panic, it pulls in *all* the panic machinery, which is very big and sad. It is also completely unnecessary, because these indirect functions do not panic, as they are just a convenient abstraction provided from the outside. By restructuring the code to remove these indirect calls, LLVM is able to fully analyze everything and see that rust_eh_personality cannot panic, and therefore remove its landing pad. With this change, exporting a panic-free function from a cdylib will only contain the function and the personality (when linked with LLD at least, with binutils ld it will only contain the function), with no panic code being present at all, which is great.
Noratrieb
force-pushed
the
personality-panicking
branch
from
82ad235 to
6b33cc6
Compare
|
std debug assertions 😔, I now made it a build-std test to deal with that which I don't love, but the alternatives seem worse (like just ignoring it for |
ChrisDenton
reviewed
Oct 26, 2025
Comment on lines
+37
to
+38
| // a collection of panic-related strings. if this appears in the output | ||
| // for other reasons than having panic symbols, I am sorry. |
There was a problem hiding this comment.
Ok, but what if these symbols aren't found for reasons other than not having panic symbols? E.g. this test gets subtly broken after a refactor or something else changes.
There was a problem hiding this comment.
It seems unlikely to me that all of these symbols would disappear for a reason other than panic machinery being gone, especially the "panic" one. But I'd be open to better suggestions for how to write it.
There was a problem hiding this comment.
Put a panic behind a cfg switch and compile it twice? Then check for presence and absence respectively?
|
The job Click to see the possible cause of the failure (guessed by this bot) |
hkBst
approved these changes
Oct 26, 2025
| use unwind as uw; | ||
|
|
||
| use super::dwarf::eh::{self, EHAction, EHContext}; | ||
| use self::eh::{EHAction, EHContext}; |
There was a problem hiding this comment.
Does this not work?
Suggested change
| use self::eh::{EHAction, EHContext}; | |
| use eh::{EHAction, EHContext}; |
There was a problem hiding this comment.
It does, but I prefer the self style because it makes it explicit that it's not a crate but a module, and makes rustfmt group it as such
Comment on lines
+37
to
+38
| // a collection of panic-related strings. if this appears in the output | ||
| // for other reasons than having panic symbols, I am sorry. |
There was a problem hiding this comment.
Put a panic behind a cfg switch and compile it twice? Then check for presence and absence respectively?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a cdylib that uses std and is free from panics in the code, the panic machinery will still be pulled in because of the personality function when using LLD.
The personality function is used by unwinding to figure out what to do when unwinding through a function. Each function that participates in unwind has an associated FDE (frame descriptor entries) in
.eh_frame. This FDE points to a CIE (common information entry), which can reference a language-specific personality function, likerust_eh_personalityin our case.As long as there is a CIE that references the personality, the function cannot be removed. If all references to a CIE get removed (because the functions and their associated FDEs have been removed), LLD will not remove the CIE, likely due to the ordering of its passes). Binutils ld will remove it.
In the case where the CIE is still around (despite not being used), it will still reference the personality function, so that will still be around. This is not great since it's a bunch of code, but also not that much. But this is where panicking comes in.
Before this change, the personality function internally made use of
dyn Fn. This caused an indirect call that LLVM was not able to analyze as guaranteed free of unwinding, even during fat LTO. This meant that aninvokewas used, with a landing pad. In anextern "C"function, which the personality function is, all landing pads callpanic_cannot_unwind, which is apanic_nounwind, which is, obviously, a panic. And as a panic, it pulls in all the panic machinery, which is very big and sad.It is also completely unnecessary, because these indirect functions do not panic, as they are just a convenient abstraction provided from the outside. By restructuring the code to remove these indirect calls, LLVM is able to fully analyze everything and see that rust_eh_personality cannot panic, and therefore remove its landing pad.
With this change, exporting a panic-free function from a cdylib will only contain the function and the personality (when linked with LLD at least, with binutils ld it will only contain the function), with no panic code being present at all, which is great.