[LLD][ELF] Refactor PltSection and IPltSection common code into a base class NFC

[peter.smith]

Summary

In a comment via email on D28956 Rafael suggested:
"Would it be possible to add a common base class to PltSection and Ipltsection and share this code?"

This is an attempt to add a common base class PltSectionBase that moves out most of the more complex code into the base class. It is using the same logic and interface as previously.

[ruiu]

It is probably my taste, but I'd try to keep the class hierarchy as shallow as possible. Looks like IPLT and PLT are pretty similar. I wonder if you can merge PltBase, Plt and Iplt into one class, add IsIplt boolean flag, and do whatever it is needed to do for IPLT according to the flag.

[peter.smith]

Updated the diff with an attempt to use a single class for both the Plt and IPlt. I've used HeaderSize to distinguish between them as that removes a few conditional statements.

[ruiu]

LGTM

ELF/SyntheticSections.h

public:
   size_t getSize() const override;
   void writeTo(uint8_t *Buf) override;
   bool empty() const override { return Entries.empty(); }
 
 private:
   std::vector<const SymbolBody *> Entries;
 };
 
-// The IgotPltSection is a Got associated with the IpltSection for GNU Ifunc
+// The IgotPltSection is a Got associated with the PltSection for GNU Ifunc
 // Symbols that will be relocated by Target->IRelativeRel.
 // On most Targets the IgotPltSection will immediately follow the GotPltSection
 // on ARM the IgotPltSection will immediately follow the GotSection.
 template <class ELFT>
 class IgotPltSection final : public SyntheticSection<ELFT> {
   typedef typename ELFT::uint uintX_t;
 
 public:
public:
   void finalize() override;
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return this->Size; }
 
 private:
   size_t Size = 0;
 };
 
