deps: V8: cherry-pick deac757

Original commit message: [debugger] Fix code coverage for break/return inside switch-case Case statements have a list of statements associated with them, but are not blocks, and were hence not fixed-up correctly for code coverage. This CL also applies the fix-up to the "body" of case statements, in this way removing ranges reported as uncovered between the final break/return in a case and the next case (or end of function). Drive-by: Add optional pretty printing to code coverage test results. Change-Id: I5f4002d4e17b7253ed516d99f7c389ab2264be10 Bug: v8:9705 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1798426 Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Jakob Gruber <[email protected]> Commit-Queue: Sigurd Schneider <[email protected]> Cr-Commit-Position: refs/heads/master@{#63719} Refs: v8/v8@deac757 PR-URL: #29626 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
nodejs · Sep 25, 2019 · c718c60 · c718c60
1 parent e4a51ad
commit c718c60
Show file tree

Hide file tree

Showing 5 changed files with 81 additions and 12 deletions.
diff --git a/common.gypi b/common.gypi
@@ -38,7 +38,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.11',
+    'v8_embedder_string': '-node.12',
 
     ##### V8 defaults for Node.js #####
 

diff --git a/deps/v8/src/ast/source-range-ast-visitor.cc b/deps/v8/src/ast/source-range-ast-visitor.cc
@@ -25,6 +25,14 @@ void SourceRangeAstVisitor::VisitBlock(Block* stmt) {
   }
 }
 
+void SourceRangeAstVisitor::VisitSwitchStatement(SwitchStatement* stmt) {
+  AstTraversalVisitor::VisitSwitchStatement(stmt);
+  ZonePtrList<CaseClause>* clauses = stmt->cases();
+  for (CaseClause* clause : *clauses) {
+    MaybeRemoveLastContinuationRange(clause->statements());
+  }
+}
+
 void SourceRangeAstVisitor::VisitFunctionLiteral(FunctionLiteral* expr) {
   AstTraversalVisitor::VisitFunctionLiteral(expr);
   ZonePtrList<Statement>* stmts = expr->body();

diff --git a/deps/v8/src/ast/source-range-ast-visitor.h b/deps/v8/src/ast/source-range-ast-visitor.h
@@ -34,6 +34,7 @@ class SourceRangeAstVisitor final
   friend class AstTraversalVisitor<SourceRangeAstVisitor>;
 
   void VisitBlock(Block* stmt);
+  void VisitSwitchStatement(SwitchStatement* stmt);
   void VisitFunctionLiteral(FunctionLiteral* expr);
   bool VisitNode(AstNode* node);
 

diff --git a/deps/v8/test/mjsunit/code-coverage-block.js b/deps/v8/test/mjsunit/code-coverage-block.js
@@ -434,8 +434,8 @@ TestCoverage(
 `,
 [{"start":0,"end":399,"count":1},
  {"start":1,"end":351,"count":1},
- {"start":154,"end":204,"count":0},
- {"start":226,"end":350,"count":0}]
+ {"start":154,"end":176,"count":0},
+ {"start":254,"end":276,"count":0}]
 );
 
 TestCoverage(
@@ -464,8 +464,8 @@ TestCoverage(
 `,
 [{"start":0,"end":999,"count":1},
  {"start":1,"end":951,"count":1},
- {"start":152,"end":202,"count":0},
- {"start":285,"end":353,"count":0}]
+ {"start":152,"end":168,"count":0},
+ {"start":287,"end":310,"count":0}]
 );
 
 TestCoverage(
@@ -1052,4 +1052,49 @@ try {                                     // 0500
  {"start":69,"end":153,"count":1}]
 );
 
+TestCoverage(
+"https://crbug.com/v8/9705",
+`
+function f(x) {                           // 0000
+  switch (x) {                            // 0050
+    case 40: nop();                       // 0100
+    case 41: nop(); return 1;             // 0150
+    case 42: nop(); break;                // 0200
+  }                                       // 0250
+  return 3;                               // 0300
+};                                        // 0350
+f(40);                                    // 0400
+f(41);                                    // 0450
+f(42);                                    // 0500
+f(43);                                    // 0550
+`,
+[{"start":0,"end":599,"count":1},
+ {"start":0,"end":351,"count":4},
+ {"start":104,"end":119,"count":1},
+ {"start":154,"end":179,"count":2},
+ {"start":204,"end":226,"count":1},
+ {"start":253,"end":350,"count":2}]
+);
+
+TestCoverage(
+"https://crbug.com/v8/9705",
+`
+function f(x) {                           // 0000
+  switch (x) {                            // 0050
+    case 40: nop();                       // 0100
+    case 41: nop(); return 1;             // 0150
+    case 42: nop(); break;                // 0200
+  }                                       // 0250
+  return 3;                               // 0300
+};                                        // 0350
+f(42);                                    // 0400
+f(43);                                    // 0450
+`,
+[{"start":0,"end":499,"count":1},
+ {"start":0,"end":351,"count":2},
+ {"start":104,"end":119,"count":0},
+ {"start":154,"end":179,"count":0},
+ {"start":204,"end":226,"count":1}]
+);
+
 %DebugToggleBlockCoverage(false);
diff --git a/deps/v8/test/mjsunit/code-coverage-utils.js b/deps/v8/test/mjsunit/code-coverage-utils.js
@@ -18,25 +18,40 @@ let gen;
     return undefined;
   };
 
-  function TestCoverageInternal(name, source, expectation, collect_garbage) {
+  function TestCoverageInternal(
+      name, source, expectation, collect_garbage, prettyPrintResults) {
     source = source.trim();
     eval(source);
     if (collect_garbage) %CollectGarbage("collect dead objects");
     var covfefe = GetCoverage(source);
     var stringified_result = JSON.stringify(covfefe);
     var stringified_expectation = JSON.stringify(expectation);
-    if (stringified_result != stringified_expectation) {
-      print(stringified_result.replace(/[}],[{]/g, "},\n {"));
+    const mismatch = stringified_result != stringified_expectation;
+    if (mismatch) {
+      console.log(stringified_result.replace(/[}],[{]/g, "},\n {"));
+    }
+    if (prettyPrintResults) {
+      console.log("=== Coverage Expectation ===")
+      for (const {start,end,count} of expectation) {
+        console.log(`Range [${start}, ${end}) (count: ${count})`);
+        console.log(source.substring(start, end));
+      }
+      console.log("=== Coverage Results ===")
+      for (const {start,end,count} of covfefe) {
+        console.log(`Range [${start}, ${end}) (count: ${count})`);
+        console.log(source.substring(start, end));
+      }
+      console.log("========================")
     }
     assertEquals(stringified_expectation, stringified_result, name + " failed");
   };
 
-  TestCoverage = function(name, source, expectation) {
-    TestCoverageInternal(name, source, expectation, true);
+  TestCoverage = function(name, source, expectation, prettyPrintResults) {
+    TestCoverageInternal(name, source, expectation, true, prettyPrintResults);
   };
 
-  TestCoverageNoGC = function(name, source, expectation) {
-    TestCoverageInternal(name, source, expectation, false);
+  TestCoverageNoGC = function(name, source, expectation, prettyPrintResults) {
+    TestCoverageInternal(name, source, expectation, false, prettyPrintResults);
   };
 
   nop = function() {};