Speed up gdb index creation

[• rafael]

Summary

This depends on https://reviews.llvm.org/D35373

With that in place we can use lld's own infrastructure for the low level detail of dwarf parsing.

With this we don't decompress sections twice, we don't scan all realocations and even with this simplistic implementation linking clang with gdb index goes from 34.09 seconds to 20.80 seconds.

[ruiu]

ELF/GdbIndex.cpp

56

auto -> LLDDWARFSection

60

Is this safe?

65

auto -> StringRef

69

Is returning a nullptr safe?

I think writing without StringSwitch is more straightforward:

if (Sec->Name == ".debug_abbrev")
  AbbrevSection = toStringRef...;
else if (Sec->Name == ".debug_gnu_pubnames")
  GnuPubNameSection = toStringRef...;
else if ...

74

I'd write a function comment.

ELF/GdbIndex.h

22–26

Are these classes designed to be inherited? I prefer delegation over inheritance and am wondering if this is a good approach.

41

explicit?

[• rafael]

Address review comments.

[ruiu]

LGTM

ELF/GdbIndex.cpp

56–64

This may look a bit dumb, but I think I prefer this style over StringSwitch:

if (Sec->Name == ".debug_info") {
  InfoSection->Data = toStringRef(Sec->Data);
  InfoSection->Sec = Sec;
} else if (Sec->Name == ".debug_ranges") {
  RangeSection->Data = toStringRef(Sec->Data);
  RangeSection->Sec = Sec;
} else if ...

ELF/InputFiles.cpp

60–61

DO sounds like a verb. How about DOjb or just Obj?

[grimar]

Looks great !

[• rafael]

Change variable name and rebase.

I have kept the StringSwitch since it looks odd to duplicate a multiline if body.

[ruiu]

Is that that odd? StringSwitch seems a bit too clever to my taste, but I'm fine in either way. LGTM

[thakis]

r308544

ELF/GdbIndex.h

 
 class InputSection;
 
+struct LLDDWARFSection final : public llvm::DWARFSection {
+  InputSectionBase *Sec = nullptr;
+};
+
+template <class ELFT> class LLDDwarfObj final : public llvm::DWARFObj {
+  elf::ObjectFile<ELFT> *Obj;
+  LLDDWARFSection InfoSection;
+  LLDDWARFSection RangeSection;
+  LLDDWARFSection LineSection;
+  StringRef AbbrevSection;
+  StringRef GnuPubNamesSection;
+  StringRef GnuPubTypesSection;
+
+  template <class RelTy>
+  llvm::Optional<llvm::RelocAddrEntry> findAux(const InputSectionBase &Sec,
+                                               uint64_t Pos,
+                                               ArrayRef<RelTy> Rels) const;
+
+public:
+  explicit LLDDwarfObj(elf::ObjectFile<ELFT> *Obj);
+  const llvm::DWARFSection &getInfoSection() const override {
+    return InfoSection;
+  }
+  const llvm::DWARFSection &getRangeSection() const override {
+    return RangeSection;
+  }
+  const llvm::DWARFSection &getLineSection() const override {
+    return LineSection;
+  }
+  StringRef getCUIndexSection() const override { return ""; }
+  StringRef getAbbrevSection() const override { return AbbrevSection; }
+  StringRef getStringSection() const override { return ""; }
+  StringRef getGnuPubNamesSection() const override {
+    return GnuPubNamesSection;
+  }
+  StringRef getGnuPubTypesSection() const override {
+    return GnuPubTypesSection;
+  }
+  bool isLittleEndian() const override {
+    return ELFT::TargetEndianness == llvm::support::little;
+  }
+  llvm::Optional<llvm::RelocAddrEntry> find(const llvm::DWARFSection &Sec,
+                                            uint64_t Pos) const override;
+};
+
 // Struct represents single entry of address area of gdb index.
 struct AddressEntry {
   InputSection *Section;

ELF/GdbIndex.cpp

     Table[I] = Sym;
   }
 }
