[llvm-dwarfdump] - Print section name and index when dumping .debug_info ranges

[grimar]

Summary

Teaches llvm-dwarfdump to print section index and name of range
when it dumps .debug_info.

[aprantl]

lib/DebugInfo/DWARF/DWARFDie.cpp

64 ↗

(On Diff #109741)

I think I would prefer an empty string instead of "N/A".

[dblaikie]

Thanks a bunch for working on this - I reckon it's a great direction!

lib/DebugInfo/DWARF/DWARFContext.cpp

1044 ↗

(On Diff #109741)

If/how do section indexes work in this codepath? Are they provided & printed but without names (because the section itself cannot be looked up)? Is there a way to support that? (probably not necessary, but consistency is nice where it's possible)

1194–1200 ↗

(On Diff #109741)

Maybe invert this for an early return:

if (!Obj)
  return "";
return std::next(Obj->sections(), Index)->getName(Ret);

lib/DebugInfo/DWARF/DWARFDie.cpp

64–67 ↗

(On Diff #109741)

Rather than creating a temporary string, maybe move this to after the format line, and print it directly to the stream with a format call as well?

test/DebugInfo/X86/dwarfdump-ranges-unrelocated.s

7–8 ↗

(On Diff #109741)

It'd be nice to columnize the section number - but maybe that's too much work? (scanning through to find the longest section name, etc)

Also, similar amount of work: Do you think it'd be worth it/good to omit the section number if the section names are not ambiguous in this list? (or perhaps necessary to only do this if the name is not ambiguous in the whole file (so you on't look at the ranges for two functions and see them describing what appears to be the same section))

& what about omitting the name or putting it somewhere else (like at the start on a separate line) if every entry is in the same section? (which will be the case for all ranges except the compile_unit ranges, most likely)

Maybe that's all overkill - wouldn't mind getting Adrian's take on this, as he was driving a bunch of the dwarfdump improvements like this inline range dumping.

test/DebugInfo/dwarfdump-ranges.test

7–8 ↗

(On Diff #109741)

Why does this print N/A in this test case?

[grimar]

• Addressed review comments.

lib/DebugInfo/DWARF/DWARFContext.cpp

1044 ↗

(On Diff #109741)

Currently I think only 2 tools use DIContext::dump() call: llvm-dwarfdump and llvm-objdump,
both are creating DWARFObjInMemory using another codepatch, where ObjectFile is provided.
If it is not, then getSectionName() (introduced in this patch) would just return empty section name
for any index, because it checks Obj member and takes section index from it.

But that is not so simple to implement because currently we take SectionIndex from relocations and
this constructor assumes to take relocated data. That mean we have no section index to take from.

Fortunately tools do not need it. I would leave this place as is.

1194–1200 ↗

(On Diff #109741)

Done.

lib/DebugInfo/DWARF/DWARFDie.cpp

64 ↗

(On Diff #109741)

Done.

64–67 ↗

(On Diff #109741)

Done.

test/DebugInfo/X86/dwarfdump-ranges-unrelocated.s

7–8 ↗

(On Diff #109741)

I would not scan over all sections of file, because it looks too much. Scanning over all sections can be slow.
I would not omit something dependent on third conditions too, because it makes logic of output unclear for
people that are not familar with tool. ("Why it prints section index for "foo" and does not for "bar" ? BUUUG !")

Also I think often I am looking for section number in readelf/objdump/other tools, so I would not omit
it to be able to find it as fast as possible without manual eye-mapping name to index by myself.

Basing on above I think I like "omitting the name or putting it somewhere else (like at the start on a separate line) if every entry is in the same section" as it avoids printing the same name+index multiple times.
It also removes need to columnize anything probably and thus scan through to find longest index/section name, what should leave code simple.

What do you think about new version of output ? (I am wondering if we still want to columnize the section index, I am not sure).

test/DebugInfo/dwarfdump-ranges.test

7–8 ↗

(On Diff #109741)

dwarfdump-test4.elf-x86-64 is not an object but DSO. It does not have .rel[a] sections
(targeted to debug sections I mean), and so all RelocAddrMaps are empty and we can not
take SectionIndex from relocation.

[grimar]

Changed inplementation.

• Do not print section/index when -brief=true
• Do not print section index when name is unique.

I am not sure last one worth all the code complication it requires,
I would probably just pring section index always. Though
such approach has its own benefits, so I think I am fine with it.

[grimar]

• Minor update.

[dblaikie]

Looks OK to me - though please refactor the section name/uniqueness gathering so it's done once (probably lazily) rather than every time a range is dumped.

lib/DebugInfo/DWARF/DWARFDie.cpp

60–71 ↗

(On Diff #110339)

Presumably this shouldn't be computed for each range that's dumped?

Maybe a lazy accessor on DWARFContext that populates the list the first time it's called?

[grimar]

In D36313#841530, @dblaikie wrote:

Looks OK to me - though please refactor the section name/uniqueness gathering so it's done once (probably lazily) rather than every time a range is dumped.

Thanks, David ! I'll post refactoring patch soon.

llvm/trunk/include/llvm/DebugInfo/DWARF/DWARFObject.h

 // implementation is eager and will call some of these methods even if the
 // result is not used.
 class DWARFObject {
   DWARFSection Dummy;
 
 public:
   virtual ~DWARFObject() = default;
   virtual StringRef getFileName() const { llvm_unreachable("unimplemented"); }
+  virtual const object::ObjectFile *getFile() const { return nullptr; }
   virtual bool isLittleEndian() const = 0;
   virtual uint8_t getAddressSize() const { llvm_unreachable("unimplemented"); }
   virtual const DWARFSection &getInfoSection() const { return Dummy; }
   virtual void
   forEachTypesSections(function_ref<void(const DWARFSection &)> F) const {}
   virtual StringRef getAbbrevSection() const { return ""; }
   virtual const DWARFSection &getLocSection() const { return Dummy; }
   virtual StringRef getARangeSection() const { return ""; }

llvm/trunk/lib/DebugInfo/DWARF/DWARFContext.cpp

struct DWARFSectionMap final : public DWARFSection {
   RelocAddrMap Relocs;
 };
 } // namespace
 
 class DWARFObjInMemory final : public DWARFObject {
   bool IsLittleEndian;
   uint8_t AddressSize;
   StringRef FileName;
+  const object::ObjectFile *Obj = nullptr;
 
   using TypeSectionMap = MapVector<object::SectionRef, DWARFSectionMap,
                                    std::map<object::SectionRef, unsigned>>;
 
   TypeSectionMap TypesSections;
   TypeSectionMap TypesDWOSections;
 
   DWARFSectionMap InfoSection;
DWARFObjInMemory(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
     for (const auto &SecIt : Sections) {
       if (StringRef *SectionData = mapSectionToMember(SecIt.first()))
         *SectionData = SecIt.second->getBuffer();
     }
   }
   DWARFObjInMemory(const object::ObjectFile &Obj, const LoadedObjectInfo *L,
                    function_ref<ErrorPolicy(Error)> HandleError)
       : IsLittleEndian(Obj.isLittleEndian()),
-        AddressSize(Obj.getBytesInAddress()), FileName(Obj.getFileName()) {
+        AddressSize(Obj.getBytesInAddress()), FileName(Obj.getFileName()),
+        Obj(&Obj) {
 
     for (const SectionRef &Section : Obj.sections()) {
       StringRef Name;
       Section.getName(Name);
       // Skip BSS and Virtual sections, they aren't interesting.
       if (Section.isBSS() || Section.isVirtual())
         continue;
 
Optional<RelocAddrEntry> find(const DWARFSection &S,
                                 uint64_t Pos) const override {
     auto &Sec = static_cast<const DWARFSectionMap &>(S);
     RelocAddrMap::const_iterator AI = Sec.Relocs.find(Pos);
     if (AI == Sec.Relocs.end())
       return None;
     return AI->second;
   }
 
+  const object::ObjectFile *getFile() const override { return Obj; }
+
   bool isLittleEndian() const override { return IsLittleEndian; }
   StringRef getAbbrevDWOSection() const override { return AbbrevDWOSection; }
   const DWARFSection &getLineDWOSection() const override {
     return LineDWOSection;
   }
   const DWARFSection &getLocDWOSection() const override {
     return LocDWOSection;
   }

llvm/trunk/lib/DebugInfo/DWARF/DWARFDebugRangeList.cpp

bool DWARFDebugRangeList::extract(const DWARFDataExtractor &data,
   clear();
   if (!data.isValidOffset(*offset_ptr))
     return false;
   AddressSize = data.getAddressSize();
   if (AddressSize != 4 && AddressSize != 8)
     return false;
   Offset = *offset_ptr;
   while (true) {
-    RangeListEntry entry;
+    RangeListEntry Entry;
+    Entry.SectionIndex = -1ULL;
+
     uint32_t prev_offset = *offset_ptr;
-    entry.StartAddress =
-        data.getRelocatedAddress(offset_ptr, &entry.SectionIndex);
-    entry.EndAddress = data.getRelocatedAddress(offset_ptr);
+    Entry.StartAddress = data.getRelocatedAddress(offset_ptr);
+    Entry.EndAddress =
+        data.getRelocatedAddress(offset_ptr, &Entry.SectionIndex);
 
     // Check that both values were extracted correctly.
     if (*offset_ptr != prev_offset + 2 * AddressSize) {
       clear();
       return false;
     }
-    if (entry.isEndOfListEntry())
+    if (Entry.isEndOfListEntry())
       break;
-    Entries.push_back(entry);
+    Entries.push_back(Entry);
   }
   return true;
 }
 
 void DWARFDebugRangeList::dump(raw_ostream &OS) const {
   for (const RangeListEntry &RLE : Entries) {
     const char *format_str = (AddressSize == 4
                               ? "%08x %08"  PRIx64 " %08"  PRIx64 "\n"

llvm/trunk/lib/DebugInfo/DWARF/DWARFDie.cpp

 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/DebugInfo/DWARF/DWARFAbbreviationDeclaration.h"
 #include "llvm/DebugInfo/DWARF/DWARFContext.h"
 #include "llvm/DebugInfo/DWARF/DWARFDebugRangeList.h"
 #include "llvm/DebugInfo/DWARF/DWARFFormValue.h"
 #include "llvm/DebugInfo/DWARF/DWARFUnit.h"
+#include "llvm/Object/ObjectFile.h"
 #include "llvm/Support/DataExtractor.h"
 #include "llvm/Support/Format.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cinttypes>
 #include <cstdint>
 #include <string>
 #include <utility>
 
 using namespace llvm;
 using namespace dwarf;
+using namespace object;
 using namespace syntax;
 
 static void dumpApplePropertyAttribute(raw_ostream &OS, uint64_t Val) {
   OS << " (";
   do {
     uint64_t Shift = countTrailingZeros(Val);
     assert(Shift < 64 && "undefined behavior");
     uint64_t Bit = 1ULL << Shift;
     auto PropName = ApplePropertyString(Bit);
     if (!PropName.empty())
       OS << PropName;
     else
       OS << format("DW_APPLE_PROPERTY_0x%" PRIx64, Bit);
     if (!(Val ^= Bit))
       break;
     OS << ", ";
   } while (true);
   OS << ")";
 }
 
-static void dumpRanges(raw_ostream &OS, const DWARFAddressRangesVector& Ranges,
-                       unsigned AddressSize, unsigned Indent) {
-  if (Ranges.empty())
-    return;
-  
-  for (const auto &Range: Ranges) {
+static void dumpRanges(const DWARFObject &Obj, raw_ostream &OS,
+                       const DWARFAddressRangesVector &Ranges,
+                       unsigned AddressSize, unsigned Indent,
+                       const DIDumpOptions &DumpOpts) {
+  StringMap<unsigned> SectionAmountMap;
+  std::vector<StringRef> SectionNames;
+  if (Obj.getFile() && !DumpOpts.Brief) {
+    for (const SectionRef &Section : Obj.getFile()->sections()) {
+      StringRef Name;
+      if (Section.getName(Name))
+        Name = "<error>";
+
+      ++SectionAmountMap[Name];
+      SectionNames.push_back(Name);
+    }
+  }
+
+  for (size_t I = 0; I < Ranges.size(); ++I) {
+    const DWARFAddressRange &R = Ranges[I];
     OS << '\n';
     OS.indent(Indent);
-    OS << format("[0x%0*" PRIx64 " - 0x%0*" PRIx64 ")",
-                 AddressSize*2, Range.LowPC,
-                 AddressSize*2, Range.HighPC);
+    OS << format("[0x%0*" PRIx64 " - 0x%0*" PRIx64 ")", AddressSize * 2,
+                 R.LowPC, AddressSize * 2, R.HighPC);
+
+    if (SectionNames.empty() || R.SectionIndex == -1ULL)
+      continue;
+
+    StringRef Name = R.SectionIndex < SectionNames.size()
+                         ? SectionNames[R.SectionIndex]
+                         : "<error>";
+    OS << format(" \"%s\"", Name.str().c_str());
+
+    // Print section index if there is more than one section with this name.
+    if (SectionAmountMap[Name] > 1)
+      OS << format(" [%u]", R.SectionIndex);
   }
 }
 
 static void dumpAttribute(raw_ostream &OS, const DWARFDie &Die,
                           uint32_t *OffsetPtr, dwarf::Attribute Attr,
                           dwarf::Form Form, unsigned Indent,
                           DIDumpOptions DumpOpts) {
   if (!Die.isValid())
static void dumpAttribute(raw_ostream &OS, const DWARFDie &Die,
   // interesting. These attributes are handled below.
   if (Attr == DW_AT_specification || Attr == DW_AT_abstract_origin) {
     if (const char *Name = Die.getAttributeValueAsReferencedDie(Attr).getName(DINameKind::LinkageName))
         OS << " \"" << Name << '\"';
   } else if (Attr == DW_AT_APPLE_property_attribute) {
     if (Optional<uint64_t> OptVal = formValue.getAsUnsignedConstant())
       dumpApplePropertyAttribute(OS, *OptVal);
   } else if (Attr == DW_AT_ranges) {
-    dumpRanges(OS, Die.getAddressRanges(), U->getAddressByteSize(),
-               sizeof(BaseIndent)+Indent+4);
+    const DWARFObject &Obj = Die.getDwarfUnit()->getContext().getDWARFObj();
+    dumpRanges(Obj, OS, Die.getAddressRanges(), U->getAddressByteSize(),
+               sizeof(BaseIndent) + Indent + 4, DumpOpts);
   }
-  
   OS << ")\n";
 }
 
 bool DWARFDie::isSubprogramDIE() const {
   return getTag() == DW_TAG_subprogram;
 }
 
 bool DWARFDie::isSubroutineDIE() const {

llvm/trunk/test/DebugInfo/X86/dwarfdump-ranges-unrelocated.s

 # RUN: llvm-mc -triple x86_64-pc-linux -filetype=obj %s -o %t
 # RUN: llvm-dwarfdump %t | FileCheck %s
 
+# CHECK: .debug_info contents:
+# CHECK: DW_TAG_compile_unit
+# CHECK: DW_AT_ranges [DW_FORM_sec_offset] (0x00000000
+# CHECK-NEXT:  [0x0000000000000000 - 0x0000000000000001) ".text.foo1"
+# CHECK-NEXT:  [0x0000000000000000 - 0x0000000000000002) ".text.foo2" [4]
+# CHECK-NEXT:  [0x0000000000000000 - 0x0000000000000003) ".text.foo2" [5])
+
 # CHECK: .debug_ranges contents:
 # CHECK:   00000000 0000000000000000 0000000000000001
 # CHECK:   00000000 0000000000000000 0000000000000002
+# CHECK:   00000000 0000000000000000 0000000000000003
 # CHECK:   00000000 <End of list>
 
-## Asm code for testcase is a reduced output from next invocation and source:
+# RUN: llvm-dwarfdump -brief=true %t | FileCheck %s --check-prefix=BRIEF
+# BRIEF: DW_TAG_compile_unit
+# BRIEF: DW_AT_ranges         (0x00000000
+# BRIEF-NEXT:  [0x0000000000000000 - 0x0000000000000001)
+# BRIEF-NEXT:  [0x0000000000000000 - 0x0000000000000002)
+# BRIEF-NEXT:  [0x0000000000000000 - 0x0000000000000003))
+
+## Asm code for testcase is a reduced and modified output from next
+## invocation and source:
 # clang test.cpp -S -o test.s -gmlt -ffunction-sections
 # test.cpp:
 #   void foo1() { }  
 #   void foo2() { }  
 
 .section .text.foo1,"ax",@progbits
 .Lfunc_begin0:
  nop
 .Lfunc_end0:
 
-.section .text.foo2,"ax",@progbits
+.section .text.foo2,"ax",@progbits, unique, 1
 .Lfunc_begin1:
  nop
  nop
 .Lfunc_end1:
 
+.section .text.foo2,"ax",@progbits, unique, 2
+.Lfunc_begin2:
+ nop
+ nop
+ nop
+.Lfunc_end2:
+
 .section .debug_abbrev,"",@progbits
 .byte 1                       # Abbreviation Code
 .byte 17                      # DW_TAG_compile_unit
 .byte 0                       # DW_CHILDREN_no
 .byte 37                      # DW_AT_producer
 .byte 14                      # DW_FORM_strp
 .byte 19                      # DW_AT_language
 .byte 5                       # DW_FORM_data2

 .long .Ldebug_ranges0         # DW_AT_ranges
 
 .section .debug_ranges,"",@progbits
 .Ldebug_ranges0:
 .quad .Lfunc_begin0
 .quad .Lfunc_end0
 .quad .Lfunc_begin1
 .quad .Lfunc_end1
+.quad .Lfunc_begin2
+.quad .Lfunc_end2
 .quad 0
 .quad 0