[ELF] --gdb-index: extract entities from .debug_info when .debug_gnu_pubnames is absent

[MaskRay]

Summary

When .debug_gnu_pubnames is absent, gold traverses .debug_info to
extract entities to make up symbol table entries. This patch implements
a subset of it to improve .gdb_index coverage.

[MaskRay]

.

[dblaikie]

Is this test case consistent with gold's behavior? (given this subprogram has no source locations here, for instance - does gold still consider such a simplified DIE to be a thing that should be indexed?)

Also would probably be good to document the differences/incompleteness - what cases does this not cover? (false negatives, where an index entry should be present but isn't - and false positives (index entries that shouldn't be present) & the details of things like the attributes for the entry?)

[echristo]

ELF/SyntheticSections.cpp

2417

Can you document what the different subset is?

2439

From a quick look: what if DW_AT_sibling isn't there? Does this still work?

[dblaikie]

ELF/SyntheticSections.cpp

2439

Yeah, not a problem, this isn't powered by DW_AT_sibling (see https://github.com/llvm-mirror/llvm/blob/master/lib/DebugInfo/DWARF/DWARFUnit.cpp#L695 ). Probably be nice if we changed this sibling based iteration to iterator-based iteration to avoid confusion like this & separate out DIE navigation from DIEs themselves.

Oh, right, we do have such an API & should probably use that here instead:

for (const DWARFDie &Child : D.children())
  extractSymbol(Child, Idx, Ret);

[MaskRay]

Add comment about the missing parts.

[MaskRay]

Is this test case consistent with gold's behavior?

It does. But I think I need to do more work to understand what gold expects (it may have issues with my other multi-cu tests).

% fllvm-mc -filetype=obj gdb-index-debug-info.s -o t.o
% gold --gdb-index t.o -o t.gold
% fld.lld --gdb-index t.o -o t.gold
% diff <(fllvm-dwarfdump -debug-info t.gold) <(fllvm-dwarfdump -debug-info t.lld)
1c1
< t.gold:       file format ELF64-x86-64
---
> t.lld:        file format ELF64-x86-64

[MaskRay]

Update comment

[ruiu]

ELF/SyntheticSections.cpp

2424

Miss -> Missinug

2426

What is the consequence of the function always returning None?

[MaskRay]

fix typo

[MaskRay]

ELF/SyntheticSections.cpp

2426

Our internal script may break weirdly. I may not sure if this information is useful and I am happy to remove it.

[ruiu]

ELF/SyntheticSections.cpp

2426

I'm not sure if this is a good thing to create a half-baked .gdb-index section for an incomplete input. Silently creating a .gdb-index that is not complete could be very confusing. I feel like if we do this, we should do this perfectly. Or we should report an warning and should refuse to create a .gdb-index section.

Can gold create a complete .gdb-index section from object files that don't have .debug_gnu_pubnames sections? What if both .debug_gnu_pubnames and .debug_gnu_pubtypes are absent? (I believe -ggnu-pubnames creates both sections, so it is hard to imagine that an object file contains only one of them though.)

If gold can still create .gdb-index sections from object files compiled without -ggnu-pubnames, then, what is the point of -ggnu-pubnames option?

[dblaikie]

ELF/SyntheticSections.cpp

2426

Yeah, I tend to agree about half-baking this. Though, equally I don't personally mind if this is done incrementally either (but I'm not sure it makes it that much easier to follow compared to implementing an attempt at the full algorithm up-front).

What's the purpose of gnu-pubnames:

• Linker/debugger (whichever consumes it) performance - when present it's faster to walk the list than to parse all the DWARF to find things
• With Split DWARF it's necessary (to some degree) - the linker can't parse the DWARF (because the linker doesn't see most of the DWARF - it's in the .dwo file/not present in the objects that are being linked) so it must rely on gnu-pubnames to build the gdb-index in that case. (I say necessary to some degree, because in theory the debugger could still go and read all the debug info in the .dwo/.dwps - it'd be insanely slow, but possible - but in fact GDB relies on the presence of the .gdb_index for correctness (When reading split DWARF it doesn't have a fallback to build the index itself from the DWARF))

[saugustine]

ELF/SyntheticSections.cpp

2426

Gold can create a correct index from object files without gnu pubnames, but it is dramatically slower.

ELF/SyntheticSections.cpp

