From e70320abeaf3b71db63a263eaec9f1480440c6c7 Mon Sep 17 00:00:00 2001 From: Jason Orendorff Date: Mon, 23 Mar 2015 14:32:30 -0500 Subject: [PATCH] Bug 1148652, part 1 - Move array-specific special cases to the top of NativeDefineProperty; update ArraySetLength to be able to cope with incomplete attrs. r=efaust. --HG-- extra : rebase_source : 4c4a58cb7c0819d6af2799e8847cde981f3528b8 --- js/src/jsapi.h | 2 +- js/src/jsarray.cpp | 91 ++++++++++++++++++++++---------------- js/src/vm/NativeObject.cpp | 52 +++++++++++++--------- 3 files changed, 84 insertions(+), 61 deletions(-) diff --git a/js/src/jsapi.h b/js/src/jsapi.h index c5b31a48261d..1051143f2008 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -2595,7 +2595,7 @@ class PropertyDescriptorOperations MOZ_ASSERT(!has(JSPROP_IGNORE_READONLY)); MOZ_ASSERT(!has(JSPROP_IGNORE_VALUE)); MOZ_ASSERT(!has(SHADOWABLE)); - MOZ_ASSERT(desc()->value.isUndefined()); + MOZ_ASSERT(value().isUndefined()); MOZ_ASSERT_IF(!has(JSPROP_GETTER), !getter()); MOZ_ASSERT_IF(!has(JSPROP_SETTER), !setter()); } else { diff --git a/js/src/jsarray.cpp b/js/src/jsarray.cpp index 72551a0fbe8e..508293ce3c75 100644 --- a/js/src/jsarray.cpp +++ b/js/src/jsarray.cpp @@ -505,7 +505,7 @@ js::CanonicalizeArrayLengthValue(JSContext* cx, HandleValue v, uint32_t* newLen) return false; } -/* ES6 20130308 draft 8.4.2.4 ArraySetLength */ +/* ES6 draft rev 34 (2015 Feb 20) 9.4.2.4 ArraySetLength */ bool js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, unsigned attrs, HandleValue value, ObjectOpResult& result) @@ -515,26 +515,26 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, if (!arr->maybeCopyElementsForWrite(cx)) return false; - /* Steps 1-2 are irrelevant in our implementation. */ - - /* Steps 3-5. */ + // Step 1. uint32_t newLen; - if (!CanonicalizeArrayLengthValue(cx, value, &newLen)) - return false; + if (attrs & JSPROP_IGNORE_VALUE) { + // The spec has us calling OrdinaryDefineOwnProperty if + // Desc.[[Value]] is absent, but our implementation is so different that + // this is impossible. Instead, set newLen to the current length and + // proceed to step 9. + newLen = arr->length(); + } else { + // Step 2 is irrelevant in our implementation. - // Abort if we're being asked to change enumerability or configurability. - // (The length property of arrays is non-configurable, so such attempts - // must fail.) This behavior is spread throughout the ArraySetLength spec - // algorithm, but we only need check it once as our array implementation - // is internally so different from the spec algorithm. (ES5 and ES6 define - // behavior by delegating to the default define-own-property algorithm -- - // OrdinaryDefineOwnProperty in ES6, the default [[DefineOwnProperty]] in - // ES5 -- but we reimplement all the conflict-detection bits ourselves here - // so that we can use a customized length representation.) - if (!(attrs & JSPROP_PERMANENT) || (attrs & JSPROP_ENUMERATE)) - return result.fail(JSMSG_CANT_REDEFINE_PROP); + // Steps 3-7. + MOZ_ASSERT_IF(attrs & JSPROP_IGNORE_VALUE, value.isUndefined()); + if (!CanonicalizeArrayLengthValue(cx, value, &newLen)) + return false; - /* Steps 6-7. */ + // Step 8 is irrelevant in our implementation. + } + + // Steps 9-11. bool lengthIsWritable = arr->lengthIsWritable(); #ifdef DEBUG { @@ -543,10 +543,21 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, MOZ_ASSERT(lengthShape->writable() == lengthIsWritable); } #endif - uint32_t oldLen = arr->length(); - /* Steps 8-9 for arrays with non-writable length. */ + // Part of steps 1.a, 12.a, and 16: Fail if we're being asked to change + // enumerability or configurability, or otherwise break the object + // invariants. (ES6 checks these by calling OrdinaryDefineOwnProperty, but + // in SM, the array length property is hardly ordinary.) + if ((attrs & (JSPROP_PERMANENT | JSPROP_IGNORE_PERMANENT)) == 0 || + (attrs & (JSPROP_ENUMERATE | JSPROP_IGNORE_ENUMERATE)) == JSPROP_ENUMERATE || + (attrs & (JSPROP_GETTER | JSPROP_SETTER)) != 0 || + (!lengthIsWritable && (attrs & (JSPROP_READONLY | JSPROP_IGNORE_READONLY)) == 0)) + { + return result.fail(JSMSG_CANT_REDEFINE_PROP); + } + + // Steps 12-13 for arrays with non-writable length. if (!lengthIsWritable) { if (newLen == oldLen) return result.succeed(); @@ -554,7 +565,7 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, return result.fail(JSMSG_CANT_REDEFINE_ARRAY_LENGTH); } - /* Step 8. */ + // Step 19. bool succeeded = true; do { // The initialized length and capacity of an array only need updating @@ -616,10 +627,10 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, // If we're removing a relatively small number of elements, just do // it exactly by the spec. while (newLen < oldLen) { - /* Step 15a. */ + // Step 15a. oldLen--; - /* Steps 15b-d. */ + // Steps 15b-d. ObjectOpResult deleteSucceeded; if (!DeleteElement(cx, arr, oldLen, deleteSucceeded)) return false; @@ -679,7 +690,7 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, MOZ_ASSERT(indexes[i] < index, "indexes should never repeat"); index = indexes[i]; - /* Steps 15b-d. */ + // Steps 15b-d. ObjectOpResult deleteSucceeded; if (!DeleteElement(cx, arr, index, deleteSucceeded)) return false; @@ -692,23 +703,25 @@ js::ArraySetLength(JSContext* cx, Handle arr, HandleId id, } } while (false); - /* Steps 12, 16. */ - - // Yes, we totally drop a non-stub getter/setter from a defineProperty - // API call on the floor here. Given that getter/setter will go away in - // the long run, with accessors replacing them both internally and at the - // API level, just run with this. - RootedShape lengthShape(cx, arr->lookup(cx, id)); - if (!NativeObject::changeProperty(cx, arr, lengthShape, - attrs | JSPROP_PERMANENT | JSPROP_SHARED | - (lengthShape->attributes() & JSPROP_READONLY), - array_length_getter, array_length_setter)) - { - return false; - } - + // Update array length. Technically we should have been doing this + // throughout the loop, in step 19.d.iii. arr->setLength(cx, newLen); + // Step 20. + if (attrs & JSPROP_READONLY) { + // Yes, we totally drop a non-stub getter/setter from a defineProperty + // API call on the floor here. Given that getter/setter will go away in + // the long run, with accessors replacing them both internally and at the + // API level, just run with this. + RootedShape lengthShape(cx, arr->lookup(cx, id)); + if (!NativeObject::changeProperty(cx, arr, lengthShape, + lengthShape->attributes() | JSPROP_READONLY, + array_length_getter, array_length_setter)) + { + return false; + } + } + // All operations past here until the |!succeeded| code must be infallible, // so that all element fields remain properly synchronized. diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 5b49084e7b09..23e2cbf0630f 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -1286,6 +1286,37 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId { desc_.assertValid(); + // Section numbers and step numbers below refer to ES6 draft rev 36 + // (17 March 2015). + // + // This function aims to implement 9.1.6 [[DefineOwnProperty]] as well as + // the [[DefineOwnProperty]] methods described in 9.4.2.1 (arrays), 9.4.4.2 + // (arguments), and 9.4.5.3 (typed array views). + + // Dispense with custom behavior of exotic native objects first. + if (obj->is()) { + // 9.4.2.1 step 2. Redefining an array's length is very special. + Rooted arr(cx, &obj->as()); + if (id == NameToId(cx->names().length)) { + if (!cx->shouldBeJSContext()) + return false; + return ArraySetLength(cx->asJSContext(), arr, id, desc_.attributes(), desc_.value(), + result); + } + + // 9.4.2.1 step 3. Don't extend a fixed-length array. + uint32_t index; + if (IdIsIndex(id, &index)) { + if (WouldDefinePastNonwritableLength(obj, index)) + return result.fail(JSMSG_CANT_DEFINE_PAST_ARRAY_LENGTH); + } + } else if (IsAnyTypedArray(obj)) { + // Don't define new indexed properties on typed arrays. + uint64_t index; + if (IsTypedArrayIndex(id, &index)) + return result.succeed(); + } + Rooted desc(cx, desc_); RootedShape shape(cx); @@ -1422,27 +1453,6 @@ js::NativeDefineProperty(ExclusiveContext* cx, HandleNativeObject obj, HandleId // clear it. desc.setAttributes(ApplyOrDefaultAttributes(desc.attributes()) & ~JSPROP_IGNORE_VALUE); - if (obj->is()) { - Rooted arr(cx, &obj->as()); - if (id == NameToId(cx->names().length)) { - if (!cx->shouldBeJSContext()) - return false; - return ArraySetLength(cx->asJSContext(), arr, id, desc.attributes(), desc.value(), - result); - } - - uint32_t index; - if (IdIsIndex(id, &index)) { - if (WouldDefinePastNonwritableLength(obj, index)) - return result.fail(JSMSG_CANT_DEFINE_PAST_ARRAY_LENGTH); - } - } else if (IsAnyTypedArray(obj)) { - // Don't define new indexed properties on typed arrays. - uint64_t index; - if (IsTypedArrayIndex(id, &index)) - return result.succeed(); - } - // At this point, no mutation has happened yet, but all ES6 error cases // have been dealt with. if (!AddOrChangeProperty(cx, obj, id, desc))