Skip to content

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

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

Open
wants to merge 1 commit into
base: release/6.1
Choose a base branch
from

Conversation

al45tair
Copy link
Contributor

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

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 a team as a code owner June 13, 2025 09:28
@al45tair al45tair added 🍒 release cherry pick Flag: Release branch cherry picks swift 6.1 labels Jun 13, 2025
@al45tair
Copy link
Contributor Author

Explanation: ELF linkers will optimize GOTPCREL relocations by replacing mov instructions with lea instructions, which enables them to remove the GOT entry and a level of indirection. That's great, but they do it in non-code sections as well, and assume that if there's an 0x8b two bytes before the relocation, it must be a mov instruction. This can cause unfortunate behaviour where they simultaneously corrupt the word before the relocation (by changing 0x8b to 0x8d) and remove the indirection while not clearing the bottom bit (because they don't know that Swift metadata uses that to indicate indirection), thus messing up our metadata.
Risk: Low. Only affects Linux, FreeBSD et al, and works by enabling code we were already using elsewhere.
Original PR: #82176
Reviewed by: @jckarter
Resolves: rdar://148168098
Tests: This affects basically all Swift code generation, since it changes the relocations we use for relative pointers in the Swift metadata. As such it's rather extensively exercised by the Swift test suite. As far as testing that the change fixes the problem, that was done manually since the actual problem doesn't always trigger (it requires an 0x8b to somehow be present two bytes before the GOTPCREL, and in the cases we've seen so far, that's happening because the linker is generating that byte as part of the offset in a previous GOTPCREL!)

@al45tair
Copy link
Contributor Author

@swift-ci Please test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 release cherry pick Flag: Release branch cherry picks swift 6.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant