Bug 1369337 - Forcibly create length and @@iterator properties for arguments when redefining. r=evilpie

--HG--
extra : rebase_source : 5bd47ea33e4274b42443662aa6477b5678dea3df

js/src/jit-test/tests/arguments/args-redefine-iterator-1.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, Symbol.iterator, { });
+  for (var i = 0; i < 5; i++)
+    assertEq(a[Symbol.iterator], Array.prototype[Symbol.iterator]);
+
+  var desc = Object.getOwnPropertyDescriptor(a, Symbol.iterator);
+  assertEq(desc.value, Array.prototype[Symbol.iterator]);
+  assertEq(desc.writable, true);
+  assertEq(desc.enumerable, false);
+  assertEq(desc.configurable, true);
+}
+t();

js/src/jit-test/tests/arguments/args-redefine-iterator-2.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, Symbol.iterator, { writable: false, enumerable: true, configurable: false });
+  for (var i = 0; i < 5; i++)
+    assertEq(a[Symbol.iterator], Array.prototype[Symbol.iterator]);
+
+  var desc = Object.getOwnPropertyDescriptor(a, Symbol.iterator);
+  assertEq(desc.value, Array.prototype[Symbol.iterator]);
+  assertEq(desc.writable, false);
+  assertEq(desc.enumerable, true);
+  assertEq(desc.configurable, false);
+}
+t();

js/src/jit-test/tests/arguments/args-redefine-length-3.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, "length", { });
+  for (var i = 0; i < 5; i++)
+    assertEq(a.length, 0);
+
+  var desc = Object.getOwnPropertyDescriptor(a, "length");
+  assertEq(desc.value, 0);
+  assertEq(desc.writable, true);
+  assertEq(desc.enumerable, false);
+  assertEq(desc.configurable, true);
+}
+t();

js/src/jit-test/tests/arguments/args-redefine-length-4.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, "length", { writable: false });
+  for (var i = 0; i < 5; i++)
+    assertEq(a.length, 0);
+
+  var desc = Object.getOwnPropertyDescriptor(a, "length");
+  assertEq(desc.value, 0);
+  assertEq(desc.writable, false);
+  assertEq(desc.enumerable, false);
+  assertEq(desc.configurable, true);
+}
+t();

js/src/jit-test/tests/arguments/args-redefine-length-5.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, "length", { enumerable: true });
+  for (var i = 0; i < 5; i++)
+    assertEq(a.length, 0);
+
+  var desc = Object.getOwnPropertyDescriptor(a, "length");
+  assertEq(desc.value, 0);
+  assertEq(desc.writable, true);
+  assertEq(desc.enumerable, true);
+  assertEq(desc.configurable, true);
+}
+t();

js/src/jit-test/tests/arguments/args-redefine-length-6.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, "length", { configurable: false });
+  for (var i = 0; i < 5; i++)
+    assertEq(a.length, 0);
+
+  var desc = Object.getOwnPropertyDescriptor(a, "length");
+  assertEq(desc.value, 0);
+  assertEq(desc.writable, true);
+  assertEq(desc.enumerable, false);
+  assertEq(desc.configurable, false);
+}
+t();

js/src/jit-test/tests/arguments/args-redefine-length-7.js

@@ -0,0 +1,14 @@
+function t()
+{
+  var a = arguments;
+  Object.defineProperty(a, "length", { value: 0 });
+  for (var i = 0; i < 5; i++)
+    assertEq(a.length, 0);
+
+  var desc = Object.getOwnPropertyDescriptor(a, "length");
+  assertEq(desc.value, 0);
+  assertEq(desc.writable, true);
+  assertEq(desc.enumerable, false);
+  assertEq(desc.configurable, true);
+}
+t();

js/src/vm/ArgumentsObject.cpp

