[coverage] Fix invalid coverage block transformation

Before reporting coverage data, we attempt to reduce clutter by
merging nested and consecutive ranges. Nested ranges are merged, if
the child range has the same execution count as the parent range.
Sibling ranges are merged, if one sibling begins where the other ends
and execution counts are identical.

This allowed an invalid transformation in which a range with an
execution count of 1 would be merged into the parent change, but the
sibling range with identical start and end points and a count of 0
would remain, effectively deleting the covered range.

For example:

{start: 0, end: 10, count: 1},
{start: 5, end:  8, count: 1},  // It's invalid to remove this.
{start: 5, end:  8, count: 0}

The fix is to separate the parent and sibling merge passes, and
removing duplicate ranges in-between.

Bug: chromium:827530
Change-Id: Ic35eae1d4a106746570ce9cb412ed6710ef6da53
Reviewed-on: https://chromium-review.googlesource.com/992114Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52352}

src/debug/debug-coverage.cc

@@ -67,6 +67,11 @@ bool CompareCoverageBlock(const CoverageBlock& a, const CoverageBlock& b) {
   return a.start < b.start;
 }
 
+void SortBlockData(std::vector<CoverageBlock>& v) {
+  // Sort according to the block nesting structure.
+  std::sort(v.begin(), v.end(), CompareCoverageBlock);
+}
+
 std::vector<CoverageBlock> GetSortedBlockData(Isolate* isolate,
                                               SharedFunctionInfo* shared) {
   DCHECK(shared->HasCoverageInfo());
@@ -86,8 +91,7 @@ std::vector<CoverageBlock> GetSortedBlockData(Isolate* isolate,
     result.emplace_back(start_pos, until_pos, count);
   }
 
-  // Sort according to the block nesting structure.
-  std::sort(result.begin(), result.end(), CompareCoverageBlock);
+  SortBlockData(result);
 
   return result;
 }
@@ -243,6 +247,21 @@ void MergeDuplicateSingletons(CoverageFunction* function) {
   }
 }
 
+void MergeDuplicateRanges(CoverageFunction* function) {
+  CoverageBlockIterator iter(function);
+
+  while (iter.Next() && iter.HasNext()) {
+    CoverageBlock& block = iter.GetBlock();
+    CoverageBlock& next_block = iter.GetNextBlock();
+
+    if (!HaveSameSourceRange(block, next_block)) continue;
+
+    DCHECK_NE(kNoSourcePosition, block.end);  // Non-singleton range.
+    next_block.count = std::max(block.count, next_block.count);
+    iter.DeleteBlock();
+  }
+}
+
 // Rewrite position singletons (produced by unconditional control flow
 // like return statements, and by continuation counters) into source
 // ranges that end at the next sibling range or the end of the parent
@@ -274,16 +293,13 @@ void RewritePositionSingletonsToRanges(CoverageFunction* function) {
   }
 }
 
-void MergeNestedAndConsecutiveRanges(CoverageFunction* function) {
+void MergeConsecutiveRanges(CoverageFunction* function) {
   CoverageBlockIterator iter(function);
 
   while (iter.Next()) {
     CoverageBlock& block = iter.GetBlock();
-    CoverageBlock& parent = iter.GetParent();
 
-    if (parent.count == block.count) {
-      iter.DeleteBlock();
-    } else if (iter.HasSiblingOrChild()) {
+    if (iter.HasSiblingOrChild()) {
       CoverageBlock& sibling = iter.GetSiblingOrChild();
       if (sibling.start == block.end && sibling.count == block.count) {
         // Best-effort: this pass may miss mergeable siblings in the presence of
@@ -295,6 +311,21 @@ void MergeNestedAndConsecutiveRanges(CoverageFunction* function) {
   }
 }
 
+void MergeNestedRanges(CoverageFunction* function) {
+  CoverageBlockIterator iter(function);
+
+  while (iter.Next()) {
+    CoverageBlock& block = iter.GetBlock();
+    CoverageBlock& parent = iter.GetParent();
+
+    if (parent.count == block.count) {
+      // Transformation may not be valid if sibling blocks exist with a
+      // differing count.
+      iter.DeleteBlock();
+    }
+  }
+}
+
 void FilterUncoveredRanges(CoverageFunction* function) {
   CoverageBlockIterator iter(function);
 
@@ -373,7 +404,15 @@ void CollectBlockCoverage(Isolate* isolate, CoverageFunction* function,
   RewritePositionSingletonsToRanges(function);
 
   // Merge nested and consecutive ranges with identical counts.
-  MergeNestedAndConsecutiveRanges(function);
+  // Note that it's necessary to merge duplicate ranges prior to merging nested
+  // changes in order to avoid invalid transformations. See crbug.com/827530.
+  MergeConsecutiveRanges(function);
+
+  SortBlockData(function->blocks);
+  MergeDuplicateRanges(function);
+  MergeNestedRanges(function);
+
+  MergeConsecutiveRanges(function);
 
   // Filter out ranges with count == 0 unless the immediate parent range has
   // a count != 0.
@@ -382,7 +421,6 @@ void CollectBlockCoverage(Isolate* isolate, CoverageFunction* function,
   // Filter out ranges of zero length.
   FilterEmptyRanges(function);
 
-
   // Reset all counters on the DebugInfo to zero.
   ResetAllBlockCounts(info);
 }

test/mjsunit/code-coverage-block.js

@@ -831,4 +831,23 @@ const i = d || c || (c ? 'left' : 'right')// 0400
  {"start":423,"end":431,"count":0}
 ]);
 
+TestCoverage(
+"https://crbug.com/827530",
+`
+Util = {};                                // 0000
+Util.escape = function UtilEscape(str) {  // 0050
+  if (!str) {                             // 0100
+    return 'if';                          // 0150
+  } else {                                // 0200
+    return 'else';                        // 0250
+  }                                       // 0300
+};                                        // 0350
+Util.escape("foo.bar");                   // 0400
+`,
+[{"start":0,"end":449,"count":1},
+ {"start":64,"end":351,"count":1},
+ {"start":112,"end":203,"count":0},
+ {"start":303,"end":350,"count":0}]
+);
+
 %DebugToggleBlockCoverage(false);