dwarfdump --statistics: Use new location list api

[labath]

Summary

This patch removes manual location list handling in the statistics code
and replaces it with the new DWARFDie api, which provides access to a
"cooked" location list. This has the following effects:

• the code now properly handles split-dwarf location lists
• it will automatically support dwarf5 location lists once support for those is added
• it properly handles location lists with base address selection entries
• it fixes a bug where the location list code was using the first DW_AT_ranges range as a "base address" of the compile unit (it should have used DW_AT_low_pc instead. The effect of this was that the computation of the start address of a variable in its scope was broken for these kinds of compile units. This only manifested itself on linked files, since in object files the first DW_AT_ranges range normally starts at 0.

Since pretty much every kind of location list was broken in some way,
it's hard to verify that the new implementation is correct -- the output
will be different in all non-trivial cases, and mostly with good reason.

Most of the existing statistics tests continue to pass though, and a
visual inspection of the statistics for non-trivial inputs shows that
the data is more "reasonable" now. I have updated the "dwo statistics"
test to include the new numbers, as the previous ones were completely
bogus, and I have added a targeted test for the "base address" bug.

[aprantl]

Thanks!

llvm/tools/llvm-dwarfdump/Statistics.cpp

247

if (!Loc) {
  consumeError()
  return;
}

llvm/test/tools/llvm-dwarfdump/X86/statistics-base-address.s

+# This tests the computation of the scope bytes covered by local variables. In
+# particular, the case when the variable starts in the middle of the enclosing
+# scope, and the compile unit has both DW_AT_ranges and DW_AT_low_pc attributes.
+
+# RUN: llvm-mc -triple x86_64-pc-linux %s -filetype=obj -o %t
+# RUN: llvm-dwarfdump --statistics %t | FileCheck %s
+
+# CHECK: "vars scope bytes total":8
+# CHECK: "vars scope bytes covered":8
+
+        .text
+
+# Add padding to ensure the function does not start at address zero.
+        .zero 256
+
+f:                                      # @f
+.Lf_begin:
+        .zero 4
+.Lx_begin:
+        .zero 8
+.Lf_end:
+
+        .section        .debug_ranges,"",@progbits
+.Ldebug_ranges:
+        .quad   .Lf_begin
+        .quad   .Lf_end
+        .quad   0
+        .quad   0
+
+        .section        .debug_loc,"",@progbits
+.Ldebug_loc:
+        .quad   .Lx_begin
+        .quad   .Lf_end
+        .short  1                       # Loc expr size
+        .byte   85                      # super-register DW_OP_reg5
+        .quad   0
+        .quad   0
+
+
+        .section        .debug_abbrev,"",@progbits
+        .byte   1                       # Abbreviation Code
+        .byte   17                      # DW_TAG_compile_unit
+        .byte   1                       # DW_CHILDREN_yes
+        .byte   37                      # DW_AT_producer
+        .byte   8                       # DW_FORM_string
+        .byte   17                      # DW_AT_low_pc
+        .byte   1                       # DW_FORM_addr
+        .byte   85                      # DW_AT_ranges
+        .byte   23                      # DW_FORM_sec_offset
+        .byte   0                       # EOM(1)
+        .byte   0                       # EOM(2)
+        .byte   2                       # Abbreviation Code
+        .byte   46                      # DW_TAG_subprogram
+        .byte   1                       # DW_CHILDREN_yes
+        .byte   17                      # DW_AT_low_pc
+        .byte   1                       # DW_FORM_addr
+        .byte   18                      # DW_AT_high_pc
+        .byte   6                       # DW_FORM_data4
+        .byte   3                       # DW_AT_name
+        .byte   8                       # DW_FORM_string
+        .byte   0                       # EOM(1)
+        .byte   0                       # EOM(2)
+        .byte   3                       # Abbreviation Code
+        .byte   52                      # DW_TAG_variable
+        .byte   0                       # DW_CHILDREN_no
+        .byte   2                       # DW_AT_location
+        .byte   23                      # DW_FORM_sec_offset
+        .byte   3                       # DW_AT_name
+        .byte   8                       # DW_FORM_string
+        .byte   73                      # DW_AT_type
+        .byte   19                      # DW_FORM_ref4
+        .byte   0                       # EOM(1)
+        .byte   0                       # EOM(2)
+        .byte   5                       # Abbreviation Code
+        .byte   36                      # DW_TAG_base_type
+        .byte   0                       # DW_CHILDREN_no
+        .byte   3                       # DW_AT_name
+        .byte   8                       # DW_FORM_string
+        .byte   62                      # DW_AT_encoding
+        .byte   11                      # DW_FORM_data1
+        .byte   11                      # DW_AT_byte_size
+        .byte   11                      # DW_FORM_data1
+        .byte   0                       # EOM(1)
+        .byte   0                       # EOM(2)
+        .byte   0                       # EOM(3)
+
+        .section        .debug_info,"",@progbits
+.Lcu_begin:
+        .long   .Ldebug_info_end-.Ldebug_info_start # Length of Unit
+.Ldebug_info_start:
+        .short  4                       # DWARF version number
+        .long   .debug_abbrev           # Offset Into Abbrev. Section
+        .byte   8                       # Address Size (in bytes)
+        .byte   1                       # Abbrev [1] 0xb:0x64 DW_TAG_compile_unit
+        .asciz  "Hand-written DWARF"    # DW_AT_producer
+        .quad   0                       # DW_AT_low_pc
+        .long   .Ldebug_ranges          # DW_AT_ranges
+        .byte   2                       # Abbrev [2] 0x2a:0x28 DW_TAG_subprogram
+        .quad   .Lf_begin               # DW_AT_low_pc
+        .long   .Lf_end-.Lf_begin       # DW_AT_high_pc
+        .asciz  "f"                     # DW_AT_name
+        .byte   3                       # Abbrev [3] 0x43:0xe DW_TAG_variable
+        .long   .Ldebug_loc             # DW_AT_location
+        .asciz  "x"                     # DW_AT_name
+        .long   .Lint                   # DW_AT_type
+        .byte   0                       # End Of Children Mark
+.Lint:
+        .byte   5                       # Abbrev [5] 0x67:0x7 DW_TAG_base_type
+        .asciz  "int"                   # DW_AT_name
+        .byte   5                       # DW_AT_encoding
+        .byte   4                       # DW_AT_byte_size
+        .byte   0                       # End Of Children Mark
+.Ldebug_info_end:

llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test

 CHECK: "inlined functions":7
 CHECK: "inlined funcs with abstract origins":7
 CHECK: "unique source variables":9
 CHECK: "source variables":30
 
 # Ideally the value below would be 33 but currently it's not.
 CHECK: "variables with location":22
 CHECK: "call site entries":7
-CHECK: "scope bytes total":2817
-CHECK: "scope bytes covered":506
+CHECK: "scope bytes total":2702
+CHECK: "scope bytes covered":1160
 CHECK: "total function size":594
 CHECK: "total inlined function size":345
 CHECK: "total formal params":12
 CHECK: "formal params with source location":12
 CHECK: "formal params with type":12
 CHECK: "formal params with binary location":12
 CHECK: "total vars":18
 CHECK: "vars with source location":18
 CHECK: "vars with type":18
 
 # Ideally the value below would be 18, but currently it's not.
 CHECK: "vars with binary location":10

llvm/tools/llvm-dwarfdump/Statistics.cpp

static void collectLocStats(uint64_t BytesCovered, uint64_t BytesInScope,
   VarParamLocStats[CoverageBucket]++;
   if (IsParam)
     ParamLocStats[CoverageBucket]++;
   else if (IsLocalVar)
     VarLocStats[CoverageBucket]++;
 }
 
 /// Collect debug info quality metrics for one DIE.
-static void collectStatsForDie(DWARFDie Die, uint64_t UnitLowPC, std::string FnPrefix,
+static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
                                std::string VarPrefix, uint64_t ScopeLowPC,
                                uint64_t BytesInScope, uint32_t InlineDepth,
                                StringMap<PerFunctionStats> &FnStatMap,
                                GlobalStats &GlobalStats,
                                LocationStats &LocStats) {
   bool HasLoc = false;
   bool HasSrcLoc = false;
   bool HasType = false;
if (Die.find(dwarf::DW_AT_const_value)) {
     HasLoc = true;
     BytesCovered = BytesInScope;
   } else {
     if (Die.getTag() == dwarf::DW_TAG_member) {
       // Non-const member.
       return;
     }
     // Handle variables and function arguments.
-    auto FormValue = Die.find(dwarf::DW_AT_location);
-    HasLoc = FormValue.hasValue();
-    if (HasLoc) {
+    if (Expected<std::vector<DWARFLocationExpression>> Loc =
+            Die.getLocations(dwarf::DW_AT_location)) {
+      HasLoc = true;
       // Get PC coverage.
-      if (auto DebugLocOffset = FormValue->getAsSectionOffset()) {
-        auto *DebugLoc = Die.getDwarfUnit()->getContext().getDebugLoc();
-        // TODO: This code does not handle DWARF5 nor DWARF4 base address
-        // selection entries. This should use a higher-level API which abstracts
-        // these away.
-        if (auto List = DebugLoc->getLocationListAtOffset(*DebugLocOffset)) {
-          ArrayRef<DWARFLocationEntry> Entries = List->Entries;
-          // Ignore end-of-list entries
-          Entries = Entries.drop_back();
-
-          for (auto Entry : Entries) {
-            uint64_t BytesEntryCovered = Entry.Value1 - Entry.Value0;
-            BytesCovered += BytesEntryCovered;
-            if (IsEntryValue(Entry.Loc))
-              BytesEntryValuesCovered += BytesEntryCovered;
-          }
-          if (Entries.size()) {
-            uint64_t FirstDef = Entries[0].Value0;
-            uint64_t UnitOfs = UnitLowPC; 
-            // Ranges sometimes start before the lexical scope.
-            if (UnitOfs + FirstDef >= ScopeLowPC)
-              OffsetToFirstDefinition = UnitOfs + FirstDef - ScopeLowPC;
-            // Or even after it. Count that as a failure.
-            if (OffsetToFirstDefinition > BytesInScope)
-              OffsetToFirstDefinition = 0;
-          }
-        }
+      auto Default = find_if(
+          *Loc, [](const DWARFLocationExpression &L) { return !L.Range; });
+      if (Default != Loc->end()) {
+        // Assume the entire range is covered by a single location.
+        BytesCovered = BytesInScope;
+      } else {
+        for (auto Entry : *Loc) {
+          uint64_t BytesEntryCovered = Entry.Range->HighPC - Entry.Range->LowPC;
+          if (IsEntryValue(Entry.Expr))
+        if (!Loc->empty()) {
+          uint64_t FirstDef = Loc->front().Range->LowPC;
+          if (FirstDef >= ScopeLowPC)
+            OffsetToFirstDefinition = FirstDef - ScopeLowPC;
         assert(BytesInScope);
-      } else {
-        // Assume the entire range is covered by a single location.
-        BytesCovered = BytesInScope;
-      }
       }
+    } else
+      consumeError(Loc.takeError());
   }
 
   // Calculate the debug location statistics.
   if (BytesInScope) {
     LocStats.NumVarParam++;
     if (IsParam)
       LocStats.NumParam++;
     else if (IsLocalVar)
if (IsParam) {
         FnStats.NumVarSourceLocations++;
       if (HasLoc)
         FnStats.NumVarLocations++;
     }
   }
 }
 
 /// Recursively collect debug info quality metrics.
-static void collectStatsRecursive(DWARFDie Die, uint64_t UnitLowPC, std::string FnPrefix,
+static void collectStatsRecursive(DWARFDie Die, std::string FnPrefix,
                                   std::string VarPrefix, uint64_t ScopeLowPC,
                                   uint64_t BytesInScope, uint32_t InlineDepth,
                                   StringMap<PerFunctionStats> &FnStatMap,
                                   GlobalStats &GlobalStats,
                                   LocationStats &LocStats) {
   // Handle any kind of lexical scope.
   const dwarf::Tag Tag = Die.getTag();
   const bool IsFunction = Tag == dwarf::DW_TAG_subprogram;
if (BytesInThisScope) {
       BytesInScope = BytesInThisScope;
       if (IsFunction)
         GlobalStats.FunctionSize += BytesInThisScope;
       else if (IsInlinedFunction && InlineDepth == 0)
         GlobalStats.InlineFunctionSize += BytesInThisScope;
     }
   } else {
     // Not a scope, visit the Die itself. It could be a variable.
-    collectStatsForDie(Die, UnitLowPC, FnPrefix, VarPrefix, ScopeLowPC, BytesInScope,
+    collectStatsForDie(Die, FnPrefix, VarPrefix, ScopeLowPC, BytesInScope,
                        InlineDepth, FnStatMap, GlobalStats, LocStats);
   }
 
   // Set InlineDepth correctly for child recursion
   if (IsFunction)
     InlineDepth = 0;
   else if (IsInlinedFunction)
     ++InlineDepth;
 
   // Traverse children.
   unsigned LexicalBlockIndex = 0;
   DWARFDie Child = Die.getFirstChild();
   while (Child) {
     std::string ChildVarPrefix = VarPrefix;
     if (Child.getTag() == dwarf::DW_TAG_lexical_block)
       ChildVarPrefix += toHex(LexicalBlockIndex++) + '.';
 
-    collectStatsRecursive(Child, UnitLowPC, FnPrefix, ChildVarPrefix, ScopeLowPC,
+    collectStatsRecursive(Child, FnPrefix, ChildVarPrefix, ScopeLowPC,
                           BytesInScope, InlineDepth, FnStatMap, GlobalStats,
                           LocStats);
     Child = Child.getSibling();
   }
 }
 
 /// Print machine-readable output.
 /// The machine-readable format is single-line JSON output.

 bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
                                Twine Filename, raw_ostream &OS) {
   StringRef FormatName = Obj.getFileFormatName();
   GlobalStats GlobalStats;
   LocationStats LocStats;
   StringMap<PerFunctionStats> Statistics;
   for (const auto &CU : static_cast<DWARFContext *>(&DICtx)->compile_units())
     if (DWARFDie CUDie = CU->getNonSkeletonUnitDIE(false))
-      collectStatsRecursive(CUDie, getLowPC(CUDie), "/", "g", 0, 0, 0,
-                            Statistics, GlobalStats, LocStats);
+      collectStatsRecursive(CUDie, "/", "g", 0, 0, 0, Statistics, GlobalStats,
+                            LocStats);
 
   /// The version number should be increased every time the algorithm is changed
   /// (including bug fixes). New metrics may be added without increasing the
   /// version.
   unsigned Version = 3;
   unsigned VarParamTotal = 0;
   unsigned VarParamUnique = 0;
   unsigned VarParamWithLoc = 0;