[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:
   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> {
+// Common base class for Plt and IPlt.
+template <class ELFT> class PltSectionBase : public SyntheticSection<ELFT> {
 public:
-  PltSection();
+  PltSectionBase(size_t HeaderSize, RelocationSection<ELFT> *PltRelSec);
   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:
+  virtual void writeHeader(uint8_t *Buf){};
+  virtual void addHeaderSymbols(){};
+  virtual unsigned getPltRelocOff() const = 0;
   std::vector<std::pair<const SymbolBody *, unsigned>> Entries;
+  // IPlt always has HeaderSize of 0
+  size_t HeaderSize;
+  RelocationSection<ELFT> *PltRelocSection;
+};
+
+// The traditional Plt section for non GNU Ifunc symbols. This always has a
+// header as the first entry to facilitate lazy binding at run time.
+template <class ELFT> class PltSection final : public PltSectionBase<ELFT> {
+public:
+  PltSection();
+
+private:
+  virtual void writeHeader(uint8_t *Buf) override;
+  virtual void addHeaderSymbols() override;
+  virtual unsigned getPltRelocOff() const override { return 0; }
 };
 
 // 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> {
+template <class ELFT> class IpltSection final : public PltSectionBase<ELFT> {
 public:
   IpltSection();
-  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;
+  virtual unsigned getPltRelocOff() const override;
 };
 
 template <class ELFT>
 class GdbIndexSection final : public SyntheticSection<ELFT> {
   typedef typename ELFT::uint uintX_t;
 
   const unsigned OffsetTypeSize = 4;
   const unsigned CuListOffset = 6 * OffsetTypeSize;

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()
+PltSectionBase<ELFT>::PltSectionBase(size_t S,
+                                     RelocationSection<ELFT> *PltRelSec)
     : SyntheticSection<ELFT>(SHF_ALLOC | SHF_EXECINSTR, SHT_PROGBITS, 16,
-                             ".plt") {}
+                             ".plt"),
+      HeaderSize(S), PltRelocSection(PltRelSec) {}
 
-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.
-  Target->writePltHeader(Buf);
-  size_t Off = Target->PltHeaderSize;
+template <class ELFT> void PltSectionBase<ELFT>::writeTo(uint8_t *Buf) {
+  // At beginning of PLT but not the IPLT, we have code to call the dynamic
+  // linker to resolve dynsyms at runtime. Write such code.
+  writeHeader(Buf);
+  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) {
+template <class ELFT> void PltSectionBase<ELFT>::addEntry(SymbolBody &Sym) {
   Sym.PltIndex = Entries.size();
-  unsigned RelOff = In<ELFT>::RelaPlt->getRelocOffset();
+  unsigned RelOff = PltRelocSection->getRelocOffset();
+  Sym.IsInIplt = HeaderSize == 0;
   Entries.push_back(std::make_pair(&Sym, RelOff));
 }
 
-template <class ELFT> size_t PltSection<ELFT>::getSize() const {
-  return Target->PltHeaderSize + Entries.size() * Target->PltEntrySize;
+template <class ELFT> size_t PltSectionBase<ELFT>::getSize() const {
+  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() {
-  Target->addPltHeaderSymbols(this);
-  size_t Off = Target->PltHeaderSize;
+template <class ELFT> void PltSectionBase<ELFT>::addSymbols() {
+  // The PLT may have symbols defined for the Header, the IPLT has no header
+  addHeaderSymbols();
+  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") {}
+PltSection<ELFT>::PltSection()
+    : PltSectionBase<ELFT>(Target->PltHeaderSize, In<ELFT>::RelaPlt) {}
 
-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 PltSection<ELFT>::writeHeader(uint8_t *Buf) {
+  Target->writePltHeader(Buf);
 }
 
-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));
+// 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>::addHeaderSymbols() {
+  Target->addPltHeaderSymbols(this);
 }
 
-template <class ELFT> size_t IpltSection<ELFT>::getSize() const {
-  return Entries.size() * Target->PltEntrySize;
-}
+template <class ELFT>
+IpltSection<ELFT>::IpltSection()
+    : PltSectionBase<ELFT>(0, In<ELFT>::RelaIplt) {}
 
-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 IpltSection<ELFT>::getPltRelocOff() const {
+  return In<ELFT>::Plt->getSize();
 }
 
 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::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::PltSectionBase<ELF32LE>;
+template class elf::PltSectionBase<ELF32BE>;
+template class elf::PltSectionBase<ELF64LE>;
+template class elf::PltSectionBase<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>;