From c7e789c84308f8a46cf12d6439cfa5a0b285ebbc Mon Sep 17 00:00:00 2001 From: Vedant Kumar Date: Fri, 4 Aug 2017 00:36:24 +0000 Subject: [PATCH] [llvm-cov] Ignore unclosed line segments when setting line counts This patch makes a slight change to the way llvm-cov determines line execution counts. If there are multiple line segments on a line, the line count is the max count among the regions which start *and* end on the line. This avoids an issue posed by deferred regions which start on the same line as a terminated region, e.g: if (false) return; //< The line count should be 0, even though a new region //< starts at the semi-colon. foo(); Another change is that counts from line segments which don't correspond to region entries are considered. This enables the first change, and corrects an outstanding issue (see the showLineExecutionCounts.cpp test change). This is related to D35925. Testing: check-profile, llvm-cov lit tests Differential Revision: https://reviews.llvm.org/D36014 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@310012 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Inputs/deferred-regions.covmapping | Bin 0 -> 688 bytes .../llvm-cov/Inputs/deferred-regions.profdata | Bin 0 -> 1208 bytes test/tools/llvm-cov/deferred-region.cpp | 79 ++++++++++++++++++ .../llvm-cov/showLineExecutionCounts.cpp | 4 +- tools/llvm-cov/SourceCoverageView.cpp | 45 ++++++++-- tools/llvm-cov/SourceCoverageView.h | 18 +--- 6 files changed, 122 insertions(+), 24 deletions(-) create mode 100644 test/tools/llvm-cov/Inputs/deferred-regions.covmapping create mode 100644 test/tools/llvm-cov/Inputs/deferred-regions.profdata create mode 100644 test/tools/llvm-cov/deferred-region.cpp diff --git a/test/tools/llvm-cov/Inputs/deferred-regions.covmapping b/test/tools/llvm-cov/Inputs/deferred-regions.covmapping new file mode 100644 index 0000000000000000000000000000000000000000..757f3eea7204b1cb3bccd68918bcbf81a03a7292 GIT binary patch literal 688 zcmZuuF>ljA6n^jQb8?Oy_nH`#mQ*CDQClf7r~)BDmmmd+0Z^(^m8EhMCyj7=ksSpg z80d<`z`#yd{s0ma0}B!>6A}Xhuy$qQor8dZ^ZR`FKHq!ad(Uw^8~4;~JdM(6Kb(eJ z?>-zqUAPAO_csShjga<(FqzSQb)b^{SgDD2E?sbJk)+T!}Dn7i5gt#|kI#!2?C&gA;XAMd}s z(cG&%nGKJ2Pqvd6-SRqd$1J-BKR-SHex?8N^S}9UJJ?C1Bn@T{f;8y`G4bPi_+{vpr;T_m5S%Sz7+JML6js>znMhCkveMiCKCiiZlWPML(GB8!DA-V zrn^PkG3a0;FK~24_B5n$H5Sm5UKZqVy6FO%+Q=?>$bxdaYLgU>CA1&}^fSKNRi%Wbe6S}iDoUxBkw=@R!zsQ>ey_8IovAd{}$siaI;~UOc0v~%;S8ZH=X?NUktwh Drrv8~ literal 0 HcmV?d00001 diff --git a/test/tools/llvm-cov/Inputs/deferred-regions.profdata b/test/tools/llvm-cov/Inputs/deferred-regions.profdata new file mode 100644 index 0000000000000000000000000000000000000000..0bacac01ccf3f2c7ef78507404e17128eddcebcf GIT binary patch literal 1208 zcma)5ze_?<82w)M!$i}hBKiSt+Gd2(5dE+}pfIR(F~iIU)|FQnK}0SMLPLc6bB*W^ zsHLH?wW)|`sK#7dD&GB$!{t86<@lX@?)i@Q`}`?aEKxLTouvQ7_@JAQ!;Gkhxb9%= zQgD##P?WzYV4oTegL|?fnh$ui{Og4bgnr0JgUgjm~vH!U;~65Yz#VDs?0om-oMD95tZ6uC*ubtv%JN5ZExN7`Uu7_ pLN2ud&VkSY@w-tY5K)LJ{O_S~!(p^`6EL4)-IbzmU@2Vg{{j_OM*IK( literal 0 HcmV?d00001 diff --git a/test/tools/llvm-cov/deferred-region.cpp b/test/tools/llvm-cov/deferred-region.cpp new file mode 100644 index 00000000000..3704d0ed515 --- /dev/null +++ b/test/tools/llvm-cov/deferred-region.cpp @@ -0,0 +1,79 @@ +// RUN: llvm-cov show %S/Inputs/deferred-regions.covmapping -instr-profile %S/Inputs/deferred-regions.profdata -show-line-counts-or-regions -dump -filename-equivalence %s 2>&1 | FileCheck %s + +void foo(int x) { + if (x == 0) { + return; // CHECK: [[@LINE]]|{{ +}}1| + } + +} // CHECK: [[@LINE]]|{{ +}}2| + +void bar() { + return; + +} // CHECK: [[@LINE]]|{{ +}}1| + +void for_loop() { + if (false) + return; // CHECK: [[@LINE]]|{{ +}}0| + + for (int i = 0; i < 10; ++i) { // CHECK: [[@LINE]]|{{ +}}2| + if (i % 2 == 0) + continue; // CHECK: [[@LINE]]|{{ +}}1| + + if (i % 5 == 0) + break; // CHECK: [[@LINE]]|{{ +}}0| + + int x = i; + return; // CHECK: [[@LINE]]|{{ +}}1| + + } // CHECK: [[@LINE]]|{{ +}}1| +} + +struct Error {}; + +void while_loop() { + if (false) + return; // CHECK: [[@LINE]]|{{ +}}0| + + int x = 0; + while (++x < 10) { // CHECK: [[@LINE]]|{{ +}}3| + if (x == 1) + continue; // CHECK: [[@LINE]]|{{ +}}1| + + while (++x < 4) { // CHECK: [[@LINE]]|{{ +}}1| + if (x == 3) + break; // CHECK: [[@LINE]]|{{ +}}1| + // CHECK: [[@LINE]]|{{ +}}0| + while (++x < 5) {} // CHECK: [[@LINE]]|{{ +}}0| + } // CHECK: [[@LINE]]|{{ +}}1| + + if (x == 0) + throw Error(); // CHECK: [[@LINE]]|{{ +}}0| + + while (++x < 9) { // CHECK: [[@LINE]]|{{ +}}6| + if (x == 0) // CHECK: [[@LINE]]|{{ +}}5| + break; // CHECK: [[@LINE]]|{{ +}}0| + + } + } +} + +void gotos() { + if (false) + goto out; // CHECK: [[@LINE]]|{{ +}}0| + + return; + +out: // CHECK: [[@LINE]]|{{ +}}1| + return; +} + +int main() { + foo(0); + foo(1); + bar(); + for_loop(); + while_loop(); + gotos(); + return 0; +} diff --git a/test/tools/llvm-cov/showLineExecutionCounts.cpp b/test/tools/llvm-cov/showLineExecutionCounts.cpp index bdb0e1bd3a9..4bc3ad5dfdc 100644 --- a/test/tools/llvm-cov/showLineExecutionCounts.cpp +++ b/test/tools/llvm-cov/showLineExecutionCounts.cpp @@ -6,7 +6,7 @@ int main() { // TEXT: [[@LINE]]| 161|int main( int x = 0; // TEXT: [[@LINE]]| 161| int x // TEXT: [[@LINE]]| 161| - if (x) { // TEXT: [[@LINE]]| 0| if (x) + if (x) { // TEXT: [[@LINE]]| 161| if (x) x = 0; // TEXT: [[@LINE]]| 0| x = 0 } else { // TEXT: [[@LINE]]| 161| } else x = 1; // TEXT: [[@LINE]]| 161| x = 1 @@ -50,7 +50,7 @@ int main() { // TEXT: [[@LINE]]| 161|int main( // HTML: 
[[@LINE-44]]
161
int main() {
 // HTML: 
[[@LINE-44]]
161
  int x = 0
 // HTML: 
[[@LINE-44]]
161
-// HTML: 
[[@LINE-44]]
0
  if (x) {
+// HTML: 
[[@LINE-44]]
161
  if (x) {
 // HTML: 
[[@LINE-44]]
0
 // HTML: 
[[@LINE-44]]
161
  }
 // HTML: 
[[@LINE-44]]
161
    x = 1;
diff --git a/tools/llvm-cov/SourceCoverageView.cpp b/tools/llvm-cov/SourceCoverageView.cpp
index 52b8ff1747f..6630f3333bd 100644
--- a/tools/llvm-cov/SourceCoverageView.cpp
+++ b/tools/llvm-cov/SourceCoverageView.cpp
@@ -83,6 +83,41 @@ CoveragePrinter::create(const CoverageViewOptions &Opts) {
   llvm_unreachable("Unknown coverage output format!");
 }
 
+LineCoverageStats::LineCoverageStats(
+    ArrayRef LineSegments,
+    const coverage::CoverageSegment *WrappedSegment) {
+  // Find the minimum number of regions which start in this line.
+  unsigned MinRegionCount = 0;
+  auto isStartOfRegion = [](const coverage::CoverageSegment *S) {
+    return S->HasCount && S->IsRegionEntry;
+  };
+  for (unsigned I = 0; I < LineSegments.size() && MinRegionCount < 2; ++I)
+    if (isStartOfRegion(LineSegments[I]))
+      ++MinRegionCount;
+
+  ExecutionCount = 0;
+  HasMultipleRegions = MinRegionCount > 1;
+  Mapped = (WrappedSegment && WrappedSegment->HasCount) || (MinRegionCount > 0);
+
+  if (!Mapped)
+    return;
+
+  // Pick the max count among regions which start and end on this line, to
+  // avoid erroneously using the wrapped count, and to avoid picking region
+  // counts which come from deferred regions.
+  if (LineSegments.size() > 1) {
+    for (unsigned I = 0; I < LineSegments.size() - 1; ++I)
+      ExecutionCount = std::max(ExecutionCount, LineSegments[I]->Count);
+    return;
+  }
+
+  // Just pick the maximum count.
+  if (WrappedSegment && WrappedSegment->HasCount)
+    ExecutionCount = WrappedSegment->Count;
+  if (!LineSegments.empty())
+    ExecutionCount = std::max(ExecutionCount, LineSegments[0]->Count);
+}
+
 unsigned SourceCoverageView::getFirstUncoveredLineNo() {
   auto CheckIfUncovered = [](const coverage::CoverageSegment &S) {
     return S.HasCount && S.Count == 0;
@@ -207,17 +242,11 @@ void SourceCoverageView::print(raw_ostream &OS, bool WholeFile,
     while (NextSegment != EndSegment && NextSegment->Line == LI.line_number())
       LineSegments.push_back(&*NextSegment++);
 
-    // Calculate a count to be for the line as a whole.
-    LineCoverageStats LineCount;
-    if (WrappedSegment && WrappedSegment->HasCount)
-      LineCount.addRegionCount(WrappedSegment->Count);
-    for (const auto *S : LineSegments)
-      if (S->HasCount && S->IsRegionEntry)
-        LineCount.addRegionStartCount(S->Count);
-
     renderLinePrefix(OS, ViewDepth);
     if (getOptions().ShowLineNumbers)
       renderLineNumberColumn(OS, LI.line_number());
+
+    LineCoverageStats LineCount{LineSegments, WrappedSegment};
     if (getOptions().ShowLineStats)
       renderLineCoverageColumn(OS, LineCount);
 
diff --git a/tools/llvm-cov/SourceCoverageView.h b/tools/llvm-cov/SourceCoverageView.h
index 9cb608fed60..c9f0c57b5cb 100644
--- a/tools/llvm-cov/SourceCoverageView.h
+++ b/tools/llvm-cov/SourceCoverageView.h
@@ -67,25 +67,15 @@ struct InstantiationView {
 /// \brief Coverage statistics for a single line.
 struct LineCoverageStats {
   uint64_t ExecutionCount;
-  unsigned RegionCount;
+  bool HasMultipleRegions;
   bool Mapped;
 
-  LineCoverageStats() : ExecutionCount(0), RegionCount(0), Mapped(false) {}
+  LineCoverageStats(ArrayRef LineSegments,
+                    const coverage::CoverageSegment *WrappedSegment);
 
   bool isMapped() const { return Mapped; }
 
-  bool hasMultipleRegions() const { return RegionCount > 1; }
-
-  void addRegionStartCount(uint64_t Count) {
-    // The max of all region starts is the most interesting value.
-    addRegionCount(RegionCount ? std::max(ExecutionCount, Count) : Count);
-    ++RegionCount;
-  }
-
-  void addRegionCount(uint64_t Count) {
-    Mapped = true;
-    ExecutionCount = Count;
-  }
+  bool hasMultipleRegions() const { return HasMultipleRegions; }
 };
 
 /// \brief A file manager that handles format-aware file creation.
-- 
2.40.0