Alternative way to detemplate GotSection

[• rafael]

Summary

GetSection is a template because write calls relocate.

relocate has two parts. The non alloc code really has to be a template, as it is looking a raw input file data.

The alloc part is only a template because of getSize.

This patch folds the value of getSize early, detemplates getRelocTargetVA and splits relocate into a templated non alloc case and a regular function for the alloc case. This has the nice advantage of making sure we collect all the information we need for relocations before getting to InputSection::relocateNonAlloc.

Since we know got is alloc, it can just call the function directly and avoid the template.

[• rafael]

Rebased.

[ruiu]

LGTM.

Nice! I like this.

[• espindola]

303355

ELF/InputSection.h

public:
   void uncompress();
 
   // Returns a source location string. Used to construct an error message.
   template <class ELFT> std::string getLocation(uint64_t Offset);
   template <class ELFT> std::string getSrcMsg(uint64_t Offset);
   template <class ELFT> std::string getObjMsg(uint64_t Offset);
 
   template <class ELFT> void relocate(uint8_t *Buf, uint8_t *BufEnd);
+  void relocateAlloc(uint8_t *Buf, uint8_t *BufEnd);
+  template <class ELFT> void relocateNonAlloc(uint8_t *Buf, uint8_t *BufEnd);
 
   std::vector<Relocation> Relocations;
 
   template <typename T> llvm::ArrayRef<T> getDataAs() const {
     size_t S = Data.size();
     assert(S % sizeof(T) == 0);
     return llvm::makeArrayRef<T>((const T *)Data.data(), S / sizeof(T));
   }

ELF/InputSection.cpp

static uint64_t getAArch64UndefinedRelativeWeakVA(uint64_t Type, uint64_t A,
   case R_AARCH64_JUMP26:
   case R_AARCH64_TSTBR14:
     return P + 4;
   default:
     return A;
   }
 }
 
-template <class ELFT>
-static typename ELFT::uint
-getRelocTargetVA(uint32_t Type, int64_t A, typename ELFT::uint P,
+static uint64_t getRelocTargetVA(uint32_t Type, int64_t A, uint64_t P,
                                  const SymbolBody &Body, RelExpr Expr) {
   switch (Expr) {
   case R_ABS:
   case R_RELAX_GOT_PC_NOPIC:
     return Body.getVA(A);
   case R_GOT:
   case R_RELAX_TLS_GD_TO_IE_ABS:
     return Body.getGotVA() + A;
   case R_GOTONLY_PC:
if (Body.isTls() && (Body.isLazy() || Body.isUndefined()) &&
       return 0;
     if (Target->TcbSize)
       return Body.getVA(A) + alignTo(Target->TcbSize, Out::TlsPhdr->p_align);
     return Body.getVA(A) - Out::TlsPhdr->p_memsz;
   case R_RELAX_TLS_GD_TO_LE_NEG:
   case R_NEG_TLS:
     return Out::TlsPhdr->p_memsz - Body.getVA(A);
   case R_SIZE:
-    return Body.getSize<ELFT>() + A;
+    return A; // Body.getSize was already folded into the addend.
   case R_TLSDESC:
     return InX::Got->getGlobalDynAddr(Body) + A;
   case R_TLSDESC_PAGE:
     return getAArch64Page(InX::Got->getGlobalDynAddr(Body) + A) -
            getAArch64Page(P);
   case R_TLSGD:
     return InX::Got->getGlobalDynOffset(Body) + A - InX::Got->getSize();
   case R_TLSGD_PC:
if (Expr != R_ABS) {
       error(this->getLocation<ELFT>(Offset) + ": has non-ABS reloc");
       return;
     }
 
     uint64_t AddrLoc = this->OutSec->Addr + Offset;
     uint64_t SymVA = 0;
     if (!Sym.isTls() || Out::TlsPhdr)
       SymVA = SignExtend64<sizeof(typename ELFT::uint) * 8>(
-          getRelocTargetVA<ELFT>(Type, Addend, AddrLoc, Sym, R_ABS));
+          getRelocTargetVA(Type, Addend, AddrLoc, Sym, R_ABS));
     Target->relocateOne(BufLoc, Type, SymVA);
   }
 }
 
 template <class ELFT> elf::ObjectFile<ELFT> *InputSectionBase::getFile() const {
   return cast_or_null<elf::ObjectFile<ELFT>>(File);
 }
 
 template <class ELFT>
 void InputSectionBase::relocate(uint8_t *Buf, uint8_t *BufEnd) {
+  if (Flags & SHF_ALLOC)
+    relocateAlloc(Buf, BufEnd);
+  else
+    relocateNonAlloc<ELFT>(Buf, BufEnd);
+}
+
+template <class ELFT>
+void InputSectionBase::relocateNonAlloc(uint8_t *Buf, uint8_t *BufEnd) {
   // scanReloc function in Writer.cpp constructs Relocations
   // vector only for SHF_ALLOC'ed sections. For other sections,
   // we handle relocations directly here.
-  auto *IS = dyn_cast<InputSection>(this);
-  if (IS && !(IS->Flags & SHF_ALLOC)) {
-    if (IS->AreRelocsRela)
-      IS->relocateNonAlloc<ELFT>(Buf, IS->template relas<ELFT>());
-    else
-      IS->relocateNonAlloc<ELFT>(Buf, IS->template rels<ELFT>());
-    return;
-  }
+  auto *IS = cast<InputSection>(this);
+  assert(!(IS->Flags & SHF_ALLOC));
 
-  const unsigned Bits = sizeof(typename ELFT::uint) * 8;
+void InputSectionBase::relocateAlloc(uint8_t *Buf, uint8_t *BufEnd) {
+  assert(Flags & SHF_ALLOC);
+  const unsigned Bits = Config->Wordsize * 8;
   for (const Relocation &Rel : Relocations) {
     uint64_t Offset = getOffset(Rel.Offset);
     uint8_t *BufLoc = Buf + Offset;
     uint32_t Type = Rel.Type;
 
     uint64_t AddrLoc = getOutputSection()->Addr + Offset;
     RelExpr Expr = Rel.Expr;
-    uint64_t TargetVA = SignExtend64<Bits>(
-        getRelocTargetVA<ELFT>(Type, Rel.Addend, AddrLoc, *Rel.Sym, Expr));
+    uint64_t TargetVA = SignExtend64(
+        getRelocTargetVA(Type, Rel.Addend, AddrLoc, *Rel.Sym, Expr), Bits);
 
     switch (Expr) {
     case R_RELAX_GOT_PC:
     case R_RELAX_GOT_PC_NOPIC:
       Target->relaxGot(BufLoc, TargetVA);
       break;
     case R_RELAX_TLS_IE_TO_LE:
       Target->relaxTlsIeToLe(BufLoc, Type, TargetVA);

ELF/Relocations.cpp

if (!needsPlt(Expr) && !needsGot(Expr) && isPreemptible(Body, Type)) {
         InX::MipsGot->addEntry(Body, Addend, Expr);
       continue;
     }
 
     // If the relocation points to something in the file, we can process it.
     bool IsConstant =
         isStaticLinkTimeConstant<ELFT>(Expr, Type, Body, Sec, Rel.r_offset);
 
+    // The size is not going to change, so we fold it in here.
+    if (Expr == R_SIZE)
+      Addend += Body.getSize<ELFT>();
+
     // If the output being produced is position independent, the final value
     // is still not known. In that case we still need some help from the
     // dynamic linker. We can however do better than just copying the incoming
     // relocation. We can process some of it and and just ask the dynamic
     // linker to add the load address.
     if (!IsConstant)
       In<ELFT>::RelaDyn->addReloc(
           {Target->RelativeRel, &Sec, Offset, true, &Body, Addend});

ELF/SyntheticSections.h

private:
   uint64_t getFdePc(uint8_t *Buf, size_t Off, uint8_t Enc);
 
   std::vector<CieRecord *> Cies;
 
   // CIE records are uniquified by their contents and personality functions.
   llvm::DenseMap<std::pair<ArrayRef<uint8_t>, SymbolBody *>, CieRecord> CieMap;
 };
 
-class GotBaseSection : public SyntheticSection {
+class GotSection : public SyntheticSection {
 public:
-  GotBaseSection();
+  GotSection();
   size_t getSize() const override { return Size; }
   void finalizeContents() override;
   bool empty() const override;
+  void writeTo(uint8_t *Buf) override;
 
   void addEntry(SymbolBody &Sym);
   bool addDynTlsEntry(SymbolBody &Sym);
   bool addTlsIndex();
   uint64_t getGlobalDynAddr(const SymbolBody &B) const;
   uint64_t getGlobalDynOffset(const SymbolBody &B) const;
 
   uint64_t getTlsIndexVA() { return this->getVA() + TlsIndexOff; }
   uint32_t getTlsIndexOff() const { return TlsIndexOff; }
 
   // Flag to force GOT to be in output if we have relocations
   // that relies on its address.
   bool HasGotOffRel = false;
 
 protected:
   size_t NumEntries = 0;
   uint32_t TlsIndexOff = -1;
   uint64_t Size = 0;
 };
 
-template <class ELFT> class GotSection final : public GotBaseSection {
-public:
-  void writeTo(uint8_t *Buf) override;
-};
-
 // .note.gnu.build-id section.
 class BuildIdSection : public SyntheticSection {
   // First 16 bytes are a header.
   static const unsigned HeaderSize = 16;
 
 public:
   BuildIdSection();
   void writeTo(uint8_t *Buf) override;
struct InX {
   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 GotSection *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;

ELF/SyntheticSections.cpp

for (EhSectionPiece *Fde : Cie->FdePieces) {
 
       // FDE's second word should have the offset to an associated CIE.
       // Write it.
       write32<E>(Buf + Off + 4, Off + 4 - CieOffset);
     }
   }
 
   for (EhInputSection *S : Sections)
-    S->template relocate<ELFT>(Buf, nullptr);
+    S->relocateAlloc(Buf, nullptr);
 
   // Construct .eh_frame_hdr. .eh_frame_hdr is a binary search table
   // to get a FDE from an address to which FDE is applied. So here
   // we obtain two addresses and pass them to EhFrameHdr object.
   if (In<ELFT>::EhFrameHdr) {
     for (CieRecord *Cie : Cies) {
       uint8_t Enc = getFdeEncoding<ELFT>(Cie->Piece);
       for (SectionPiece *Fde : Cie->FdePieces) {
         uint64_t Pc = getFdePc(Buf, Fde->OutputOff, Enc);
         uint64_t FdeVA = this->OutSec->Addr + Fde->OutputOff;
         In<ELFT>::EhFrameHdr->addFde(Pc, FdeVA);
       }
     }
   }
 }
 
-GotBaseSection::GotBaseSection()
+GotSection::GotSection()
     : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS,
                        Target->GotEntrySize, ".got") {}
 
-void GotBaseSection::addEntry(SymbolBody &Sym) {
+void GotSection::addEntry(SymbolBody &Sym) {
   Sym.GotIndex = NumEntries;
   ++NumEntries;
 }
 
-bool GotBaseSection::addDynTlsEntry(SymbolBody &Sym) {
+bool GotSection::addDynTlsEntry(SymbolBody &Sym) {
   if (Sym.GlobalDynIndex != -1U)
     return false;
   Sym.GlobalDynIndex = NumEntries;
   // Global Dynamic TLS entries take two GOT slots.
   NumEntries += 2;
   return true;
 }
 
 // Reserves TLS entries for a TLS module ID and a TLS block offset.
 // In total it takes two GOT slots.
-bool GotBaseSection::addTlsIndex() {
+bool GotSection::addTlsIndex() {
   if (TlsIndexOff != uint32_t(-1))
     return false;
   TlsIndexOff = NumEntries * Config->Wordsize;
   NumEntries += 2;
   return true;
 }
 
-uint64_t GotBaseSection::getGlobalDynAddr(const SymbolBody &B) const {
+uint64_t GotSection::getGlobalDynAddr(const SymbolBody &B) const {
   return this->getVA() + B.GlobalDynIndex * Config->Wordsize;
 }
 
-uint64_t GotBaseSection::getGlobalDynOffset(const SymbolBody &B) const {
+uint64_t GotSection::getGlobalDynOffset(const SymbolBody &B) const {
   return B.GlobalDynIndex * Config->Wordsize;
 }
 
-void GotBaseSection::finalizeContents() {
-  Size = NumEntries * Config->Wordsize;
-}
+void GotSection::finalizeContents() { Size = NumEntries * Config->Wordsize; }
 
-bool GotBaseSection::empty() const {
+bool GotSection::empty() const {
   // If we have a relocation that is relative to GOT (such as GOTOFFREL),
   // we need to emit a GOT even if it's empty.
   return NumEntries == 0 && !HasGotOffRel;
 }
 
-template <class ELFT> void GotSection<ELFT>::writeTo(uint8_t *Buf) {
-  this->template relocate<ELFT>(Buf, Buf + Size);
-}
+void GotSection::writeTo(uint8_t *Buf) { relocateAlloc(Buf, Buf + Size); }
 
 MipsGotSection::MipsGotSection()
     : SyntheticSection(SHF_ALLOC | SHF_WRITE | SHF_MIPS_GPREL, SHT_PROGBITS, 16,
                        ".got") {}
 
 void MipsGotSection::addEntry(SymbolBody &Sym, int64_t Addend, RelExpr Expr) {
   // For "true" local symbols which can be referenced from the same module
   // only compiler creates two instructions for address loading:

 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;
+GotSection *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;

 template class elf::MipsOptionsSection<ELF64LE>;
 template class elf::MipsOptionsSection<ELF64BE>;
 
 template class elf::MipsReginfoSection<ELF32LE>;
 template class elf::MipsReginfoSection<ELF32BE>;
 template class elf::MipsReginfoSection<ELF64LE>;
 template class elf::MipsReginfoSection<ELF64BE>;
 
-template class elf::GotSection<ELF32LE>;
-template class elf::GotSection<ELF32BE>;
-template class elf::GotSection<ELF64LE>;
-template class elf::GotSection<ELF64BE>;
-
 template class elf::DynamicSection<ELF32LE>;
 template class elf::DynamicSection<ELF32BE>;
 template class elf::DynamicSection<ELF64LE>;
 template class elf::DynamicSection<ELF64BE>;
 
 template class elf::RelocationSection<ELF32LE>;
 template class elf::RelocationSection<ELF32BE>;
 template class elf::RelocationSection<ELF64LE>;

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
   }
 
   // Add .got. MIPS' .got is so different from the other archs,
   // it has its own class.
   if (Config->EMachine == EM_MIPS) {
     InX::MipsGot = make<MipsGotSection>();
     Add(InX::MipsGot);
   } else {
-    InX::Got = make<GotSection<ELFT>>();
+    InX::Got = make<GotSection>();
     Add(InX::Got);
   }
 
   InX::GotPlt = make<GotPltSection>();
   Add(InX::GotPlt);
   InX::IgotPlt = make<IgotPltSection>();
   Add(InX::IgotPlt);