Handle statics and TLS in raw-dylib for ELF#153580
Draft
mati865 wants to merge 3 commits intorust-lang:mainfrom
Draft
Handle statics and TLS in raw-dylib for ELF#153580mati865 wants to merge 3 commits intorust-lang:mainfrom
mati865 wants to merge 3 commits intorust-lang:mainfrom
Conversation
Collaborator
|
r? @jieyouxu rustbot has assigned @jieyouxu. Use Why was this reviewer chosen?The reviewer was selected based on:
|
Member
Author
|
cc @bjorn3 |
Member
|
Probably, r? bjorn3 |
Member
Author
|
Shoot, forgot to change the title yesterday. |
bjorn3
reviewed
Mar 9, 2026
| DllImportSymbolType::Static | ||
| } | ||
| } else { | ||
| DllImportSymbolType::Unknown |
Member
There was a problem hiding this comment.
In what cases does this get reached? Can this be a bug!()?
Member
Author
There was a problem hiding this comment.
Hopefully never, I was worried to break Windows impl because previous code distinguished only functions and everything else.
bjorn3
approved these changes
Mar 9, 2026
Member
|
Actually, for PIE executables we do also need to store the symbol size for statics and maybe thread locals. This because of copy relocations. Basically the linker reserves space in the PIE executable for all statics coming from other DSOs and then at runtime the dynamic linker copies the data from the DSO that the symbol resolves to in the end into the reserved memory. Without the symbol size, the linker can't know how much memory to reserve. |
Member
Author
|
Copy relocations would definitely require known size. I couldn't manage to write Rust code that would make LLVM emit direct access with PIE or PIC, it would use GOT indirection instead which prevents copy relocations. However, I sort of succeeded with static relocation model:
Dunno about TLS, I'm not familiar enough with how it's handled. More detailsI have used WILD_SAVE_BASE to make debugging easier. While at it, posting linker-diff for the future reference: |
Member
Author
|
(forgot to write about it in the previous comment) I have no idea how we can deal with the unknown size. Perhaps we can somehow avoid direct access to raw-dylib statics? |
Member
|
We can derive the size of statics from the type layout, right? We don't support unsized statics. |
Member
Author
|
Right, I feel so dumb. For some reason I thought that size of "string" objects would store the length of the underlying string rather than pointer size 🤦🏻♂️. |
f8a5ba3 to
9cb0ca1
Compare
9cb0ca1 to
2e52da9
Compare
Member
Author
To my defence, this is the case with Added size with implementation partially based on GNU ld doesn't look half bad, it didn't emit any copy relocations. One big issue standing out is putting data into Somehow this works, except for LLD didn't like our raw-dylib at all. With sizes set, it no longer rejects linkage, but it'd be better if it did rather than creating this abomination: Yeah, this has zero chance of working. Wild starts off with warnings when linking the same code: And looks like we have a deja vu: How about making static relocation model unsupported for now (with an error emitted) and creating an issue to track fixing it? |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot)For more information how to resolve CI failures of this job, visit this link. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #153090