Skip to content

Commit 471fef0

Browse files
hashseedCommit Bot
hashseed
authored and
Commit Bot
committed
[coverage] change block range to avoid ambiguity.
By moving the block range end to left of closing bracket, we can avoid ambiguity where an open-ended singleton range could be both interpreted as inside the parent range, or next to it. [email protected] Bug: v8:8237 Change-Id: Ibc9412b31efe900b6d8bff0d8fa8c52ddfbf460a Reviewed-on: https://chromium-review.googlesource.com/1254127 Reviewed-by: Georg Neis <[email protected]> Commit-Queue: Yang Guo <[email protected]> Cr-Commit-Position: refs/heads/master@{#56347}
1 parent 4130d61 commit 471fef0

5 files changed

Lines changed: 70 additions & 6 deletions

File tree

‎src/debug/debug-coverage.cc‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ std::vector<CoverageBlock> GetSortedBlockData(SharedFunctionInfo* shared) {
8181
std::vector<CoverageBlock> result;
8282
if (coverage_info->SlotCount() == 0) return result;
8383

84+
if (FLAG_trace_block_coverage) {
85+
PrintF("Collecting coverage data\n");
86+
coverage_info->Print(shared->DebugName()->ToCString());
87+
}
88+
8489
for (int i = 0; i < coverage_info->SlotCount(); i++) {
8590
const int start_pos = coverage_info->StartSourcePosition(i);
8691
const int until_pos = coverage_info->EndSourcePosition(i);

‎src/objects/debug-objects.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ void CoverageInfo::Print(std::unique_ptr<char[]> function_name) {
375375

376376
for (int i = 0; i < SlotCount(); i++) {
377377
os << "{" << StartSourcePosition(i) << "," << EndSourcePosition(i) << "}"
378-
<< std::endl;
378+
<< ": " << BlockCount(i) << std::endl;
379379
}
380380
}
381381

‎src/parsing/parser-base.h‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5093,6 +5093,7 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseBlock(
50935093
Expect(Token::LBRACE, CHECK_OK_CUSTOM(NullStatement));
50945094
{
50955095
BlockState block_state(zone(), &scope_);
5096+
// Scope starts before opening brace.
50965097
scope()->set_start_position(scanner()->location().beg_pos);
50975098
typename Types::Target target(this, body);
50985099

@@ -5104,9 +5105,10 @@ typename ParserBase<Impl>::BlockT ParserBase<Impl>::ParseBlock(
51045105
}
51055106

51065107
Expect(Token::RBRACE, CHECK_OK_CUSTOM(NullStatement));
5107-
int end_pos = end_position();
5108-
scope()->set_end_position(end_pos);
5109-
impl()->RecordBlockSourceRange(body, end_pos);
5108+
// Scope ends after closing brace.
5109+
scope()->set_end_position(scanner()->location().end_pos);
5110+
// Coverage range uses position before closing brace.
5111+
impl()->RecordBlockSourceRange(body, scanner()->location().beg_pos);
51105112
body->set_scope(scope()->FinalizeBlockScope());
51115113
}
51125114
return body;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ TestCoverage(
471471
{"start":472,"end":503,"count":0},
472472
{"start":626,"end":653,"count":0},
473473
{"start":768,"end":803,"count":0},
474-
{"start":867,"end":869,"count":0}]
474+
{"start":867,"end":868,"count":0}]
475475
);
476476

477477
TestCoverage(
@@ -847,7 +847,7 @@ Util.escape("foo.bar"); // 0400
847847
[{"start":0,"end":449,"count":1},
848848
{"start":64,"end":351,"count":1},
849849
{"start":112,"end":203,"count":0},
850-
{"start":303,"end":350,"count":0}]
850+
{"start":268,"end":350,"count":0}]
851851
);
852852

853853
%DebugToggleBlockCoverage(false);

‎test/mjsunit/regress/regress-8237.js‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Copyright 2018 the V8 project authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
// Flags: --allow-natives-syntax --no-always-opt
6+
// Files: test/mjsunit/code-coverage-utils.js
7+
8+
%DebugToggleBlockCoverage(true);
9+
10+
TestCoverage(
11+
"Repro for the bug",
12+
`
13+
function lib (n) { // 0000
14+
if (n >= 0) { // 0050
15+
if (n < 0) { // 0100
16+
return; // 0150
17+
} // 0200
18+
} else if (foo()) { // 0250
19+
} // 0300
20+
} // 0350
21+
function foo () { // 0400
22+
console.log('foo') // 0450
23+
return false // 0500
24+
} // 0550
25+
lib(1) // 0600
26+
`,
27+
[{"start":0,"end":649,"count":1},
28+
{"start":0,"end":351,"count":1},
29+
{"start":115,"end":205,"count":0},
30+
{"start":253,"end":303,"count":0},
31+
{"start":400,"end":551,"count":0}]
32+
);
33+
34+
TestCoverage(
35+
"Variant with omitted brackets",
36+
`
37+
function lib (n) { // 0000
38+
if (n >= 0) { // 0050
39+
if (n < 0) // 0100
40+
return; // 0150
41+
} // 0200
42+
else if (foo()); // 0250
43+
} // 0300
44+
function foo () { // 0350
45+
console.log('foo') // 0400
46+
return false // 0450
47+
} // 0500
48+
lib(1) // 0550
49+
`,
50+
[{"start":0,"end":599,"count":1},
51+
{"start":0,"end":301,"count":1},
52+
{"start":156,"end":163,"count":0},
53+
{"start":203,"end":268,"count":0},
54+
{"start":350,"end":501,"count":0}]
55+
);
56+
57+
%DebugToggleBlockCoverage(false);

0 commit comments

Comments
 (0)