[llvm] 3ff4d75 - [llvm-dwarfdump] Fix misleading scope byte coverage statistics
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 25 01:38:12 PDT 2020
Author: OCHyams
Date: 2020-08-25T06:40:11+01:00
New Revision: 3ff4d75c9cbffb12c5c690c389e3977ea6811042
URL: https://github.com/llvm/llvm-project/commit/3ff4d75c9cbffb12c5c690c389e3977ea6811042
DIFF: https://github.com/llvm/llvm-project/commit/3ff4d75c9cbffb12c5c690c389e3977ea6811042.diff
LOG: [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.
Added:
llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml
Modified:
llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test
llvm/test/tools/llvm-dwarfdump/X86/statistics-v3.test
llvm/test/tools/llvm-dwarfdump/X86/statistics.ll
llvm/tools/llvm-dwarfdump/Statistics.cpp
Removed:
################################################################################
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test b/llvm/test/tools/llvm-dwarfdump/X86/statistics-dwo.test
index b55ca657ee32..a5b716802f75 100644
--- 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,
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/statistics-v3.test b/llvm/test/tools/llvm-dwarfdump/X86/statistics-v3.test
index 26a8d8448705..bb7d3dec710b 100644
--- 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,
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/statistics.ll b/llvm/test/tools/llvm-dwarfdump/X86/statistics.ll
index b4057413dd05..7bd43308a0cd 100644
--- 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;
diff --git a/llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml b/llvm/test/tools/llvm-dwarfdump/X86/stats-scope-bytes-covered.yaml
new file mode 100644
index 000000000000..1e99fd5debce
--- /dev/null
+++ 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
diff --git a/llvm/tools/llvm-dwarfdump/Statistics.cpp b/llvm/tools/llvm-dwarfdump/Statistics.cpp
index 2f080fe9ef3b..82da06eab1d6 100644
--- 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)",
More information about the llvm-commits
mailing list