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

Miscompilation on i686-pc-windows-msvc with Fat LTO #112576

Closed
kdy1 opened this issue Jun 13, 2023 · 8 comments
Closed

Miscompilation on i686-pc-windows-msvc with Fat LTO #112576

kdy1 opened this issue Jun 13, 2023 · 8 comments
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-x86_32 Target: x86 processors, 32 bit (like i686-*) S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress.

Comments

@kdy1
Copy link
Contributor

kdy1 commented Jun 13, 2023

I tried this code:

https://github.com/swc-project/swc/blob/275db1baec0046c15e35456c268295c5e7a42517/crates/swc_ecma_codegen/src/lib.rs#L2435-L2463

(May not be exact, just a guess based on the test failure)

I expected to see this happen: Binaries behave identically regardless of CPU arch.

Instead, this happened: CI for publishing action failed.

Meta

I'm trying to enable LTO for release builds of swc, with swc-project/swc#7509. But it seems like rustc miscompiles on some platforms if I enable LTO.

Full log: https://github.com/swc-project/swc/actions/runs/5212808850/jobs/9406918334?pr=7509

rustc --version --verbose:

rustc 1.70.0-nightly (da7c50c08 2023-03-19)
binary: rustc
commit-hash: da7c50c089d5db2d3ebaf227fe075bb1346bfaec
commit-date: 2023-03-19
host: aarch64-apple-darwin
release: 1.70.0-nightly
LLVM version: 15.0.7
@kdy1 kdy1 added the C-bug Category: This is a bug. label Jun 13, 2023
@Noratrieb Noratrieb added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. O-x86 I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Jun 13, 2023
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jun 13, 2023
@Noratrieb
Copy link
Member

may or may not be related to #109114
it would be nice to have a more minimal example to rule out undefined behavior in SWC

@lqd
Copy link
Member

lqd commented Jun 13, 2023

There's no mention of -Zdylib-lto in this project and the PR looks to be enabling fat lto ? There are/were other issues for ThinLTO + LLD on msvc.

@apiraino apiraino added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Jun 13, 2023
@kdy1
Copy link
Contributor Author

kdy1 commented Jun 14, 2023

@lqd Do you mean I should use -Zdylib-lto if I want to enable the fat LTO?

@Noratrieb
Copy link
Member

no, but that flag is known to be broken already

@lqd
Copy link
Member

lqd commented Jun 14, 2023

@lqd Do you mean I should use -Zdylib-lto if I want to enable the fat LTO?

No, it's an internal flag (basically only intended for rustc to use) that has known issues on windows.

I meant that:

@kdy1 kdy1 changed the title Miscompilation on i686-pc-windows-msvc with Thin LTO Miscompilation on i686-pc-windows-msvc with Fat LTO Jun 14, 2023
@Noratrieb Noratrieb added O-x86_32 Target: x86 processors, 32 bit (like i686-*) and removed O-x86-all labels Oct 25, 2023
@apiraino
Copy link
Contributor

@kdy1 are you still affected by this issue? Can you provide an update or close - if it's the case? Thanks

@kpreid
Copy link
Contributor

kpreid commented Dec 25, 2023

Relabeling issues which don't have a runnable reproduction (as opposed to not having a minimized one) to the new label S-needs-repro.
@rustbot label +S-needs-repro -E-needs-mcve

@rustbot rustbot added S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress. and removed E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example labels Dec 25, 2023
@kdy1
Copy link
Contributor Author

kdy1 commented Dec 25, 2023

Closing as I don't have time to create a minimal repro

@kdy1 kdy1 closed this as completed Dec 25, 2023
@apiraino apiraino removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness O-x86_32 Target: x86 processors, 32 bit (like i686-*) S-needs-repro Status: This issue has no reproduction and needs a reproduction to make progress.
Projects
None yet
Development

No branches or pull requests

6 participants