-
Notifications
You must be signed in to change notification settings - Fork 13.5k
Fix backtraces with -C panic=abort
on linux; emit unwind tables by default
#143613
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @petrochenkov. Use |
These commits modify compiler targets. This PR modifies cc @jieyouxu |
-Cpanic=abort
on linux-C panic=abort
on linux; emit unwind tables by default
Note that it’s currently pretty tricky to change this setting from whatever the default is on a given target, since there’s no Cargo profile setting for it (unlike the panic strategy), cc rust-lang/cargo#15333 This observation can probably be used as argument for or against this change. |
If you don't want unwind tables to save space you probably want to strip it out after linking anyway. |
If there's a way to drop all unwind tables that can safely be dropped but no more, then that would be a nice new feature ( |
…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. 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 24cc38e). In 1.45 (see cda9946) a workaround in the form of `-C force-unwind-tables=yes` was added. `-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.
d113be9
to
826aa60
Compare
Changes since the initial version d113be9:
I don't think ordinary users want to go back to the regression introduced in Rust 1.23, because I expect ordinary users to want backtraces to work. For advanced users I don't think it's too much to ask to require building with |
Stripping unwind tables is going to change the behaviour if you link in some foreign static library (e.g. a C++ library) that depends on unwinding.
I think there is no soundness concern, it's just correctness? Unwinding will stop with error if it cannot find unwind tables. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change looks correct to me.
Right, only keeping |
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 #81902 which is regression-from-stable-to-stable
Closes #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 #45031 and #81902 (comment)).In 1.45 a workaround in the form of
-C force-unwind-tables=yes
was added (see #69984).-C panic=abort
was added in Rust 1.10 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
-C panic=abort
on Linux by generating unwind tables by default. Build with-C force-unwind-tables=no
to keep omitting unwind tables.