- 
                Notifications
    You must be signed in to change notification settings 
- Fork 13.9k
          rustc: Always emit the uwtable attribute on Windows
          #40549
        
          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
Conversation
| r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) | 
| 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. | 
b5f9a9b    to
    165a295      
    Compare
  
    | @bors r+ | 
| 📌 Commit 165a295 has been approved by  | 
| 🔒 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
165a295    to
    ef90d32      
    Compare
  
    | @bors r=eddyb | 
| 📌 Commit ef90d32 has been approved by  | 
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
| ☀️ Test successful - status-appveyor, status-travis | 
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
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
This commit alters the translation layer to unconditionally emit the
uwtableLLVM attribute on Windows regardless of the
no_landing_padssetting.Previously I believe we omitted this attribute as an optimization when the
-Cpanic=abortflag was passed, but this unfortunately caused problems forGecko.
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
uwtableattribute, 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, sothis is still somewhat working on just suspicion.