[lld] r292910 - [ELF] Correct sh_info for static symbol table

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 27 05:40:28 PST 2017


> But building packages found a regression in r292789, which I fixed in
> r293177.
>
>
> I will try back porting without it and report.

It is possible, but I actually have way more confidence in backporting
the pair of patches from trunk and avoiding the conflict. That is the
combinations that so far has built the most of freebsd and should make
it easier to port any other patches if we need to.

So, is the attached patch OK?

Cheers,
Rafael
-------------- next part --------------
commit 1131c71cf5b5144d0e8934d8637e523565f81829
Author: Rafael Avila de Espindola <rafael.espindola at gmail.com>
Date:   Fri Jan 27 05:33:17 2017 -0800

    Port r292789 and r293177.
    
    r293177:
        Fix -r when the input has a relocation with no symbol.
    
        Should fix a few freebsd packages with dtrace.
    
    r292789:
        [ELF] - Stop handling local symbols in a special way.
    
        Previously we stored kept locals in a KeptLocalSyms arrays,
        belonged to files.
    
        Patch makes SymbolTableSection to store locals in Symbols member,
        that already present and was used for globals.
        SymbolTableSection already had NumLocals counter member, so change
        itself is trivial.
    
        That allows to simplify handling of -r,
        Body::DynsymIndex is no more used as "symbol table index" for relocatable
        output.
    
        Change was suggested during review of D28773 and opens road for D28612.
    
        Differential revision: https://reviews.llvm.org/D29021

diff --git a/ELF/InputFiles.h b/ELF/InputFiles.h
index 73dda7b..9588806 100644
--- a/ELF/InputFiles.h
+++ b/ELF/InputFiles.h
@@ -180,10 +180,6 @@ public:
   // R_MIPS_GPREL16 / R_MIPS_GPREL32 relocations.
   uint32_t MipsGp0 = 0;
 
-  // The number is the offset in the string table. It will be used as the
-  // st_name of the symbol.
-  std::vector<std::pair<const DefinedRegular<ELFT> *, unsigned>> KeptLocalSyms;
-
   // Name of source file obtained from STT_FILE symbol value,
   // or empty string if there is no such symbol in object file
   // symbol table.
diff --git a/ELF/InputSection.cpp b/ELF/InputSection.cpp
index 3580042..6b1e928 100644
--- a/ELF/InputSection.cpp
+++ b/ELF/InputSection.cpp
@@ -246,7 +246,8 @@ void InputSection<ELFT>::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
     if (Config->Rela)
       P->r_addend = getAddend<ELFT>(Rel);
     P->r_offset = RelocatedSection->getOffset(Rel.r_offset);
-    P->setSymbolAndType(Body.DynsymIndex, Type, Config->Mips64EL);
+    P->setSymbolAndType(In<ELFT>::SymTab->getSymbolIndex(&Body), Type,
+                        Config->Mips64EL);
   }
 }
 
diff --git a/ELF/SyntheticSections.cpp b/ELF/SyntheticSections.cpp
index f09b60b..5e3e634 100644
--- a/ELF/SyntheticSections.cpp
+++ b/ELF/SyntheticSections.cpp
@@ -1065,22 +1065,19 @@ template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
   this->OutSec->Info = this->Info = NumLocals + 1;
   this->OutSec->Entsize = this->Entsize;
 
-  if (Config->Relocatable) {
-    size_t I = NumLocals;
-    for (const SymbolTableEntry &S : Symbols)
-      S.Symbol->DynsymIndex = ++I;
+  if (Config->Relocatable)
     return;
-  }
 
   if (!StrTabSec.isDynamic()) {
-    std::stable_sort(
-        Symbols.begin(), Symbols.end(),
-        [](const SymbolTableEntry &L, const SymbolTableEntry &R) {
-          return L.Symbol->symbol()->computeBinding() == STB_LOCAL &&
-                 R.Symbol->symbol()->computeBinding() != STB_LOCAL;
-        });
+    auto GlobBegin = Symbols.begin() + NumLocals;
+    std::stable_sort(GlobBegin, Symbols.end(), [](const SymbolTableEntry &L,
+                                                  const SymbolTableEntry &R) {
+      return L.Symbol->symbol()->computeBinding() == STB_LOCAL &&
+             R.Symbol->symbol()->computeBinding() != STB_LOCAL;
+    });
     return;
   }
