Bug 1149797 - Use the loop's *static* block object when freshening a loop's block object, then copy in values from the old cloned block. Using the old cloned block directly isn't valid when the cloned block might be extended with additional variables created by eval or added by nested function statements. r=shu, a=lizzard
authorJeff Walden <jwalden@mit.edu>
Wed, 01 Apr 2015 16:39:33 -0400
changeset 265416 8cef59e11f864087e003e737c6a5f74b8fa067bc
parent 265415 e4aef493a47043a53e23be7555130557d61e75db
child 265417 c846da7a03a606375ad80deb983f0a4284bd994e
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu, lizzard
bugs1149797
milestone39.0a2
Bug 1149797 - Use the loop's *static* block object when freshening a loop's block object, then copy in values from the old cloned block. Using the old cloned block directly isn't valid when the cloned block might be extended with additional variables created by eval or added by nested function statements. r=shu, a=lizzard
js/src/tests/browser.js
js/src/tests/ecma_6/Class/shell.js
js/src/tests/ecma_6/LexicalEnvironment/for-loop-with-bindings-added-at-runtime.js
js/src/tests/ecma_6/extensions/for-loop-with-lexical-declaration-and-nested-function-statement.js
js/src/tests/ecma_6/extensions/shell.js
js/src/vm/ScopeObject.cpp
js/src/vm/ScopeObject.h
--- a/js/src/tests/browser.js
+++ b/js/src/tests/browser.js
@@ -331,16 +331,20 @@ function jsTestDriverBrowserInit()
     else if (properties.test.match(/^ecma_6\/LexicalEnvironment/))
     {
       properties.version = '1.8';
     }
     else if (properties.test.match(/^ecma_6\/Class/))
     {
       properties.version = '1.8';
     }
+    else if (properties.test.match(/^ecma_6\/extensions/))
+    {
+      properties.version = '1.8';
+    }
   }
 
   // default to language=type;text/javascript. required for
   // reftest style manifests.
   if (!properties.language)
   {
     properties.language = 'type';
     properties.mimetype = 'text/javascript';
--- a/js/src/tests/ecma_6/Class/shell.js
+++ b/js/src/tests/ecma_6/Class/shell.js
@@ -1,9 +1,11 @@
-// Enable "let" in shell builds. So silly.
+// NOTE: This only turns on 1.8.5 in shell builds.  The browser requires the
+//       futzing in js/src/tests/browser.js (which only turns on 1.8, the most
+//       the browser supports).
 if (typeof version != 'undefined')
   version(185);
 
 function classesEnabled() {
     try {
         new Function("class Foo { constructor() { } }");
         return true;
     } catch (e if e instanceof SyntaxError) {
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/LexicalEnvironment/for-loop-with-bindings-added-at-runtime.js
@@ -0,0 +1,125 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile = "for-loop-with-bindings-added-at-runtime.js";
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 1149797;
+var summary =
+  "Don't assert when freshening the scope chain for a for-loop whose head " +
+  "contains a lexical declaration, where the loop body might add more " +
+  "bindings at runtime";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+for (let x = 0; x < 9; ++x)
+  eval("var y");
+
+{
+  for (let x = 0; x < 9; ++x)
+    eval("var y");
+}
+
+function f1()
+{
+  for (let x = 0; x < 9; ++x)
+    eval("var y");
+}
+f1();
+
+function f2()
+{
+  {
+    for (let x = 0; x < 9; ++x)
+      eval("var y");
+  }
+}
+f2();
+
+for (let x = 0; x < 9; ++x)
+{
+  // deliberately inside a block statement
+  eval("var y");
+}
+
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    eval("var y");
+  }
+}
+
+function g1()
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    eval("var y");
+  }
+}
+g1();
+
+function g2()
+{
+  {
+    for (let x = 0; x < 9; ++x)
+    {
+      // deliberately inside a block statement
+      eval("var y");
+    }
+  }
+}
+g2();
+
+for (let x = 0; x < 9; ++x) {
+  (function() {
+      eval("var y");
+  })();
+}
+
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    (function() {
+        eval("var y");
+    })();
+  }
+}
+
+function h1()
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    (function() {
+        eval("var y");
+    })();
+  }
+}
+h1();
+
+function h2()
+{
+  {
+    for (let x = 0; x < 9; ++x)
+    {
+      // deliberately inside a block statement
+      (function() { eval("var y"); })();
+    }
+  }
+}
+h2();
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
new file mode 100644
--- /dev/null
+++ b/js/src/tests/ecma_6/extensions/for-loop-with-lexical-declaration-and-nested-function-statement.js
@@ -0,0 +1,126 @@
+/*
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/licenses/publicdomain/
+ */
+
+var gTestfile =
+  "for-loop-with-lexical-declaration-and-nested-function-statement.js";
+//-----------------------------------------------------------------------------
+var BUGNUMBER = 1149797;
+var summary =
+  "Don't assert when freshening the scope chain for a for-loop whose head " +
+  "contains a lexical declaration, where the loop body might add more " +
+  "bindings at runtime";
+
+print(BUGNUMBER + ": " + summary);
+
+/**************
+ * BEGIN TEST *
+ **************/
+
+for (let x = 0; x < 9; ++x)
+  function q1() {}
+
+{
+  for (let x = 0; x < 9; ++x)
+    function q2() {}
+}
+
+function f1()
+{
+  for (let x = 0; x < 9; ++x)
+    function q3() {}
+}
+f1();
+
+function f2()
+{
+  {
+    for (let x = 0; x < 9; ++x)
+      function q4() {}
+  }
+}
+f2();
+
+for (let x = 0; x < 9; ++x)
+{
+  // deliberately inside a block statement
+  function q5() {}
+}
+
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    function q6() {}
+  }
+}
+
+function g1()
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    function q7() {}
+  }
+}
+g1();
+
+function g2()
+{
+  {
+    for (let x = 0; x < 9; ++x)
+    {
+      // deliberately inside a block statement
+      function q8() {}
+    }
+  }
+}
+g2();
+
+for (let x = 0; x < 9; ++x) {
+  (function() {
+    eval("function q9() {}");
+  })();
+}
+
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    (function() {
+        eval("function q10() {}");
+    })();
+  }
+}
+
+function h1()
+{
+  for (let x = 0; x < 9; ++x)
+  {
+    // deliberately inside a block statement
+    (function() {
+        eval("function q11() {}");
+    })();
+  }
+}
+h1();
+
+function h2()
+{
+  {
+    for (let x = 0; x < 9; ++x)
+    {
+      // deliberately inside a block statement
+      (function() { eval("function q12() {}"); })();
+    }
+  }
+}
+h2();
+
+/******************************************************************************/
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);
+
+print("Tests complete");
--- a/js/src/tests/ecma_6/extensions/shell.js
+++ b/js/src/tests/ecma_6/extensions/shell.js
@@ -0,0 +1,5 @@
+// NOTE: This only turns on 1.8.5 in shell builds.  The browser requires the
+//       futzing in js/src/tests/browser.js (which only turns on 1.8, the most
+//       the browser supports).
+if (typeof version != 'undefined')
+  version(185);
--- a/js/src/vm/ScopeObject.cpp
+++ b/js/src/vm/ScopeObject.cpp
@@ -656,42 +656,29 @@ ClonedBlockObject::copyUnaliasedValues(A
         if (!block.isAliased(i)) {
             Value& val = frame.unaliasedLocal(block.blockIndexToLocalIndex(i));
             setVar(i, val, DONT_CHECK_ALIASING);
         }
     }
 }
 
 /* static */ ClonedBlockObject*