-template <class ELFT> class PltSection final : public SyntheticSection<ELFT> {
+// The PltSection is used for both the Plt and Iplt. The former always has a
+// header as its first entry that is used at run-time to resolve lazy binding.
+// The latter is used for GNU Ifunc symbols, that will be subject to a
+// Target->IRelativeRel.
+template <class ELFT> class PltSection : public SyntheticSection<ELFT> {
 public:
-  PltSection();
-  void writeTo(uint8_t *Buf) override;
-  size_t getSize() const override;
-  void addEntry(SymbolBody &Sym);
-  bool empty() const override { return Entries.empty(); }
-  void addSymbols();
-
-private:
-  std::vector<std::pair<const SymbolBody *, unsigned>> Entries;
-};
-
-// The IpltSection is a variant of Plt for recording entries for GNU Ifunc
-// symbols that will be subject to a Target->IRelativeRel
-// The IpltSection immediately follows the Plt section in the Output Section
-template <class ELFT> class IpltSection final : public SyntheticSection<ELFT> {
-public:
-  IpltSection();
+  PltSection(size_t HeaderSize);
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override;
   void addEntry(SymbolBody &Sym);
   bool empty() const override { return Entries.empty(); }
   void addSymbols();
 
 private:
+  void writeHeader(uint8_t *Buf){};
+  void addHeaderSymbols(){};
+  unsigned getPltRelocOff() const;
   std::vector<std::pair<const SymbolBody *, unsigned>> Entries;
+  // Iplt always has HeaderSize of 0, the Plt HeaderSize is always non-zero
+  size_t HeaderSize;
 };
 
 template <class ELFT>
 class GdbIndexSection final : public SyntheticSection<ELFT> {
   typedef typename ELFT::uint uintX_t;
 
   const unsigned OffsetTypeSize = 4;
   const unsigned CuListOffset = 6 * OffsetTypeSize;
template <class ELFT> struct In {
   static GotSection<ELFT> *Got;
   static MipsGotSection<ELFT> *MipsGot;
   static GotPltSection<ELFT> *GotPlt;
   static IgotPltSection<ELFT> *IgotPlt;
   static HashTableSection<ELFT> *HashTab;
   static InputSection<ELFT> *Interp;
   static MipsRldMapSection<ELFT> *MipsRldMap;
   static PltSection<ELFT> *Plt;
-  static IpltSection<ELFT> *Iplt;
+  static PltSection<ELFT> *Iplt;
   static RelocationSection<ELFT> *RelaDyn;
   static RelocationSection<ELFT> *RelaPlt;
   static RelocationSection<ELFT> *RelaIplt;
   static StringTableSection<ELFT> *ShStrTab;
   static StringTableSection<ELFT> *StrTab;
   static SymbolTableSection<ELFT> *SymTab;
   static VersionDefinitionSection<ELFT> *VerDef;
   static VersionTableSection<ELFT> *VerSym;

 template <class ELFT> GotSection<ELFT> *In<ELFT>::Got;
 template <class ELFT> MipsGotSection<ELFT> *In<ELFT>::MipsGot;
 template <class ELFT> GotPltSection<ELFT> *In<ELFT>::GotPlt;
 template <class ELFT> IgotPltSection<ELFT> *In<ELFT>::IgotPlt;
 template <class ELFT> HashTableSection<ELFT> *In<ELFT>::HashTab;
 template <class ELFT> InputSection<ELFT> *In<ELFT>::Interp;
 template <class ELFT> MipsRldMapSection<ELFT> *In<ELFT>::MipsRldMap;
 template <class ELFT> PltSection<ELFT> *In<ELFT>::Plt;
-template <class ELFT> IpltSection<ELFT> *In<ELFT>::Iplt;
+template <class ELFT> PltSection<ELFT> *In<ELFT>::Iplt;
 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> StringTableSection<ELFT> *In<ELFT>::ShStrTab;
 template <class ELFT> StringTableSection<ELFT> *In<ELFT>::StrTab;
 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

ELF/SyntheticSections.cpp

for (const SymbolTableEntry &S : In<ELFT>::DynSymTab->getSymbols()) {
     unsigned I = Body->DynsymIndex;
     uint32_t Hash = hashSysV(Name) % NumSymbols;
     Chains[I] = Buckets[Hash];
     Buckets[Hash] = I;
   }
 }
 
 template <class ELFT>
-PltSection<ELFT>::PltSection()
+PltSection<ELFT>::PltSection(size_t S)
     : SyntheticSection<ELFT>(SHF_ALLOC | SHF_EXECINSTR, SHT_PROGBITS, 16,
-                             ".plt") {}
+                             ".plt"),
+      HeaderSize(S) {}
 
 template <class ELFT> void PltSection<ELFT>::writeTo(uint8_t *Buf) {
-  // At beginning of PLT, we have code to call the dynamic linker
-  // to resolve dynsyms at runtime. Write such code.
+  // At beginning of PLT but not the IPLT, we have code to call the dynamic
+  // linker to resolve dynsyms at runtime. Write such code.
+  if (HeaderSize != 0)
     Target->writePltHeader(Buf);
-  size_t Off = Target->PltHeaderSize;
+  size_t Off = HeaderSize;
+  // The IPlt is immediately after the Plt, account for this in RelOff
+  unsigned PltOff = getPltRelocOff();
 
   for (auto &I : Entries) {
     const SymbolBody *B = I.first;
-    unsigned RelOff = I.second;
+    unsigned RelOff = I.second + PltOff;
     uint64_t Got = B->getGotPltVA<ELFT>();
     uint64_t Plt = this->getVA() + Off;
     Target->writePlt(Buf + Off, Got, Plt, B->PltIndex, RelOff);
     Off += Target->PltEntrySize;
   }
 }
 
 template <class ELFT> void PltSection<ELFT>::addEntry(SymbolBody &Sym) {
   Sym.PltIndex = Entries.size();
-  unsigned RelOff = In<ELFT>::RelaPlt->getRelocOffset();
+  RelocationSection<ELFT> *PltRelocSection = In<ELFT>::RelaPlt;
+  if (HeaderSize == 0) {
+    PltRelocSection = In<ELFT>::RelaIplt;
+    Sym.IsInIplt = true;
+  }
+  unsigned RelOff = PltRelocSection->getRelocOffset();
   Entries.push_back(std::make_pair(&Sym, RelOff));
 }
 
 template <class ELFT> size_t PltSection<ELFT>::getSize() const {
-  return Target->PltHeaderSize + Entries.size() * Target->PltEntrySize;
+  return HeaderSize + Entries.size() * Target->PltEntrySize;
 }
 
 // Some architectures such as additional symbols in the PLT section. For
 // example ARM uses mapping symbols to aid disassembly
 template <class ELFT> void PltSection<ELFT>::addSymbols() {
+  // The PLT may have symbols defined for the Header, the IPLT has no header
+  if (HeaderSize != 0)
     Target->addPltHeaderSymbols(this);
-  size_t Off = Target->PltHeaderSize;
+  size_t Off = HeaderSize;
   for (size_t I = 0; I < Entries.size(); ++I) {
     Target->addPltSymbols(this, Off);
     Off += Target->PltEntrySize;
   }
 }
 
-template <class ELFT>
-IpltSection<ELFT>::IpltSection()
-    : SyntheticSection<ELFT>(SHF_ALLOC | SHF_EXECINSTR, SHT_PROGBITS, 16,
-                             ".plt") {}
-
-template <class ELFT> void IpltSection<ELFT>::writeTo(uint8_t *Buf) {
-  // The IRelative relocations do not support lazy binding so no header is
-  // needed
-  size_t Off = 0;
-  for (auto &I : Entries) {
-    const SymbolBody *B = I.first;
-    unsigned RelOff = I.second + In<ELFT>::Plt->getSize();
-    uint64_t Got = B->getGotPltVA<ELFT>();
-    uint64_t Plt = this->getVA() + Off;
-    Target->writePlt(Buf + Off, Got, Plt, B->PltIndex, RelOff);
-    Off += Target->PltEntrySize;
-  }
-}
-
-template <class ELFT> void IpltSection<ELFT>::addEntry(SymbolBody &Sym) {
-  Sym.PltIndex = Entries.size();
-  Sym.IsInIplt = true;
-  unsigned RelOff = In<ELFT>::RelaIplt->getRelocOffset();
-  Entries.push_back(std::make_pair(&Sym, RelOff));
-}
-
-template <class ELFT> size_t IpltSection<ELFT>::getSize() const {
-  return Entries.size() * Target->PltEntrySize;
-}
-
-template <class ELFT> void IpltSection<ELFT>::addSymbols() {
-  size_t Off = 0;
-  for (size_t I = 0; I < Entries.size(); ++I) {
-    Target->addPltSymbols(this, Off);
-    Off += Target->PltEntrySize;
-  }
+template <class ELFT> unsigned PltSection<ELFT>::getPltRelocOff() const {
+  return (HeaderSize == 0) ? In<ELFT>::Plt->getSize() : 0;
 }
 
 template <class ELFT>
 GdbIndexSection<ELFT>::GdbIndexSection()
     : SyntheticSection<ELFT>(0, SHT_PROGBITS, 1, ".gdb_index"),
       StringPool(llvm::StringTableBuilder::ELF) {}
 
 template <class ELFT> void GdbIndexSection<ELFT>::parseDebugSections() {

 template class elf::HashTableSection<ELF64LE>;
 template class elf::HashTableSection<ELF64BE>;
 
 template class elf::PltSection<ELF32LE>;
 template class elf::PltSection<ELF32BE>;
 template class elf::PltSection<ELF64LE>;
 template class elf::PltSection<ELF64BE>;
 
-template class elf::IpltSection<ELF32LE>;
-template class elf::IpltSection<ELF32BE>;
-template class elf::IpltSection<ELF64LE>;
-template class elf::IpltSection<ELF64BE>;
-
 template class elf::GdbIndexSection<ELF32LE>;
 template class elf::GdbIndexSection<ELF32BE>;
 template class elf::GdbIndexSection<ELF64LE>;
 template class elf::GdbIndexSection<ELF64BE>;
 
 template class elf::EhFrameHeader<ELF32LE>;
 template class elf::EhFrameHeader<ELF32BE>;
 template class elf::EhFrameHeader<ELF64LE>;

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
 
   // The RelaIplt immediately follows .rel.plt (.rel.dyn for ARM) to ensure
   // that the IRelative relocations are processed last by the dynamic loader
   In<ELFT>::RelaIplt = make<RelocationSection<ELFT>>(
       (Config->EMachine == EM_ARM) ? ".rel.dyn" : In<ELFT>::RelaPlt->Name,
       false /*Sort*/);
   Add(In<ELFT>::RelaIplt);
 
-  In<ELFT>::Plt = make<PltSection<ELFT>>();
+  In<ELFT>::Plt = make<PltSection<ELFT>>(Target->PltHeaderSize);
   Add(In<ELFT>::Plt);
-  In<ELFT>::Iplt = make<IpltSection<ELFT>>();
+  In<ELFT>::Iplt = make<PltSection<ELFT>>(0);
   Add(In<ELFT>::Iplt);
 
   if (Config->EhFrameHdr) {
     In<ELFT>::EhFrameHdr = make<EhFrameHeader<ELFT>>();
     Add(In<ELFT>::EhFrameHdr);
   }
 
   if (In<ELFT>::SymTab)