[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 (file)
index 0000000..03b37cd
--- /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<typename T>
+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 (file)
index 0000000..eeaa4eb
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 (file)
index 0000000..835db29
--- /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 (file)
index 0000000..641bdb4
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 (file)
index 0000000..5296559
--- /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 (file)
index 0000000..f26caea
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 (file)
index 0000000..f23a16d
--- /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 ec263c8c987f6ee1a9ac501d884e9b37e523d4aa..16ae8aaa4493192478c5d984a48f8439fdc35093 100644 (file)
--- 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<uint64_t, 16> 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 b3385fcb3277c58ccae0579448a0f37f9617e111..48cd63a21ddb8276a243aad0781ab63298cea72f 100644 (file)
--- 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;