[ELF] - Detemplate GnuHashTableSection and SymbolTableSection sections.

[grimar]

Summary

This change tries to do the same D31467 did 2 month ago,
but solution now is consistent to what recent commits to LLD do.

SymbolTableBaseSection was introduced.

Detemplation of SymbolTableSection should allow to detemplate more things.

[ruiu]

I'm not in favor of this change. I don't want to introduce a new class in general unless it is worthwhile. It seems this patch introduced a new class hierarchy too casually.

[ruiu]

Okay, it is not at least as concerning as I first thought. LGTM.

ELF/SyntheticSections.h

413 ↗

(On Diff #98748)

Do you need this->?

[grimar]

In D33124#755537, @ruiu wrote:

Okay, it is not at least as concerning as I first thought. LGTM.

Thanks !

lld/trunk/ELF/SyntheticSections.h

private:
   std::vector<DynamicReloc> Relocs;
 };
 
 struct SymbolTableEntry {
   SymbolBody *Symbol;
   size_t StrTabOffset;
 };
 
-template <class ELFT> class SymbolTableSection final : public SyntheticSection {
+class SymbolTableBaseSection : public SyntheticSection {
 public:
-  typedef typename ELFT::Sym Elf_Sym;
-
-  SymbolTableSection(StringTableSection &StrTabSec);
-
+  SymbolTableBaseSection(StringTableSection &StrTabSec);
   void finalizeContents() override;
   void postThunkContents() override;
-  void writeTo(uint8_t *Buf) override;
-  size_t getSize() const override { return getNumSymbols() * sizeof(Elf_Sym); }
+  size_t getSize() const override { return getNumSymbols() * Entsize; }
   void addSymbol(SymbolBody *Body);
   unsigned getNumSymbols() const { return Symbols.size() + 1; }
   size_t getSymbolIndex(SymbolBody *Body);
   ArrayRef<SymbolTableEntry> getSymbols() const { return Symbols; }
 
-private:
+protected:
   // A vector of symbols and their string table offsets.
   std::vector<SymbolTableEntry> Symbols;
 
   StringTableSection &StrTabSec;
 };
 
+template <class ELFT>
+class SymbolTableSection final : public SymbolTableBaseSection {
+  typedef typename ELFT::Sym Elf_Sym;
+
+public:
+  SymbolTableSection(StringTableSection &StrTabSec);
+  void writeTo(uint8_t *Buf) override;
+};
+
 // Outputs GNU Hash section. For detailed explanation see:
 // https://blogs.oracle.com/ali/entry/gnu_hash_elf_sections
-template <class ELFT>
 class GnuHashTableSection final : public SyntheticSection {
 public:
   GnuHashTableSection();
   void finalizeContents() override;
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return Size; }
 
   // Adds symbols to the hash table.

 struct InX {
   static InputSection *ARMAttributes;
   static BssSection *Bss;
   static BssSection *BssRelRo;
   static BuildIdSection *BuildId;
   static InputSection *Common;
   static SyntheticSection *Dynamic;
   static StringTableSection *DynStrTab;
+  static SymbolTableBaseSection *DynSymTab;
+  static GnuHashTableSection *GnuHashTab;
   static InputSection *Interp;
   static GdbIndexSection *GdbIndex;
   static GotBaseSection *Got;
   static GotPltSection *GotPlt;
   static IgotPltSection *IgotPlt;
   static MipsGotSection *MipsGot;
   static MipsRldMapSection *MipsRldMap;
   static PltSection *Plt;
   static PltSection *Iplt;
   static StringTableSection *ShStrTab;
   static StringTableSection *StrTab;
+  static SymbolTableBaseSection *SymTab;
 };
 
 template <class ELFT> struct In : public InX {
-  static SymbolTableSection<ELFT> *DynSymTab;
   static EhFrameHeader<ELFT> *EhFrameHdr;
-  static GnuHashTableSection<ELFT> *GnuHashTab;
   static EhFrameSection<ELFT> *EhFrame;
   static HashTableSection<ELFT> *HashTab;
   static RelocationSection<ELFT> *RelaDyn;
   static RelocationSection<ELFT> *RelaPlt;
   static RelocationSection<ELFT> *RelaIplt;
-  static SymbolTableSection<ELFT> *SymTab;
   static VersionDefinitionSection<ELFT> *VerDef;
   static VersionTableSection<ELFT> *VerSym;
   static VersionNeedSection<ELFT> *VerNeed;
 };
 
-template <class ELFT> SymbolTableSection<ELFT> *In<ELFT>::DynSymTab;
 template <class ELFT> EhFrameHeader<ELFT> *In<ELFT>::EhFrameHdr;
-template <class ELFT> GnuHashTableSection<ELFT> *In<ELFT>::GnuHashTab;
 template <class ELFT> EhFrameSection<ELFT> *In<ELFT>::EhFrame;
 template <class ELFT> HashTableSection<ELFT> *In<ELFT>::HashTab;
 template <class ELFT> RelocationSection<ELFT> *In<ELFT>::RelaDyn;
 template <class ELFT> RelocationSection<ELFT> *In<ELFT>::RelaPlt;
 template <class ELFT> RelocationSection<ELFT> *In<ELFT>::RelaIplt;
-template <class ELFT> SymbolTableSection<ELFT> *In<ELFT>::SymTab;
 template <class ELFT> VersionDefinitionSection<ELFT> *In<ELFT>::VerDef;
 template <class ELFT> VersionTableSection<ELFT> *In<ELFT>::VerSym;
 template <class ELFT> VersionNeedSection<ELFT> *In<ELFT>::VerNeed;
 } // namespace elf
 } // namespace lld
 
 #endif

lld/trunk/ELF/SyntheticSections.cpp

 template <class ELFT> void RelocationSection<ELFT>::finalizeContents() {
   this->Link = In<ELFT>::DynSymTab ? In<ELFT>::DynSymTab->OutSec->SectionIndex
                                    : In<ELFT>::SymTab->OutSec->SectionIndex;
 
   // Set required output section properties.
   this->OutSec->Link = this->Link;
 }
 
-template <class ELFT>
-SymbolTableSection<ELFT>::SymbolTableSection(StringTableSection &StrTabSec)
+SymbolTableBaseSection::SymbolTableBaseSection(StringTableSection &StrTabSec)
     : SyntheticSection(StrTabSec.isDynamic() ? (uint64_t)SHF_ALLOC : 0,
                        StrTabSec.isDynamic() ? SHT_DYNSYM : SHT_SYMTAB,
                        Config->Wordsize,
                        StrTabSec.isDynamic() ? ".dynsym" : ".symtab"),
-      StrTabSec(StrTabSec) {
-  this->Entsize = sizeof(Elf_Sym);
-}
+      StrTabSec(StrTabSec) {}
 
 // Orders symbols according to their positions in the GOT,
 // in compliance with MIPS ABI rules.
 // See "Global Offset Table" in Chapter 5 in the following document
 // for detailed description:
 // ftp://www.linux-mips.org/pub/linux/mips/doc/ABI/mipsabi.pdf
 static bool sortMipsSymbols(const SymbolTableEntry &L,
                             const SymbolTableEntry &R) {
   // Sort entries related to non-local preemptible symbols by GOT indexes.
   // All other entries go to the first part of GOT in arbitrary order.
   bool LIsInLocalGot = !L.Symbol->IsInGlobalMipsGot;
   bool RIsInLocalGot = !R.Symbol->IsInGlobalMipsGot;
   if (LIsInLocalGot || RIsInLocalGot)
     return !RIsInLocalGot;
   return L.Symbol->GotIndex < R.Symbol->GotIndex;
 }
 
 // Finalize a symbol table. The ELF spec requires that all local
 // symbols precede global symbols, so we sort symbol entries in this
 // function. (For .dynsym, we don't do that because symbols for
 // dynamic linking are inherently all globals.)
