Skip to content

[IRGen] Don't use GOTPCREL relocations for x86 ELF. #82176

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

Merged
merged 1 commit into from
Jun 13, 2025

Conversation

al45tair
Copy link
Contributor

@al45tair al45tair commented Jun 11, 2025

Unfortunately, x86 ELF linkers like to optimize GOTPCREL relocations by replacing mov instructions that go via the GOT with lea instructions that do not.

That would be fine, but they aren't very selective and will happily perform this transformation in non-code sections if they think that the bytes before a relocation look like a mov instruction.

This corrupts our metadata.

rdar://148168098

Unforunately, x86 ELF linkers like to optimize GOTPCREL relocations by
replacing `mov` instructions that go via the GOT with `lea` instructions
that do not.

That would be fine, but they aren't very selective and will happily
perform this transformation in non-code sections if they think that
the bytes before a relocation look like a `mov` instruction.

This corrupts our metadata.

rdar://148168098
@al45tair al45tair requested a review from rjmccall as a code owner June 11, 2025 11:06
@al45tair al45tair requested a review from jckarter June 11, 2025 11:06
@al45tair
Copy link
Contributor Author

I already know the PR testing will fail because we've currently got half of the changes for the new OS version numbering. However:

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

I mean, I hadn't expected it to fail quite that quickly. Let's try again.

@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test

@al45tair
Copy link
Contributor Author

Please test with the following PR:
swiftlang/llvm-project#10823

@swift-ci Please smoke test

if (!IGM.getOptions().UseJIT &&
(!IGM.Triple.isOSDarwin() || IGM.Triple.getArch() != llvm::Triple::x86)) {
(!IGM.Triple.isOSDarwin() || IGM.Triple.getArch() != llvm::Triple::x86) &&
(!IGM.Triple.isOSBinFormatELF() || !IGM.Triple.isX86())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you happen to know offhand if the linker on other ELF platforms (like FreeBSD) is also affected by this bug?

Copy link
Contributor Author

@al45tair al45tair Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FreeBSD uses lld, which does have this problem (and, unlike the BFD linker, while it understands -no-relax and won't complain about it, it ignores it for the purposes of this particular optimisation).

Basically, lld, gold and the BFD linker all do this; only the BFD linker has a way to disable it, and none of them test whether they're looking at a code section first. They do all have a handful of things they exempt from this behaviour (though the exact set differs; BFD and lld have more exceptions than gold does), but sadly none of them check whether they're in a code section first.

I'm aware that in principle there might be a platform using some other ELF linker that doesn't do this, but even in that case, someone might choose to use one of the three linkers above that all do.

(It looks to me like mold may be such a linker, just at a quick glance.)

@al45tair
Copy link
Contributor Author

Please test with the following PR:
swiftlang/llvm-project#10823

@swift-ci Please smoke test Windows platform

@al45tair
Copy link
Contributor Author

@swift-ci Please smoke test Windows platform

@al45tair al45tair merged commit 6936e2b into swiftlang:main Jun 13, 2025
3 checks passed
Sign up for free to 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.

2 participants