@@ -510,7 +510,7 @@ MappedArgSetter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue
     /*
      * For simplicity we use delete/define to replace the property with a
      * simple data property. Note that we rely on ArgumentsObject::obj_delProperty
-     * to clear the corresponding reserved slot so the GC can collect its value.
+     * to set the corresponding override-bit.
      * Note also that we must define the property instead of setting it in case
      * the user has changed the prototype to an object that has a setter for
      * this id.
@@ -529,10 +529,37 @@ DefineArgumentsIterator(JSContext* cx, Handle<ArgumentsObject*> argsobj)
     RootedValue val(cx);
     if (!GlobalObject::getSelfHostedFunction(cx, cx->global(), shName, name, 0, &val))
         return false;
-
     return NativeDefineProperty(cx, argsobj, iteratorId, val, nullptr, nullptr, JSPROP_RESOLVING);
 }
 
+/* static */ bool
+ArgumentsObject::reifyLength(JSContext* cx, Handle<ArgumentsObject*> obj)
+{
+    if (obj->hasOverriddenLength())
+        return true;
+
+    RootedId id(cx, NameToId(cx->names().length));
+    RootedValue val(cx, Int32Value(obj->initialLength()));
+    if (!NativeDefineProperty(cx, obj, id, val, nullptr, nullptr, JSPROP_RESOLVING))
+        return false;
+
+    obj->markLengthOverridden();
+    return true;
+}
+
+/* static */ bool
+ArgumentsObject::reifyIterator(JSContext* cx, Handle<ArgumentsObject*> obj)
+{
+    if (obj->hasOverriddenIterator())
+        return true;
+
+    if (!DefineArgumentsIterator(cx, obj))
+        return false;
+
+    obj->markIteratorOverridden();
+    return true;
+}
+
 /* static */ bool
 MappedArgumentsObject::obj_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp)
 {
@@ -714,7 +741,7 @@ UnmappedArgSetter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleVal
     /*
      * For simplicity we use delete/define to replace the property with a
      * simple data property. Note that we rely on ArgumentsObject::obj_delProperty
-     * to clear the corresponding reserved slot so the GC can collect its value.
+     * to set the corresponding override-bit.
      */
     ObjectOpResult ignored;
     return NativeDeleteProperty(cx, argsobj, id, ignored) &&

js/src/vm/ArgumentsObject.h

@@ -52,12 +52,10 @@ class RareArgumentsData
     }
 };
 
-/*
- * ArgumentsData stores the initial indexed arguments provided to the
- * corresponding and that function itself.  It is used to store arguments[i]
- * and arguments.callee -- up until the corresponding property is modified,
- * when the relevant value is flagged to memorialize the modification.
- */
+// ArgumentsData stores the initial indexed arguments provided to a function
+// call. It is used to store arguments[i] -- up until the corresponding
+// property is modified, when the relevant value is flagged to memorialize the
+// modification.
 struct ArgumentsData
 {
     /*
@@ -235,6 +233,11 @@ class ArgumentsObject : public NativeObject
         setFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(v));
     }
 
+    /*
+     * Create the default "length" property and set LENGTH_OVERRIDDEN_BIT.
+     */
+    static bool reifyLength(JSContext* cx, Handle<ArgumentsObject*> obj);
+
     /* True iff arguments[@@iterator] has been assigned or its attributes
      * changed. */
     bool hasOverriddenIterator() const {
@@ -247,6 +250,11 @@ class ArgumentsObject : public NativeObject
         setFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(v));
     }
 
+    /*
+     * Create the default @@iterator property and set ITERATOR_OVERRIDDEN_BIT.
+     */
+    static bool reifyIterator(JSContext* cx, Handle<ArgumentsObject*> obj);
+
     /* True iff any element has been assigned or its attributes
      * changed. */
     bool hasOverriddenElement() const {

js/src/vm/NativeObject.cpp

@@ -1606,21 +1606,26 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id,
             return DefineTypedArrayElement(cx, obj, index, desc_, result);
         }
     } else if (obj->is<ArgumentsObject>()) {
+        Rooted<ArgumentsObject*> argsobj(cx, &obj->as<ArgumentsObject>());
         if (id == NameToId(cx->names().length)) {
-            // Either we are resolving the .length property on this object, or
-            // redefining it. In the latter case only, we must set a bit. To
-            // distinguish the two cases, we note that when resolving, the
-            // JSPROP_RESOLVING mask is set; whereas the first time it is
-            // redefined, it isn't set.
-            if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
-                obj->as<ArgumentsObject>().markLengthOverridden();
+            // Either we are resolving the .length property on this object,
+            // or redefining it. In the latter case only, we must reify the
+            // property. To distinguish the two cases, we note that when
+            // resolving, the JSPROP_RESOLVING mask is set; whereas the first
+            // time it is redefined, it isn't set.
+            if ((desc_.attributes() & JSPROP_RESOLVING) == 0) {
+                if (!ArgumentsObject::reifyLength(cx, argsobj))
+                    return false;
+            }
         } else if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) {
             // Do same thing as .length for [@@iterator].
-            if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
-                obj->as<ArgumentsObject>().markIteratorOverridden();
+            if ((desc_.attributes() & JSPROP_RESOLVING) == 0) {
+                if (!ArgumentsObject::reifyIterator(cx, argsobj))
+                    return false;
+            }
         } else if (JSID_IS_INT(id)) {
             if ((desc_.attributes() & JSPROP_RESOLVING) == 0)
-                obj->as<ArgumentsObject>().markElementOverridden();
+                argsobj->markElementOverridden();
         }
     }