Skip to content
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.

Sign up for GitHub

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

Jump to bottom

-C panic=abort doesn't generate EHABI information on arm #49867

Closed
makotokato opened this issue Apr 11, 2018 · 4 comments · Fixed by #50093
Closed

-C panic=abort doesn't generate EHABI information on arm #49867

makotokato opened this issue Apr 11, 2018 · 4 comments · Fixed by #50093
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state

Comments

@makotokato
Copy link
Contributor

From https://bugzilla.mozilla.org/show_bug.cgi?id=1453220

Gecko profiler uses EHABI information to walk stack. But since rustc doesn't generate EHABI, it cannot walk stack on generated code by rust.

@glandium says, "-C panic=abort" causes that EHABI isn't emitted", so we need a option not to remove it (or don't remove EHABI even if -C panic=abort).
@glandium
Copy link
Contributor

Note the same actually applies to EH_FRAME info on x86-64, which iirc the Firefox profiler also uses on x86-64. More generally, it can be desirable to have unwind info even when panic doesn't unwind.

@luser
Copy link
Contributor

luser commented Apr 11, 2018

For C/C++ code in Firefox we use -funwind-tables to ensure that the compiler generates these when --enable-profiling is on:
https://dxr.mozilla.org/mozilla-central/rev/0a2dae2d8cf9f628c55668514c54a23da446d5de/build/autoconf/frameptr.m4#12

The equivalent for x86-64 is -fasynchronous-unwind-tables which is apparently on by default.

It seems like having a compiler option to enable this would be good enough, since the behavior here mirrors what gcc does: only emit unwind info if needed for exceptions/unwinding.

@pietroalbini pietroalbini added C-enhancement Category: An issue proposing an enhancement or a PR with one. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state labels Apr 11, 2018
@makotokato
Copy link
Contributor Author

LLVM seems to turn on EHABI as default by https://reviews.llvm.org/D2627 (EnableARMEHABI is removed by this landing, but src/rustllvm/PassWrapper.cpp still has it)
#40549 is similar issue for Win64 ABI.

@alexcrichton
Copy link
Member

This may or may not be related to #45031, but I can prepare a patch like #40549 for the targets here. @makotokato do you know what targets are in use by Firefox and would need this treatment?

@alexcrichton alexcrichton mentioned this issue Apr 20, 2018
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this issue Apr 21, 2018
@alexcrichton
rustc: Always emit uwtable on Android
f7439a5 
Long ago (rust-lang#40549) we enabled the `uwtable` attribute on Windows by default
(even with `-C panic=abort`) to allow unwinding binaries for [stack unwinding
information][winstack]. It looks like this same issue is [plaguing][arm1]
Gecko's Android platforms [as well][arm2]. This commit applies the same fix
as rust-lang#40549 except that this time it's applied for all Android targets.

Generating a `-C panic=abort` binary for `armv7-linux-androideabi` before this
commit generated a number of `cantunwind` functions (detected with `readelf -u`)
but after this commit they all list appropriate unwind information.

Closes rust-lang#49867

[winstack]: https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
[arm1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1453220
[arm2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1451741
bors added a commit that referenced this issue Apr 21, 2018
@bors
Auto merge of #50093 - alexcrichton:android-uwtable, r=michaelwoerister
d2577ca 
rustc: Always emit `uwtable` on Android

Long ago (#40549) we enabled the `uwtable` attribute on Windows by default
(even with `-C panic=abort`) to allow unwinding binaries for [stack unwinding
information][winstack]. It looks like this same issue is [plaguing][arm1]
Gecko's Android platforms [as well][arm2]. This commit applies the same fix
as #40549 except that this time it's applied for all Android targets.

Generating a `-C panic=abort` binary for `armv7-linux-androideabi` before this
commit generated a number of `cantunwind` functions (detected with `readelf -u`)
but after this commit they all list appropriate unwind information.

Closes #49867

[winstack]: https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
[arm1]: https://bugzilla.mozilla.org/show_bug.cgi?id=1453220
[arm2]: https://bugzilla.mozilla.org/show_bug.cgi?id=1451741
@ghost ghost mentioned this issue Mar 22, 2021
Merged
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 24, 2021
@Dylan-DPC
Rollup merge of rust-lang#83391 - hyd-dev:uwtable, r=alexcrichton
30db261 
Allow not emitting `uwtable` on Android

`uwtable` is marked as required on Android, so it can't be disabled via `-C force-unwind-tables=no`. However, I found that the reason it's marked as required was to resolve a [backtrace issue in Gecko](rust-lang#49867), and I haven't find any other reasons that make it required ([yet](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Unwind.20tables.20are.20strictly.20required.20on.20Windows.20and.20Android)). Therefore, I assume it's safe to turn it off if a (nice) backtrace is not needed, and submit this PR to allow `-C force-unwind-tables=no` when targeting Android.

Note that I haven't tested this change on Android as I don't have an Android environment for testing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. O-Arm Target: 32-bit Arm processors (armv6, armv7, thumb...), including 64-bit Arm in AArch32 state
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rustc: Always emit uwtable on Android alexcrichton/rust
5 participants
@alexcrichton @luser @makotokato @glandium @pietroalbini