Reviewers: ulan,

Description:
Fix lazy compilation for strict eval scopes.

This prevents lazy compilation of functions that have an outer context
containing a strict eval scope. Such a scope potentially contains
context allocated variables in an artificial function scope that is not
deserialized correctly.

[email protected]
BUG=chromium:135066
TEST=mjsunit/regress/regress-crbug-135066


Please review this at https://chromiumcodereview.appspot.com/10704058/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/scopes.h
  M src/scopes.cc
  A + test/mjsunit/regress/regress-crbug-135066.js


Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index faedb5f085871a358807675d50a543666ae27bb9..b0fd10e0e1668b151a7ed695e94be9ebfedf2191 100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -662,28 +662,36 @@ bool Scope::HasTrivialOuterContext() const {
 }


-bool Scope::AllowsLazyCompilation() const {
-  return !force_eager_compilation_ &&
-         !TrivialDeclarationScopesBeforeWithScope();
+bool Scope::HasLazyCompilableOuterContext() const {
+  Scope* outer = outer_scope_;
+  if (outer == NULL) return true;
+  // There are several reasons that prevent lazy compilation:
+  // - This scope is inside a with scope and all declaration scopes between
+  //   them have empty contexts. Such declaration scopes become invisible
+  //   during scope info deserialization.
+  // - This scope is inside a strict eval scope with variables that are
+  //   potentially context allocated in an artificial function scope that
+  //   is not deserialized correctly.
+  outer = outer->DeclarationScope();
+  bool found_non_trivial_declarations = false;
+ for (const Scope* scope = outer; scope != NULL; scope = scope->outer_scope_) {
+    if (scope->is_eval_scope()) return false;
+ if (scope->is_with_scope() && !found_non_trivial_declarations) return false;
+    if (scope->is_declaration_scope() && scope->num_heap_slots() > 0) {
+      found_non_trivial_declarations = true;
+    }
+  }
+  return true;
 }


-bool Scope::AllowsLazyCompilationWithoutContext() const {
-  return !force_eager_compilation_ && HasTrivialOuterContext();
+bool Scope::AllowsLazyCompilation() const {
+  return !force_eager_compilation_ && HasLazyCompilableOuterContext();
 }


-bool Scope::TrivialDeclarationScopesBeforeWithScope() const {
-  Scope* outer = outer_scope_;
-  if (outer == NULL) return false;
-  outer = outer->DeclarationScope();
-  while (outer != NULL) {
-    if (outer->is_with_scope()) return true;
-    if (outer->is_declaration_scope() && outer->num_heap_slots() > 0)
-      return false;
-    outer = outer->outer_scope_;
-  }
-  return false;
+bool Scope::AllowsLazyCompilationWithoutContext() const {
+  return !force_eager_compilation_ && HasTrivialOuterContext();
 }


Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index 2868cdeecb37dd776f8386c813afe19338dc6b14..42339a98e58ca9daaed84f773138bb75120a6938 100644
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -380,10 +380,8 @@ class Scope: public ZoneObject {
   // True if the outer context of this scope is always the global context.
   bool HasTrivialOuterContext() const;

-  // True if this scope is inside a with scope and all declaration scopes
-  // between them have empty contexts. Such declaration scopes become
-  // invisible during scope info deserialization.
-  bool TrivialDeclarationScopesBeforeWithScope() const;
+  // True if the outer context allows lazy compilation of this scope.
+  bool HasLazyCompilableOuterContext() const;

   // The number of contexts between this and scope; zero if this == scope.
   int ContextChainLength(Scope* scope);
Index: test/mjsunit/regress/regress-crbug-135066.js
diff --git a/test/mjsunit/regress/regress-crbug-135008.js b/test/mjsunit/regress/regress-crbug-135066.js
similarity index 82%
copy from test/mjsunit/regress/regress-crbug-135008.js
copy to test/mjsunit/regress/regress-crbug-135066.js
index 2be396e8055a916b1f7c7370ff32ff0d40eea762..1aeca8b1a32d678ba7274c60230a77fdda97f6aa 100644
--- a/test/mjsunit/regress/regress-crbug-135008.js
+++ b/test/mjsunit/regress/regress-crbug-135066.js
@@ -28,18 +28,26 @@
 // Filler long enough to trigger lazy parsing.
 var filler = "//" + new Array(1024).join('x');

-var scope = { x:23 };
+// Test strict eval in global context.
+eval(
+  "'use strict';" +
+  "var x = 23;" +
+  "var f = function bozo1() {" +
+  "  return x;" +
+  "};" +
+  "assertSame(23, f());" +
+  filler
+);

-with(scope) {
+// Test default eval in strict context.
+(function() {
+  "use strict";
   eval(
-    "scope.f = (function outer() {" +
-    "  function inner() {" +
-    "    return x;" +
-    "  }" +
-    "  return inner;" +
-    "})();" +
+    "var y = 42;" +
+    "var g = function bozo2() {" +
+    "  return y;" +
+    "};" +
+    "assertSame(42, g());" +
     filler
   );
-};
-
-assertSame(23, scope.f());
+})();


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to