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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

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



================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:1044
 public:
   DWARFObjInMemory(const StringMap<std::unique_ptr<MemoryBuffer>> &Sections,
                    uint8_t AddrSize, bool IsLittleEndian)
----------------
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)


================
Comment at: lib/DebugInfo/DWARF/DWARFContext.cpp:1194-1200
+    StringRef Ret;
+    if (Obj) {
+      auto SecI = Obj->section_begin();
+      std::advance(SecI, Index);
+      (*SecI).getName(Ret);
+    }
+    return Ret;
----------------
Maybe invert this for an early return:

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


================
Comment at: lib/DebugInfo/DWARF/DWARFDie.cpp:64-67
+    std::string NameIndex = "N/A";
+    if (Range.SectionIndex != -1ULL)
+      NameIndex = Obj.getSectionName(Range.SectionIndex).str() + "(" +
+                  std::to_string(Range.SectionIndex) + ")";
----------------
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?


================
Comment at: test/DebugInfo/X86/dwarfdump-ranges-unrelocated.s:7-8
+# CHECK: DW_AT_ranges [DW_FORM_sec_offset] (0x00000000
+# CHECK-NEXT:  [0x0000000000000000 - 0x0000000000000001) .text.foo1(3)
+# CHECK-NEXT:  [0x0000000000000000 - 0x0000000000000002) .text.foo2(4))
+
----------------
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.


================
Comment at: test/DebugInfo/dwarfdump-ranges.test:7-8
 CHECK:  DW_AT_ranges [DW_FORM_data4]      (0x00000000
-CHECK-NEXT:          [0x000000000000062c - 0x0000000000000637)
-CHECK-NEXT:          [0x0000000000000637 - 0x000000000000063d))
+CHECK-NEXT:          [0x000000000000062c - 0x0000000000000637) N/A
+CHECK-NEXT:          [0x0000000000000637 - 0x000000000000063d) N/A)
 
----------------
Why does this print N/A in this test case?


https://reviews.llvm.org/D36313