-template <class ELFT> void SymbolTableSection<ELFT>::finalizeContents() {
+void SymbolTableBaseSection::finalizeContents() {
   this->OutSec->Link = StrTabSec.OutSec->SectionIndex;
 
   // If it is a .dynsym, there should be no local symbols, but we need
   // to do a few things for the dynamic linker.
   if (this->Type == SHT_DYNSYM) {
     // Section's Info field has the index of the first non-local symbol.
     // Because the first symbol entry is a null entry, 1 is the first.
     this->OutSec->Info = 1;
 
-    if (In<ELFT>::GnuHashTab) {
+    if (InX::GnuHashTab) {
       // NB: It also sorts Symbols to meet the GNU hash table requirements.
-      In<ELFT>::GnuHashTab->addSymbols(Symbols);
+      InX::GnuHashTab->addSymbols(Symbols);
     } else if (Config->EMachine == EM_MIPS) {
       std::stable_sort(Symbols.begin(), Symbols.end(), sortMipsSymbols);
     }
 
     size_t I = 0;
     for (const SymbolTableEntry &S : Symbols)
       S.Symbol->DynsymIndex = ++I;
     return;
   }
 }
 
-template <class ELFT> void SymbolTableSection<ELFT>::postThunkContents() {
+void SymbolTableBaseSection::postThunkContents() {
   if (this->Type == SHT_DYNSYM)
     return;
   // move all local symbols before global symbols.
   auto It = std::stable_partition(
       Symbols.begin(), Symbols.end(), [](const SymbolTableEntry &S) {
         return S.Symbol->isLocal() ||
                S.Symbol->symbol()->computeBinding() == STB_LOCAL;
       });
   size_t NumLocals = It - Symbols.begin();
   this->OutSec->Info = NumLocals + 1;
 }
 
-template <class ELFT> void SymbolTableSection<ELFT>::addSymbol(SymbolBody *B) {
+void SymbolTableBaseSection::addSymbol(SymbolBody *B) {
   // Adding a local symbol to a .dynsym is a bug.
   assert(this->Type != SHT_DYNSYM || !B->isLocal());
 
   bool HashIt = B->isLocal();
   Symbols.push_back({B, StrTabSec.addString(B->getName(), HashIt)});
 }
 
-template <class ELFT>
-size_t SymbolTableSection<ELFT>::getSymbolIndex(SymbolBody *Body) {
+size_t SymbolTableBaseSection::getSymbolIndex(SymbolBody *Body) {
   auto I = llvm::find_if(Symbols, [&](const SymbolTableEntry &E) {
     if (E.Symbol == Body)
       return true;
     // This is used for -r, so we have to handle multiple section
     // symbols being combined.
     if (Body->Type == STT_SECTION && E.Symbol->Type == STT_SECTION)
       return cast<DefinedRegular>(Body)->Section->getOutputSection() ==
              cast<DefinedRegular>(E.Symbol)->Section->getOutputSection();
     return false;
   });
   if (I == Symbols.end())
     return 0;
   return I - Symbols.begin() + 1;
 }
 
+template <class ELFT>
+SymbolTableSection<ELFT>::SymbolTableSection(StringTableSection &StrTabSec)
+    : SymbolTableBaseSection(StrTabSec) {
+  this->Entsize = sizeof(Elf_Sym);
+}
+
 // Write the internal symbol table contents to the output symbol table.
 template <class ELFT> void SymbolTableSection<ELFT>::writeTo(uint8_t *Buf) {
   // The first entry is a null entry as per the ELF spec.
   Buf += sizeof(Elf_Sym);
 
   auto *ESym = reinterpret_cast<Elf_Sym *>(Buf);
 
   for (SymbolTableEntry &Ent : Symbols) {

 //
 // Due to historical reasons, we have two different hash tables, .hash
 // and .gnu.hash. They are for the same purpose, and .gnu.hash is a new
 // and better version of .hash. .hash is just an on-disk hash table, but
 // .gnu.hash has a bloom filter in addition to a hash table to skip
 // DSOs very quickly. If you are sure that your dynamic linker knows
 // about .gnu.hash, you want to specify -hash-style=gnu. Otherwise, a
 // safe bet is to specify -hash-style=both for backward compatibilty.
-template <class ELFT>
-GnuHashTableSection<ELFT>::GnuHashTableSection()
+GnuHashTableSection::GnuHashTableSection()
     : SyntheticSection(SHF_ALLOC, SHT_GNU_HASH, Config->Wordsize, ".gnu.hash") {
 }
 
-template <class ELFT> void GnuHashTableSection<ELFT>::finalizeContents() {
-  this->OutSec->Link = In<ELFT>::DynSymTab->OutSec->SectionIndex;
+void GnuHashTableSection::finalizeContents() {
+  this->OutSec->Link = InX::DynSymTab->OutSec->SectionIndex;
 
   // Computes bloom filter size in word size. We want to allocate 8
   // bits for each symbol. It must be a power of two.
   if (Symbols.empty())
     MaskWords = 1;
   else
     MaskWords = NextPowerOf2((Symbols.size() - 1) / Config->Wordsize);
 
   Size = 16;                            // Header
   Size += Config->Wordsize * MaskWords; // Bloom filter
   Size += NBuckets * 4;                 // Hash buckets
   Size += Symbols.size() * 4;           // Hash values
 }
 
-template <class ELFT>
-void GnuHashTableSection<ELFT>::writeTo(uint8_t *Buf) {
+void GnuHashTableSection::writeTo(uint8_t *Buf) {
   // Write a header.
   write32(Buf, NBuckets, Config->Endianness);
-  write32(Buf + 4, In<ELFT>::DynSymTab->getNumSymbols() - Symbols.size(),
+  write32(Buf + 4, InX::DynSymTab->getNumSymbols() - Symbols.size(),
           Config->Endianness);
   write32(Buf + 8, MaskWords, Config->Endianness);
   write32(Buf + 12, getShift2(), Config->Endianness);
   Buf += 16;
 
   // Write a bloom filter and a hash table.
   writeBloomFilter(Buf);
   Buf += Config->Wordsize * MaskWords;
   writeHashTable(Buf);
 }
 
 // This function writes a 2-bit bloom filter. This bloom filter alone
 // usually filters out 80% or more of all symbol lookups [1].
 // The dynamic linker uses the hash table only when a symbol is not
 // filtered out by a bloom filter.
 //
 // [1] Ulrich Drepper (2011), "How To Write Shared Libraries" (Ver. 4.1.2),
 //     p.9, https://www.akkadia.org/drepper/dsohowto.pdf
-template <class ELFT>
-void GnuHashTableSection<ELFT>::writeBloomFilter(uint8_t *Buf) {
+void GnuHashTableSection::writeBloomFilter(uint8_t *Buf) {
   const unsigned C = Config->Wordsize * 8;
   for (const Entry &Sym : Symbols) {
     size_t I = (Sym.Hash / C) & (MaskWords - 1);
     uint64_t Val = readUint(Buf + I * Config->Wordsize);
     Val |= uint64_t(1) << (Sym.Hash % C);
     Val |= uint64_t(1) << ((Sym.Hash >> getShift2()) % C);
     writeUint(Buf + I * Config->Wordsize, Val);
   }
 }
 
-template <class ELFT>
-void GnuHashTableSection<ELFT>::writeHashTable(uint8_t *Buf) {
+void GnuHashTableSection::writeHashTable(uint8_t *Buf) {
   // Group symbols by hash value.
   std::vector<std::vector<Entry>> Syms(NBuckets);
   for (const Entry &Ent : Symbols)
     Syms[Ent.Hash % NBuckets].push_back(Ent);
 
   // Write hash buckets. Hash buckets contain indices in the following
   // hash value table.
   uint32_t *Buckets = reinterpret_cast<uint32_t *>(Buf);
for (size_t N : {131071, 65521, 32749, 16381, 8191, 4093, 2039, 1021, 509,
     if (N <= NumSymbols)
       return N;
   return 0;
 }
 
 // Add symbols to this symbol hash table. Note that this function
 // destructively sort a given vector -- which is needed because
 // GNU-style hash table places some sorting requirements.
-template <class ELFT>
-void GnuHashTableSection<ELFT>::addSymbols(std::vector<SymbolTableEntry> &V) {
+void GnuHashTableSection::addSymbols(std::vector<SymbolTableEntry> &V) {
   // We cannot use 'auto' for Mid because GCC 6.1 cannot deduce
   // its type correctly.
   std::vector<SymbolTableEntry>::iterator Mid =
       std::stable_partition(V.begin(), V.end(), [](const SymbolTableEntry &S) {
         return S.Symbol->isUndefined();
       });
   if (Mid == V.end())
     return;

 
 InputSection *InX::ARMAttributes;
 BssSection *InX::Bss;
 BssSection *InX::BssRelRo;
 BuildIdSection *InX::BuildId;
 InputSection *InX::Common;
 SyntheticSection *InX::Dynamic;
 StringTableSection *InX::DynStrTab;
+SymbolTableBaseSection *InX::DynSymTab;
 InputSection *InX::Interp;
 GdbIndexSection *InX::GdbIndex;
 GotBaseSection *InX::Got;
 GotPltSection *InX::GotPlt;
+GnuHashTableSection *InX::GnuHashTab;
 IgotPltSection *InX::IgotPlt;
 MipsGotSection *InX::MipsGot;
 MipsRldMapSection *InX::MipsRldMap;
 PltSection *InX::Plt;
 PltSection *InX::Iplt;
 StringTableSection *InX::ShStrTab;
 StringTableSection *InX::StrTab;
+SymbolTableBaseSection *InX::SymTab;
 
 template void PltSection::addEntry<ELF32LE>(SymbolBody &Sym);
 template void PltSection::addEntry<ELF32BE>(SymbolBody &Sym);
 template void PltSection::addEntry<ELF64LE>(SymbolBody &Sym);
 template void PltSection::addEntry<ELF64BE>(SymbolBody &Sym);
 
 template InputSection *elf::createCommonSection<ELF32LE>();
 template InputSection *elf::createCommonSection<ELF32BE>();

 template class elf::RelocationSection<ELF64LE>;
 template class elf::RelocationSection<ELF64BE>;
 
 template class elf::SymbolTableSection<ELF32LE>;
 template class elf::SymbolTableSection<ELF32BE>;
 template class elf::SymbolTableSection<ELF64LE>;
 template class elf::SymbolTableSection<ELF64BE>;
 
-template class elf::GnuHashTableSection<ELF32LE>;
-template class elf::GnuHashTableSection<ELF32BE>;
-template class elf::GnuHashTableSection<ELF64LE>;
-template class elf::GnuHashTableSection<ELF64BE>;
-
 template class elf::HashTableSection<ELF32LE>;
 template class elf::HashTableSection<ELF32BE>;
 template class elf::HashTableSection<ELF64LE>;
 template class elf::HashTableSection<ELF64BE>;
 
 template class elf::EhFrameHeader<ELF32LE>;
 template class elf::EhFrameHeader<ELF32BE>;
 template class elf::EhFrameHeader<ELF64LE>;

lld/trunk/ELF/Writer.cpp

if (needsInterpSection<ELFT>()) {
     InX::Interp = nullptr;
   }
 
   if (!Config->Relocatable)
     Add(createCommentSection<ELFT>());
 
   if (Config->Strip != StripPolicy::All) {
     InX::StrTab = make<StringTableSection>(".strtab", false);
-    In<ELFT>::SymTab = make<SymbolTableSection<ELFT>>(*InX::StrTab);
+    InX::SymTab = make<SymbolTableSection<ELFT>>(*InX::StrTab);
   }
 
   if (Config->BuildId != BuildIdKind::None) {
     InX::BuildId = make<BuildIdSection>();
     Add(InX::BuildId);
   }
 
   InX::Common = createCommonSection<ELFT>();
if (!Config->VersionDefinitions.empty()) {
       In<ELFT>::VerDef = make<VersionDefinitionSection<ELFT>>();
       Add(In<ELFT>::VerDef);
     }
 
     In<ELFT>::VerNeed = make<VersionNeedSection<ELFT>>();
     Add(In<ELFT>::VerNeed);
 
     if (Config->GnuHash) {
-      In<ELFT>::GnuHashTab = make<GnuHashTableSection<ELFT>>();
+      In<ELFT>::GnuHashTab = make<GnuHashTableSection>();
       Add(In<ELFT>::GnuHashTab);
     }
 
     if (Config->SysvHash) {
       In<ELFT>::HashTab = make<HashTableSection<ELFT>>();
       Add(In<ELFT>::HashTab);
     }