[PATCH] D54624: [LLD][ELF] Error if _GLOBAL_OFFSET_TABLE_ is defined in input objects

[Peter Smith via Phabricator via llvm-commits]

peter.smith created this revision.
peter.smith added reviewers: ruiu, grimar.
Herald added subscribers: kristof.beyls, arichardson, javed.absar, emaste.
Herald added a reviewer: espindola.

The _GLOBAL_OFFSET_TABLE_ is a linker defined symbol that is placed at some location relative to the .got, .got.plt or .toc section. On some targets such as Arm the correctness of some code sequences using a relocation to _GLOBAL_OFFSET_TABLE_ depend on the value of the symbol being in the linker defined place. Follow the ld.gold example and give a multiple symbol definition error. The ld.bfd behaviour is to ignore the definition in the input object and redefine it, which seems like it could be more surprising.

I've restricted the effect of the change to _GLOBAL_OFFSET_TABLE_ rather than making it universal for all linker defined symbols as there are some existing tests that explicitly redefine some linker defined symbols in input objects citing some previous problem. My justification for special casing _GLOBAL_OFFSET_TABLE_ is that it is the only symbol that the linker uses the value of internally, the use of other linker defined symbols is defined by the input objects only.

A test program to check the behaviour of ld.gold and ld.bfd using the symbol names in addReservedSymbols() showed that ld.bfd would just redefine the symbols and ignore the input definitions. ld.gold would error if there was a non-common definition of the symbol.

fixes pr39587 (turning incorrect output into an error message)


https://reviews.llvm.org/D54624

Files:
  ELF/Writer.cpp
  test/ELF/global-offset-table-position-redef-err.s


Index: test/ELF/global-offset-table-position-redef-err.s
===================================================================
--- /dev/null
+++ test/ELF/global-offset-table-position-redef-err.s
@@ -0,0 +1,13 @@
+# REQUIRES x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
+
+# On some targets the location of the _GLOBAL_OFFSET_TABLE_ symbol table can
+# matter for the correctness of some relocations. Follow the example of ld.gold
+# and give a multiple definition error if input objects attempt to redefine it.
+
+# CHECK: ld.lld: error: duplicate symbol: _GLOBAL_OFFSET_TABLE_
+.data
+.global _GLOBAL_OFFSET_TABLE_
+_GLOBAL_OFFSET_TABLE_:
+.word 0
Index: ELF/Writer.cpp
===================================================================
--- ELF/Writer.cpp
+++ ELF/Writer.cpp
@@ -168,9 +168,10 @@
 
 static Defined *addOptionalRegular(StringRef Name, SectionBase *Sec,
                                    uint64_t Val, uint8_t StOther = STV_HIDDEN,
-                                   uint8_t Binding = STB_GLOBAL) {
+                                   uint8_t Binding = STB_GLOBAL,
+                                   bool AllowMuldef = true) {
   Symbol *S = Symtab->find(Name);
-  if (!S || S->isDefined())
+  if (!S || (AllowMuldef && S->isDefined()))
     return nullptr;
   Symbol *Sym = Symtab->addDefined(Name, StOther, STT_NOTYPE, Val,
                                    /*Size=*/0, Binding, Sec,
@@ -213,9 +214,12 @@
   // _GLOBAL_OFFSET_TABLE_ and _SDA_BASE_ from the 32-bit ABI. It is used to
   // represent the TOC base which is offset by 0x8000 bytes from the start of
   // the .got section.
+  // We do not _GLOBAL_OFFSET_TABLE_ to be defined by input objects as the
+  // correctness of some relocations depends on its value.
   ElfSym::GlobalOffsetTable = addOptionalRegular(
       (Config->EMachine == EM_PPC64) ? ".TOC." : "_GLOBAL_OFFSET_TABLE_",
-      Out::ElfHeader, Target->GotBaseSymOff);
+      Out::ElfHeader, Target->GotBaseSymOff, STV_HIDDEN, STB_GLOBAL,
+      /*AllowMulDef*/ false);
 
   // __ehdr_start is the location of ELF file headers. Note that we define
   // this symbol unconditionally even when using a linker script, which


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D54624.174348.patch
Type: text/x-patch
Size: 2254 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181116/86cf813f/attachment.bin>