Reviewers: rossberg,
Description:
Fix declarations escaping global strict eval.
According to ES5 10.4.2(3), eval calls of strict code always require
their own lexical and variable environment. For now we just add a new
scope when we parse the strict mode directive. The clean solution would
be to always have this sope present (even for global eval calls) and
adapt variable binding to cope with that.
[email protected]
BUG=v8:1624
TEST=mjsunit/regress/regress-1624,test262/S10.4.2.1_A1
Please review this at https://chromiumcodereview.appspot.com/9703021/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files:
M src/parser.h
M src/parser.cc
A test/mjsunit/regress/regress-1624.js
M test/test262/test262.status
Index: src/parser.cc
diff --git a/src/parser.cc b/src/parser.cc
index
90dd6a7c43ab728613008dd4c9af5014aad2c69b..73a635add5106bcddb57c068068741f82972ace9
100644
--- a/src/parser.cc
+++ b/src/parser.cc
@@ -604,7 +604,7 @@ FunctionLiteral*
Parser::DoParseProgram(CompilationInfo* info,
FunctionLiteral* result = NULL;
{ Scope* scope = NewScope(top_scope_, GLOBAL_SCOPE);
info->SetGlobalScope(scope);
- if (!info->is_global() &&
+ if (info->is_eval() && !info->is_global() &&
(info->shared_info().is_null() ||
info->shared_info()->is_function())) {
scope = Scope::DeserializeScopeChain(*info->calling_context(),
scope);
scope = NewScope(scope, EVAL_SCOPE);
@@ -616,13 +616,13 @@ FunctionLiteral*
Parser::DoParseProgram(CompilationInfo* info,
ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(16);
bool ok = true;
int beg_loc = scanner().location().beg_pos;
- ParseSourceElements(body, Token::EOS, &ok);
+ ParseSourceElements(body, Token::EOS, info->is_eval(), &ok);
if (ok && !top_scope_->is_classic_mode()) {
CheckOctalLiteral(beg_loc, scanner().location().end_pos, &ok);
}
if (ok && is_extended_mode()) {
- CheckConflictingVarDeclarations(scope, &ok);
+ CheckConflictingVarDeclarations(top_scope_, &ok);
}
if (ok) {
@@ -1096,6 +1096,7 @@ class ThisNamedPropertyAssignmentFinder : public
ParserFinder {
void* Parser::ParseSourceElements(ZoneList<Statement*>* processor,
int end_token,
+ bool is_eval,
bool* ok) {
// SourceElements ::
// (ModuleElement)* <end_token>
@@ -1138,6 +1139,17 @@ void*
Parser::ParseSourceElements(ZoneList<Statement*>* processor,
directive->Equals(isolate()->heap()->use_strict()) &&
token_loc.end_pos - token_loc.beg_pos ==
isolate()->heap()->use_strict()->length() + 2) {
+ // TODO(mstarzinger): Global strict eval calls, need their own
scope
+ // as specified in ES5 10.4.2(3). The correct fix would be to
always
+ // add this scope in DoParseProgram(), but that requires
adaptations
+ // all over the code base, so we go with a quick-fix for now.
+ if (is_eval && !top_scope_->is_eval_scope()) {
+ ASSERT(top_scope_->is_global_scope());
+ Scope* scope = NewScope(top_scope_, EVAL_SCOPE);
+ scope->set_start_position(top_scope_->start_position());
+ scope->set_end_position(top_scope_->end_position());
+ top_scope_ = scope;
+ }
// TODO(ES6): Fix entering extended mode, once it is specified.
top_scope_->SetLanguageMode(FLAG_harmony_scoping
? EXTENDED_MODE : STRICT_MODE);
@@ -4548,7 +4560,7 @@ FunctionLiteral*
Parser::ParseFunctionLiteral(Handle<String> function_name,
factory()->NewThisFunction(),
RelocInfo::kNoPosition)));
}
- ParseSourceElements(body, Token::RBRACE, CHECK_OK);
+ ParseSourceElements(body, Token::RBRACE, false, CHECK_OK);
materialized_literal_count =
function_state.materialized_literal_count();
expected_property_count = function_state.expected_property_count();
Index: src/parser.h
diff --git a/src/parser.h b/src/parser.h
index
227344f32019aa89d3631baa0aba97bdc44678e5..b4d88255f7aaa729422549c7de84a44a938c8aa0
100644
--- a/src/parser.h
+++ b/src/parser.h
@@ -580,7 +580,7 @@ class Parser {
// By making the 'exception handling' explicit, we are forced to check
// for failure at the call sites.
void* ParseSourceElements(ZoneList<Statement*>* processor,
- int end_token, bool* ok);
+ int end_token, bool is_eval, bool* ok);
Statement* ParseModuleElement(ZoneStringList* labels, bool* ok);
Block* ParseModuleDeclaration(ZoneStringList* names, bool* ok);
Module* ParseModule(bool* ok);
Index: test/mjsunit/regress/regress-1624.js
diff --git a/test/mjsunit/regress/regress-1624.js
b/test/mjsunit/regress/regress-1624.js
new file mode 100644
index
0000000000000000000000000000000000000000..e65d74dafc88e7c1125835f01b467d709096f3b5
--- /dev/null
+++ b/test/mjsunit/regress/regress-1624.js
@@ -0,0 +1,48 @@
+// Copyright 2012 the V8 project authors. All rights reserved.
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+// * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+// * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following
+// disclaimer in the documentation and/or other materials provided
+// with the distribution.
+// * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived
+// from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+// Test that global eval calls of strict code (independent from whether
being
+// direct or indirect) have their own lexical and variable environment.
+
+var no_touch = 0;
+var evil = eval;
+
+// Test global direct strict eval (expects new environment).
+eval('"use strict"; var no_touch = 1;');
+assertSame(0, no_touch);
+
+// Test global indirect strict eval (expects new environment).
+evil('"use strict"; var no_touch = 2;');
+assertSame(0, no_touch);
+
+// Test global direct non-strict eval (expects global environment).
+eval('var no_touch = 3;');
+assertSame(3, no_touch);
+
+// Test global indirect non-strict eval (expects global environment).
+evil('var no_touch = 4;');
+assertSame(4, no_touch);
Index: test/test262/test262.status
diff --git a/test/test262/test262.status b/test/test262/test262.status
index
1a8a8dc7ddd6aef2e517530581b3aba166e191ce..fa832c59899742ee74ca6a882e84c4fa1d50771f
100644
--- a/test/test262/test262.status
+++ b/test/test262/test262.status
@@ -33,9 +33,6 @@ def FAIL_OK = FAIL, OKAY
# '__proto__' should be treated as a normal property in JSON.
S15.12.2_A1: FAIL
-# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1624
-S10.4.2.1_A1: FAIL
-
# V8 Bug: http://code.google.com/p/v8/issues/detail?id=1475
15.2.3.6-4-405: FAIL
15.2.3.6-4-410: FAIL
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
