Bug 1369337 - Forcibly create length and @@iterator properties for arguments when redefining. r=evilpie
authorAndré Bargull <andre.bargull@gmail.com>
Fri, 02 Jun 2017 12:04:41 +0200
changeset 362586 e0d976b9bf417edda6892091237534db34dd08c0
parent 362585 f2737791ec8e14a03feec64dff33d374f9cbca25
child 362587 4d81e7dfd02068a3c5f2da58449d3f8c55f93ed0
push id31983
push userkwierso@gmail.com
push dateWed, 07 Jun 2017 00:19:30 +0000
treeherdermozilla-central@5801aa478de1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersevilpie
bugs1369337
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1369337 - Forcibly create length and @@iterator properties for arguments when redefining. r=evilpie
js/src/jit-test/tests/arguments/args-redefine-iterator-1.js
js/src/jit-test/tests/arguments/args-redefine-iterator-2.js
js/src/jit-test/tests/arguments/args-redefine-length-3.js
js/src/jit-test/tests/arguments/args-redefine-length-4.js
js/src/jit-test/tests/arguments/args-redefine-length-5.js
js/src/jit-test/tests/arguments/args-redefine-length-6.js
js/src/jit-test/tests/arguments/args-redefine-length-7.js
js/src/vm/ArgumentsObject.cpp
js/src/vm/ArgumentsObject.h
js/src/vm/NativeObject.cpp
new file mode 100644
--- /dev/null
+++ b/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();
new file mode 100644
--- /dev/null
+++ b/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();
new file mode 100644
--- /dev/null
+++ b/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();
new file mode 100644
--- /dev/null
+++ b/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();
new file mode 100644
--- /dev/null
+++ b/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();
new file mode 100644
--- /dev/null
+++ b/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();
new file mode 100644
--- /dev/null
+++ b/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();
--- a/js/src/vm/ArgumentsObject.cpp
+++ b/js/src/vm/ArgumentsObject.cpp
@@ -505,17 +505,17 @@ MappedArgSetter(JSContext* cx, HandleObj
         }
     } else {
         MOZ_ASSERT(JSID_IS_ATOM(id, cx->names().length) || JSID_IS_ATOM(id, cx->names().callee));
     }
 
     /*
      * 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.
      */
     ObjectOpResult ignored;
     return NativeDeleteProperty(cx, argsobj, id, ignored) &&
            NativeDefineProperty(cx, argsobj, id, vp, nullptr, nullptr, attrs, result);
 }
@@ -524,18 +524,45 @@ static bool
 DefineArgumentsIterator(JSContext* cx, Handle<ArgumentsObject*> argsobj)
 {
     RootedId iteratorId(cx, SYMBOL_TO_JSID(cx->wellKnownSymbols().iterator));
     HandlePropertyName shName = cx->names().ArrayValues;
     RootedAtom name(cx, cx->names().values);
     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);
+}
 
-    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)
 {
     Rooted<MappedArgumentsObject*> argsobj(cx, &obj->as<MappedArgumentsObject>());
 
     if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) {
@@ -709,17 +736,17 @@ UnmappedArgSetter(JSContext* cx, HandleO
         }
     } else {
         MOZ_ASSERT(JSID_IS_ATOM(id, cx->names().length));
     }
 
     /*
      * 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) &&
            NativeDefineProperty(cx, argsobj, id, vp, nullptr, nullptr, attrs, result);
 }
 
 /* static */ bool
 UnmappedArgumentsObject::obj_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp)
--- a/js/src/vm/ArgumentsObject.h
+++ b/js/src/vm/ArgumentsObject.h
@@ -47,22 +47,20 @@ class RareArgumentsData
         return IsBitArrayElementSet(deletedBits_, len, i);
     }
     void markElementDeleted(size_t len, size_t i) {
         MOZ_ASSERT(i < len);
         SetBitArrayElement(deletedBits_, len, i);
     }
 };
 
-/*
- * 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
 {
     /*
      * numArgs = Max(numFormalArgs, numActualArgs)
      * The array 'args' has numArgs elements.
      */
     uint32_t    numArgs;
 
@@ -230,28 +228,38 @@ class ArgumentsObject : public NativeObj
         return v.toInt32() & LENGTH_OVERRIDDEN_BIT;
     }
 
     void markLengthOverridden() {
         uint32_t v = getFixedSlot(INITIAL_LENGTH_SLOT).toInt32() | LENGTH_OVERRIDDEN_BIT;
         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 {
         const Value& v = getFixedSlot(INITIAL_LENGTH_SLOT);
         return v.toInt32() & ITERATOR_OVERRIDDEN_BIT;
     }
 
     void markIteratorOverridden() {
         uint32_t v = getFixedSlot(INITIAL_LENGTH_SLOT).toInt32() | ITERATOR_OVERRIDDEN_BIT;
         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 {
         const Value& v = getFixedSlot(INITIAL_LENGTH_SLOT);
         return v.toInt32() & ELEMENT_OVERRIDDEN_BIT;
     }
 
     void markElementOverridden() {
--- a/js/src/vm/NativeObject.cpp
+++ b/js/src/vm/NativeObject.cpp
@@ -1601,31 +1601,36 @@ js::NativeDefineProperty(JSContext* cx, 
     } else if (obj->is<TypedArrayObject>()) {
         // 9.4.5.3 step 3. Indexed properties of typed arrays are special.
         uint64_t index;
         if (IsTypedArrayIndex(id, &index)) {
             MOZ_ASSERT(!cx->helperThread());
             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();
         }
     }
 
     // 9.1.6.1 OrdinaryDefineOwnProperty step 1.
     Rooted<PropertyResult> prop(cx);
     if (desc_.attributes() & JSPROP_RESOLVING) {
         // We are being called from a resolve or enumerate hook to reify a
         // lazily-resolved property. To avoid reentering the resolve hook and