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.

R=rossberg@chromium.org
BUG=v8:1624
TEST=mjsunit/regress/regress-1624,test262/S10.4.2.1_A1

Review URL: https://chromiumcodereview.appspot.com/9703021

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@11057 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/parser.cc

@@ -604,11 +604,15 @@ FunctionLiteral* Parser::DoParseProgram(CompilationInfo* info,
   FunctionLiteral* result = NULL;
   { Scope* scope = NewScope(top_scope_, GLOBAL_SCOPE);
     info->SetGlobalScope(scope);
-    if (!info->is_global() &&
-        (info->shared_info().is_null() || info->shared_info()->is_function())) {
+    if (info->is_eval()) {
+      Handle<SharedFunctionInfo> shared = info->shared_info();
+      if (!info->is_global() && (shared.is_null() || shared->is_function())) {
         scope = Scope::DeserializeScopeChain(*info->calling_context(), scope);
+      }
+      if (!scope->is_global_scope() || info->language_mode() != CLASSIC_MODE) {
         scope = NewScope(scope, EVAL_SCOPE);
       }
+    }
     scope->set_start_position(0);
     scope->set_end_position(source->length());
     FunctionState function_state(this, scope, isolate());
@@ -616,13 +620,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 +1100,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 +1143,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 +4564,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();

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);

test/mjsunit/regress/regress-1624-strict.js

+// 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.
+
+"use strict";
+var evil = eval;
+
+// Test global direct strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+eval('"use strict"; var no_touch = 1;');
+assertSame(0, no_touch);
+
+// Test global indirect strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+evil('"use strict"; var no_touch = 2;');
+assertSame(0, no_touch);
+
+// Test global direct non-strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+eval('var no_touch = 3;');
+assertSame(0, no_touch);
+
+// Test global indirect non-strict eval in strict script.
+// Expects global environment.
+var no_touch = 0;
+evil('var no_touch = 4;');
+assertSame(4, no_touch);
+
+// Test non-global direct strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  eval('"use strict"; var no_touch = 5;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  evil('"use strict"; var no_touch = 6;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global direct non-strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  eval('var no_touch = 7;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect non-strict eval in strict script.
+// Expects global environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  evil('var no_touch = 8;');
+  assertSame(0, no_touch);
+})()
+assertSame(8, no_touch);
+
+// Test non-global direct strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  eval('"use strict"; var no_touch = 9;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  evil('"use strict"; var no_touch = 10;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global direct non-strict eval in strict script.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  eval('var no_touch = 11;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect non-strict eval in strict script.
+// Expects global environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  evil('var no_touch = 12;');
+  assertSame(0, no_touch);
+})()
+assertSame(12, no_touch);

test/mjsunit/regress/regress-1624.js

+// 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 evil = eval;
+
+// Test global direct strict eval.
+// Expects new environment.
+var no_touch = 0;
+eval('"use strict"; var no_touch = 1;');
+assertSame(0, no_touch);
+
+// Test global indirect strict eval.
+// Expects new environment.
+var no_touch = 0;
+evil('"use strict"; var no_touch = 2;');
+assertSame(0, no_touch);
+
+// Test global direct non-strict eval.
+// Expects global environment.
+var no_touch = 0;
+eval('var no_touch = 3;');
+assertSame(3, no_touch);
+
+// Test global indirect non-strict eval.
+// Expects global environment.
+var no_touch = 0;
+evil('var no_touch = 4;');
+assertSame(4, no_touch);
+
+// Test non-global direct strict eval in non-strict function.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  eval('"use strict"; var no_touch = 5;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect strict eval in non-strict function.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  evil('"use strict"; var no_touch = 6;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global direct non-strict eval in non-strict function.
+// Expects function environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  eval('var no_touch = 7;');
+  assertSame(7, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect non-strict eval in non-strict function.
+// Expects global environment.
+var no_touch = 0;
+(function() {
+  var no_touch = 0;
+  evil('var no_touch = 8;');
+  assertSame(0, no_touch);
+})()
+assertSame(8, no_touch);
+
+// Test non-global direct strict eval in strict function.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  eval('"use strict"; var no_touch = 9;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect strict eval in strict function.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  evil('"use strict"; var no_touch = 10;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global direct non-strict eval in strict function.
+// Expects new environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  eval('var no_touch = 11;');
+  assertSame(0, no_touch);
+})()
+assertSame(0, no_touch);
+
+// Test non-global indirect non-strict eval in strict function.
+// Expects global environment.
+var no_touch = 0;
+(function() {
+  "use strict";
+  var no_touch = 0;
+  evil('var no_touch = 12;');
+  assertSame(0, no_touch);
+})()
+assertSame(12, no_touch);

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