dwarfdump --statistics: Use new location list api

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.

Reviewers: dblaikie, cmtice, vsk

Subscribers: aprantl, SouraVX, JDevlieghere, djtodoro, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D70444

1 files changed, 32 insertions, 38 deletions

diff --git a/llvm/tools/llvm-dwarfdump/Statistics.cpp b/llvm/tools/llvm-dwarfdump/Statistics.cpp
index 8b92aab194d..fa39e76cd4e 100644
--- a/llvm/tools/llvm-dwarfdump/Statistics.cpp
+++ b/llvm/tools/llvm-dwarfdump/Statistics.cpp
@@ -176,7 +176,7 @@ static void collectLocStats(uint64_t BytesCovered, uint64_t BytesInScope,
 }
 
 /// 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,
@@ -243,41 +243,35 @@ static void collectStatsForDie(DWARFDie Die, uint64_t UnitLowPC, std::string FnP
       return;
     }
     // Handle variables and function arguments.
-    auto FormValue = Die.find(dwarf::DW_AT_location);
-    HasLoc = FormValue.hasValue();
-    if (HasLoc) {
+    Expected<std::vector<DWARFLocationExpression>> Loc =
+        Die.getLocations(dwarf::DW_AT_location);
+    if (!Loc) {
+      consumeError(Loc.takeError());
+    } else {
+      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;
-          }
-        }
-        assert(BytesInScope);
-      } else {
+      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;
+          BytesCovered += BytesEntryCovered;
+          if (IsEntryValue(Entry.Expr))
+            BytesEntryValuesCovered += BytesEntryCovered;
+        }
+        if (!Loc->empty()) {
+          uint64_t FirstDef = Loc->front().Range->LowPC;
+          // Ranges sometimes start before the lexical scope.
+          if (FirstDef >= ScopeLowPC)
+            OffsetToFirstDefinition = FirstDef - ScopeLowPC;
+          // Or even after it. Count that as a failure.
+          if (OffsetToFirstDefinition > BytesInScope)
+            OffsetToFirstDefinition = 0;
+        }
+        assert(BytesInScope);
       }
     }
   }
@@ -356,7 +350,7 @@ static void collectStatsForDie(DWARFDie Die, uint64_t UnitLowPC, std::string FnP
 }
 
 /// 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,
@@ -429,7 +423,7 @@ static void collectStatsRecursive(DWARFDie Die, uint64_t UnitLowPC, std::string
     }
   } 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);
   }
 
@@ -447,7 +441,7 @@ static void collectStatsRecursive(DWARFDie Die, uint64_t UnitLowPC, std::string
     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();
@@ -502,8 +496,8 @@ bool collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
   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