+
+template <class ELFT>
+LLDDwarfObj<ELFT>::LLDDwarfObj(elf::ObjectFile<ELFT> *Obj) : Obj(Obj) {
+  for (InputSectionBase *Sec : Obj->getSections()) {
+    if (!Sec)
+      continue;
+    if (LLDDWARFSection *M = StringSwitch<LLDDWARFSection *>(Sec->Name)
+                                 .Case(".debug_info", &InfoSection)
+                                 .Case(".debug_ranges", &RangeSection)
+                                 .Case(".debug_line", &LineSection)
+                                 .Default(nullptr)) {
+      M->Data = toStringRef(Sec->Data);
+      M->Sec = Sec;
+      continue;
+    }
+    if (Sec->Name == ".debug_abbrev")
+      AbbrevSection = toStringRef(Sec->Data);
+    else if (Sec->Name == ".debug_gnu_pubnames")
+      GnuPubNamesSection = toStringRef(Sec->Data);
+    else if (Sec->Name == ".debug_gnu_pubtypes")
+      GnuPubTypesSection = toStringRef(Sec->Data);
+  }
+}
+
+// Find if there is a relocation at Pos in Sec.  The code is a bit
+// more complicated than usual because we need to pass a section index
+// to llvm since it has no idea about InputSection.
+template <class ELFT>
+template <class RelTy>
+Optional<RelocAddrEntry>
+LLDDwarfObj<ELFT>::findAux(const InputSectionBase &Sec, uint64_t Pos,
+                           ArrayRef<RelTy> Rels) const {
+  auto I = llvm::find_if(Rels,
+                         [=](const RelTy &Rel) { return Rel.r_offset == Pos; });
+  if (I == Rels.end())
+    return None;
+  const RelTy &Rel = *I;
+  const elf::ObjectFile<ELFT> *File = Sec.getFile<ELFT>();
+  uint32_t SymIndex = Rel.getSymbol(Config->IsMips64EL);
+  const typename ELFT::Sym &Sym = File->getELFSymbols()[SymIndex];
+  uint32_t SecIndex = File->getSectionIndex(Sym);
+  SymbolBody &B = File->getRelocTargetSym(Rel);
+  auto &DR = cast<DefinedRegular>(B);
+  uint64_t Val = DR.Value + getAddend<ELFT>(Rel);
+
+  // FIXME: We should be consistent about always adding the file
+  // offset or not.
+  if (DR.Section->Flags & ELF::SHF_ALLOC)
+    Val += cast<InputSection>(DR.Section)->getOffsetInFile();
+
+  RelocAddrEntry Ret{SecIndex, Val};
+  return Ret;
+}
+
+template <class ELFT>
+Optional<RelocAddrEntry> LLDDwarfObj<ELFT>::find(const llvm::DWARFSection &S,
+                                                 uint64_t Pos) const {
+  auto &Sec = static_cast<const LLDDWARFSection &>(S);
+  if (Sec.Sec->AreRelocsRela)
+    return findAux(*Sec.Sec, Pos, Sec.Sec->template relas<ELFT>());
+  return findAux(*Sec.Sec, Pos, Sec.Sec->template rels<ELFT>());
+}
+
+template class elf::LLDDwarfObj<ELF32LE>;
+template class elf::LLDDwarfObj<ELF32BE>;
+template class elf::LLDDwarfObj<ELF64LE>;
+template class elf::LLDDwarfObj<ELF64BE>;

ELF/InputFiles.h

   uint32_t getSectionIndex(const Elf_Sym &Sym) const;
 
   Elf_Sym_Range getGlobalSymbols();
+  Elf_Sym_Range getELFSymbols() const { return Symbols; }
 
 protected:
   ArrayRef<Elf_Sym> Symbols;

ELF/InputFiles.cpp

 
 InputFile::InputFile(Kind K, MemoryBufferRef M) : MB(M), FileKind(K) {}
 
