Do not assign positions to parser-generated desugarings.
The root cause for the bug is that the positions assigned to desugared code was inconsistent with the source ranges of block scopes. Since the fact that the position is assigned causes the debugger to break at the parser-generated statement, the fix is to remove positions from those nodes that we do not want to break on. The CL also teaches Hydrogen to tolerate these cases. R=adamk@chromium.org,rossberg@chromium.org BUG=chromium:468661 LOG=Y Review URL: https://codereview.chromium.org/1032653002 Cr-Commit-Position: refs/heads/master@{#27424}
This commit is contained in:
parent
5c47c1c0d3
commit
49c3a60651
@ -1868,8 +1868,10 @@ class HGraphBuilder {
|
|||||||
|
|
||||||
protected:
|
protected:
|
||||||
void SetSourcePosition(int position) {
|
void SetSourcePosition(int position) {
|
||||||
DCHECK(position != RelocInfo::kNoPosition);
|
if (position != RelocInfo::kNoPosition) {
|
||||||
position_.set_position(position - start_position_);
|
position_.set_position(position - start_position_);
|
||||||
|
}
|
||||||
|
// Otherwise position remains unknown.
|
||||||
}
|
}
|
||||||
|
|
||||||
void EnterInlinedSource(int start_position, int id) {
|
void EnterInlinedSource(int start_position, int id) {
|
||||||
|
@ -3201,7 +3201,6 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
|
|
||||||
Block* inner_block = factory()->NewBlock(NULL, names->length() + 4, false,
|
Block* inner_block = factory()->NewBlock(NULL, names->length() + 4, false,
|
||||||
RelocInfo::kNoPosition);
|
RelocInfo::kNoPosition);
|
||||||
int pos = scanner()->location().beg_pos;
|
|
||||||
ZoneList<Variable*> inner_vars(names->length(), zone());
|
ZoneList<Variable*> inner_vars(names->length(), zone());
|
||||||
|
|
||||||
// For each let variable x:
|
// For each let variable x:
|
||||||
@ -3214,8 +3213,9 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
Declare(declaration, true, CHECK_OK);
|
Declare(declaration, true, CHECK_OK);
|
||||||
inner_vars.Add(declaration->proxy()->var(), zone());
|
inner_vars.Add(declaration->proxy()->var(), zone());
|
||||||
VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
|
VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
|
||||||
Assignment* assignment = factory()->NewAssignment(
|
Assignment* assignment =
|
||||||
is_const ? Token::INIT_CONST : Token::INIT_LET, proxy, temp_proxy, pos);
|
factory()->NewAssignment(is_const ? Token::INIT_CONST : Token::INIT_LET,
|
||||||
|
proxy, temp_proxy, RelocInfo::kNoPosition);
|
||||||
Statement* assignment_statement =
|
Statement* assignment_statement =
|
||||||
factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
|
factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
|
||||||
proxy->var()->set_initializer_position(init->position());
|
proxy->var()->set_initializer_position(init->position());
|
||||||
@ -3230,8 +3230,8 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
{
|
{
|
||||||
Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
|
Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
|
||||||
VariableProxy* first_proxy = factory()->NewVariableProxy(first);
|
VariableProxy* first_proxy = factory()->NewVariableProxy(first);
|
||||||
compare =
|
compare = factory()->NewCompareOperation(Token::EQ, first_proxy, const1,
|
||||||
factory()->NewCompareOperation(Token::EQ, first_proxy, const1, pos);
|
RelocInfo::kNoPosition);
|
||||||
}
|
}
|
||||||
Statement* clear_first = NULL;
|
Statement* clear_first = NULL;
|
||||||
// Make statement: first = 0.
|
// Make statement: first = 0.
|
||||||
@ -3243,8 +3243,8 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
clear_first =
|
clear_first =
|
||||||
factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
|
factory()->NewExpressionStatement(assignment, RelocInfo::kNoPosition);
|
||||||
}
|
}
|
||||||
Statement* clear_first_or_next = factory()->NewIfStatement(
|
Statement* clear_first_or_next =
|
||||||
compare, clear_first, next, RelocInfo::kNoPosition);
|
factory()->NewIfStatement(compare, clear_first, next, next->position());
|
||||||
inner_block->AddStatement(clear_first_or_next, zone());
|
inner_block->AddStatement(clear_first_or_next, zone());
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -3265,8 +3265,8 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
{
|
{
|
||||||
Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
|
Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
|
||||||
VariableProxy* flag_proxy = factory()->NewVariableProxy(flag);
|
VariableProxy* flag_proxy = factory()->NewVariableProxy(flag);
|
||||||
flag_cond =
|
flag_cond = factory()->NewCompareOperation(Token::EQ, flag_proxy, const1,
|
||||||
factory()->NewCompareOperation(Token::EQ, flag_proxy, const1, pos);
|
RelocInfo::kNoPosition);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Create chain of expressions "flag = 0, temp_x = x, ..."
|
// Create chain of expressions "flag = 0, temp_x = x, ..."
|
||||||
@ -3282,9 +3282,11 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Make the comma-separated list of temp_x = x assignments.
|
// Make the comma-separated list of temp_x = x assignments.
|
||||||
|
int inner_var_proxy_pos = scanner()->location().beg_pos;
|
||||||
for (int i = 0; i < names->length(); i++) {
|
for (int i = 0; i < names->length(); i++) {
|
||||||
VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
|
VariableProxy* temp_proxy = factory()->NewVariableProxy(temps.at(i));
|
||||||
VariableProxy* proxy = factory()->NewVariableProxy(inner_vars.at(i), pos);
|
VariableProxy* proxy =
|
||||||
|
factory()->NewVariableProxy(inner_vars.at(i), inner_var_proxy_pos);
|
||||||
Assignment* assignment = factory()->NewAssignment(
|
Assignment* assignment = factory()->NewAssignment(
|
||||||
Token::ASSIGN, temp_proxy, proxy, RelocInfo::kNoPosition);
|
Token::ASSIGN, temp_proxy, proxy, RelocInfo::kNoPosition);
|
||||||
compound_next = factory()->NewBinaryOperation(
|
compound_next = factory()->NewBinaryOperation(
|
||||||
@ -3318,8 +3320,8 @@ Statement* Parser::DesugarLexicalBindingsInForStatement(
|
|||||||
{
|
{
|
||||||
Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
|
Expression* const1 = factory()->NewSmiLiteral(1, RelocInfo::kNoPosition);
|
||||||
VariableProxy* flag_proxy = factory()->NewVariableProxy(flag);
|
VariableProxy* flag_proxy = factory()->NewVariableProxy(flag);
|
||||||
compare =
|
compare = factory()->NewCompareOperation(Token::EQ, flag_proxy, const1,
|
||||||
factory()->NewCompareOperation(Token::EQ, flag_proxy, const1, pos);
|
RelocInfo::kNoPosition);
|
||||||
}
|
}
|
||||||
Statement* stop =
|
Statement* stop =
|
||||||
factory()->NewBreakStatement(outer_loop, RelocInfo::kNoPosition);
|
factory()->NewBreakStatement(outer_loop, RelocInfo::kNoPosition);
|
||||||
|
75
test/mjsunit/es6/regress/regress-468661.js
Normal file
75
test/mjsunit/es6/regress/regress-468661.js
Normal file
@ -0,0 +1,75 @@
|
|||||||
|
// Copyright 2015 the V8 project authors. All rights reserved.
|
||||||
|
// Use of this source code is governed by a BSD-style license that can be
|
||||||
|
// found in the LICENSE file.
|
||||||
|
|
||||||
|
// Flags: --expose-debug-as debug
|
||||||
|
|
||||||
|
Debug = debug.Debug
|
||||||
|
var exception = null;
|
||||||
|
var break_count = 0;
|
||||||
|
|
||||||
|
var expected_values =
|
||||||
|
[ReferenceError, ReferenceError, 0, 0, 0, 0, 0, 1, ReferenceError, ReferenceError];
|
||||||
|
|
||||||
|
function listener(event, exec_state, event_data, data) {
|
||||||
|
try {
|
||||||
|
if (event == Debug.DebugEvent.Break) {
|
||||||
|
assertTrue(exec_state.frameCount() != 0, "FAIL: Empty stack trace");
|
||||||
|
// Count number of expected breakpoints in this source file.
|
||||||
|
if (!break_count) {
|
||||||
|
var source_text = exec_state.frame(0).func().script().source();
|
||||||
|
expected_breaks = source_text.match(/\/\/\s*Break\s+\d+\./g).length;
|
||||||
|
print("Expected breaks: " + expected_breaks);
|
||||||
|
}
|
||||||
|
var frameMirror = exec_state.frame(0);
|
||||||
|
|
||||||
|
var v = null;;
|
||||||
|
try {
|
||||||
|
v = frameMirror.evaluate('i').value();
|
||||||
|
} catch(e) {
|
||||||
|
v = e;
|
||||||
|
}
|
||||||
|
|
||||||
|
var source = frameMirror.sourceLineText();
|
||||||
|
print("paused at: " + source);
|
||||||
|
assertTrue(source.indexOf("// Break " + break_count + ".") > 0,
|
||||||
|
"Unexpected pause at: " + source + "\n" +
|
||||||
|
"Expected: // Break " + break_count + ".");
|
||||||
|
if (expected_values[break_count] === ReferenceError) {
|
||||||
|
assertTrue(v instanceof ReferenceError);
|
||||||
|
} else {
|
||||||
|
assertSame(expected_values[break_count], v);
|
||||||
|
}
|
||||||
|
++break_count;
|
||||||
|
|
||||||
|
if (break_count !== expected_breaks) {
|
||||||
|
exec_state.prepareStep(Debug.StepAction.StepIn, 1);
|
||||||
|
print("Next step prepared");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch(e) {
|
||||||
|
exception = e;
|
||||||
|
print(e, e.stack);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
Debug.setListener(listener);
|
||||||
|
|
||||||
|
var sum = 0;
|
||||||
|
(function (){
|
||||||
|
'use strict';
|
||||||
|
|
||||||
|
debugger; // Break 0.
|
||||||
|
|
||||||
|
for (let i=0; // Break 1.
|
||||||
|
i < 1; // Break 2. // Break 3. // Break 6. // Break 7.
|
||||||
|
i++) {
|
||||||
|
let key = i; // Break 4.
|
||||||
|
sum += key; // Break 5.
|
||||||
|
}
|
||||||
|
}()); // Break 8.
|
||||||
|
|
||||||
|
assertNull(exception); // Break 9.
|
||||||
|
assertEquals(expected_breaks, break_count);
|
||||||
|
|
||||||
|
Debug.setListener(null);
|
@ -113,6 +113,7 @@
|
|||||||
'es6/debug-stepnext-for': [PASS, NO_VARIANTS],
|
'es6/debug-stepnext-for': [PASS, NO_VARIANTS],
|
||||||
'es6/debug-evaluate-blockscopes': [PASS, NO_VARIANTS],
|
'es6/debug-evaluate-blockscopes': [PASS, NO_VARIANTS],
|
||||||
'regress/regress-crbug-259300': [PASS, NO_VARIANTS],
|
'regress/regress-crbug-259300': [PASS, NO_VARIANTS],
|
||||||
|
'es6/regress/regress-468661': [PASS, NO_VARIANTS],
|
||||||
|
|
||||||
##############################################################################
|
##############################################################################
|
||||||
# Too slow in debug mode with --stress-opt mode.
|
# Too slow in debug mode with --stress-opt mode.
|
||||||
|
Loading…
Reference in New Issue
Block a user