Skip to content

Commit

Permalink
deps: V8: cherry-pick deac757
Browse files Browse the repository at this point in the history
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 <verwaest@chromium.org>
    Reviewed-by: Jakob Gruber <jgruber@chromium.org>
    Commit-Queue: Sigurd Schneider <sigurds@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#63719}

Refs: v8/v8@deac757

PR-URL: nodejs#29626
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
bcoe authored and targos committed Oct 25, 2019
1 parent b85ac9a commit e07885e
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 12 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -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 #####

Expand Down
8 changes: 8 additions & 0 deletions deps/v8/src/ast/source-range-ast-visitor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/ast/source-range-ast-visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
53 changes: 49 additions & 4 deletions deps/v8/test/mjsunit/code-coverage-block.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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);
29 changes: 22 additions & 7 deletions deps/v8/test/mjsunit/code-coverage-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {};
Expand Down

0 comments on commit e07885e

Please sign in to comment.