Page MenuHomePhabricator

[ARM][LLD] ARM TLS shouldn't use relaxations
ClosedPublic

Authored by peter.smith on Sep 22 2016, 6:58 AM.

Details

Summary

Disable all relaxations for ARM TLS

The ARM TLS relocations are placed on literal data and not the code-sequence, it is therefore not possible to implement the relaxTls* functions. The Mips Target also does not do TLS relaxation and has a separate handleMipsTlsRelocation() function to keep handleTlsRelocation() clean from target specific hooks. I've chosen to rename handleMipsTlsRelocation() to handleNoRelaxTlsRelocation() so that it can be used for both ARM and Mips.

Unfortunately there are some differences in ARM and Mips target handling as there are some cases where Mips can omit a dynamic relocation where ARM cannot. I tried some experiments to see if I could get away without these changes, but they caused segfaults at runtime. The Mips code should be logically identical to what it was before.

There is still some work to be done for ARM TLS and static linking, I'll address this separately as it is a smaller use case.

The 4 test cases will fail assertions without these changes.

This should address PR30218 (https://llvm.org/bugs/show_bug.cgi?id=30218)

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith retitled this revision from to [ARM][LLD] ARM TLS shouldn't use relaxations.
peter.smith updated this object.
peter.smith added reviewers: ruiu, rafael.
peter.smith added subscribers: atanasyan, ed, llvm-commits.
emaste added a subscriber: emaste.Sep 22 2016, 7:27 AM
emaste added inline comments.
ELF/Relocations.cpp
88 ↗(On Diff #72160)

typo "intothe"

ruiu added inline comments.Sep 22 2016, 10:45 AM
ELF/Relocations.cpp
89 ↗(On Diff #72160)

Please also briefly mention that the difference between ARM and MIPS in this comment.

ed added a comment.Sep 22 2016, 2:20 PM

Not that it helps much, but with this patch I can at least get all of the packages for CloudABI to build on ARMv6. I can't test whether the resulting binaries work yet, as I'm at a conference. I will be able to test this for you on Monday.

Updated diff to fix typo and explain the differences between ARM and Mips.

I've put the comment as a FIXME as some more work is needed to support static linking. In effect we need to hard-code the module index (1 for applications) in the GOT rather than getting the dynamic linker to do it for us. I think that this should be done in a separate patch as part of supporting and testing static linking.

Ed, I think ARMv6 for LLD won't work very well as there are some assumptions that ARMv7 instructions are available in interworking Thunks and in the encoding of Thumb2 branches. If you can I would recommend ARMv7 as a baseline if using LLD. I would like to add this support at some point, but it will need some thought as to the best way of doing it.

I'll be at a conference myself next week, but I should have some spare time to work on stuff as well.

This revision was automatically updated to reflect the committed changes.

Committed as r282250, so I've got a chance of fixing problems before end of work day. Happy to make any further updates later.