This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] Define _GLOBAL_OFFSET_TABLE_ to base of .got for ARM
ClosedPublic

Authored by peter.smith on Jun 19 2017, 9:54 AM.

Details

Summary

On many architectures gcc and clang will recognize _GLOBAL_OFFSET_TABLE_ - . and produce a relocation that can be processed without needing to know the value of _GLOBAL_OFFSET_TABLE_. For ARM gcc produces R_ARM_BASE_PREL but clang produces the more general R_ARM_REL32 to _GLOBAL_OFFSET_TABLE_. To evaluate this relocation correctly _GLOBAL_OFFSET_TABLE_ must be defined to be the base of the GOT (R_ARM_BASE_PREL evaluates to this value).

If/when llvm-mc is changed to recognize _GLOBAL_OFFSET_TABLE_ - . this change will not be necessary for new objects. However there may still be old objects and versions of clang so I think it is worth making the change in lld as well as fixing llvm-mc (pr33511).

This partially fixes pr31159, the R_ARM_REL32 relocation error messages disappear but there are a couple more relocation errors relating to R_ARM_GOTOFF32, I think that these are unrelated to _GLOBAL_OFFSET_TABLE_ though.

Diff Detail

Repository
rL LLVM

Event Timeline

peter.smith created this revision.Jun 19 2017, 9:54 AM
peter.smith edited the summary of this revision. (Show Details)Jun 19 2017, 9:55 AM

I've updated the patch to support the general case of _GLOBAL_OFFSET_TABLE_ across the supported architectures.

Based on binutils the x86 and x86_64 architectures traditionally have the _GLOBAL_OFFSET_TABLE_ set to the base of the .got.plt which by default in gold and bfd puts .got immediately before .got.plt. In lld .got may be separate to .got.plt so I've used the end of .got in the same way as the _FROM_END relocations in x86 and x86_64. All other architectures use the base of the .got. Of the supported architectures only ARM and X86 seem to make a lot of use of _GLOBAL_OFFSET_TABLE_.
In addition:

  • Defining _GLOBAL_OFFSET_TABLE_ as a symbol reveals that R_GOTONLY_PC is missing from isRelExprOneOf in isStaticLinkTimeConstant
  • _GLOBAL_OFFSET_TABLE_ can't be used absolutely in tests with --shared (this is the same with gold and bfd)
  • I've added tests for all the architectures that use _GLOBAL_OFFSET_TABLE_

I've not added a test for PPC32 as I don't think the port is complete enough for GOT generation anyway. PPC64 uses .TOC. instead of _GLOBAL_OFFSET_TABLE_.

Although I've not done so in this patch, adding the target specific GotBaseSymOff would allow us to merge the RelExpr GOT_ONLYPC and GOT_ONLYPC_FROM_END and the other _FROM_END relocations.

This should make lld able to handle older objects including those from the current version of clang better. If this is not considered worthwhile let me know and I'll abandon the revision.

ruiu edited edge metadata.Jun 22 2017, 9:35 AM

Yes, overall looking good, but I have one question.

ELF/Writer.cpp
1145–1147 ↗(On Diff #103381)

Doesn't this always succeed unless the -r option is given? It seems _GLOBAL_OFFSET_TABLE_ is added unconditionally, so find should return something non-null.

I've updated the diff to record that we've added the _GLOBAL_OFFSET_TABLE_ symbol (addOptionalRegular will only define it if it is referenced). Assuming there are no more comments I'll commit it on Monday.

This revision was automatically updated to reflect the committed changes.

As per message on Friday I've committed 306282. I'm happy to make further changes or revert quickly if this causes any problems.

ELF/Writer.cpp
1145–1147 ↗(On Diff #103381)

The code that adds the _GLOBAL_OFFSET_TABLE_ symbol uses addOptionalRegular which will only define the symbol if it is referenced so the find can return null. In any case, as per Rafael's suggestion I've recorded that the symbol was added so we don't need to look it up at this point.