-ClonedBlockObject::clone(ExclusiveContext* cx, Handle<ClonedBlockObject*> block)
+ClonedBlockObject::clone(JSContext* cx, Handle<ClonedBlockObject*> clonedBlock)
 {
-    RootedObject enclosing(cx, &block->enclosingScope());
-
-    MOZ_ASSERT(block->getClass() == &BlockObject::class_);
-
-    RootedObjectGroup cloneGroup(cx, block->group());
-    RootedShape cloneShape(cx, block->lastProperty());
-
-    JSObject* obj = JSObject::create(cx, FINALIZE_KIND, gc::TenuredHeap, cloneShape, cloneGroup);
-    if (!obj)
+    Rooted<StaticBlockObject*> staticBlock(cx, &clonedBlock->staticBlock());
+    RootedObject enclosing(cx, &clonedBlock->enclosingScope());
+
+    Rooted<ClonedBlockObject*> copy(cx, create(cx, staticBlock, enclosing));
+    if (!copy)
         return nullptr;
 
-    ClonedBlockObject& copy = obj->as<ClonedBlockObject>();
-
-    MOZ_ASSERT(!copy.inDictionaryMode());
-    MOZ_ASSERT(copy.isDelegate());
-    MOZ_ASSERT(block->slotSpan() == copy.slotSpan());
-    MOZ_ASSERT(copy.slotSpan() >= copy.numVariables() + RESERVED_SLOTS);
-
-    copy.setReservedSlot(SCOPE_CHAIN_SLOT, block->getReservedSlot(SCOPE_CHAIN_SLOT));
-
-    for (uint32_t i = 0, count = copy.numVariables(); i < count; i++)
-        copy.setVar(i, block->var(i, DONT_CHECK_ALIASING), DONT_CHECK_ALIASING);
-
-    return &copy;
+    for (uint32_t i = 0, count = staticBlock->numVariables(); i < count; i++)
+        copy->setVar(i, clonedBlock->var(i, DONT_CHECK_ALIASING), DONT_CHECK_ALIASING);
+
+    return copy;
 }
 
 StaticBlockObject*
 StaticBlockObject::create(ExclusiveContext* cx)
 {
     RootedObjectGroup group(cx, ObjectGroup::defaultNewGroup(cx, &BlockObject::class_,
                                                              TaggedProto(nullptr)));
     if (!group)
--- a/js/src/vm/ScopeObject.h
+++ b/js/src/vm/ScopeObject.h
@@ -669,17 +669,17 @@ class ClonedBlockObject : public BlockOb
 
     /* Copy in all the unaliased formals and locals. */
     void copyUnaliasedValues(AbstractFramePtr frame);
 
     /*
      * Create a new ClonedBlockObject with the same enclosing scope and
      * variable values as this.
      */
-    static ClonedBlockObject* clone(ExclusiveContext* cx, Handle<ClonedBlockObject*> block);
+    static ClonedBlockObject* clone(JSContext* cx, Handle<ClonedBlockObject*> block);
 };
 
 // Internal scope object used by JSOP_BINDNAME upon encountering an
 // uninitialized lexical slot.
 //
 // ES6 lexical bindings cannot be accessed in any way (throwing
 // ReferenceErrors) until initialized. Normally, NAME operations
 // unconditionally check for uninitialized lexical slots. When getting or