Skip to content

Commit deac757

Browse files
sigurdschneiderCommit Bot
sigurdschneider
authored and
Commit Bot
committed
[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}
1 parent 8e26b12 commit deac757

4 files changed

Lines changed: 80 additions & 11 deletions

File tree

‎src/ast/source-range-ast-visitor.cc‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ void SourceRangeAstVisitor::VisitBlock(Block* stmt) {
2525
}
2626
}
2727

28+
void SourceRangeAstVisitor::VisitSwitchStatement(SwitchStatement* stmt) {
29+
AstTraversalVisitor::VisitSwitchStatement(stmt);
30+
ZonePtrList<CaseClause>* clauses = stmt->cases();
31+
for (CaseClause* clause : *clauses) {
32+
MaybeRemoveLastContinuationRange(clause->statements());
33+
}
34+
}
35+
2836
void SourceRangeAstVisitor::VisitFunctionLiteral(FunctionLiteral* expr) {
2937
AstTraversalVisitor::VisitFunctionLiteral(expr);
3038
ZonePtrList<Statement>* stmts = expr->body();

‎src/ast/source-range-ast-visitor.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ class SourceRangeAstVisitor final
3434
friend class AstTraversalVisitor<SourceRangeAstVisitor>;
3535

3636
void VisitBlock(Block* stmt);
37+
void VisitSwitchStatement(SwitchStatement* stmt);
3738
void VisitFunctionLiteral(FunctionLiteral* expr);
3839
bool VisitNode(AstNode* node);
3940
void VisitTryCatchStatement(TryCatchStatement* stmt);

‎test/mjsunit/code-coverage-block.js‎

Lines changed: 49 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -419,8 +419,8 @@ TestCoverage(
419419
`,
420420
[{"start":0,"end":399,"count":1},
421421
{"start":1,"end":351,"count":1},
422-
{"start":154,"end":204,"count":0},
423-
{"start":226,"end":350,"count":0}]
422+
{"start":154,"end":176,"count":0},
423+
{"start":254,"end":276,"count":0}]
424424
);
425425

426426
TestCoverage(
@@ -449,8 +449,8 @@ TestCoverage(
449449
`,
450450
[{"start":0,"end":999,"count":1},
451451
{"start":1,"end":951,"count":1},
452-
{"start":152,"end":202,"count":0},
453-
{"start":285,"end":353,"count":0}]
452+
{"start":152,"end":168,"count":0},
453+
{"start":287,"end":310,"count":0}]
454454
);
455455

456456
TestCoverage(
@@ -1023,4 +1023,49 @@ try { // 0500
10231023
{"start":69,"end":153,"count":1}]
10241024
);
10251025

1026+
TestCoverage(
1027+
"https://crbug.com/v8/9705",
1028+
`
1029+
function f(x) { // 0000
1030+
switch (x) { // 0050
1031+
case 40: nop(); // 0100
1032+
case 41: nop(); return 1; // 0150
1033+
case 42: nop(); break; // 0200
1034+
} // 0250
1035+
return 3; // 0300
1036+
}; // 0350
1037+
f(40); // 0400
1038+
f(41); // 0450
1039+
f(42); // 0500
1040+
f(43); // 0550
1041+
`,
1042+
[{"start":0,"end":599,"count":1},
1043+
{"start":0,"end":351,"count":4},
1044+
{"start":104,"end":119,"count":1},
1045+
{"start":154,"end":179,"count":2},
1046+
{"start":204,"end":226,"count":1},
1047+
{"start":253,"end":350,"count":2}]
1048+
);
1049+
1050+
TestCoverage(
1051+
"https://crbug.com/v8/9705",
1052+
`
1053+
function f(x) { // 0000
1054+
switch (x) { // 0050
1055+
case 40: nop(); // 0100
1056+
case 41: nop(); return 1; // 0150
1057+
case 42: nop(); break; // 0200
1058+
} // 0250
1059+
return 3; // 0300
1060+
}; // 0350
1061+
f(42); // 0400
1062+
f(43); // 0450
1063+
`,
1064+
[{"start":0,"end":499,"count":1},
1065+
{"start":0,"end":351,"count":2},
1066+
{"start":104,"end":119,"count":0},
1067+
{"start":154,"end":179,"count":0},
1068+
{"start":204,"end":226,"count":1}]
1069+
);
1070+
10261071
%DebugToggleBlockCoverage(false);

‎test/mjsunit/code-coverage-utils.js‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,25 +18,40 @@ let gen;
1818
return undefined;
1919
};
2020

21-
function TestCoverageInternal(name, source, expectation, collect_garbage) {
21+
function TestCoverageInternal(
22+
name, source, expectation, collect_garbage, prettyPrintResults) {
2223
source = source.trim();
2324
eval(source);
2425
if (collect_garbage) %CollectGarbage("collect dead objects");
2526
var covfefe = GetCoverage(source);
2627
var stringified_result = JSON.stringify(covfefe);
2728
var stringified_expectation = JSON.stringify(expectation);
28-
if (stringified_result != stringified_expectation) {
29-
print(stringified_result.replace(/[}],[{]/g, "},\n {"));
29+
const mismatch = stringified_result != stringified_expectation;
30+
if (mismatch) {
31+
console.log(stringified_result.replace(/[}],[{]/g, "},\n {"));
32+
}
33+
if (prettyPrintResults) {
34+
console.log("=== Coverage Expectation ===")
35+
for (const {start,end,count} of expectation) {
36+
console.log(`Range [${start}, ${end}) (count: ${count})`);
37+
console.log(source.substring(start, end));
38+
}
39+
console.log("=== Coverage Results ===")
40+
for (const {start,end,count} of covfefe) {
41+
console.log(`Range [${start}, ${end}) (count: ${count})`);
42+
console.log(source.substring(start, end));
43+
}
44+
console.log("========================")
3045
}
3146
assertEquals(stringified_expectation, stringified_result, name + " failed");
3247
};
3348

34-
TestCoverage = function(name, source, expectation) {
35-
TestCoverageInternal(name, source, expectation, true);
49+
TestCoverage = function(name, source, expectation, prettyPrintResults) {
50+
TestCoverageInternal(name, source, expectation, true, prettyPrintResults);
3651
};
3752

38-
TestCoverageNoGC = function(name, source, expectation) {
39-
TestCoverageInternal(name, source, expectation, false);
53+
TestCoverageNoGC = function(name, source, expectation, prettyPrintResults) {
54+
TestCoverageInternal(name, source, expectation, false, prettyPrintResults);
4055
};
4156

4257
nop = function() {};

0 commit comments

Comments
 (0)