[lld] r328579 - Refactor SharedFile::parseRest. NFC.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 26 12:57:38 PDT 2018


Author: ruiu
Date: Mon Mar 26 12:57:38 2018
New Revision: 328579

URL: http://llvm.org/viewvc/llvm-project?rev=328579&view=rev
Log:
Refactor SharedFile::parseRest. NFC.

SharedFile::parseRest function grew organically and got a bit hard to
understand. This patch refactor it. This patch also adds comments.

Differential Revision: https://reviews.llvm.org/D44860

Modified:
    lld/trunk/ELF/InputFiles.cpp
    lld/trunk/ELF/InputFiles.h

Modified: lld/trunk/ELF/InputFiles.cpp
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=328579&r1=328578&r2=328579&view=diff
==============================================================================
--- lld/trunk/ELF/InputFiles.cpp (original)
+++ lld/trunk/ELF/InputFiles.cpp Mon Mar 26 12:57:38 2018
@@ -782,23 +782,32 @@ template <class ELFT> void SharedFile<EL
   }
 }
 
+// Parses ".gnu.version" section which is a parallel array for the symbol table.
+// If a given file doesn't have ".gnu.version" section, returns VER_NDX_GLOBAL.
+template <class ELFT> std::vector<uint32_t> SharedFile<ELFT>::parseVersyms() {
+  size_t Size = this->ELFSyms.size() - this->FirstNonLocal;
+  if (!VersymSec)
+    return std::vector<uint32_t>(Size, VER_NDX_GLOBAL);
+
+  const char *Base = this->MB.getBuffer().data();
+  const Elf_Versym *Versym =
+      reinterpret_cast<const Elf_Versym *>(Base + VersymSec->sh_offset) +
+      this->FirstNonLocal;
+
+  std::vector<uint32_t> Ret(Size);
+  for (size_t I = 0; I < Size; ++I)
+    Ret[I] = Versym[I].vs_index;
+  return Ret;
+}
+
 // Parse the version definitions in the object file if present. Returns a vector
 // whose nth element contains a pointer to the Elf_Verdef for version identifier
 // n. Version identifiers that are not definitions map to nullptr.
 template <class ELFT>
