[ELF] - Fixed bug leading to miss of tls relocation when @tlsgd and @gottpoff relocations were used at the same time.

[grimar]

Summary

Combination of @tlsgd and @gottpoff at the same time leads to miss of R_X86_64_TPOFF64 dynamic relocation. Patch fixes that.

@tlsgd(%rip) Allocate two contiguous entries in the GOT to hold a tls index
structure (for passing to tls get addr).

@gottpoff(%rip) Allocate one GOT entry to hold a variable offset in initial TLS
block (relative to TLS block end, %fs:0).

The same situation can be observed at for x86 (probably others too, not sure) with corresponding for that target relocations: @tlsgd, @gotntpoff.

[ruiu]

ELF/Symbols.h

93 ↗

(On Diff #41480)

Glob -> Global

ELF/Writer.cpp

227 ↗

(On Diff #41480)

Can you add a one-liner function for this comparison to SymbolBody, instead of comparing a value with -1?

[grimar]

Review comments addressed.

[grimar]

ELF/Symbols.h

93 ↗

(On Diff #41480)

Done.

ELF/Writer.cpp

227 ↗

(On Diff #41480)

Done. But also simplified a little in the same way how was done for addLocalModelTlsIndex.

[ruiu]

LGTM

lld/trunk/ELF/InputSection.cpp

if (SymIndex < SymTab->sh_info) {
       continue;
     }
 
     SymbolBody &Body = *File->getSymbolBody(SymIndex)->repl();
 
     if (Target->isTlsGlobalDynamicReloc(Type) &&
         !Target->isTlsOptimized(Type, &Body)) {
       Target->relocateOne(BufLoc, BufEnd, Type, AddrLoc,
-                          Out<ELFT>::Got->getEntryAddr(Body) +
+                          Out<ELFT>::Got->getGlobalDynAddr(Body) +
                               getAddend<ELFT>(RI));
       continue;
     }
 
     if (Target->isTlsOptimized(Type, &Body)) {
       // By optimizing TLS relocations, it is sometimes needed to skip
       // relocations that immediately follow TLS relocations. This function
       // knows how many slots we need to skip.

lld/trunk/ELF/OutputSections.h

template <class ELFT> class GotSection final : public OutputSectionBase<ELFT> {
   typedef OutputSectionBase<ELFT> Base;
   typedef typename Base::uintX_t uintX_t;
 
 public:
   GotSection();
   void finalize() override;
   void writeTo(uint8_t *Buf) override;
   void addEntry(SymbolBody *Sym);
-  void addDynTlsEntry(SymbolBody *Sym);
+  bool addDynTlsEntry(SymbolBody *Sym);
   bool addLocalModelTlsIndex();
   bool empty() const { return Entries.empty(); }
   uintX_t getEntryAddr(const SymbolBody &B) const;
+  uintX_t getGlobalDynAddr(const SymbolBody &B) const;
 
   // Returns the symbol which corresponds to the first entry of the global part
   // of GOT on MIPS platform. It is required to fill up MIPS-specific dynamic
   // table properties.
   // Returns nullptr if the global part is empty.
   const SymbolBody *getMipsFirstGlobalEntry() const;
 
   // Returns the number of entries in the local part of GOT including

lld/trunk/ELF/OutputSections.cpp

GotSection<ELFT>::GotSection()
   this->Header.sh_addralign = sizeof(uintX_t);
 }
 
 template <class ELFT> void GotSection<ELFT>::addEntry(SymbolBody *Sym) {
   Sym->GotIndex = Target->getGotHeaderEntriesNum() + Entries.size();
   Entries.push_back(Sym);
 }
 
-template <class ELFT> void GotSection<ELFT>::addDynTlsEntry(SymbolBody *Sym) {
-  Sym->GotIndex = Target->getGotHeaderEntriesNum() + Entries.size();
+template <class ELFT> bool GotSection<ELFT>::addDynTlsEntry(SymbolBody *Sym) {
+  if (Sym->hasGlobalDynIndex())
+    return false;
+  Sym->GlobalDynIndex = Target->getGotHeaderEntriesNum() + Entries.size();
   // Global Dynamic TLS entries take two GOT slots.
   Entries.push_back(Sym);
   Entries.push_back(nullptr);
+  return true;
 }
 
 template <class ELFT> bool GotSection<ELFT>::addLocalModelTlsIndex() {
   if (LocalTlsIndexOff != uint32_t(-1))
     return false;
   Entries.push_back(nullptr);
   Entries.push_back(nullptr);
   LocalTlsIndexOff = (Entries.size() - 2) * sizeof(uintX_t);
   return true;
 }
 
 template <class ELFT>
 typename GotSection<ELFT>::uintX_t
 GotSection<ELFT>::getEntryAddr(const SymbolBody &B) const {
   return this->getVA() + B.GotIndex * sizeof(uintX_t);
 }
 
 template <class ELFT>
+typename GotSection<ELFT>::uintX_t
+GotSection<ELFT>::getGlobalDynAddr(const SymbolBody &B) const {
+  return this->getVA() + B.GlobalDynIndex * sizeof(uintX_t);
+}
+
+template <class ELFT>
 const SymbolBody *GotSection<ELFT>::getMipsFirstGlobalEntry() const {
   return Entries.empty() ? nullptr : Entries.front();
 }
 
 template <class ELFT>
 unsigned GotSection<ELFT>::getMipsLocalEntriesNum() const {
   // TODO: Update when the suppoort of GOT entries for local symbols is added.
   return Target->getGotHeaderEntriesNum();
if (Target->isTlsLocalDynamicReloc(Type)) {
     P->setSymbolAndType(0, Target->getTlsModuleIndexReloc(), Config->Mips64EL);
     P->r_offset = Out<ELFT>::Got->getLocalTlsIndexVA();
     return true;
   }
 
   if (Body && Target->isTlsGlobalDynamicReloc(Type)) {
     P->setSymbolAndType(Body->getDynamicSymbolTableIndex(),
                         Target->getTlsModuleIndexReloc(), Config->Mips64EL);
-    P->r_offset = Out<ELFT>::Got->getEntryAddr(*Body);
+    P->r_offset = Out<ELFT>::Got->getGlobalDynAddr(*Body);
     N->setSymbolAndType(Body->getDynamicSymbolTableIndex(),
                         Target->getTlsOffsetReloc(), Config->Mips64EL);
-    N->r_offset = Out<ELFT>::Got->getEntryAddr(*Body) + sizeof(uintX_t);
+    N->r_offset = Out<ELFT>::Got->getGlobalDynAddr(*Body) + sizeof(uintX_t);
     return true;
   }
   return false;
 }
 
 template <class ELFT> void RelocationSection<ELFT>::writeTo(uint8_t *Buf) {
   const unsigned EntrySize = IsRela ? sizeof(Elf_Rela) : sizeof(Elf_Rel);
   for (const DynamicReloc<ELFT> &Rel : Relocs) {

lld/trunk/ELF/Symbols.h

public:
 
   uint8_t getVisibility() const { return Visibility; }
 
   unsigned getDynamicSymbolTableIndex() const {
     return DynamicSymbolTableIndex;
   }
   void setDynamicSymbolTableIndex(unsigned V) { DynamicSymbolTableIndex = V; }
 
+  uint32_t GlobalDynIndex = -1;
   uint32_t GotIndex = -1;
   uint32_t GotPltIndex = -1;
   uint32_t PltIndex = -1;
+  bool hasGlobalDynIndex() { return GlobalDynIndex != uint32_t(-1); }
   bool isInGot() const { return GotIndex != -1U; }
   bool isInGotPlt() const { return GotPltIndex != -1U; }
   bool isInPlt() const { return PltIndex != -1U; }
 
   // A SymbolBody has a backreference to a Symbol. Originally they are
   // doubly-linked. A backreference will never change. But the pointer
   // in the Symbol may be mutated by the resolver. If you have a
   // pointer P to a SymbolBody and are not sure whether the resolver

lld/trunk/ELF/Writer.cpp

if (Body && Body->isUndefined() && !Body->isWeak())
         S->File->IsUsed = true;
 
     if (Body)
       Body = Body->repl();
 
     if (Body && Body->isTLS() && Target->isTlsGlobalDynamicReloc(Type)) {
       if (Target->isTlsOptimized(Type, Body))
         continue;
-      if (Body->isInGot())
-        continue;
-      Out<ELFT>::Got->addDynTlsEntry(Body);
+      if (Out<ELFT>::Got->addDynTlsEntry(Body)) {
         Out<ELFT>::RelaDyn->addReloc({&C, &RI});
         Out<ELFT>::RelaDyn->addReloc({nullptr, nullptr});
         Body->setUsedInDynamicReloc();
+      }
       continue;
     }
 
     if (Body && Body->isTLS() && !Target->isTlsDynReloc(Type))
       continue;
 
     bool NeedsGot = false;
     bool NeedsPlt = false;

lld/trunk/test/ELF/tls-dynamic.s

// RUN: llvm-objdump -d %tout | FileCheck %s --check-prefix=DIS
   callq __tls_get_addr@PLT
   leaq  a@dtpoff(%rax), %rcx
   leaq  b@dtpoff(%rax), %rcx
   .long b@dtpoff, 0
   leaq  c@tlsgd(%rip), %rdi
   rex64
   callq __tls_get_addr@PLT
   leaq  c@dtpoff(%rax), %rcx
+  // Initial Exec Model Code Sequence, II
+  movq c@gottpoff(%rip),%rax
+  movq %fs:(%rax),%rax
 
   .global a
   .hidden a
   .section .tbss,"awT",@nobits
   .align 4
 a:
   .long 0
 

 // CHECK:          Name: .got
 // CHECK-NEXT:     Type: SHT_PROGBITS
 // CHECK-NEXT:     Flags [
 // CHECK-NEXT:       SHF_ALLOC
 // CHECK-NEXT:       SHF_WRITE
 // CHECK-NEXT:     ]
 // CHECK-NEXT:     Address: 0x20D0
 // CHECK-NEXT:     Offset:
-// CHECK-NEXT:     Size: 32
+// CHECK-NEXT:     Size: 40
 
 // CHECK:      Relocations [
 // CHECK:        Section ({{.+}}) .rela.dyn {
 // CHECK-NEXT:     0x20D0 R_X86_64_DTPMOD64 - 0x0
 // CHECK-NEXT:     0x20E0 R_X86_64_DTPMOD64 c 0x0
 // CHECK-NEXT:     0x20E8 R_X86_64_DTPOFF64 c 0x0
+// CHECK-NEXT:     0x20F0 R_X86_64_TPOFF64 c 0x0
 // CHECK-NEXT:   }
 
 // 4297 = (0x20D0 + -4) - (0x1000 + 3) // PC relative offset to got entry.
 // 4285 = (0x20D0 + -4) - (0x100c + 3) // PC relative offset to got entry.
 // 4267 = (0x20E0 + -4) - (0x102e + 3) // PC relative offset to got entry.
+// 4263 = (0x20F0 + -4) - (0x1042 + 3) // PC relative offset to got entry.
 
 // DIS:      Disassembly of section .text:
 // DIS-NEXT: .text:
 // DIS-NEXT:     1000: {{.+}} leaq    4297(%rip), %rdi
 // DIS-NEXT:     1007: {{.+}} callq
 // DIS-NEXT:     100c: {{.+}} leaq    4285(%rip), %rdi
 // DIS-NEXT:     1013: {{.+}} callq
 // DIS-NEXT:     1018: {{.+}} leaq    (%rax), %rcx
 // DIS-NEXT:     101f: {{.+}} leaq    4(%rax), %rcx
 // DIS-NEXT:     1026: 04 00
 // DIS-NEXT:     1028: 00 00
 // DIS-NEXT:     102a: 00 00
 // DIS-NEXT:     102c: 00 00
 // DIS-NEXT:     102e: {{.+}} leaq    4267(%rip), %rdi
 // DIS-NEXT:     1035: {{.+}} callq
 // DIS-NEXT:     103b: {{.+}} leaq    8(%rax), %rcx
+// DIS-NEXT:     1042: {{.+}} movq    4263(%rip), %rax
+// DIS-NEXT:     1049: {{.+}} movq    %fs:(%rax), %rax