From 01d19f706ceb293d0449d80d0db7389cc509aea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marja=20H=C3=B6ltt=C3=A4?= Date: Tue, 11 Apr 2017 09:35:23 +0200 Subject: [PATCH] Merged: Squashed multiple commits. Merged: [parser] Fix crash when lazy arrow func params contain destructuring assignments. Revision: bc39a5148a3824ea948fd7725674945ca0b1c56a Merged: [parser] don't rewrite destructuring assignments in params for lazy top level arrow functions Revision: 5f782db9541eca72dc6cbe3c6f5e8dc52ff7c16f BUG=chromium:704811,chromium:706234,chromium:706761,v8:6182 LOG=N NOTRY=true NOPRESUBMIT=true NOTREECHECKS=true Change-Id: If5c04c3b9f6ac9c6879052b6a34446f895624200 Reviewed-on: https://chromium-review.googlesource.com/474746 Reviewed-by: Michael Achenbach Cr-Commit-Position: refs/branch-heads/5.8@{#58} Cr-Branched-From: eda659cc5e307f20ac1ad542ba12ab32eaf4c7ef-refs/heads/5.8.283@{#1} Cr-Branched-From: 4310cd02d2160b1457baed81a2f40063eb264a21-refs/heads/master@{#43429} --- src/ast/scopes.cc | 6 +- src/parsing/parser-base.h | 35 +++++++++- src/parsing/parser.cc | 9 ++- test/mjsunit/regress/regress-704811.js | 88 ++++++++++++++++++++++++ test/mjsunit/regress/regress-706234-2.js | 37 ++++++++++ test/mjsunit/regress/regress-706234.js | 8 +++ 6 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 test/mjsunit/regress/regress-704811.js create mode 100644 test/mjsunit/regress/regress-706234-2.js create mode 100644 test/mjsunit/regress/regress-706234.js diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc index 2bc9b3af64a..225793c7bbc 100644 --- a/src/ast/scopes.cc +++ b/src/ast/scopes.cc @@ -1430,7 +1430,11 @@ void DeclarationScope::ResetAfterPreparsing(AstValueFactory* ast_value_factory, DCHECK(is_function_scope()); // Reset all non-trivial members. - params_.Clear(); + if (!aborted || !IsArrowFunction(function_kind_)) { + // Do not remove parameters when lazy parsing an Arrow Function has failed, + // as the formal parameters are not re-parsed. + params_.Clear(); + } decls_.Clear(); locals_.Clear(); inner_scope_ = nullptr; diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h index 869fd2efbd5..cf56c53a8e9 100644 --- a/src/parsing/parser-base.h +++ b/src/parsing/parser-base.h @@ -395,6 +395,17 @@ class ParserBase { return scope()->promise_var(); } + void RewindDestructuringAssignments(int pos) { + destructuring_assignments_to_rewrite_.Rewind(pos); + } + + void SetDestructuringAssignmentsScope(int pos, Scope* scope) { + for (int i = pos; i < destructuring_assignments_to_rewrite_.length(); + ++i) { + destructuring_assignments_to_rewrite_[i].scope = scope; + } + } + const ZoneList& destructuring_assignments_to_rewrite() const { return destructuring_assignments_to_rewrite_; @@ -1138,9 +1149,14 @@ class ParserBase { ExpressionT ParseMemberExpression(bool* is_async, bool* ok); ExpressionT ParseMemberExpressionContinuation(ExpressionT expression, bool* is_async, bool* ok); + + // `rewritable_length`: length of the destructuring_assignments_to_rewrite() + // queue in the parent function state, prior to parsing of formal parameters. + // If the arrow function is lazy, any items added during formal parameter + // parsing are removed from the queue. ExpressionT ParseArrowFunctionLiteral(bool accept_IN, const FormalParametersT& parameters, - bool* ok); + int rewritable_length, bool* ok); void ParseAsyncFunctionBody(Scope* scope, StatementListT body, FunctionKind kind, FunctionBodyType type, bool accept_IN, int pos, bool* ok); @@ -2679,6 +2695,8 @@ ParserBase::ParseAssignmentExpression(bool accept_IN, bool* ok) { this, classifier()->duplicate_finder()); Scope::Snapshot scope_snapshot(scope()); + int rewritable_length = + function_state_->destructuring_assignments_to_rewrite().length(); bool is_async = peek() == Token::ASYNC && !scanner()->HasAnyLineTerminatorAfterNext() && @@ -2732,6 +2750,7 @@ ParserBase::ParseAssignmentExpression(bool accept_IN, bool* ok) { this->scope()->PropagateUsageFlagsToScope(scope); scope_snapshot.Reparent(scope); + function_state_->SetDestructuringAssignmentsScope(rewritable_length, scope); FormalParametersT parameters(scope); if (!classifier()->is_simple_parameter_list()) { @@ -2748,7 +2767,8 @@ ParserBase::ParseAssignmentExpression(bool accept_IN, bool* ok) { if (duplicate_loc.IsValid()) { classifier()->RecordDuplicateFormalParameterError(duplicate_loc); } - expression = ParseArrowFunctionLiteral(accept_IN, parameters, CHECK_OK); + expression = ParseArrowFunctionLiteral(accept_IN, parameters, + rewritable_length, CHECK_OK); impl()->Discard(); classifier()->RecordPatternError(arrow_loc, MessageTemplate::kUnexpectedToken, @@ -4097,7 +4117,8 @@ bool ParserBase::IsTrivialExpression() { template typename ParserBase::ExpressionT ParserBase::ParseArrowFunctionLiteral( - bool accept_IN, const FormalParametersT& formal_parameters, bool* ok) { + bool accept_IN, const FormalParametersT& formal_parameters, + int rewritable_length, bool* ok) { const RuntimeCallStats::CounterId counters[2][2] = { {&RuntimeCallStats::ParseBackgroundArrowFunctionLiteral, &RuntimeCallStats::ParseArrowFunctionLiteral}, @@ -4228,6 +4249,14 @@ ParserBase::ParseArrowFunctionLiteral( } impl()->CheckConflictingVarDeclarations(formal_parameters.scope, CHECK_OK); + if (is_lazy_top_level_function) { + FunctionState* parent_state = function_state.outer(); + DCHECK_NOT_NULL(parent_state); + DCHECK_GE(parent_state->destructuring_assignments_to_rewrite().length(), + rewritable_length); + parent_state->RewindDestructuringAssignments(rewritable_length); + } + impl()->RewriteDestructuringAssignments(); } diff --git a/src/parsing/parser.cc b/src/parsing/parser.cc index b5241dd6768..cc6b6a260b2 100644 --- a/src/parsing/parser.cc +++ b/src/parsing/parser.cc @@ -880,6 +880,8 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info, scope->set_start_position(info->start_position()); ExpressionClassifier formals_classifier(this); ParserFormalParameters formals(scope); + int rewritable_length = + function_state.destructuring_assignments_to_rewrite().length(); Checkpoint checkpoint(this); { // Parsing patterns as variable reference expression creates @@ -916,7 +918,8 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info, // Pass `accept_IN=true` to ParseArrowFunctionLiteral --- This should // not be observable, or else the preparser would have failed. - Expression* expression = ParseArrowFunctionLiteral(true, formals, &ok); + Expression* expression = + ParseArrowFunctionLiteral(true, formals, rewritable_length, &ok); if (ok) { // Scanning must end at the same position that was recorded // previously. If not, parsing has been interrupted due to a stack @@ -929,6 +932,10 @@ FunctionLiteral* Parser::DoParseFunction(ParseInfo* info, // must produce a FunctionLiteral. DCHECK(expression->IsFunctionLiteral()); result = expression->AsFunctionLiteral(); + // Rewrite destructuring assignments in the parameters. (The ones + // inside the function body are rewritten by + // ParseArrowFunctionLiteral.) + RewriteDestructuringAssignments(); } else { ok = false; } diff --git a/test/mjsunit/regress/regress-704811.js b/test/mjsunit/regress/regress-704811.js new file mode 100644 index 00000000000..dcdeb4e930f --- /dev/null +++ b/test/mjsunit/regress/regress-704811.js @@ -0,0 +1,88 @@ +// Copyright 2017 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. + +// The bug was that destructuring assignments which occur inside a lazy arrow +// function parameter list were not rewritten. + +// Repro from the bug (slightly modified so that it doesn't produce a run-time +// exception). +(({x = {} = {}}) => {})({}); + +// ... and without the parens. +let a0 = ({x = {} = {}}) => {}; +a0({}); + +// Testing that the destructuring assignments also work properly. The semantics +// are: The value of the destructuring assignment is an object {myprop: 2115} +// and 2115 also gets assigned to global_side_assignment. So the default value +// for x is {myprop: 2115}. This is the value which x will have if the function +// is called with an object which doesn't have property x. +let called = false; +let global_side_assignment = undefined; +(({x = {myprop: global_side_assignment} = {myprop: 2115}}) => { + assertTrue('myprop' in x); + assertEquals(2115, x.myprop); + called = true; +})({}); +assertTrue(called); +assertEquals(2115, global_side_assignment); + +// If the parameter is an object which has property x, the default value is not +// used. +called = false; +global_side_assignment = undefined; +(({x = {myprop: global_side_assignment} = {myprop: 2115}}) => { + assertEquals(3000, x); + called = true; +})({x: 3000}); +assertTrue(called); +// Global side assignment doesn't happen, since the default value was not used. +assertEquals(undefined, global_side_assignment); + +// Different kinds of lazy arrow functions (it's actually a bit weird that the +// above functions are lazy, since they are parenthesized). +called = false; +global_side_assignment = undefined; +let a1 = ({x = {myprop: global_side_assignment} = {myprop: 2115}}) => { + assertTrue('myprop' in x); + assertEquals(2115, x.myprop); + called = true; +} +a1({}); +assertTrue(called); +assertEquals(2115, global_side_assignment); + +called = false; +global_side_assignment = undefined; +let a2 = ({x = {myprop: global_side_assignment} = {myprop: 2115}}) => { + assertEquals(3000, x); + called = true; +} +a2({x: 3000}); +assertTrue(called); +assertEquals(undefined, global_side_assignment); + +// We never had a problem with non-arrow functions, but testing them too for +// completeness. +called = false; +global_side_assignment = undefined; +function f1({x = {myprop: global_side_assignment} = {myprop: 2115}}) { + assertTrue('myprop' in x); + assertEquals(2115, x.myprop); + assertEquals(2115, global_side_assignment); + called = true; +} +f1({}); +assertTrue(called); +assertEquals(2115, global_side_assignment); + +called = false; +global_side_assignment = undefined; +function f2({x = {myprop: global_side_assignment} = {myprop: 2115}}) { + assertEquals(3000, x); + called = true; +} +f2({x: 3000}); +assertTrue(called); +assertEquals(undefined, global_side_assignment); diff --git a/test/mjsunit/regress/regress-706234-2.js b/test/mjsunit/regress/regress-706234-2.js new file mode 100644 index 00000000000..ed5facf7f44 --- /dev/null +++ b/test/mjsunit/regress/regress-706234-2.js @@ -0,0 +1,37 @@ +// Copyright 2017 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. + +// Lazy top-level arrow function which must be re-parsed and eagerly compiled. +var f = ({ x } = { x: 1 }) => { + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; + x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x;x; +}; + +f(); diff --git a/test/mjsunit/regress/regress-706234.js b/test/mjsunit/regress/regress-706234.js new file mode 100644 index 00000000000..aee31ae8330 --- /dev/null +++ b/test/mjsunit/regress/regress-706234.js @@ -0,0 +1,8 @@ +// Copyright 2017 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. + +var fn = ({foo = {} = {}}) => { return foo; } +if (true) { + fn({}); +}