[llvm-dwarfdump] Fix misleading scope byte coverage statistics

[Orlando]

Summary

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.

[dstenb]

There are no FileCheck checks for the new metric, as far as I can tell?

I guess it could be good to add a lit test where the two metrics differ.

[Orlando]

Hi @dstenb, thanks for taking a look. I've updated the diff to include a test but it feels a little unwieldy. Is there a better way to test this?

I've also fixed a bug (good idea to request a test...) and renamed the stat to better reflect its purpose.

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.s

1–13 ↗

(On Diff #284641)

Probably worth simplifying this test - it seems really long if it's only testing this property. (& if possible, including source code/steps to generate it - though probably better/easier to hand-craft DWARF for a case like this, since any case where LLVM produces locations wider than their scope is buggy & if that can't be tickled in a small example, maybe worth some hand-writing/tweaking of the DWARF or IR)

llvm/tools/llvm-dwarfdump/Statistics.cpp

300–305

FWIW, I hope it does emit DWARF like this in the not-too-distant future. (specifically, if the variable's location is described using an entry_value, it might be good to use that entry_value for the entire scope of the function, even if the variable's value is available through other means as well - it'd be a more efficient encoding than having two separate ranges that use entry_value split because there's a range that uses an immediate value in between)

[Orlando]

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.s

1–13 ↗

(On Diff #284641)

I agree that the test is clunky, but I'm a bit stumped on what a good test would look like.

I would like to avoid testing from IR/MIR because, as you point out, the situation we are checking for is buggy output from clang. The output is prone to getting fixed and could cause a false failure in the future. That is why I chose to use assembly in the test, but it's not very human friendly, even with a smaller very contrived input MIR to generate the assembly.

yaml2obj seems like a promising solution but the yaml generated by obj2yaml is doesn't convey the purpose of the test either IMO.

So IMO a non-contrived (and more complex) yaml or .s test with the source in the comments is the way forward here, wdyt?

[aprantl]

LGTM modulo David's comment about the test.

[jhenderson]

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.s

1–13 ↗

(On Diff #284641)

I looked at this test, and I honestly can't see the wood from the trees. There's a lot of fluff in it that you can delete, as it has zero relevance to what you are testing. A YAML version might make it easier to trim it down further, although the DWARF support for ELF yaml2obj is still undergoing refinement - see the various patches by @Higuoxing in recent months. It might be worth switching to YAML, and then working with @Higuoxing to highlight where improvements could be made to yaml2obj to allow removing much of the useless bits - this is one of the aims of yaml2obj: to allow you to write concise tests that do what you need without things getting too verbose. The same can sometimes be achieved in assembly too, but raw assembly isn't that readable for DWARF, in my opinion.

Having the input code might help, but there's a risk it will go stale in the future.

5–6 ↗

(On Diff #284641)

Tip: Something I'm trying to encourage is to use ## for comments in lit tests (or equivalent for other comment markers), because it helps the comments stand out from the RUN and CHECK lines. Indeed, there's some suggestion that this should be adopted as "standard" practice to help FileCheck diagnostics (which for example here might complain about the use of "Check" in the first line not being a real CHECK directive).

[Orlando]

Thanks @aprantl and @jhenderson.

I'd be happy to write a yaml2obj test. The situation that I want to represent is this (note the var location range and the lexical block high_pc and low_pc):

DW_TAG_compile_unit
  DW_AT_producer    ("clang version 11.0.0")
  DW_AT_language    (DW_LANG_C99)
  DW_AT_name        ("example.c")
  DW_AT_low_pc      (0x0000000000000000)
  DW_AT_high_pc     (0x000000000000000b)

  DW_TAG_subprogram
    DW_AT_low_pc    (0x0000000000000000)
    DW_AT_high_pc   (0x000000000000000b)
    DW_AT_name      ("fun")
    DW_AT_type      (0x00000061 "int")

    DW_TAG_lexical_block
      DW_AT_low_pc  (0x0000000000000005)
      DW_AT_high_pc (0x000000000000000a)

      DW_TAG_variable
        DW_AT_location      (0x00000000: 
           [0x0000000000000000, 0x0000000000000005): DW_OP_reg5 RDI)
        DW_AT_name  ("local")
        DW_AT_type  (0x00000061 "int")

      NULL

    NULL

  DW_TAG_base_type
    DW_AT_name      ("int")
    DW_AT_encoding  (DW_ATE_signed)
    DW_AT_byte_size (0x04)

  NULL

@jhenderson or @Higuoxing, is that something we're able to do with yaml2obj at the moment? Round-tripping my built test file through obj2yaml and yaml2obj (tools built at f5a252ed681) and using llvm-dwarfdump shows:

return.o:	file format elf64-x86-64

.debug_info contents:

In the mean time I've shortened the assembly test and added a comment showing the llvm-dwarfdump output.

I've also changed the new stat name back to 'sum_all_variables(#bytes in any scope covered by DW_AT_location)'. I changed this in the last update to look more like 'sum_all_variables(#bytes in parent scope)', but I misunderstood the meaning of that stat. The most similar stat is "sum_all_variables(#bytes in any scope covered by DW_AT_location)". The name isn't great because both the new stat and this one both include DW_AT_const_value locations AFAICT. I think "sum_all_variables(#bytes in any scope covered by a location description) might be better, but I wanted to keep it in line with the existing stats.

[Higuoxing]

Hi,

Currently, obj2yaml only supports dumping the .debug_aranges section. You might not be able to convert the object file to YAML. I'm very happy to handcraft one for you.

I stripped most of the unused attributes and DIEs. The following YAML can be accepted by yaml2obj. (yaml2obj doesn't support emitting the .debug_loc section, so we have to use raw content to generate it.)
Please let me know if it doesn't work.

It looks like:

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_low_pc  (0x0000000000000005)
      DW_AT_high_pc (0x000000000000000a)

      DW_TAG_variable
        DW_AT_location      (0x00000000: 
           [0x0000000000000000, 0x0000000000000005): DW_OP_reg5 RDI)

      NULL

    NULL

  NULL

--- !ELF
FileHeader:
  Class:   ELFCLASS64
  Data:    ELFDATA2LSB
  Type:    ET_EXEC
  Machine: EM_X86_64
Sections:
  - Name:         .debug_loc
    Type:         SHT_PROGBITS
    AddressAlign: 0x01
    Content:      '0000000000000000050000000000000001005500000000000000000000000000000000'
DWARF:
  debug_abbrev:
    - 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_low_pc
          Form:      DW_FORM_addr
        - Attribute: DW_AT_high_pc
          Form:      DW_FORM_data4
    - 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: 0x05 ## DW_AT_low_pc
            - Value: 0x05 ## DW_AT_high_pc
        - AbbrCode: 4 ## DW_TAG_variable
          Values:
            - Value: 0x00 ## DW_AT_sec_offset
        - AbbrCode: 0 ## NULL
        - AbbrCode: 0 ## NULL
        - AbbrCode: 0 ## NULL

[Orlando]

Thank you so much @Higuoxing, that's very kind!

The test now uses yaml2obj with the yaml provided by @Higuoxing.

[jhenderson]

Thanks. The newer test feels much clearer and more maintainable to me. I don't have enough knowledge in this area to provide more feedback on the patch as a whole though.

[Orlando]

Thanks @jhenderson. I think this should have also addressed @dblaikie's concerns regarding the test, so I will aim to land this in a couple of days if there are no objections.

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml

17–23

Might be worth making this test case a bit more interesting by having /some/ overlap (and some non-overlap) regions.

llvm/tools/llvm-dwarfdump/Statistics.cpp

296–312

Little concerned about the N^2 complexity of this approach, and wondering if an alternative approach might also address the limitation regarding overlapping locations.

Start with a range of intervals (without overlaps) for the enclosing scope. Then intersect any location ranges with that collection of intervals and store those in another non-overlapping interval range.

A small data structure that lets you build a non-overlapping list of intervals (merging intervals, etc), keeping them sorted - then it'll be easier to find other overlapping regions.

eg:

Intervals Scope;
for (address ranges in scope)
  Scope.insert(R);
Intervals Loc;
for (address ranges in location) {
  R' = Scope.intersection(R);
  Loc.insert(R'); 
}
scope bytes covered = Loc.total_coverage/size/something

But, yeah, maybe the implementation of that's more complicated than is justified for this for now.

[Orlando]

llvm/tools/llvm-dwarfdump/Statistics.cpp

296–312

Yeah, that does look like a much better solution. If we want to go in that direction I might need to come back to this after some higher priority work (though I'd be happy to do so!).

A middle ground option might be to use IntervalMap. It's not perfect for this case though because it maps intervals to values, and only coalesces intervals if they have the same value. We don't have 'values' to map here - we only care about the intervals themselves. So we'd have to fudge it slightly by giving every interval some constant value (e.g. 0). I'm not really a fan of this idea, but I thought it'd be at least worth bringing up.

[dblaikie]

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml

17–23

Ping on this.

llvm/tools/llvm-dwarfdump/Statistics.cpp

296–312

Yeah, I looked at/considered IntervalMap but I'm not sure it provides an easy way to get the intersection of one interval with any number of intervals in the map, for instance - or intersectionally reassign the value (if it could do that (eg: given a map with [1, 5) -> 0, [7, 10) -> 0 and an interval [3, 8) -> 1, it could produce [1, 3) -> 0, [3, 5) -> 1, [7, 8) -> 1, [8, 10) -> 0, then you could use it to do this work & do a linear scan over the map summing up all ranges that -> 1. Maybe it's viable to add that functionality to IntervalMap, I'm not sure.)

But I'm OK with that being a follow-up/future work.

[Higuoxing]

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml

43–72

Hi, yaml2obj now supports emitting multiple abbrev tables after 290e399f967390648ec7cf53c4b75a883a90b609 and f7ff0ace96db9164dcde232c36cab6519ea4fce8. We should insert an additional key Table: in debug_abbrev:. yaml2obj and obj2yaml are heavily developed recently, if something doesn't work please feel free to ping me, thanks!

[Orlando]

+ Rebase
+ Update test for latest yaml2obj with change suggested by @Higuoxing (thank you!).
+ Make the test more interesting, @dblaikie wdyt?

[dblaikie]

Looks good, thanks!

[djtodoro]

@Orlando Thanks for doing this!

Have you tested llvm/utils/llvm-locstats/llvm-locstats.py (along with its tests)? Do we need to change something there as well?

Otherwise, lgtm!

[Orlando]

Hi @djtodoro thanks for taking a look!

Have you tested llvm/utils/llvm-locstats/llvm-locstats.py (along with its tests)?

Yes, all tests pass currently.

Do we need to change something there as well?

We don't need to update llvm-locstats because the stat "#bytes in parent scope covered by DW_AT_location" - which locstats reads - has been updated here. And IMO the new stat "sum_all_variables(#bytes in any scope covered by DW_AT_location)" is not something we need locstats to display because it doesn't affect coverage.

[Orlando]

Oops, I didn't include the Phabricator revision line in the commit message.

3ff4d75c9cbffb12c5c690c389e3977ea6811042.

[djtodoro]

Please close the revision as well.

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

 #   printf ("fibonacci(9) = %d\n", result);
 #   result = fib(10);
 #   printf ("fibonacci(10) = %d\n", result);
 # 
 #   return 0;
 # }
 #
 
-CHECK: "version":5
+CHECK: "version":6
 CHECK: "#functions":3
 CHECK: "#functions with location":3
 CHECK: "#inlined functions":7
 CHECK: "#inlined functions 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.

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

 #   printf ("fibonacci(9) = %d\n", result);
 #   result = fib(10);
 #   printf ("fibonacci(10) = %d\n", result);
 # 
 #   return 0;
 # }
 #
 
-CHECK: "version":5
+CHECK: "version":6
 CHECK: "#functions":3
 CHECK: "#functions with location":3
 CHECK: "#inlined functions":8
 CHECK: "#inlined functions with abstract origins":8
 CHECK: "#unique source variables":9
 CHECK: "#source variables":33
 
 # Ideally the value below would be 33 but currently it's not.

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

 ; RUN: llc -O0 %s -o - -filetype=obj \
 ; RUN:   | llvm-dwarfdump -statistics - | FileCheck %s
-; CHECK: "version":5
+; CHECK: "version":6
 
 ; namespace test {
 ;  extern int a;
 ; }
 ; using test::a;
 ;
 ; int GlobalConst = 42;
 ; int Global;

llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml

+# 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_low_pc (0x0000000000000005)
+##       DW_AT_high_pc  (0x000000000000000a)
+##
+##       DW_TAG_variable
+##         DW_AT_location (0x00000000:
+##            [0x0000000000000000, 0x0000000000000005): DW_OP_reg5 RDI)
+
+# CHECK:      "version":6,
+# CHECK-SAME: "sum_all_variables(#bytes in parent scope)":5,
+# CHECK-SAME: "sum_all_variables(#bytes in any scope covered by DW_AT_location)":5,
+# CHECK-SAME: "sum_all_variables(#bytes in parent scope covered by DW_AT_location)":0
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_X86_64
+Sections:
+  - Name:         .debug_loc
+    Type:         SHT_PROGBITS
+    AddressAlign: 0x01
+    Content:      '0000000000000000050000000000000001005500000000000000000000000000000000'
+DWARF:
+  debug_abbrev:
+    - 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_low_pc
+          Form:      DW_FORM_addr
+        - Attribute: DW_AT_high_pc
+          Form:      DW_FORM_data4
+    - 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: 0x05 ## DW_AT_low_pc
+            - Value: 0x05 ## DW_AT_high_pc
+        - AbbrCode: 4 ## DW_TAG_variable
+          Values:
+            - Value: 0x00 ## DW_AT_sec_offset
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 0 ## NULL
+        - AbbrCode: 0 ## NULL

llvm/tools/llvm-dwarfdump/Statistics.cpp

struct PerFunctionStats {
   unsigned NumLocalVarTypes = 0;
   /// Number of local variables with DW_AT_location.
   unsigned NumLocalVarLocations = 0;
 };
 
 /// 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;
   /// Total number of PC range bytes covered by DW_AT_locations with
   /// the debug entry values (DW_OP_entry_value).
   unsigned ScopeEntryValueBytesCovered = 0;
   /// Total number of PC range bytes covered by DW_AT_locations of
   /// formal parameters.
struct LocationStats {
   /// Total number of formal parameters processed.
   unsigned NumParam = 0;
   /// Total number of local variables processed.
   unsigned NumVar = 0;
 };
 } // 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;
   };
 
   unsigned CoverageBucket = getCoverageBucket();
   VarParamLocStats[CoverageBucket]++;
   if (IsParam)
     ParamLocStats[CoverageBucket]++;
if (const auto *LT = U->getContext().getLineTableForUnit(U))
         File = std::string(sys::path::filename(File));
   }
   ID << ":" << (File.empty() ? "/" : File);
   ID << ":"
      << dwarf::toUnsigned(Die.findRecursively(dwarf::DW_AT_decl_line), 0);
   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,
                                uint32_t InlineDepth,
                                StringMap<PerFunctionStats> &FnStatMap,
                                GlobalStats &GlobalStats,
                                LocationStats &LocStats) {
   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;
   bool IsLocalVar = Die.getTag() == dwarf::DW_TAG_variable;
   bool IsConstantMember = Die.getTag() == dwarf::DW_TAG_member &&
                           Die.find(dwarf::DW_AT_const_value);
 
   if (Die.getTag() == dwarf::DW_TAG_call_site ||
return llvm::any_of(Expression, [](DWARFExpression::Operation &Op) {
       return Op.getCode() == dwarf::DW_OP_entry_value ||
              Op.getCode() == dwarf::DW_OP_GNU_entry_value;
     });
   };
 
   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 =
         Die.getLocations(dwarf::DW_AT_location);
     if (!Loc) {
       consumeError(Loc.takeError());
     } else {
       HasLoc = true;
       // Get PC coverage.
       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;
+        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;
         }
       }
     }
   }
 
   // Calculate the debug location statistics.
   if (BytesInScope) {
     LocStats.NumVarParam++;
     if (IsParam)
       LocStats.NumParam++;
     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);
   }
 
   // Collect PC range coverage data.
   if (DWARFDie D =
           Die.getAttributeValueAsReferencedDie(dwarf::DW_AT_abstract_origin))
     Die = D;
 
   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;
     }
     assert(GlobalStats.ScopeBytesCovered <= GlobalStats.ScopeBytes);
   }
 
   if (IsConstantMember) {
bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
 
   /// Collect the sizes of debug sections.
   SectionSizes Sizes;
   calculateSectionSizes(Obj, Sizes, Filename);
 
   /// 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;
   unsigned NumFunctions = 0;
   unsigned NumInlinedFunctions = 0;
   unsigned NumFuncsWithSrcLoc = 0;
   unsigned NumAbstractOrigins = 0;
   unsigned ParamTotal = 0;
bool dwarfdump::collectStatsForObjectFile(ObjectFile &Obj, DWARFContext &DICtx,
 
   printDatum(OS, "#call site entries", GlobalStats.CallSiteEntries);
   printDatum(OS, "#call site DIEs", GlobalStats.CallSiteDIEs);
   printDatum(OS, "#call site parameter DIEs", GlobalStats.CallSiteParamDIEs);
 
   printDatum(OS, "sum_all_variables(#bytes in parent scope)",
              GlobalStats.ScopeBytes);
   printDatum(OS,
+             "sum_all_variables(#bytes in any scope covered by DW_AT_location)",
+             GlobalStats.TotalBytesCovered);
+  printDatum(OS,
              "sum_all_variables(#bytes in parent scope covered by "
              "DW_AT_location)",
              GlobalStats.ScopeBytesCovered);
   printDatum(OS,
              "sum_all_variables(#bytes in parent scope covered by "
              "DW_OP_entry_value)",
              GlobalStats.ScopeEntryValueBytesCovered);