+
   if (In<ELFT>::GnuHashTab)
     // NB: It also sorts Symbols to meet the GNU hash table requirements.
     In<ELFT>::GnuHashTab->addSymbols(Symbols);
@@ -1094,10 +1091,25 @@ template <class ELFT> void SymbolTableSection<ELFT>::finalize() {
     S.Symbol->DynsymIndex = ++I;
 }
 
-template <class ELFT> void SymbolTableSection<ELFT>::addSymbol(SymbolBody *B) {
+template <class ELFT> void SymbolTableSection<ELFT>::addGlobal(SymbolBody *B) {
   Symbols.push_back({B, StrTabSec.addString(B->getName(), false)});
 }
 
+template <class ELFT> void SymbolTableSection<ELFT>::addLocal(SymbolBody *B) {
+  assert(!StrTabSec.isDynamic());
+  ++NumLocals;
+  Symbols.push_back({B, StrTabSec.addString(B->getName())});
+}
+
+template <class ELFT>
+size_t SymbolTableSection<ELFT>::getSymbolIndex(SymbolBody *Body) {
+  auto I = llvm::find_if(
+      Symbols, [&](const SymbolTableEntry &E) { return E.Symbol == Body; });
+  if (I == Symbols.end())
+    return 0;
+  return I - Symbols.begin() + 1;
+}
+
 template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *Buf) {
   Buf += sizeof(Elf_Sym);
 
@@ -1113,26 +1125,24 @@ template <class ELFT>
 void SymbolTableSection<ELFT>::writeLocalSymbols(uint8_t *&Buf) {
   // Iterate over all input object files to copy their local symbols
   // to the output symbol table pointed by Buf.
-  for (ObjectFile<ELFT> *File : Symtab<ELFT>::X->getObjectFiles()) {
-    for (const std::pair<const DefinedRegular<ELFT> *, size_t> &P :
-         File->KeptLocalSyms) {
-      const DefinedRegular<ELFT> &Body = *P.first;
-      InputSectionBase<ELFT> *Section = Body.Section;
-      auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
-
-      if (!Section) {
-        ESym->st_shndx = SHN_ABS;
-        ESym->st_value = Body.Value;
-      } else {
-        const OutputSectionBase *OutSec = Section->OutSec;
-        ESym->st_shndx = OutSec->SectionIndex;
-        ESym->st_value = OutSec->Addr + Section->getOffset(Body);
-      }
-      ESym->st_name = P.second;
-      ESym->st_size = Body.template getSize<ELFT>();
-      ESym->setBindingAndType(STB_LOCAL, Body.Type);
-      Buf += sizeof(*ESym);
+
+  for (auto I = Symbols.begin(); I != Symbols.begin() + NumLocals; ++I) {
+    const DefinedRegular<ELFT> &Body = *cast<DefinedRegular<ELFT>>(I->Symbol);
+    InputSectionBase<ELFT> *Section = Body.Section;
+    auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
+
+    if (!Section) {
+      ESym->st_shndx = SHN_ABS;
+      ESym->st_value = Body.Value;
+    } else {
+      const OutputSectionBase *OutSec = Section->OutSec;
+      ESym->st_shndx = OutSec->SectionIndex;
+      ESym->st_value = OutSec->Addr + Section->getOffset(Body);
     }
+    ESym->st_name = I->StrTabOffset;
+    ESym->st_size = Body.template getSize<ELFT>();
+    ESym->setBindingAndType(STB_LOCAL, Body.Type);
+    Buf += sizeof(*ESym);
   }
 }
 
@@ -1141,7 +1151,9 @@ void SymbolTableSection<ELFT>::writeGlobalSymbols(uint8_t *Buf) {
   // Write the internal symbol table contents to the output symbol table
   // pointed by Buf.
   auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
-  for (const SymbolTableEntry &S : Symbols) {
+
+  for (auto I = Symbols.begin() + NumLocals; I != Symbols.end(); ++I) {
+    const SymbolTableEntry &S = *I;
     SymbolBody *Body = S.Symbol;
     size_t StrOff = S.StrTabOffset;
 
diff --git a/ELF/SyntheticSections.h b/ELF/SyntheticSections.h
index f7e891e..df67e07 100644
--- a/ELF/SyntheticSections.h
+++ b/ELF/SyntheticSections.h
@@ -366,23 +366,26 @@ public:
   void finalize() override;
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return getNumSymbols() * sizeof(Elf_Sym); }
-  void addSymbol(SymbolBody *Body);
+  void addGlobal(SymbolBody *Body);
+  void addLocal(SymbolBody *Body);
   StringTableSection<ELFT> &getStrTabSec() const { return StrTabSec; }
-  unsigned getNumSymbols() const { return NumLocals + Symbols.size() + 1; }
+  unsigned getNumSymbols() const { return Symbols.size() + 1; }
+  size_t getSymbolIndex(SymbolBody *Body);
 
   ArrayRef<SymbolTableEntry> getSymbols() const { return Symbols; }
 
   static const OutputSectionBase *getOutputSection(SymbolBody *Sym);
 
-  unsigned NumLocals = 0;
-  StringTableSection<ELFT> &StrTabSec;
-
 private:
   void writeLocalSymbols(uint8_t *&Buf);
   void writeGlobalSymbols(uint8_t *Buf);
 
   // A vector of symbols and their string table offsets.
   std::vector<SymbolTableEntry> Symbols;
+
+  StringTableSection<ELFT> &StrTabSec;
+
+  unsigned NumLocals = 0;
 };
 
 // Outputs GNU Hash section. For detailed explanation see:
diff --git a/ELF/Writer.cpp b/ELF/Writer.cpp
index 4f2f919..fbb05ef 100644
--- a/ELF/Writer.cpp
+++ b/ELF/Writer.cpp
@@ -455,11 +455,7 @@ template <class ELFT> void Writer<ELFT>::copyLocalSymbols() {
       InputSectionBase<ELFT> *Sec = DR->Section;
       if (!shouldKeepInSymtab<ELFT>(Sec, B->getName(), *B))
         continue;
-      ++In<ELFT>::SymTab->NumLocals;
-      if (Config->Relocatable)
-        B->DynsymIndex = In<ELFT>::SymTab->NumLocals;
-      F->KeptLocalSyms.push_back(std::make_pair(
-          DR, In<ELFT>::SymTab->StrTabSec.addString(B->getName())));
+      In<ELFT>::SymTab->addLocal(B);
     }
   }
 }
@@ -1024,10 +1020,10 @@ template <class ELFT> void Writer<ELFT>::finalizeSections() {
     if (!includeInSymtab<ELFT>(*Body))
       continue;
     if (In<ELFT>::SymTab)
-      In<ELFT>::SymTab->addSymbol(Body);
+      In<ELFT>::SymTab->addGlobal(Body);
 
     if (In<ELFT>::DynSymTab && S->includeInDynsym()) {
-      In<ELFT>::DynSymTab->addSymbol(Body);
+      In<ELFT>::DynSymTab->addGlobal(Body);
       if (auto *SS = dyn_cast<SharedSymbol<ELFT>>(Body))
         if (SS->file()->isNeeded())
           In<ELFT>::VerNeed->addSymbol(SS);
diff --git a/test/ELF/Inputs/dtrace-r.o b/test/ELF/Inputs/dtrace-r.o
new file mode 100644
index 0000000..ce742de
Binary files /dev/null and b/test/ELF/Inputs/dtrace-r.o differ
diff --git a/test/ELF/dtrace-r.test b/test/ELF/dtrace-r.test
new file mode 100644
index 0000000..2b6d885
--- /dev/null
+++ b/test/ELF/dtrace-r.test
@@ -0,0 +1,8 @@
+RUN: ld.lld -r -o %t.o %p/Inputs/dtrace-r.o
+RUN: llvm-readobj -r %t.o | FileCheck %s
+
+CHECK:      Relocations [
+CHECK-NEXT:   Section ({{.*}}) .rela.text {
+CHECK-NEXT:     0x0 R_X86_64_NONE - 0x0
+CHECK-NEXT:   }
+CHECK-NEXT: ]


More information about the llvm-commits mailing list