for (DWARFAddressRange &R : Ranges) {
       uint64_t Offset = IS->getOffsetInFile();
       Ret.push_back({IS, R.LowPC - Offset, R.HighPC - Offset, CuIdx});
     }
     ++CuIdx;
   }
   return Ret;
 }
 
+// gold/gdb-index.cc Gdb_index_info_reader::visit_die traverses .debug_info to
+// extract symbols when .debug_gnu_pubnames is absent. This function implements
+// a subset of it to improve coverage of the .gdb_index symbol table.
+static void extractSymbol(DWARFDie D, uint32_t Idx,
+                          std::vector<GdbIndexSection::NameAttrEntry> &Ret) {
+  switch (D.getTag()) {
+  case DW_TAG_constant:
+  case DW_TAG_enumerator:
+  case DW_TAG_subprogram:
+  case DW_TAG_variable: {
+    // Ignore mangled names for now.
+    if (StringRef(D.getName(DINameKind::LinkageName)).startswith("_Z"))
+      break;
+    // As of 2018-11, gold uses 0 as the attributes part. We follow it here.
+    if (const char *Name = D.getName(DINameKind::ShortName))
+      Ret.push_back({{Name, computeGdbHash(Name)}, Idx});
+    break;
+  }
+  default:
+    break;
+  }
+  DWARFDie Child = D.getFirstChild();
+  while (Child) {
+    extractSymbol(Child, Idx, Ret);
+    Child = Child.getSibling();
+  }
+}
+
 template <class ELFT>
 static std::vector<GdbIndexSection::NameAttrEntry>
 readPubNamesAndTypes(const LLDDwarfObj<ELFT> &Obj,
                      const std::vector<GdbIndexSection::CuEntry> &CUs) {
   const DWARFSection &PubNames = Obj.getGnuPubNamesSection();
   const DWARFSection &PubTypes = Obj.getGnuPubTypesSection();
 
   std::vector<GdbIndexSection::NameAttrEntry> Ret;
parallelForEachN(0, Sections.size(), [&](size_t I) {
     DWARFContext Dwarf(make_unique<LLDDwarfObj<ELFT>>(File));
 
     Chunks[I].Sec = Sections[I];
     Chunks[I].CompilationUnits = readCuList(Dwarf);
     Chunks[I].AddressAreas = readAddressAreas(Dwarf, Sections[I]);
     NameAttrs[I] = readPubNamesAndTypes<ELFT>(
         static_cast<const LLDDwarfObj<ELFT> &>(Dwarf.getDWARFObj()),
         Chunks[I].CompilationUnits);
+    if (NameAttrs[I].empty()) {
+      uint32_t Idx = 0;
+      for (const auto &U : Dwarf.info_section_units())
+        if (DWARFDie CUDie = U->getUnitDIE(false))
+          extractSymbol(CUDie, Idx, NameAttrs[Idx]);
+      ++Idx;
+    }
   });
 
   auto *Ret = make<GdbIndexSection>();
   Ret->Chunks = std::move(Chunks);
   Ret->Symbols = createSymbols(NameAttrs, Ret->Chunks);
   Ret->initOutputSize();
   return Ret;
 }

test/ELF/gdb-index-debug-info.s

+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
+# RUN: ld.lld --gdb-index %t.o -o %t
+# RUN: llvm-dwarfdump -gdb-index %t | FileCheck %s
+
+# CHECK: String name: __libc_csu_init
+
+.section .debug_abbrev,"",@progbits
+  .byte 1              # Abbreviation Code
+  .byte 17             # DW_TAG_compile_unit
+  .byte 1              # DW_CHILDREN_yes
+  .byte 0              # EOM(1)
+  .byte 0              # EOM(2)
+  .byte 2              # Abbreviation Code
+  .byte 46             # DW_TAG_subprogram
+  .byte 0              # DW_CHILDREN_no
+  .byte 3              # DW_AT_name
+  .byte 8              # DW_FORM_string
+  .byte 0              # EOM(1)
+  .byte 0              # EOM(2)
+  .byte 0
+
+.section .debug_info,"",@progbits
+.Lcu_begin0:
+  .long .Lcu_end0 - .Lcu_begin0 - 4
+  .short  4              # DWARF version number
+  .long 0              # Offset Into Abbrev. Section
+  .byte 4              # Address Size
+.Ldie:
+  .byte 1              # Abbrev [1] DW_TAG_compile_unit
+  .byte 2              # Abbrev [2] DW_TAG_subprogram
+  .asciz  "__libc_csu_init"  # DW_AT_name
+  .byte 0
+.Lcu_end0: