Skip to content

rustc: Always emit the uwtable attribute on Windows - #40549

Merged
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:uwtable-everywhere
Mar 23, 2017
Merged

rustc: Always emit the uwtable attribute on Windows#40549
bors merged 1 commit into
rust-lang:masterfrom
alexcrichton:uwtable-everywhere

Conversation

@alexcrichton

Copy link
Copy Markdown
Member

This commit alters the translation layer to unconditionally emit the uwtable
LLVM attribute on Windows regardless of the no_landing_pads setting.
Previously I believe we omitted this attribute as an optimization when the
-Cpanic=abort flag was passed, but this unfortunately caused problems for
Gecko.

It was discovered that there was trouble unwinding through Rust functions due
to foreign exceptions such as illegal instructions or otherwise in-practice
methods used to abort a process. In testing it looked like the major difference
between a working binary and a non-working binary is indeed this uwtable
attribute, but this PR has unfortunately not been thoroughly tested in terms of
compiling Gecko with -C panic=abortand this PR to see whether it works, so
this is still somewhat working on just suspicion.

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@retep998

Copy link
Copy Markdown
Contributor

Even if you never have C++ exceptions or Rust panics, you can still have SEH exceptions and those use the same unwinding mechanism and thus require unwinding tables.

@alexcrichton
alexcrichtonforce-pushed the uwtable-everywhere branch 2 times, most recently from b5f9a9b to 165a295CompareMarch 15, 2017 22:10
@eddyb

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Mar 21, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit 165a295 has been approved by eddyb

@bors

bors commented Mar 21, 2017

Copy link
Copy Markdown
Collaborator

🔒 Merge conflict

This commit alters the translation layer to unconditionally emit the `uwtable`
LLVM attribute on Windows regardless of the `no_landing_pads` setting.
Previously I believe we omitted this attribute as an optimization when the
`-Cpanic=abort` flag was passed, but this unfortunately caused problems for
Gecko.
It [was discovered] that there was trouble unwinding through Rust functions due
to foreign exceptions such as illegal instructions or otherwise in-practice
methods used to abort a process. In testing it looked like the major difference
between a working binary and a non-working binary is indeed this `uwtable`
attribute, but this PR has unfortunately not been thoroughly tested in terms of
compiling Gecko with `-C panic=abort` *and* this PR to see whether it works, so
this is still somewhat working on just suspicion.
[was discovered]: https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
@bstrie

Copy link
Copy Markdown
Contributor

@bors r=eddyb

@bors

bors commented Mar 21, 2017

Copy link
Copy Markdown
Collaborator

📌 Commit ef90d32 has been approved by eddyb

@bors

bors commented Mar 23, 2017

Copy link
Copy Markdown
Collaborator

⌛ Testing commit ef90d32 with merge 90346ea...

bors added a commit that referenced this pull request Mar 23, 2017
rustc: Always emit the `uwtable` attribute on Windows
This commit alters the translation layer to unconditionally emit the `uwtable`
LLVM attribute on Windows regardless of the `no_landing_pads` setting.
Previously I believe we omitted this attribute as an optimization when the
`-Cpanic=abort` flag was passed, but this unfortunately caused problems for
Gecko.
It [was discovered] that there was trouble unwinding through Rust functions due
to foreign exceptions such as illegal instructions or otherwise in-practice
methods used to abort a process. In testing it looked like the major difference
between a working binary and a non-working binary is indeed this `uwtable`
attribute, but this PR has unfortunately not been thoroughly tested in terms of
compiling Gecko with `-C panic=abort` *and* this PR to see whether it works, so
this is still somewhat working on just suspicion.
[was discovered]: https://bugzilla.mozilla.org/show_bug.cgi?id=1302078
@bors

bors commented Mar 23, 2017

Copy link
Copy Markdown
Collaborator

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 90346ea to master...

@bors
bors merged commit ef90d32 into rust-lang:masterMar 23, 2017
@alexcrichton
alexcrichton deleted the uwtable-everywhere branch April 4, 2017 23:08
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Apr 21, 2018
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.
Closesrust-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 pull request Apr 21, 2018
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
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@alexcrichton@rust-highfive@retep998@eddyb@bors@bstrie@arielb1