From c43f5be7cf7d22d56ce0a905238f7c2fe3f5db95 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Thu, 19 Sep 2019 19:50:23 -0700 Subject: [PATCH] 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 Reviewed-by: Jakob Gruber Commit-Queue: Sigurd Schneider Cr-Commit-Position: refs/heads/master@{#63719} Refs: https://github.com/v8/v8/commit/deac757bc76d207845375340bcea090c774ad9d0 Backport-PR-URL: https://github.com/nodejs/node/pull/30109 PR-URL: https://github.com/nodejs/node/pull/29626 Reviewed-By: Rich Trott Reviewed-By: Richard Lau Reviewed-By: Shelley Vohr Reviewed-By: Anna Henningsen --- common.gypi | 2 +- deps/v8/src/ast/source-range-ast-visitor.cc | 8 ++++ deps/v8/src/ast/source-range-ast-visitor.h | 1 + deps/v8/test/mjsunit/code-coverage-block.js | 53 +++++++++++++++++++-- deps/v8/test/mjsunit/code-coverage-utils.js | 29 ++++++++--- 5 files changed, 81 insertions(+), 12 deletions(-) diff --git a/common.gypi b/common.gypi index d90daf84abaa22..c1445595209fc7 100644 --- 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.9', + 'v8_embedder_string': '-node.10', ##### 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 index d171e30587584f..2fcf151999ace0 100644 --- 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* clauses = stmt->cases(); + for (CaseClause* clause : *clauses) { + MaybeRemoveLastContinuationRange(clause->statements()); + } +} + void SourceRangeAstVisitor::VisitFunctionLiteral(FunctionLiteral* expr) { AstTraversalVisitor::VisitFunctionLiteral(expr); ZonePtrList* 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 index 4ea36a947f58e6..4ba5feb2d299f9 100644 --- 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; 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 index c441342cdfbeed..a7bad5bf11fbc4 100644 --- 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 index 57833902220b7f..4164f5a314f157 100644 --- 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() {};