From ed080e69665c96f9e5e61cf1d0e1ce377fc5aca6 Mon Sep 17 00:00:00 2001 From: marja Date: Thu, 15 Dec 2016 06:26:58 -0800 Subject: [PATCH] Disable lazy parsing inside eval (see bug). If the eval contains a let, we need to know whether an inner function refers to the variable to be able to decide its context allocation status. The added test needs https://codereview.chromium.org/2435023002/ too in order to pass. BUG=v8:5736 Review-Url: https://codereview.chromium.org/2574753002 Cr-Commit-Position: refs/heads/master@{#41723} --- src/ast/scopes.cc | 5 +--- test/mjsunit/bugs/bug-2728.js | 39 ---------------------------- test/mjsunit/regress/regress-5736.js | 23 ++++++++++++++++ 3 files changed, 24 insertions(+), 43 deletions(-) delete mode 100644 test/mjsunit/bugs/bug-2728.js create mode 100644 test/mjsunit/regress/regress-5736.js diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc index acba74d699..59b0b357d4 100644 --- a/src/ast/scopes.cc +++ b/src/ast/scopes.cc @@ -1179,10 +1179,7 @@ bool Scope::AllowsLazyParsingWithoutUnresolvedVariables( // the parse, since context allocation of those variables is already // guaranteed to be correct. for (const Scope* s = this; s != outer; s = s->outer_scope_) { - // Eval forces context allocation on all outer scopes, so we don't need to - // look at those scopes. Sloppy eval makes all top-level variables dynamic, - // whereas strict-mode requires context allocation. - if (s->is_eval_scope()) return !is_strict(s->language_mode()); + if (s->is_eval_scope()) return false; // Catch scopes force context allocation of all variables. if (s->is_catch_scope()) continue; // With scopes do not introduce variables that need allocation. diff --git a/test/mjsunit/bugs/bug-2728.js b/test/mjsunit/bugs/bug-2728.js deleted file mode 100644 index 4e0cb59190..0000000000 --- a/test/mjsunit/bugs/bug-2728.js +++ /dev/null @@ -1,39 +0,0 @@ -// Copyright 2016 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. - -// from test/webkit/fast/js/kde/parse.js -assertThrows("function test() { while(0) break lab; } lab: 1"); -assertThrows("function test() { while(0) continue lab; } lab: 1"); -assertThrows("function test() { while(0) break lab } lab: 1"); -assertThrows("function test() { while(0) continue lab } lab: 1"); - -// from test/webkit/fast/js/parser-syntax-check.js -assertThrows("break ; break your_limits ; continue ; continue living ; debugger"); -assertThrows("function f() { break ; break your_limits ; continue ; continue living ; debugger }"); -assertThrows("try { break } catch(e) {}"); -assertThrows("function f() { try { break } catch(e) {} }"); -assertThrows("L: L: ;"); -assertThrows("function f() { L: L: ; }"); -assertThrows("L: L1: L: ;"); -assertThrows("function f() { L: L1: L: ; }"); -assertThrows("L: L1: L2: L3: L4: L: ;"); -assertThrows("function f() { L: L1: L2: L3: L4: L: ; }"); - -// from test/mjsunit/es6/async-destructuring.js -assertThrows("'use strict'; async function f(x) { let x = 0; }", SyntaxError); -assertThrows("'use strict'; async function f({x}) { let x = 0; }", SyntaxError); -assertThrows("'use strict'; async function f(x) { const x = 0; }", SyntaxError); -assertThrows("'use strict'; async function f({x}) { const x = 0; }", SyntaxError); - -// from test/mjsunit/es6/destructuring.js -assertThrows("'use strict'; function f(x) { let x = 0; }", SyntaxError); -assertThrows("'use strict'; function f({x}) { let x = 0; }", SyntaxError); -assertThrows("'use strict'; function f(x) { const x = 0; }", SyntaxError); -assertThrows("'use strict'; function f({x}) { const x = 0; }", SyntaxError); - -// from test/mjsunit/es6/generator-destructuring.js -assertThrows("'use strict'; function* f(x) { let x = 0; }", SyntaxError); -assertThrows("'use strict'; function* f({x}) { let x = 0; }", SyntaxError); -assertThrows("'use strict'; function* f(x) { const x = 0; }", SyntaxError); -assertThrows("'use strict'; function* f({x}) { const x = 0; }", SyntaxError); diff --git a/test/mjsunit/regress/regress-5736.js b/test/mjsunit/regress/regress-5736.js new file mode 100644 index 0000000000..6dcc0de7f4 --- /dev/null +++ b/test/mjsunit/regress/regress-5736.js @@ -0,0 +1,23 @@ +// Copyright 2016 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 my_global = 0; + +// The problem was that we allowed lazy functions inside evals, but did not +// force context allocation on the eval scope. Thus, foo was not context +// allocated since we didn't realize that a lazy function referred to it. +eval(`let foo = 1; + let maybe_lazy = function() { foo = 2; } + maybe_lazy(); + my_global = foo;`); +assertEquals(2, my_global); + +(function TestVarInStrictEval() { + "use strict"; + eval(`var foo = 3; + let maybe_lazy = function() { foo = 4; } + maybe_lazy(); + my_global = foo;`); + assertEquals(4, my_global); +})();