-namespace {
-// In ELF object file all section addresses are zero. If we have multiple
-// .text sections (when using -ffunction-section or comdat group) then
-// LLVM DWARF parser will not be able to parse .debug_line correctly, unless
-// we assign each section some unique address. This callback method assigns
-// each section an address equal to its offset in ELF object file.
-class ObjectInfo : public LoadedObjectInfoHelper<ObjectInfo> {
-public:
-  uint64_t getSectionLoadAddress(const object::SectionRef &Sec) const override {
-    return static_cast<const ELFSectionRef &>(Sec).getOffset();
-  }
-};
-}
-
 Optional<MemoryBufferRef> elf::readFile(StringRef Path) {
   log(Path);
   auto MBOrErr = MemoryBuffer::getFile(Path);

 }
 
 template <class ELFT> void elf::ObjectFile<ELFT>::initializeDwarfLine() {
-  std::unique_ptr<object::ObjectFile> Obj =
-      check(object::ObjectFile::createObjectFile(this->MB), toString(this));
-
-  ObjectInfo ObjInfo;
-  DWARFContextInMemory Dwarf(*Obj, &ObjInfo);
+  DWARFContext Dwarf(make_unique<LLDDwarfObj<ELFT>>(this));
+  const DWARFObj &Obj = Dwarf.getDWARFObj();
   DwarfLine.reset(new DWARFDebugLine);
-  DWARFDataExtractor LineData(Dwarf.getLineSection(), Config->IsLE,
+  DWARFDataExtractor LineData(Obj, Obj.getLineSection(), Config->IsLE,
                               Config->Wordsize);
 
   // The second parameter is offset in .debug_line section

   // If that's the case, we want to eliminate .debug_gnu_pub{names,types}
   // because they are redundant and can waste large amount of disk space
   // (for example, they are about 400 MiB in total for a clang debug build.)
+  // We still create the section and mark it dead so that the gdb index code
+  // can use the InputSection to access the data.
   if (Config->GdbIndex &&
-      (Name == ".debug_gnu_pubnames" || Name == ".debug_gnu_pubtypes"))
-    return &InputSection::Discarded;
+      (Name == ".debug_gnu_pubnames" || Name == ".debug_gnu_pubtypes")) {
+    auto *Ret = make<InputSection>(this, &Sec, Name);
+    Script->discard({Ret});
+    return Ret;
+  }
 
   // The linkonce feature is a sort of proto-comdat. Some glibc i386 object
   // files contain definitions of symbol "__x86.get_pc_thunk.bx" in linkonce

ELF/SyntheticSections.cpp

       // Range list with zero size has no effect.
       if (R.LowPC == R.HighPC)
         continue;
-      Ret.push_back({cast<InputSection>(S), R.LowPC, R.HighPC, CurrentCu});
+      auto *IS = cast<InputSection>(S);
+      uint64_t Offset = IS->getOffsetInFile();
+      Ret.push_back({IS, R.LowPC - Offset, R.HighPC - Offset, CurrentCu});
     }
     ++CurrentCu;
   }

 
 static std::vector<NameTypeEntry> readPubNamesAndTypes(DWARFContext &Dwarf,
                                                        bool IsLE) {
-  StringRef Data[] = {Dwarf.getGnuPubNamesSection(),
-                      Dwarf.getGnuPubTypesSection()};
+  StringRef Data[] = {Dwarf.getDWARFObj().getGnuPubNamesSection(),
+                      Dwarf.getDWARFObj().getGnuPubTypesSection()};
 
   std::vector<NameTypeEntry> Ret;
   for (StringRef D : Data) {

   }
 }
 
-static GdbIndexChunk readDwarf(DWARFContextInMemory &Dwarf, InputSection *Sec) {
+static GdbIndexChunk readDwarf(DWARFContext &Dwarf, InputSection *Sec) {
   GdbIndexChunk Ret;
   Ret.DebugInfoSec = Sec;
   Ret.CompilationUnits = readCuList(Dwarf);

 template <class ELFT> GdbIndexSection *elf::createGdbIndex() {
   std::vector<GdbIndexChunk> Chunks;
   for (InputSection *Sec : getDebugInfoSections()) {
-    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;
-    });
+    elf::ObjectFile<ELFT> *F = Sec->getFile<ELFT>();
+    DWARFContext Dwarf(make_unique<LLDDwarfObj<ELFT>>(F));
     Chunks.push_back(readDwarf(Dwarf, Sec));
   }
   return make<GdbIndexSection>(std::move(Chunks));

test/ELF/invalid/invalid-debug-relocations.test

 # RUN: yaml2obj %s -o %t.o
 # RUN: not ld.lld -gdb-index %t.o -o %t.exe 2>&1 | FileCheck %s
 
-# CHECK:      error: {{.*}}.o: error parsing DWARF data:
-# CHECK-NEXT: >>> failed to compute relocation: Unknown, Invalid data was encountered while parsing the file
+# CHECK: error: {{.*}}invalid-debug-relocations.test.tmp.o: unknown relocation type: Unknown (255)
 
 !ELF
 FileHeader: