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.

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

.eh_frame is emitted for -C panic=abort on (at least) i686-pc-windows-gnu #128136

Closed
jieyouxu opened this issue Jul 24, 2024 · 5 comments
Closed
Labels
A-codegen Area: Code generation A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-panic Area: Panicking machinery C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jieyouxu
Copy link
Member

#112403 tried to prevent .eh_frame from being emitted for -Cpanic=abort and added a test (tests/run-make/panic-abort-eh_frame/Makefile) to check unnecessary landing pads related to unwinding do not appear in the object file when -Cpanic=abort.

The original Makefile test was only-linux, and when @Oneirical tried to run the (roughly) equivalent test on Windows (see #127523), we found that this "no unnecessary landing pads" property (whichever property is checked by proxy via testing if DW_CFA exists) is not preserved at least on i686-pc-windows-gnu.

I don't understand much of this test, so I'll ask those who are more knowledgeable: for people more familiar with this test cc @nbdd0121 and @Nilstrieb, is this property also intended to be upheld on Windows and not just Linux?

@jieyouxu jieyouxu added O-windows Operating system: Windows A-codegen Area: Code generation T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html C-bug Category: This is a bug. A-panic Area: Panicking machinery labels Jul 24, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 24, 2024
@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jul 24, 2024
@jieyouxu jieyouxu changed the title .eh_frame is emitted for -C panic=abort on (at least) i686-pc-windows-gnu .eh_frame is emitted for -C panic=abort on (at least) i686-pc-windows-gnu Jul 24, 2024
@nbdd0121
Copy link
Contributor

We currently force uwtables to be emitted on Windows:

@jieyouxu
Copy link
Member Author

We currently force uwtables to be emitted on Windows:

Thank you, that's the context I'm missing 👍

@jieyouxu
Copy link
Member Author

Actually re-opening because

the test does pass on MSVC (see rust-lang-ci/rust/actions/runs/10017286801/job/27691281649, it's not even getting ignored). It even passes on x86 gnu-windows. It is only this one specific architecture (i686-mingw) that fails.

So I'm really unsure what the intended behavior of this test is (on the i686-mingw CI job at least).

@jieyouxu jieyouxu reopened this Jul 25, 2024
@nbdd0121
Copy link
Contributor

On windows-gnu we also force uwtables.

What's the issue of keeping only-linux?

@jieyouxu
Copy link
Member Author

jieyouxu commented Jul 25, 2024

Hm, it was strange that only i686-mingw had the panic symbol but not other windows runners. Anyway, we can keep it only-linux. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-MIR Area: Mid-level IR (MIR) - https://blog.rust-lang.org/2016/04/19/MIR.html A-panic Area: Panicking machinery C-bug Category: This is a bug. O-windows Operating system: Windows T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants