From: Vedant Kumar Date: Fri, 15 Sep 2017 23:00:02 +0000 (+0000) Subject: [llvm-cov] Avoid over-counting covered lines and regions X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=310c2806731c6a23d9769da3c519cd03251ba638;p=llvm [llvm-cov] Avoid over-counting covered lines and regions * Fix an unsigned integer overflow in the logic that computes the number of uncovered lines in a function. * When aggregating region and line coverage summaries, take into account that different instantiations may have a different number of regions. The new test case provides test coverage for both bugs. I also verified this change by preparing a coverage report for a stage2 build of llc -- the new assertions should detect any outstanding over-counting bugs. Fixes PR34613. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@313417 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/test/tools/llvm-cov/Inputs/multiple_objects/header.h b/test/tools/llvm-cov/Inputs/multiple_objects/header.h new file mode 100644 index 00000000000..03b37cd2dbb --- /dev/null +++ b/test/tools/llvm-cov/Inputs/multiple_objects/header.h @@ -0,0 +1,29 @@ +static inline void f1() { +#ifdef DEF + if (false && false) + return; + + if (true || false || true) + return; + + if (true && false) + return; +#endif +} + +template +void f2(T **x) { +#ifdef DEF + if (false && false) + *x = nullptr; + + if (true || false || true) + *x = nullptr; + + if (true && false) + *x = nullptr; +#endif +} + +static inline void f3() { +} diff --git a/test/tools/llvm-cov/Inputs/multiple_objects/merged.profdata b/test/tools/llvm-cov/Inputs/multiple_objects/merged.profdata new file mode 100644 index 00000000000..eeaa4eb45c8 Binary files /dev/null and b/test/tools/llvm-cov/Inputs/multiple_objects/merged.profdata differ diff --git a/test/tools/llvm-cov/Inputs/multiple_objects/use_1.cc b/test/tools/llvm-cov/Inputs/multiple_objects/use_1.cc new file mode 100644 index 00000000000..835db293f69 --- /dev/null +++ b/test/tools/llvm-cov/Inputs/multiple_objects/use_1.cc @@ -0,0 +1,14 @@ +#define DEF +#include "header.h" + +int main() { + f1(); + + int *x; + f2(&x); + + float *y; + f2(&y); + + return 0; +} diff --git a/test/tools/llvm-cov/Inputs/multiple_objects/use_1.covmapping b/test/tools/llvm-cov/Inputs/multiple_objects/use_1.covmapping new file mode 100644 index 00000000000..641bdb441c2 Binary files /dev/null and b/test/tools/llvm-cov/Inputs/multiple_objects/use_1.covmapping differ diff --git a/test/tools/llvm-cov/Inputs/multiple_objects/use_2.cc b/test/tools/llvm-cov/Inputs/multiple_objects/use_2.cc new file mode 100644 index 00000000000..52965599a3a --- /dev/null +++ b/test/tools/llvm-cov/Inputs/multiple_objects/use_2.cc @@ -0,0 +1,20 @@ +#undef DEF +#include "header.h" + +static int foo() { + return 0; +} + +int main() { + f1(); + + long *x; + f2(&x); + + double *y; + f2(&y); + + f3(); + + return foo(); +} diff --git a/test/tools/llvm-cov/Inputs/multiple_objects/use_2.covmapping b/test/tools/llvm-cov/Inputs/multiple_objects/use_2.covmapping new file mode 100644 index 00000000000..f26caea7480 Binary files /dev/null and b/test/tools/llvm-cov/Inputs/multiple_objects/use_2.covmapping differ diff --git a/test/tools/llvm-cov/multiple-objects.test b/test/tools/llvm-cov/multiple-objects.test new file mode 100644 index 00000000000..f23a16d4cac --- /dev/null +++ b/test/tools/llvm-cov/multiple-objects.test @@ -0,0 +1,17 @@ +RUN: llvm-cov report -instr-profile %S/Inputs/multiple_objects/merged.profdata \ +RUN: %S/Inputs/multiple_objects/use_2.covmapping \ +RUN: -object %S/Inputs/multiple_objects/use_1.covmapping | FileCheck -check-prefix=REPORT %s + +REPORT: Filename{{ +}}Regions{{ +}}Missed Regions{{ +}}Cover +REPORT-NEXT: --- +REPORT-NEXT: header.h{{ +}}25{{ +}}14{{ +}}44.00% + +Instructions for regenerating the test: + +clang -std=c++11 -mllvm -enable-name-compression=false -fprofile-instr-generate -fcoverage-mapping use_1.cc -o use_1 +clang -std=c++11 -mllvm -enable-name-compression=false -fprofile-instr-generate -fcoverage-mapping use_2.cc -o use_2 +LLVM_PROFILE_FILE="use_1.raw" ./use_1 +LLVM_PROFILE_FILE="use_2.raw" ./use_2 +llvm-profdata merge use_{1,2}.raw -o merged.profdata +llvm-cov convert-for-testing ./use_1 -o ./use_1.covmapping +llvm-cov convert-for-testing ./use_2 -o ./use_2.covmapping diff --git a/tools/llvm-cov/CoverageSummaryInfo.cpp b/tools/llvm-cov/CoverageSummaryInfo.cpp index ec263c8c987..16ae8aaa449 100644 --- a/tools/llvm-cov/CoverageSummaryInfo.cpp +++ b/tools/llvm-cov/CoverageSummaryInfo.cpp @@ -29,6 +29,9 @@ FunctionCoverageSummary::get(const coverage::FunctionRecord &Function) { ++CoveredRegions; } + // TODO: This logic is incorrect and needs to be removed (PR34615). We need + // to use the segment builder to get accurate line execution counts. + // // Compute the line coverage size_t NumLines = 0, CoveredLines = 0; for (unsigned FileID = 0, E = Function.Filenames.size(); FileID < E; @@ -43,26 +46,33 @@ FunctionCoverageSummary::get(const coverage::FunctionRecord &Function) { LineStart = std::min(LineStart, CR.LineStart); LineEnd = std::max(LineEnd, CR.LineEnd); } + assert(LineStart <= LineEnd && "Function contains spurious file"); unsigned LineCount = LineEnd - LineStart + 1; // Get counters llvm::SmallVector ExecutionCounts; ExecutionCounts.resize(LineCount, 0); + unsigned LinesNotSkipped = LineCount; for (auto &CR : Function.CountedRegions) { if (CR.FileID != FileID) continue; // Ignore the lines that were skipped by the preprocessor. auto ExecutionCount = CR.ExecutionCount; if (CR.Kind == CounterMappingRegion::SkippedRegion) { - LineCount -= CR.LineEnd - CR.LineStart + 1; + unsigned SkippedLines = CR.LineEnd - CR.LineStart + 1; + assert((SkippedLines <= LinesNotSkipped) && + "Skipped region larger than file containing it"); + LinesNotSkipped -= SkippedLines; ExecutionCount = 1; } for (unsigned I = CR.LineStart; I <= CR.LineEnd; ++I) ExecutionCounts[I - LineStart] = ExecutionCount; } - CoveredLines += LineCount - std::count(ExecutionCounts.begin(), - ExecutionCounts.end(), 0); - NumLines += LineCount; + unsigned UncoveredLines = + std::min(std::count(ExecutionCounts.begin(), ExecutionCounts.end(), 0), + (long)LinesNotSkipped); + CoveredLines += LinesNotSkipped - UncoveredLines; + NumLines += LinesNotSkipped; } return FunctionCoverageSummary( Function.Name, Function.ExecutionCount, diff --git a/tools/llvm-cov/CoverageSummaryInfo.h b/tools/llvm-cov/CoverageSummaryInfo.h index b3385fcb327..48cd63a21dd 100644 --- a/tools/llvm-cov/CoverageSummaryInfo.h +++ b/tools/llvm-cov/CoverageSummaryInfo.h @@ -32,7 +32,9 @@ public: RegionCoverageInfo() : Covered(0), NumRegions(0) {} RegionCoverageInfo(size_t Covered, size_t NumRegions) - : Covered(Covered), NumRegions(NumRegions) {} + : Covered(Covered), NumRegions(NumRegions) { + assert(Covered <= NumRegions && "Covered regions over-counted"); + } RegionCoverageInfo &operator+=(const RegionCoverageInfo &RHS) { Covered += RHS.Covered; @@ -42,6 +44,7 @@ public: void merge(const RegionCoverageInfo &RHS) { Covered = std::max(Covered, RHS.Covered); + NumRegions = std::max(NumRegions, RHS.NumRegions); } size_t getCovered() const { return Covered; } @@ -51,6 +54,7 @@ public: bool isFullyCovered() const { return Covered == NumRegions; } double getPercentCovered() const { + assert(Covered <= NumRegions && "Covered regions over-counted"); if (NumRegions == 0) return 0.0; return double(Covered) / double(NumRegions) * 100.0; @@ -69,7 +73,9 @@ public: LineCoverageInfo() : Covered(0), NumLines(0) {} LineCoverageInfo(size_t Covered, size_t NumLines) - : Covered(Covered), NumLines(NumLines) {} + : Covered(Covered), NumLines(NumLines) { + assert(Covered <= NumLines && "Covered lines over-counted"); + } LineCoverageInfo &operator+=(const LineCoverageInfo &RHS) { Covered += RHS.Covered; @@ -79,6 +85,7 @@ public: void merge(const LineCoverageInfo &RHS) { Covered = std::max(Covered, RHS.Covered); + NumLines = std::max(NumLines, RHS.NumLines); } size_t getCovered() const { return Covered; } @@ -88,6 +95,7 @@ public: bool isFullyCovered() const { return Covered == NumLines; } double getPercentCovered() const { + assert(Covered <= NumLines && "Covered lines over-counted"); if (NumLines == 0) return 0.0; return double(Covered) / double(NumLines) * 100.0; @@ -121,6 +129,7 @@ public: bool isFullyCovered() const { return Executed == NumFunctions; } double getPercentCovered() const { + assert(Executed <= NumFunctions && "Covered functions over-counted"); if (NumFunctions == 0) return 0.0; return double(Executed) / double(NumFunctions) * 100.0; @@ -135,7 +144,7 @@ struct FunctionCoverageSummary { LineCoverageInfo LineCoverage; FunctionCoverageSummary(const std::string &Name) - : Name(Name), ExecutionCount(0) {} + : Name(Name), ExecutionCount(0), RegionCoverage(), LineCoverage() {} FunctionCoverageSummary(const std::string &Name, uint64_t ExecutionCount, const RegionCoverageInfo &RegionCoverage, @@ -163,7 +172,9 @@ struct FileCoverageSummary { FunctionCoverageInfo FunctionCoverage; FunctionCoverageInfo InstantiationCoverage; - FileCoverageSummary(StringRef Name) : Name(Name) {} + FileCoverageSummary(StringRef Name) + : Name(Name), RegionCoverage(), LineCoverage(), FunctionCoverage(), + InstantiationCoverage() {} void addFunction(const FunctionCoverageSummary &Function) { RegionCoverage += Function.RegionCoverage;