-std::vector<const typename ELFT::Verdef *>
-SharedFile<ELFT>::parseVerdefs(const Elf_Versym *&Versym) {
-  // We only need to process symbol versions for this DSO if it has both a
-  // versym and a verdef section, which indicates that the DSO contains symbol
-  // version definitions.
-  if (!VersymSec || !VerdefSec)
+std::vector<const typename ELFT::Verdef *> SharedFile<ELFT>::parseVerdefs() {
+  if (!VerdefSec)
     return {};
 
-  // The location of the first global versym entry.
-  const char *Base = this->MB.getBuffer().data();
-  Versym = reinterpret_cast<const Elf_Versym *>(Base + VersymSec->sh_offset) +
-           this->FirstNonLocal;
-
   // We cannot determine the largest verdef identifier without inspecting
   // every Elf_Verdef, but both bfd and gold assign verdef identifiers
   // sequentially starting from 1, so we predict that the largest identifier
@@ -808,6 +817,7 @@ SharedFile<ELFT>::parseVerdefs(const Elf
 
   // Build the Verdefs array by following the chain of Elf_Verdef objects
   // from the start of the .gnu.version_d section.
+  const char *Base = this->MB.getBuffer().data();
   const char *Verdef = Base + VerdefSec->sh_offset;
   for (unsigned I = 0; I != VerdefCount; ++I) {
     auto *CurVerdef = reinterpret_cast<const Elf_Verdef *>(Verdef);
@@ -821,24 +831,49 @@ SharedFile<ELFT>::parseVerdefs(const Elf
   return Verdefs;
 }
 
+// We do not usually care about alignments of data in shared object
+// files because the loader takes care of it. However, if we promote a
+// DSO symbol to point to .bss due to copy relocation, we need to keep
+// the original alignment requirements. We infer it in this function.
+template <class ELFT>
+uint32_t SharedFile<ELFT>::getAlignment(ArrayRef<Elf_Shdr> Sections,
+                                        const Elf_Sym &Sym) {
+  uint64_t Ret = 1;
+  if (Sym.st_value)
+    Ret = 1ULL << countTrailingZeros((uint64_t)Sym.st_value);
+  if (0 < Sym.st_shndx && Sym.st_shndx < Sections.size())
+    Ret = std::min<uint64_t>(Ret, Sections[Sym.st_shndx].sh_addralign);
+
+  if (Ret > UINT32_MAX)
+    error(toString(this) + ": alignment too large: " +
+          CHECK(Sym.getName(this->StringTable), this));
+  return Ret;
+}
+
 // Fully parse the shared object file. This must be called after parseSoName().
+//
+// This function parses symbol versions. If a DSO has version information,
+// the file has a ".gnu.version_d" section which contains symbol version
+// definitions. Each symbol is associated to one version through a table in
+// ".gnu.version" section. That table is a parallel array for the symbol
+// table, and each table entry contains an index in ".gnu.version_d".
+//
+// The special index 0 is reserved for VERF_NDX_LOCAL and 1 is for
+// VER_NDX_GLOBAL. There's no table entry for these special versions in
+// ".gnu.version_d".
+//
+// The file format for symbol versioning is perhaps a bit more complicated
+// than necessary, but you can easily understand the code if you wrap your
+// head around the data structure described above.
 template <class ELFT> void SharedFile<ELFT>::parseRest() {
-  // Create mapping from version identifiers to Elf_Verdef entries.
-  const Elf_Versym *Versym = nullptr;
-  Verdefs = parseVerdefs(Versym);
-
+  Verdefs = parseVerdefs();                       // parse .gnu.version_d
+  std::vector<uint32_t> Versyms = parseVersyms(); // parse .gnu.version
   ArrayRef<Elf_Shdr> Sections = CHECK(this->getObj().sections(), this);
 
   // Add symbols to the symbol table.
-  Elf_Sym_Range Syms = this->getGlobalELFSyms();
-  for (const Elf_Sym &Sym : Syms) {
-    unsigned VersymIndex = VER_NDX_GLOBAL;
-    if (Versym) {
-      VersymIndex = Versym->vs_index;
-      ++Versym;
-    }
-    bool Hidden = VersymIndex & VERSYM_HIDDEN;
-    VersymIndex = VersymIndex & ~VERSYM_HIDDEN;
+  ArrayRef<Elf_Sym> Syms = this->getGlobalELFSyms();
+  for (size_t I = 0; I < Syms.size(); ++I) {
+    const Elf_Sym &Sym = Syms[I];
 
     StringRef Name = CHECK(Sym.getName(this->StringTable), this);
     if (Sym.isUndefined()) {
@@ -862,45 +897,31 @@ template <class ELFT> void SharedFile<EL
     // MIPS BFD linker puts _gp_disp symbol into DSO files and incorrectly
     // assigns VER_NDX_LOCAL to this section global symbol. Here is a
     // workaround for this bug.
-    if (Config->EMachine == EM_MIPS && VersymIndex == VER_NDX_LOCAL &&
+    uint32_t Idx = Versyms[I] & ~VERSYM_HIDDEN;
+    if (Config->EMachine == EM_MIPS && Idx == VER_NDX_LOCAL &&
         Name == "_gp_disp")
       continue;
 
-    const Elf_Verdef *Ver = nullptr;
-    if (VersymIndex != VER_NDX_GLOBAL) {
-      if (VersymIndex >= Verdefs.size() || VersymIndex == VER_NDX_LOCAL) {
-        error("corrupt input file: version definition index " +
-              Twine(VersymIndex) + " for symbol " + Name +
-              " is out of bounds\n>>> defined in " + toString(this));
-        continue;
-      }
-      Ver = Verdefs[VersymIndex];
-    }
-
-    // We do not usually care about alignments of data in shared object
-    // files because the loader takes care of it. However, if we promote a
-    // DSO symbol to point to .bss due to copy relocation, we need to keep
-    // the original alignment requirements. We infer it here.
-    uint64_t Alignment = 1;
-    if (Sym.st_value)
-      Alignment = 1ULL << countTrailingZeros((uint64_t)Sym.st_value);
-    if (0 < Sym.st_shndx && Sym.st_shndx < Sections.size()) {
-      uint64_t SecAlign = Sections[Sym.st_shndx].sh_addralign;
-      Alignment = std::min(Alignment, SecAlign);
-    }
-    if (Alignment > UINT32_MAX)
-      error(toString(this) + ": alignment too large: " + Name);
-
-    if (!Hidden)
-      Symtab->addShared(Name, *this, Sym, Alignment, VersymIndex);
+    uint64_t Alignment = getAlignment(Sections, Sym);
+    if (!(Versyms[I] & VERSYM_HIDDEN))
+      Symtab->addShared(Name, *this, Sym, Alignment, Idx);
 
     // Also add the symbol with the versioned name to handle undefined symbols
     // with explicit versions.
-    if (Ver) {
-      StringRef VerName = this->StringTable.data() + Ver->getAux()->vda_name;
-      Name = Saver.save(Name + "@" + VerName);
-      Symtab->addShared(Name, *this, Sym, Alignment, VersymIndex);
+    if (Idx == VER_NDX_GLOBAL)
+      continue;
+
+    if (Idx >= Verdefs.size() || Idx == VER_NDX_LOCAL) {
+      error("corrupt input file: version definition index " + Twine(Idx) +
+            " for symbol " + Name + " is out of bounds\n>>> defined in " +
+            toString(this));
+      continue;
     }
+
+    StringRef VerName =
+        this->StringTable.data() + Verdefs[Idx]->getAux()->vda_name;
+    Name = Saver.save(Name + "@" + VerName);
+    Symtab->addShared(Name, *this, Sym, Alignment, Idx);
   }
 }
 

Modified: lld/trunk/ELF/InputFiles.h
URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.h?rev=328579&r1=328578&r2=328579&view=diff
==============================================================================
--- lld/trunk/ELF/InputFiles.h (original)
+++ lld/trunk/ELF/InputFiles.h Mon Mar 26 12:57:38 2018
@@ -308,7 +308,9 @@ public:
 
   void parseSoName();
   void parseRest();
-  std::vector<const Elf_Verdef *> parseVerdefs(const Elf_Versym *&Versym);
+  uint32_t getAlignment(ArrayRef<Elf_Shdr> Sections, const Elf_Sym &Sym);
+  std::vector<const Elf_Verdef *> parseVerdefs();
+  std::vector<uint32_t> parseVersyms();
 
   struct NeededVer {
     // The string table offset of the version name in the output file.




More information about the llvm-commits mailing list