[llvm-dwarfdump] Fix misleading scope byte coverage statistics

Fixes PR46575. Bump statistics version to 6. Without this patch, for a variable described with a location list the stat 'sum_all_variables(#bytes in parent scope covered by DW_AT_location)' is calculated by summing all bytes covered by the location ranges in the list and capping the result to the number of bytes in the parent scope. With the patch, only bytes which overlap with the parent DIE scope address ranges contribute to the stat. A new stat 'sum_all_variables(#bytes in any scope covered by DW_AT_location)' has been added which displays the total bytes covered when ignoring scopes.
2020-08-25 06:19:00 +01:00 · 2020-08-25 06:19:00 +01:00 · 3ff4d75c9c
parent 7b64765cd1
commit 3ff4d75c9c
5 changed files with 161 additions and 23 deletions
--- a/llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test
+++ b/llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test
@ -69,7 +69,7 @@ RUN: llvm-dwarfdump --statistics statistics-fib.split-dwarf.o | FileCheck %s
 # }
 #

-CHECK:      "version": 5,
+CHECK:      "version": 6,
 CHECK:      "#functions": 3,
 CHECK:      "#functions with location": 3,
 CHECK:      "#inlined functions": 7,
--- a/llvm/test/tools/llvm-dwarfdump/X86/statistics-v3.test
+++ b/llvm/test/tools/llvm-dwarfdump/X86/statistics-v3.test
@ -64,7 +64,7 @@ RUN: llvm-dwarfdump --statistics %t-statistics-fib.o | FileCheck %s
 # }
 #

-CHECK:      "version": 5,
+CHECK:      "version": 6,
 CHECK:      "#functions": 3,
 CHECK:      "#functions with location": 3,
 CHECK:      "#inlined functions": 8,
--- a/llvm/test/tools/llvm-dwarfdump/X86/statistics.ll
+++ b/llvm/test/tools/llvm-dwarfdump/X86/statistics.ll
@ -1,6 +1,6 @@
 ; RUN: llc -O0 %s -o - -filetype=obj \
 ; RUN:   | llvm-dwarfdump -statistics - | FileCheck %s
-; CHECK: "version": 5,
+; CHECK: "version": 6,

 ; namespace test {
 ;  extern int a;
--- a/llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml
+++ b/llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml
@ -0,0 +1,110 @@
+# RUN: yaml2obj %s | llvm-dwarfdump --statistics - | FileCheck %s
+
+## Check that coverage for variable locations which do not cover the parent
+## scope is tracked separately in "sum_all_variables(#bytes in any scope
+## covered by DW_AT_location)".
+##
+## The yaml represents this DWARF:
+##
+## DW_TAG_compile_unit
+##   DW_AT_low_pc	(0x0000000000000000)
+##   DW_AT_high_pc	(0x000000000000000b)
+##
+##   DW_TAG_subprogram
+##     DW_AT_low_pc	(0x0000000000000000)
+##     DW_AT_high_pc	(0x000000000000000b)
+##
+##     DW_TAG_lexical_block
+##       DW_AT_ranges	(0x00000000
+##          [0x0000000000000000, 0x0000000000000003)
+##          [0x0000000000000005, 0x0000000000000008))
+##
+##       // #bytes in parent scope: 6
+##       // #bytes in any scope covered by DW_AT_location: 6
+##       // #bytes in parent scope covered by DW_AT_location: 4
+##       DW_TAG_variable
+##         DW_AT_location	(0x00000000:
+##            [0x0000000000000000, 0x0000000000000006): DW_OP_reg5 RDI)
+##
+##       // #bytes in parent scope: 6
+##       // #bytes in any scope covered by DW_AT_location: 2
+##       // #bytes in parent scope covered by DW_AT_location: 0
+##       DW_TAG_variable
+##         DW_AT_location	(0x00000023:
+##            [0x0000000000000003, 0x0000000000000005): DW_OP_reg2 RCX)
+
+# CHECK: "version": 6,
+# CHECK: "sum_all_variables(#bytes in parent scope)": 12,
+# CHECK: "sum_all_variables(#bytes in any scope covered by DW_AT_location)": 8
+# CHECK: "sum_all_variables(#bytes in parent scope covered by DW_AT_location)": 4
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:         .debug_loc
+    Type:         SHT_PROGBITS
+    AddressAlign: 0x01
+    Content:      '00000000000000000600000000000000010055000000000000000000000000000000000300000000000000050000000000000001005200000000000000000000000000000000'
+  - Name:         .debug_ranges
+    Type:         SHT_PROGBITS
+    AddressAlign: 0x01
+    Content:      '000000000000000003000000000000000500000000000000080000000000000000000000000000000000000000000000'
+DWARF:
+  debug_abbrev:
+    - Table:
+      - Code:     1
+        Tag:      DW_TAG_compile_unit
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_low_pc
+            Form:      DW_FORM_addr
+          - Attribute: DW_AT_high_pc
+            Form:      DW_FORM_data4
+      - Code:     2
+        Tag:      DW_TAG_subprogram
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_low_pc
+            Form:      DW_FORM_addr
+          - Attribute: DW_AT_high_pc
+            Form:      DW_FORM_data4
+      - Code:     3
+        Tag:      DW_TAG_lexical_block
+        Children: DW_CHILDREN_yes
+        Attributes:
+          - Attribute: DW_AT_ranges
+            Form:      DW_FORM_sec_offset
+      - Code:     4
+        Tag:      DW_TAG_variable
+        Children: DW_CHILDREN_no
+        Attributes:
+          - Attribute: DW_AT_location
+            Form:      DW_FORM_sec_offset
+  debug_info:
+    - Version:    4
+      AbbrOffset: 0x00
+      Entries:
+        - AbbrCode: 1 ## DW_TAG_compile_unit
+          Values:
+            - Value: 0x00 ## DW_AT_low_pc
+            - Value: 0x0b ## DW_AT_high_pc
+        - AbbrCode: 2 ## DW_TAG_subprogram
+          Values:
+            - Value: 0x00 ## DW_AT_low_pc
+            - Value: 0x0b ## DW_AT_high_pc
+        - AbbrCode: 3 ## DW_TAG_lexical_block
+          Values:
+            - Value: 0x00 ## DW_AT_ranges
+        - AbbrCode: 4 ## DW_TAG_variable
+          Values:
+            - Value: 0x00 ## DW_AT_sec_offset
+        - AbbrCode: 4 ## DW_TAG_variable
+          Values:
+            - Value: 0x23 ## DW_AT_sec_offset
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 0 ## NULL
--- a/llvm/tools/llvm-dwarfdump/Statistics.cpp
+++ b/llvm/tools/llvm-dwarfdump/Statistics.cpp
@ -71,6 +71,8 @@ struct PerFunctionStats {
 /// Holds accumulated global statistics about DIEs.
 struct GlobalStats {
  /// Total number of PC range bytes covered by DW_AT_locations.
+  unsigned TotalBytesCovered = 0;
+  /// Total number of parent DIE PC range bytes covered by DW_AT_Locations.
  unsigned ScopeBytesCovered = 0;
  /// Total number of PC range bytes in each variable's enclosing scope.
  unsigned ScopeBytes = 0;
@ -143,20 +145,20 @@ struct LocationStats {
 } // namespace

 /// Collect debug location statistics for one DIE.
-static void collectLocStats(uint64_t BytesCovered, uint64_t BytesInScope,
+static void collectLocStats(uint64_t ScopeBytesCovered, uint64_t BytesInScope,
                            std::vector<unsigned> &VarParamLocStats,
                            std::vector<unsigned> &ParamLocStats,
                            std::vector<unsigned> &LocalVarLocStats,
                            bool IsParam, bool IsLocalVar) {
-  auto getCoverageBucket = [BytesCovered, BytesInScope]() -> unsigned {
+  auto getCoverageBucket = [ScopeBytesCovered, BytesInScope]() -> unsigned {
    // No debug location at all for the variable.
-    if (BytesCovered == 0)
+    if (ScopeBytesCovered == 0)
      return 0;
    // Fully covered variable within its scope.
-    if (BytesCovered >= BytesInScope)
+    if (ScopeBytesCovered >= BytesInScope)
      return NumOfCoverageCategories - 1;
    // Get covered range (e.g. 20%-29%).
-    unsigned LocBucket = 100 * (double)BytesCovered / BytesInScope;
+    unsigned LocBucket = 100 * (double)ScopeBytesCovered / BytesInScope;
    LocBucket /= 10;
    return LocBucket + 1;
  };
@ -198,6 +200,15 @@ static std::string constructDieID(DWARFDie Die,
  return ID.str();
 }

+/// Return the number of bytes in the overlap of ranges A and B.
+static uint64_t calculateOverlap(DWARFAddressRange A, DWARFAddressRange B) {
+  uint64_t Lower = std::max(A.LowPC, B.LowPC);
+  uint64_t Upper = std::min(A.HighPC, B.HighPC);
+  if (Lower >= Upper)
+    return 0;
+  return Upper - Lower;
+}
+
 /// Collect debug info quality metrics for one DIE.
 static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
                               std::string VarPrefix, uint64_t BytesInScope,
@ -208,7 +219,8 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
  bool HasLoc = false;
  bool HasSrcLoc = false;
  bool HasType = false;
-  uint64_t BytesCovered = 0;
+  uint64_t TotalBytesCovered = 0;
+  uint64_t ScopeBytesCovered = 0;
  uint64_t BytesEntryValuesCovered = 0;
  auto &FnStats = FnStatMap[FnPrefix];
  bool IsParam = Die.getTag() == dwarf::DW_TAG_formal_parameter;
@ -261,7 +273,8 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
  if (Die.find(dwarf::DW_AT_const_value)) {
    // This catches constant members *and* variables.
    HasLoc = true;
-    BytesCovered = BytesInScope;
+    ScopeBytesCovered = BytesInScope;
+    TotalBytesCovered = BytesInScope;
  } else {
    // Handle variables and function arguments.
    Expected<std::vector<DWARFLocationExpression>> Loc =
@ -275,13 +288,27 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
          *Loc, [](const DWARFLocationExpression &L) { return !L.Range; });
      if (Default != Loc->end()) {
        // Assume the entire range is covered by a single location.
-        BytesCovered = BytesInScope;
+        ScopeBytesCovered = BytesInScope;
+        TotalBytesCovered = BytesInScope;
      } else {
+        // Caller checks this Expected result already, it cannot fail.
+        auto ScopeRanges = cantFail(Die.getParent().getAddressRanges());
        for (auto Entry : *Loc) {
-          uint64_t BytesEntryCovered = Entry.Range->HighPC - Entry.Range->LowPC;
-          BytesCovered += BytesEntryCovered;
+          TotalBytesCovered += Entry.Range->HighPC - Entry.Range->LowPC;
+          uint64_t ScopeBytesCoveredByEntry = 0;
+          // Calculate how many bytes of the parent scope this entry covers.
+          // FIXME: In section 2.6.2 of the DWARFv5 spec it says that "The
+          // address ranges defined by the bounded location descriptions of a
+          // location list may overlap". So in theory a variable can have
+          // multiple simultaneous locations, which would make this calculation
+          // misleading because we will count the overlapped areas
+          // twice. However, clang does not currently emit DWARF like this.
+          for (DWARFAddressRange R : ScopeRanges) {
+            ScopeBytesCoveredByEntry += calculateOverlap(*Entry.Range, R);
+          }
+          ScopeBytesCovered += ScopeBytesCoveredByEntry;
          if (IsEntryValue(Entry.Expr))
-            BytesEntryValuesCovered += BytesEntryCovered;
+            BytesEntryValuesCovered += ScopeBytesCoveredByEntry;
        }
      }
    }
@ -295,11 +322,11 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
    else if (IsLocalVar)
      LocStats.NumVar++;

-    collectLocStats(BytesCovered, BytesInScope, LocStats.VarParamLocStats,
+    collectLocStats(ScopeBytesCovered, BytesInScope, LocStats.VarParamLocStats,
                    LocStats.ParamLocStats, LocStats.LocalVarLocStats, IsParam,
                    IsLocalVar);
    // Non debug entry values coverage statistics.
-    collectLocStats(BytesCovered - BytesEntryValuesCovered, BytesInScope,
+    collectLocStats(ScopeBytesCovered - BytesEntryValuesCovered, BytesInScope,
                    LocStats.VarParamNonEntryValLocStats,
                    LocStats.ParamNonEntryValLocStats,
                    LocStats.LocalVarNonEntryValLocStats, IsParam, IsLocalVar);
@ -313,19 +340,17 @@ static void collectStatsForDie(DWARFDie Die, std::string FnPrefix,
  std::string VarID = constructDieID(Die, VarPrefix);
  FnStats.VarsInFunction.insert(VarID);

+  GlobalStats.TotalBytesCovered += TotalBytesCovered;
  if (BytesInScope) {
-    // Turns out we have a lot of ranges that extend past the lexical scope.
-    GlobalStats.ScopeBytesCovered += std::min(BytesInScope, BytesCovered);
+    GlobalStats.ScopeBytesCovered += ScopeBytesCovered;
    GlobalStats.ScopeBytes += BytesInScope;
    GlobalStats.ScopeEntryValueBytesCovered += BytesEntryValuesCovered;
    if (IsParam) {
-      GlobalStats.ParamScopeBytesCovered +=
-          std::min(BytesInScope, BytesCovered);
+      GlobalStats.ParamScopeBytesCovered += ScopeBytesCovered;
      GlobalStats.ParamScopeBytes += BytesInScope;
      GlobalStats.ParamScopeEntryValueBytesCovered += BytesEntryValuesCovered;
    } else if (IsLocalVar) {
-      GlobalStats.LocalVarScopeBytesCovered +=
-          std::min(BytesInScope, BytesCovered);
+      GlobalStats.LocalVarScopeBytesCovered += ScopeBytesCovered;
      GlobalStats.LocalVarScopeBytes += BytesInScope;
      GlobalStats.LocalVarScopeEntryValueBytesCovered +=
          BytesEntryValuesCovered;
@ -545,7 +570,7 @@ bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
  /// 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 = 5;
+  unsigned Version = 6;
  unsigned VarParamTotal = 0;
  unsigned VarParamUnique = 0;
  unsigned VarParamWithLoc = 0;
@ -617,6 +642,9 @@ bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,

  printDatum(J, "sum_all_variables(#bytes in parent scope)",
             GlobalStats.ScopeBytes);
+  printDatum(J,
+             "sum_all_variables(#bytes in any scope covered by DW_AT_location)",
+             GlobalStats.TotalBytesCovered);
  printDatum(J,
             "sum_all_variables(#bytes in parent scope covered by "
             "DW_AT_location)",