fix: make three more x86_64 relaxations reqired for static binaries#1654
Conversation
davidlattimore
left a comment
There was a problem hiding this comment.
Thanks for the fix. Would it be possible to add a test? The static no-relax combination seems like it would be pretty straightforward
Otherwise, linking C++ hello world with `-static -Wl,--no-relax` fails with undefined references to `__tls_get_addr`.
This matches GNU ld behaviour according to linker-diff when linking C++ hello world with `-static -Wl,--no-relax`.
176ba40 to
b5d4a23
Compare
7194e6e to
c1363cb
Compare
|
My changes fixed the issue on Arch Linux, but all other test runners still hit it: Welp, at least I didn't regress anything, although failing to link is a bar set pretty low. |
|
Quite a bumpy ride but added test that now passes the CI. |
davidlattimore
left a comment
There was a problem hiding this comment.
Thanks for the test. A couple of minor comments, but fine to submit as-is if you prefer
| //#LinkArgs:-no-pie -Wl,-z,now | ||
| //#EnableLinker:lld | ||
|
|
||
| //#Config:static:default |
There was a problem hiding this comment.
Perhaps call the config "static-no-relax" to better capture what we're testing?
| //#LinkerDriver:g++ | ||
| //#LinkArgs:-static -Wl,-z,now,-no-relax | ||
| //#DiffIgnore:rel.extra-got-plt-got | ||
| //#DiffIgnore:init_array |
There was a problem hiding this comment.
Do you think any of these ignores are things that we should look at in future? If so, perhaps add a TODO. The init_array diff especially stands out to me
There was a problem hiding this comment.
I thought init_array diff is a known defect, I get it even when linking dynamic executables with linker-diff enabled. For example, when linking Release profile (without debug info) Clang.
FWIW, this is the diff from this test:
wild: /home/mateusz/Projects/wild/wild/tests/build/cpp-integration.cc/static-no-relax-host/cpp-integration.cc.wild
ld: /home/mateusz/Projects/wild/wild/tests/build/cpp-integration.cc/static-no-relax-host/cpp-integration.cc.ld
init_array
┌──────────────────────────────────────┬──────────────────────────────────────┐
│ wild │ ld │
├──────────────────────────────────────┼──────────────────────────────────────┤
│ _GLOBAL__sub_I.00090_globals_io.cc │ _GLOBAL__sub_I.00090_globals_io.cc │
│ frame_dummy │ frame_dummy │
│ _GLOBAL__sub_I_eh_alloc.cc │ _GLOBAL__sub_I_cxx11_wlocale_inst.cc │
│ _GLOBAL__sub_I_cxx11_locale_inst.cc │ _GLOBAL__sub_I_locale_inst.cc │
│ _GLOBAL__sub_I_cxx11_wlocale_inst.cc │ _GLOBAL__sub_I_wlocale_inst.cc │
│ _GLOBAL__sub_I_locale_inst.cc │ _GLOBAL__sub_I_eh_alloc.cc │
│ _GLOBAL__sub_I_wlocale_inst.cc │ _GLOBAL__sub_I_cxx11_locale_inst.cc │
│ _IO_stdfiles_init │ _IO_stdfiles_init │
└──────────────────────────────────────┴──────────────────────────────────────┘
There was a problem hiding this comment.
It was a known issue, but I thought it was fixed in #588, but it sounds like there might be more to do.
There was a problem hiding this comment.
For some reason I thought it wasn't supposed to be a full fix.
This change fixes linking of C++ hello world with
-static -Wl,--no-relaxand addresses one of linker-diff's errors about missing relaxation.Wild still applies too many relaxations as shown in linker-diff output, but disabling some of them results in broken binaries, so I gave up on touching that. Also, in the output there are not yet implemented
endbr64relaxations.