From b2ce10de422e6e99ab774cdf3fdeca870fa12d30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Bargull?= Date: Fri, 2 Jun 2017 12:04:41 +0200 Subject: [PATCH] Bug 1369337 - Forcibly create length and @@iterator properties for arguments when redefining. r=evilpie --HG-- extra : rebase_source : 5bd47ea33e4274b42443662aa6477b5678dea3df --- .../arguments/args-redefine-iterator-1.js | 14 ++++++++ .../arguments/args-redefine-iterator-2.js | 14 ++++++++ .../tests/arguments/args-redefine-length-3.js | 14 ++++++++ .../tests/arguments/args-redefine-length-4.js | 14 ++++++++ .../tests/arguments/args-redefine-length-5.js | 14 ++++++++ .../tests/arguments/args-redefine-length-6.js | 14 ++++++++ .../tests/arguments/args-redefine-length-7.js | 14 ++++++++ js/src/vm/ArgumentsObject.cpp | 33 +++++++++++++++++-- js/src/vm/ArgumentsObject.h | 20 +++++++---- js/src/vm/NativeObject.cpp | 25 ++++++++------ 10 files changed, 157 insertions(+), 19 deletions(-) create mode 100644 js/src/jit-test/tests/arguments/args-redefine-iterator-1.js create mode 100644 js/src/jit-test/tests/arguments/args-redefine-iterator-2.js create mode 100644 js/src/jit-test/tests/arguments/args-redefine-length-3.js create mode 100644 js/src/jit-test/tests/arguments/args-redefine-length-4.js create mode 100644 js/src/jit-test/tests/arguments/args-redefine-length-5.js create mode 100644 js/src/jit-test/tests/arguments/args-redefine-length-6.js create mode 100644 js/src/jit-test/tests/arguments/args-redefine-length-7.js diff --git a/js/src/jit-test/tests/arguments/args-redefine-iterator-1.js b/js/src/jit-test/tests/arguments/args-redefine-iterator-1.js new file mode 100644 index 000000000000..77241a6ae79a --- /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(); diff --git a/js/src/jit-test/tests/arguments/args-redefine-iterator-2.js b/js/src/jit-test/tests/arguments/args-redefine-iterator-2.js new file mode 100644 index 000000000000..186be930c4a5 --- /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(); diff --git a/js/src/jit-test/tests/arguments/args-redefine-length-3.js b/js/src/jit-test/tests/arguments/args-redefine-length-3.js new file mode 100644 index 000000000000..a4d6823acfa5 --- /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(); diff --git a/js/src/jit-test/tests/arguments/args-redefine-length-4.js b/js/src/jit-test/tests/arguments/args-redefine-length-4.js new file mode 100644 index 000000000000..1029e8fca32b --- /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(); diff --git a/js/src/jit-test/tests/arguments/args-redefine-length-5.js b/js/src/jit-test/tests/arguments/args-redefine-length-5.js new file mode 100644 index 000000000000..2ae30bf0f79f --- /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(); diff --git a/js/src/jit-test/tests/arguments/args-redefine-length-6.js b/js/src/jit-test/tests/arguments/args-redefine-length-6.js new file mode 100644 index 000000000000..a4b768d4bd14 --- /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(); diff --git a/js/src/jit-test/tests/arguments/args-redefine-length-7.js b/js/src/jit-test/tests/arguments/args-redefine-length-7.js new file mode 100644 index 000000000000..042b224740ab --- /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(); diff --git a/js/src/vm/ArgumentsObject.cpp b/js/src/vm/ArgumentsObject.cpp index 93c7ac622637..7fdf5b60346a 100644 --- a/js/src/vm/ArgumentsObject.cpp +++ b/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 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 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 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) && diff --git a/js/src/vm/ArgumentsObject.h b/js/src/vm/ArgumentsObject.h index 482f2b3ca4d5..d0e645b3a70d 100644 --- a/js/src/vm/ArgumentsObject.h +++ b/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 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 obj); + /* True iff any element has been assigned or its attributes * changed. */ bool hasOverriddenElement() const { diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index fb152110187c..82b601b09f9a 100644 --- a/js/src/vm/NativeObject.cpp +++ b/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()) { + Rooted argsobj(cx, &obj->as()); 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().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().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().markElementOverridden(); + argsobj->markElementOverridden(); } }