Refactor gdb index creation

[• rafael]

Summary

I have a patch to let DwarfContext defer to lld for getting section contents and relocations.

That is a pretty big performance improvement.

This is just a refactoring to make that easier to do.

This change makes the *creation* of gdb index a dedicated step and makes that templated. That is so that we can uses Elf_Rel in the code.

The change also removes the pubnames and pubtypes later, so that there is an InputSection for them that we can use.

[• rafael]

With the full patch (not just this refactoring) linking clang with debug index goes from 33.39 seconds to 20.59 seconds.

[grimar]

ELF/GdbIndex.h

48

DebugInfo/DebugInfoSec may be ? Sec name has unclear meaning.

ELF/SyntheticSections.cpp

1763

Since this method now not only returns .debug_info sections, but also
drops .debug_gnu_pub* sections, should it have different name may be ?

1785

Probably you can call

Script->discard({S}};

for .debug_gnu_pub* sections instead of modifying InputSections.

[ruiu]

ELF/InputFiles.cpp

504 ↗

(On Diff #105956)

I think this is cleaner way of eliminating .debug_gnu_{pubnames,pubtypes} than doing it later. Is there any reason to move this code?

ELF/SyntheticSections.h

506

Don't you want to add &&?

[• rafael]

Address review comments.

This is now just about moving the section creation earlier where it can be easily templated.

[• rafael]

Remove leftover changes.

[• rafael]

Upload correct version. Sorry about the noise.

[ruiu]

LGTM

ELF/SyntheticSections.cpp

1811

Looks like you can remove V as it is used only once.

ELF/SyntheticSections.h

539

nit: add a blank line before this line.

[thakis]

(landed in r307867)

ELF/GdbIndex.h

struct NameTypeEntry {
   StringRef Name;
   uint8_t Type;
 };
 
 // We fill one GdbIndexDataChunk for each object where scan of
 // debug information performed. That information futher used
 // for filling gdb index section areas.
 struct GdbIndexChunk {
+  InputSection *DebugInfoSec;
   std::vector<AddressEntry> AddressArea;
   std::vector<CompilationUnitEntry> CompilationUnits;
   std::vector<NameTypeEntry> NamesAndTypes;
 };
 
 // Element of GdbHashTab hash table.
 struct GdbSymbol {
   GdbSymbol(uint32_t Hash, size_t Offset)

ELF/SyntheticSections.h

 class GdbIndexSection final : public SyntheticSection {
   const unsigned OffsetTypeSize = 4;
   const unsigned CuListOffset = 6 * OffsetTypeSize;
   const unsigned CompilationUnitSize = 16;
   const unsigned AddressEntrySize = 16 + OffsetTypeSize;
   const unsigned SymTabEntrySize = 2 * OffsetTypeSize;
 
 public:
-  GdbIndexSection();
+  GdbIndexSection(std::vector<GdbIndexChunk> &&Chunks);
   void finalizeContents() override;
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override;
   bool empty() const override;
 
   // Symbol table is a hash table for types and names.
   // It is the area of gdb index.
   GdbHashTab SymbolTable;
 
   // CU vector is a part of constant pool area of section.
   std::vector<std::set<uint32_t>> CuVectors;
 
   // String pool is also a part of constant pool, it follows CU vectors.
   llvm::StringTableBuilder StringPool;
 
   // Each chunk contains information gathered from a debug sections of single
   // object and used to build different areas of gdb index.
   std::vector<GdbIndexChunk> Chunks;
 
 private:
-  GdbIndexChunk readDwarf(InputSection *Sec);
   void buildIndex();
 
   uint32_t CuTypesOffset;
   uint32_t SymTabOffset;
   uint32_t ConstantPoolOffset;
   uint32_t StringPoolOffset;
 
   size_t CuVectorsSize = 0;
   std::vector<size_t> CuVectorsOffset;
 
   bool Finalized = false;
 };
+template <class ELFT> GdbIndexSection *createGdbIndex();
 
 // --eh-frame-hdr option tells linker to construct a header for all the
 // .eh_frame sections. This header is placed to a section named .eh_frame_hdr
 // and also to a PT_GNU_EH_FRAME segment.
 // At runtime the unwinder then can find all the PT_GNU_EH_FRAME segments by
 // calling dl_iterate_phdr.
 // This section contains a lookup table for quick binary search of FDEs.
 // Detailed info about internals can be found in Ian Lance Taylor's blog:

ELF/SyntheticSections.cpp

for (size_t I = 0; I < Entries.size(); ++I) {
     Off += Target->PltEntrySize;
   }
 }
 
 unsigned PltSection::getPltRelocOff() const {
   return (HeaderSize == 0) ? InX::Plt->getSize() : 0;
 }
 
-GdbIndexSection::GdbIndexSection()
+GdbIndexSection::GdbIndexSection(std::vector<GdbIndexChunk> &&Chunks)
     : SyntheticSection(0, SHT_PROGBITS, 1, ".gdb_index"),
-      StringPool(llvm::StringTableBuilder::ELF) {}
+      StringPool(llvm::StringTableBuilder::ELF), Chunks(std::move(Chunks)) {}
 
 // Iterative hash function for symbol's name is described in .gdb_index format
 // specification. Note that we use one for version 5 to 7 here, it is different
 // for version 4.
 static uint32_t hash(StringRef Str) {
   uint32_t R = 0;
   for (uint8_t C : Str)
     R = R * 67 + tolower(C) - 113;
   return R;
 }
 
-static std::vector<CompilationUnitEntry> readCuList(DWARFContext &Dwarf,
-                                                    InputSection *Sec) {
+static std::vector<CompilationUnitEntry> readCuList(DWARFContext &Dwarf) {
   std::vector<CompilationUnitEntry> Ret;
   for (std::unique_ptr<DWARFCompileUnit> &CU : Dwarf.compile_units())
-    Ret.push_back({Sec->OutSecOff + CU->getOffset(), CU->getLength() + 4});
+    Ret.push_back({CU->getOffset(), CU->getLength() + 4});
   return Ret;
 }
 
 static std::vector<AddressEntry> readAddressArea(DWARFContext &Dwarf,
                                                  InputSection *Sec) {
   std::vector<AddressEntry> Ret;
 
   uint32_t CurrentCu = 0;
for (StringRef D : Data) {
     DWARFDebugPubTable PubTable(D, IsLE, true);
     for (const DWARFDebugPubTable::Set &Set : PubTable.getData())
       for (const DWARFDebugPubTable::Entry &Ent : Set.Entries)
         Ret.push_back({Ent.Name, Ent.Descriptor.toBits()});
   }
   return Ret;
 }
 
 static std::vector<InputSection *> getDebugInfoSections() {
   std::vector<InputSection *> Ret;
   for (InputSectionBase *S : InputSections)
     if (InputSection *IS = dyn_cast<InputSection>(S))
-      if (IS->getParent() && IS->Name == ".debug_info")
+      if (IS->Name == ".debug_info")
         Ret.push_back(IS);
   return Ret;
 }
 
 void GdbIndexSection::buildIndex() {
-  std::vector<InputSection *> V = getDebugInfoSections();
-  if (V.empty())
+  if (Chunks.empty())
     return;
 
-  for (InputSection *Sec : V)
-    Chunks.push_back(readDwarf(Sec));
-
   uint32_t CuId = 0;
   for (GdbIndexChunk &D : Chunks) {
     for (AddressEntry &E : D.AddressArea)
       E.CuIndex += CuId;
 
     // Populate constant pool area.
     for (NameTypeEntry &NameType : D.NamesAndTypes) {
       uint32_t Hash = hash(NameType.Name);
       size_t Offset = StringPool.add(NameType.Name);
 
       bool IsNew;
       GdbSymbol *Sym;
       std::tie(IsNew, Sym) = SymbolTable.add(Hash, Offset);
       if (IsNew) {
         Sym->CuVectorIndex = CuVectors.size();
         CuVectors.resize(CuVectors.size() + 1);
       }
 
       CuVectors[Sym->CuVectorIndex].insert(CuId | (NameType.Type << 24));
     }
 
     CuId += D.CompilationUnits.size();
   }
 }
 
-GdbIndexChunk GdbIndexSection::readDwarf(InputSection *Sec) {
-  Expected<std::unique_ptr<object::ObjectFile>> Obj =
-      object::ObjectFile::createObjectFile(Sec->File->MB);
-  if (!Obj) {
-    error(toString(Sec->File) + ": error creating DWARF context");
-    return {};
-  }
+static GdbIndexChunk readDwarf(DWARFContextInMemory &Dwarf, InputSection *Sec) {
+  GdbIndexChunk Ret;
+  Ret.DebugInfoSec = Sec;
+  Ret.CompilationUnits = readCuList(Dwarf);
+  Ret.AddressArea = readAddressArea(Dwarf, Sec);
+  Ret.NamesAndTypes = readPubNamesAndTypes(Dwarf, Config->IsLE);
+  return Ret;
 
-  DWARFContextInMemory Dwarf(*Obj.get(), nullptr, [&](Error E) {
-    error(toString(Sec->File) + ": error parsing DWARF data:\n>>> " +
+template <class ELFT> GdbIndexSection *elf::createGdbIndex() {
+  std::vector<InputSection *> V = getDebugInfoSections();
+  std::vector<GdbIndexChunk> Chunks;
+  for (InputSection *Sec : V) {
+    InputFile *F = Sec->File;
+    std::error_code EC;
+    ELFObjectFile<ELFT> Obj(F->MB, EC);
+    if (EC)
+      fatal(EC.message());
+    DWARFContextInMemory Dwarf(Obj, nullptr, [&](Error E) {
+      error(toString(F) + ": error parsing DWARF data:\n>>> " +
             toString(std::move(E)));
       return ErrorPolicy::Continue;
     });
-
-  GdbIndexChunk Ret;
-  Ret.CompilationUnits = readCuList(Dwarf, Sec);
-  Ret.AddressArea = readAddressArea(Dwarf, Sec);
-  Ret.NamesAndTypes = readPubNamesAndTypes(Dwarf, Config->IsLE);
-  return Ret;
+    Chunks.push_back(readDwarf(Dwarf, Sec));
+  }
+  return make<GdbIndexSection>(std::move(Chunks));
 }
 
 static size_t getCuSize(std::vector<GdbIndexChunk> &C) {
   size_t Ret = 0;
   for (GdbIndexChunk &D : C)
     Ret += D.CompilationUnits.size();
   return Ret;
 }
void GdbIndexSection::writeTo(uint8_t *Buf) {
   write32le(Buf + 12, CuTypesOffset);      // Address area offset.
   write32le(Buf + 16, SymTabOffset);       // Symbol table offset.
   write32le(Buf + 20, ConstantPoolOffset); // Constant pool offset.
   Buf += 24;
 
   // Write the CU list.
   for (GdbIndexChunk &D : Chunks) {
     for (CompilationUnitEntry &Cu : D.CompilationUnits) {
-      write64le(Buf, Cu.CuOffset);
+      write64le(Buf, D.DebugInfoSec->OutSecOff + Cu.CuOffset);
       write64le(Buf + 8, Cu.CuLength);
       Buf += 16;
     }
   }
 
   // Write the address area.
   for (GdbIndexChunk &D : Chunks) {
     for (AddressEntry &E : D.AddressArea) {

 MipsGotSection *InX::MipsGot;
 MipsRldMapSection *InX::MipsRldMap;
 PltSection *InX::Plt;
 PltSection *InX::Iplt;
 StringTableSection *InX::ShStrTab;
 StringTableSection *InX::StrTab;
 SymbolTableBaseSection *InX::SymTab;
 
+template GdbIndexSection *elf::createGdbIndex<ELF32LE>();
+template GdbIndexSection *elf::createGdbIndex<ELF32BE>();
+template GdbIndexSection *elf::createGdbIndex<ELF64LE>();
+template GdbIndexSection *elf::createGdbIndex<ELF64BE>();
+
 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 InputSection *elf::createCommonSection<ELF64LE>();

ELF/Writer.cpp

template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
   }
 
   InX::GotPlt = make<GotPltSection>();
   Add(InX::GotPlt);
   InX::IgotPlt = make<IgotPltSection>();
   Add(InX::IgotPlt);
 
   if (Config->GdbIndex) {
-    InX::GdbIndex = make<GdbIndexSection>();
+    InX::GdbIndex = createGdbIndex<ELFT>();
     Add(InX::GdbIndex);
   }
 
   // We always need to add rel[a].plt to output if it has entries.
   // Even for static linking it can contain R_[*]_IRELATIVE relocations.
   In<ELFT>::RelaPlt = make<RelocationSection<ELFT>>(
       Config->IsRela ? ".rela.plt" : ".rel.plt", false /*Sort*/);
   Add(In<ELFT>::RelaPlt);