Staticlib rename internal symbols: add COFF support - #160679
Conversation
rustbot
added
A-run-make
3a890b2 to
3d2cea7
Compare
This comment has been minimized.
This comment has been minimized.
| strip_underscore: bool, | ||
| ) -> Option<Vec<u8>> { | ||
| // COFF has no visibility concept, so hide is a no-op. | ||
| let _ = (hide, exported); |
There was a problem hiding this comment.
Maybe assert!(!hide)?
There was a problem hiding this comment.
Done.
While adding it I noticed that link_staticlib passed the raw hide flag down, so passing both -Zstaticlib-hide-internal-symbols and -Zstaticlib-rename-internal-symbols on a COFF target would have tripped this assert as an ICE(Because COFF doesn't support hide flag yet).
I've clamped it to hide && hide_supported when constructing ArchiveSymbols (the warning is already emitted above), and added a test case passing both flags that checks the warning is emitted and renaming still applies.
There was a problem hiding this comment.
I assumed those diagnostics were errors, not warnings.
There was a problem hiding this comment.
Yes, they are warnings, so the backend of COFF will still be reached—which is why I clamped hide and added both-flags tests.
bjorn3
added
S-waiting-on-author
3d2cea7 to
a2d1b27
Compare
02dd471 to
e03085d
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
|
It has been rebaseed to upstream. |
3 participants
Follow-up to #156950.
-Zstaticlib-rename-internal-symbolsnow also works on COFF targets (Windows). Renaming only rewrites symbol names, so unlike hide it needs no visibility concept.COFF objects keep their string table at the end of the file, so renames append the new names there and patch the 4-byte length prefix plus each symbol's name offset in place. Both regular and bigobj objects are handled; on i686 a leading underscore is stripped when matching against the exported set. The archive format differs (GNU ar on windows-gnu, COFF on windows-msvc) but the members are always COFF objects, so the existing archive code is unchanged.
Supported on ELF, Apple, and COFF targets.
-Zstaticlib-hide-internal-symbolsremains ELF/Apple-only and still warns on Windows.A run-make test
staticlib-rename-internal-symbols-coffmirrors the existing ELF and Mach-O tests.r? @bjorn3