diff --git a/js/src/jit-test/tests/collections/constructor-errors.js b/js/src/jit-test/tests/collections/constructor-errors.js index d5cbab9e7..6a145180f 100644 --- a/js/src/jit-test/tests/collections/constructor-errors.js +++ b/js/src/jit-test/tests/collections/constructor-errors.js @@ -2,12 +2,9 @@ load(libdir + "asserts.js"); -function argsobj() { return arguments; } - var misc = [ {}, {x: 1}, Math, isNaN, Object.create(null), - argsobj(0, 1, 2), true, 0, 3.1416, new Boolean(true), new Number(0), {iterator: function () { return undefined; }}, @@ -19,4 +16,4 @@ var misc = [ for (var v of misc) { assertThrowsInstanceOf(function () { new Set(v); }, TypeError); assertThrowsInstanceOf(function () { new Map(v); }, TypeError); -} \ No newline at end of file +} diff --git a/js/src/jit-test/tests/for-of/non-iterable.js b/js/src/jit-test/tests/for-of/non-iterable.js index 319e622ef..99f6b0780 100644 --- a/js/src/jit-test/tests/for-of/non-iterable.js +++ b/js/src/jit-test/tests/for-of/non-iterable.js @@ -2,12 +2,9 @@ load(libdir + "asserts.js"); -function argsobj() { return arguments; } - var misc = [ {}, {x: 1}, Math, isNaN, Object.create(null), - argsobj(0, 1, 2), null, undefined, true, 0, 3.1416, new Boolean(true), new Number(0), diff --git a/js/src/jit-test/tests/ion/bug832058.js b/js/src/jit-test/tests/ion/bug832058.js index e6877e5bf..22dd37a4e 100644 --- a/js/src/jit-test/tests/ion/bug832058.js +++ b/js/src/jit-test/tests/ion/bug832058.js @@ -1,4 +1,4 @@ -// |jit-test| error: TypeError +// |jit-test| function f(c) { var b = arguments; if (c == 1) @@ -9,6 +9,7 @@ function f(c) { evaluate("f('a', 'b', 'c', 'd', 'e');"); function test(){ var args = f('a', (0), 'c'); + var s; for (var v of args) s += v; } diff --git a/js/src/jit/BaselineIC.cpp b/js/src/jit/BaselineIC.cpp index 24437ae47..c6441090f 100644 --- a/js/src/jit/BaselineIC.cpp +++ b/js/src/jit/BaselineIC.cpp @@ -1667,7 +1667,9 @@ TryAttachGetElemStub(JSContext* cx, JSScript* script, jsbytecode* pc, ICGetElem_ RootedObject obj(cx, &lhs.toObject()); // Check for ArgumentsObj[int] accesses - if (obj->is() && rhs.isInt32()) { + if (obj->is() && rhs.isInt32() && + !obj->as().hasOverriddenElement()) + { ICGetElem_Arguments::Which which = ICGetElem_Arguments::Mapped; if (obj->is()) which = ICGetElem_Arguments::Unmapped; @@ -2568,10 +2570,11 @@ ICGetElem_Arguments::Compiler::generateStubCode(MacroAssembler& masm) // Get initial ArgsObj length value. masm.unboxInt32(Address(objReg, ArgumentsObject::getInitialLengthSlotOffset()), scratchReg); - // Test if length has been overridden. + // Test if length or any element have been overridden. masm.branchTest32(Assembler::NonZero, scratchReg, - Imm32(ArgumentsObject::LENGTH_OVERRIDDEN_BIT), + Imm32(ArgumentsObject::LENGTH_OVERRIDDEN_BIT | + ArgumentsObject::ELEMENT_OVERRIDDEN_BIT), &failure); // Length has not been overridden, ensure that R1 is an integer and is <= length. diff --git a/js/src/jit/IonCaches.cpp b/js/src/jit/IonCaches.cpp index 869652138..1d1ff652b 100644 --- a/js/src/jit/IonCaches.cpp +++ b/js/src/jit/IonCaches.cpp @@ -535,6 +535,9 @@ IsOptimizableArgumentsObjectForGetElem(JSObject* obj, Value idval) if (argsObj.isAnyElementDeleted()) return false; + if (argsObj.hasOverriddenElement()) + return false; + if (!idval.isInt32()) return false; @@ -4210,9 +4213,12 @@ GetPropertyIC::tryAttachArgumentsElement(JSContext* cx, HandleScript outerScript masm.branchTestObjClass(Assembler::NotEqual, object(), tmpReg, obj->getClass(), &failures); - // Get initial ArgsObj length value, test if length has been overridden. + // Get initial ArgsObj length value, test if length or any element have + // been overridden. masm.unboxInt32(Address(object(), ArgumentsObject::getInitialLengthSlotOffset()), tmpReg); - masm.branchTest32(Assembler::NonZero, tmpReg, Imm32(ArgumentsObject::LENGTH_OVERRIDDEN_BIT), + masm.branchTest32(Assembler::NonZero, tmpReg, + Imm32(ArgumentsObject::LENGTH_OVERRIDDEN_BIT | + ArgumentsObject::ELEMENT_OVERRIDDEN_BIT), &failures); masm.rshiftPtr(Imm32(ArgumentsObject::PACKED_BITS_COUNT), tmpReg); diff --git a/js/src/tests/ecma_6/Function/arguments-extra-property.js b/js/src/tests/ecma_6/Function/arguments-extra-property.js new file mode 100644 index 000000000..017734382 --- /dev/null +++ b/js/src/tests/ecma_6/Function/arguments-extra-property.js @@ -0,0 +1,33 @@ +var BUGNUMBER = 1263811; +var summary = "GetElem for modified arguments shouldn't be optimized to get original argument."; + +print(BUGNUMBER + ": " + summary); + +function testModifyFirst() { + function f() { + Object.defineProperty(arguments, 1, { value: 30 }); + assertEq(arguments[1], 30); + } + for (let i = 0; i < 10; i++) + f(10, 20); +} +testModifyFirst(); + +function testModifyLater() { + function f() { + var ans = 20; + for (let i = 0; i < 10; i++) { + if (i == 5) { + Object.defineProperty(arguments, 1, { value: 30 }); + ans = 30; + } + assertEq(arguments[1], ans); + } + } + for (let i = 0; i < 10; i++) + f(10, 20); +} +testModifyLater(); + +if (typeof reportCompare === "function") + reportCompare(true, true); diff --git a/js/src/tests/ecma_6/Function/arguments-iterator.js b/js/src/tests/ecma_6/Function/arguments-iterator.js new file mode 100644 index 000000000..5610b8a24 --- /dev/null +++ b/js/src/tests/ecma_6/Function/arguments-iterator.js @@ -0,0 +1,167 @@ +var BUGNUMBER = 992617; +var summary = "Implement arguments[@@iterator]."; + +print(BUGNUMBER + ": " + summary); + +// MappedArgumentsObject +let mapped = [ + function(a, b, c) { + assertEq(Symbol.iterator in arguments, true); + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function(a, b, c) { + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function(a, b, c) { + arguments[Symbol.iterator] = 10; + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function(a, b, c) { + Object.defineProperty(arguments, Symbol.iterator, { + value: 10, writable: true, enumerable: true, configurable: true + }); + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function(a, b, c) { + assertEq(arguments[Symbol.iterator], Array.prototype[Symbol.iterator]); + }, + function(a, b, c) { + assertEq(arguments[Symbol.iterator].name, "values"); + }, + function(a, b, c) { + var desc = Object.getOwnPropertyDescriptor(arguments, Symbol.iterator); + assertEq("value" in desc, true); + assertEq(desc.value, Array.prototype[Symbol.iterator]); + assertEq(desc.writable, true); + assertEq(desc.enumerable, false); + assertEq(desc.configurable, true); + }, + function(a, b, c) { + var iter = arguments[Symbol.iterator](); + assertDeepEq(iter.next(), { value: 10, done: false }); + assertDeepEq(iter.next(), { value: 20, done: false }); + assertDeepEq(iter.next(), { value: 30, done: false }); + assertDeepEq(iter.next(), { value: undefined, done: true }); + }, + function(a, b, c) { + assertDeepEq([...arguments], [10, 20, 30]); + }, + function(a, b, c) { + b = 40; + assertDeepEq([...arguments], [10, 40, 30]); + }, + function(a, b, c) { + arguments.length = 4; + assertDeepEq([...arguments], [10, 20, 30, undefined]); + }, + function(a, b, c) { + arguments[5] = 50; + assertDeepEq([...arguments], [10, 20, 30]); + }, + function(a, b, c) { + arguments[Symbol.iterator] = function*() { + yield 40; + yield 50; + yield 60; + }; + assertDeepEq([...arguments], [40, 50, 60]); + }, +]; +for (let f of mapped) { + f(10, 20, 30); +} + +var g1 = newGlobal(); +assertEq(g1.eval(` +function f(a, b, c) { + return arguments[Symbol.iterator].name; +} +f(1, 2, 3); +`), "values"); + +// UnmappedArgumentsObject +let unmapped = [ + function([a], b, c) { + assertEq(Symbol.iterator in arguments, true); + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function([a], b, c) { + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function([a], b, c) { + arguments[Symbol.iterator] = 10; + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function([a], b, c) { + Object.defineProperty(arguments, Symbol.iterator, { + value: 10, writable: true, enumerable: true, configurable: true + }); + delete arguments[Symbol.iterator]; + assertEq(Symbol.iterator in arguments, false); + }, + function([a], b, c) { + assertEq(arguments[Symbol.iterator], Array.prototype[Symbol.iterator]); + }, + function([a], b, c) { + assertEq(arguments[Symbol.iterator].name, "values"); + }, + function([a], b, c) { + var desc = Object.getOwnPropertyDescriptor(arguments, Symbol.iterator); + assertEq("value" in desc, true); + assertEq(desc.value, Array.prototype[Symbol.iterator]); + assertEq(desc.writable, true); + assertEq(desc.enumerable, false); + assertEq(desc.configurable, true); + }, + function([a], b, c) { + var iter = arguments[Symbol.iterator](); + assertDeepEq(iter.next(), { value: [10], done: false }); + assertDeepEq(iter.next(), { value: 20, done: false }); + assertDeepEq(iter.next(), { value: 30, done: false }); + assertDeepEq(iter.next(), { value: undefined, done: true }); + }, + function([a], b, c) { + assertDeepEq([...arguments], [[10], 20, 30]); + }, + function([a], b, c) { + b = 40; + assertDeepEq([...arguments], [[10], 20, 30]); + }, + function([a], b, c) { + arguments.length = 4; + assertDeepEq([...arguments], [[10], 20, 30, undefined]); + }, + function([a], b, c) { + arguments[5] = 50; + assertDeepEq([...arguments], [[10], 20, 30]); + }, + function([a], b, c) { + arguments[Symbol.iterator] = function*() { + yield 40; + yield 50; + yield 60; + }; + assertDeepEq([...arguments], [40, 50, 60]); + }, +]; +for (let f of unmapped) { + f([10], 20, 30); +} + +var g2 = newGlobal(); +assertEq(g2.eval(` +function f([a], b, c) { + return arguments[Symbol.iterator].name; +} +f([1], 2, 3); +`), "values"); + +if (typeof reportCompare === "function") + reportCompare(true, true); diff --git a/js/src/vm/ArgumentsObject.cpp b/js/src/vm/ArgumentsObject.cpp index 45b2c7cc2..a6ff42a4d 100644 --- a/js/src/vm/ArgumentsObject.cpp +++ b/js/src/vm/ArgumentsObject.cpp @@ -328,6 +328,8 @@ ArgumentsObject::obj_delProperty(JSContext* cx, HandleObject obj, HandleId id, argsobj.markLengthOverridden(); } else if (JSID_IS_ATOM(id, cx->names().callee)) { argsobj.as().clearCallee(); + } else if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) { + argsobj.markIteratorOverridden(); } return result.succeed(); } @@ -398,11 +400,34 @@ MappedArgSetter(JSContext* cx, HandleObject obj, HandleId id, MutableHandleValue NativeDefineProperty(cx, argsobj, id, vp, nullptr, nullptr, attrs, result); } +static bool +DefineArgumentsIterator(JSContext* cx, Handle 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); +} + /* static */ bool MappedArgumentsObject::obj_resolve(JSContext* cx, HandleObject obj, HandleId id, bool* resolvedp) { Rooted argsobj(cx, &obj->as()); + if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) { + if (argsobj->hasOverriddenIterator()) + return true; + + if (!DefineArgumentsIterator(cx, argsobj)) + return false; + *resolvedp = true; + return true; + } + unsigned attrs = JSPROP_SHARED | JSPROP_SHADOWABLE | JSPROP_RESOLVING; if (JSID_IS_INT(id)) { uint32_t arg = uint32_t(JSID_TO_INT(id)); @@ -448,6 +473,10 @@ MappedArgumentsObject::obj_enumerate(JSContext* cx, HandleObject obj) if (!HasProperty(cx, argsobj, id, &found)) return false; + id = SYMBOL_TO_JSID(cx->wellKnownSymbols().iterator); + if (!HasProperty(cx, argsobj, id, &found)) + return false; + for (unsigned i = 0; i < argsobj->initialLength(); i++) { id = INT_TO_JSID(i); if (!HasProperty(cx, argsobj, id, &found)) @@ -519,6 +548,16 @@ UnmappedArgumentsObject::obj_resolve(JSContext* cx, HandleObject obj, HandleId i { Rooted argsobj(cx, &obj->as()); + if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) { + if (argsobj->hasOverriddenIterator()) + return true; + + if (!DefineArgumentsIterator(cx, argsobj)) + return false; + *resolvedp = true; + return true; + } + unsigned attrs = JSPROP_SHARED | JSPROP_SHADOWABLE; GetterOp getter = UnmappedArgGetter; SetterOp setter = UnmappedArgSetter; @@ -570,6 +609,10 @@ UnmappedArgumentsObject::obj_enumerate(JSContext* cx, HandleObject obj) if (!HasProperty(cx, argsobj, id, &found)) return false; + id = SYMBOL_TO_JSID(cx->wellKnownSymbols().iterator); + if (!HasProperty(cx, argsobj, id, &found)) + return false; + for (unsigned i = 0; i < argsobj->initialLength(); i++) { id = INT_TO_JSID(i); if (!HasProperty(cx, argsobj, id, &found)) diff --git a/js/src/vm/ArgumentsObject.h b/js/src/vm/ArgumentsObject.h index 0eecff6d6..de928beb2 100644 --- a/js/src/vm/ArgumentsObject.h +++ b/js/src/vm/ArgumentsObject.h @@ -109,8 +109,9 @@ static const unsigned ARGS_LENGTH_MAX = 500 * 1000; * * INITIAL_LENGTH_SLOT * Stores the initial value of arguments.length, plus a bit indicating - * whether arguments.length has been modified. Use initialLength() and - * hasOverriddenLength() to access these values. If arguments.length has + * whether arguments.length and/or arguments[@@iterator] have been + * modified. Use initialLength(), hasOverriddenLength(), and + * hasOverriddenIterator() to access these values. If arguments.length has * been modified, then the current value of arguments.length is stored in * another slot associated with a new property. * DATA_SLOT @@ -125,7 +126,12 @@ class ArgumentsObject : public NativeObject public: static const uint32_t LENGTH_OVERRIDDEN_BIT = 0x1; - static const uint32_t PACKED_BITS_COUNT = 1; + static const uint32_t ITERATOR_OVERRIDDEN_BIT = 0x2; + static const uint32_t ELEMENT_OVERRIDDEN_BIT = 0x4; + static const uint32_t PACKED_BITS_COUNT = 3; + + static_assert(ARGS_LENGTH_MAX <= (UINT32_MAX >> PACKED_BITS_COUNT), + "Max arguments length must fit in available bits"); protected: template @@ -185,6 +191,30 @@ class ArgumentsObject : public NativeObject setFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(v)); } + /* 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)); + } + + /* 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() { + uint32_t v = getFixedSlot(INITIAL_LENGTH_SLOT).toInt32() | ELEMENT_OVERRIDDEN_BIT; + setFixedSlot(INITIAL_LENGTH_SLOT, Int32Value(v)); + } + /* * Because the arguments object is a real object, its elements may be * deleted. This is implemented by setting a 'deleted' flag for the arg diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index eb4c967e1..51a5dafd2 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -1514,6 +1514,13 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId // redefined, it will. if ((desc_.attributes() & JSPROP_RESOLVING) == 0) obj->as().markLengthOverridden(); + } 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(); + } else if (JSID_IS_INT(id)) { + if ((desc_.attributes() & JSPROP_RESOLVING) == 0) + obj->as().markElementOverridden(); } }