]> granicus.if.org Git - llvm/commitdiff
[llvm-cov] Ignore unclosed line segments when setting line counts
authorVedant Kumar <vsk@apple.com>
Fri, 4 Aug 2017 00:36:24 +0000 (00:36 +0000)
committerVedant Kumar <vsk@apple.com>
Fri, 4 Aug 2017 00:36:24 +0000 (00:36 +0000)
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

test/tools/llvm-cov/Inputs/deferred-regions.covmapping [new file with mode: 0644]
test/tools/llvm-cov/Inputs/deferred-regions.profdata [new file with mode: 0644]
test/tools/llvm-cov/deferred-region.cpp [new file with mode: 0644]
test/tools/llvm-cov/showLineExecutionCounts.cpp
tools/llvm-cov/SourceCoverageView.cpp
tools/llvm-cov/SourceCoverageView.h

diff --git a/test/tools/llvm-cov/Inputs/deferred-regions.covmapping b/test/tools/llvm-cov/Inputs/deferred-regions.covmapping
new file mode 100644 (file)
index 0000000..757f3ee
Binary files /dev/null and b/test/tools/llvm-cov/Inputs/deferred-regions.covmapping differ
diff --git a/test/tools/llvm-cov/Inputs/deferred-regions.profdata b/test/tools/llvm-cov/Inputs/deferred-regions.profdata
new file mode 100644 (file)
index 0000000..0bacac0
Binary files /dev/null and b/test/tools/llvm-cov/Inputs/deferred-regions.profdata differ
diff --git a/test/tools/llvm-cov/deferred-region.cpp b/test/tools/llvm-cov/deferred-region.cpp
new file mode 100644 (file)
index 0000000..3704d0e
--- /dev/null
@@ -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;
+}
index bdb0e1bd3a90165ea70b1442f003c004dd6ac02f..4bc3ad5dfdc0a2aa4c8b5310b2a026e978416930 100644 (file)
@@ -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: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>int main() {
 // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>  int x = 0
 // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>
-// HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='uncovered-line'><pre>0</pre></td><td class='code'><pre>  if (x) {
+// HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>  if (x) {
 // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='uncovered-line'><pre>0</pre></td><td class='code'><pre>
 // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre><span class='red'>  }</span>
 // HTML: <td class='line-number'><a name='L[[@LINE-44]]' href='#L[[@LINE-44]]'><pre>[[@LINE-44]]</pre></a></td><td class='covered-line'><pre>161</pre></td><td class='code'><pre>    x = 1;
index 52b8ff1747fe57f5f597e43beca7bceffe8697c9..6630f3333bd98a58f3f441af7a7526471e043e93 100644 (file)
@@ -83,6 +83,41 @@ CoveragePrinter::create(const CoverageViewOptions &Opts) {
   llvm_unreachable("Unknown coverage output format!");
 }
 
+LineCoverageStats::LineCoverageStats(
+    ArrayRef<const coverage::CoverageSegment *> 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);
 
index 9cb608fed6081ae81b671c0dc6be20219a4737c2..c9f0c57b5cb2f1395bbc1dd3356ddd234e297e89 100644 (file)
@@ -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<const coverage::CoverageSegment *> 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.