diff --git a/js/src/vm/NativeObject.cpp b/js/src/vm/NativeObject.cpp index 60eb2dd4fcee..8c1b410c61d0 100644 --- a/js/src/vm/NativeObject.cpp +++ b/js/src/vm/NativeObject.cpp @@ -1408,9 +1408,8 @@ GetExistingPropertyValue(JSContext* cx, HandleNativeObject obj, HandleId id, } /* - * If ES6 draft rev 37 9.1.6.3 ValidateAndApplyPropertyDescriptor step 4 would - * return early, because desc is redundant with an existing own property obj[id], - * then set *redundant = true and return true. + * If desc is redundant with an existing own property obj[id], then set + * |*redundant = true| and return true. */ static bool DefinePropertyIsRedundant(JSContext* cx, HandleNativeObject obj, HandleId id, @@ -1419,14 +1418,14 @@ DefinePropertyIsRedundant(JSContext* cx, HandleNativeObject obj, HandleId id, { *redundant = false; - if (desc.hasConfigurable() && desc.configurable() != ((shapeAttrs & JSPROP_PERMANENT) == 0)) + if (desc.hasConfigurable() && desc.configurable() != IsConfigurable(shapeAttrs)) return true; - if (desc.hasEnumerable() && desc.enumerable() != ((shapeAttrs & JSPROP_ENUMERATE) != 0)) + if (desc.hasEnumerable() && desc.enumerable() != IsEnumerable(shapeAttrs)) return true; if (desc.isDataDescriptor()) { - if ((shapeAttrs & (JSPROP_GETTER | JSPROP_SETTER)) != 0) + if (IsAccessorDescriptor(shapeAttrs)) return true; - if (desc.hasWritable() && desc.writable() != ((shapeAttrs & JSPROP_READONLY) == 0)) + if (desc.hasWritable() && desc.writable() != IsWritable(shapeAttrs)) return true; if (desc.hasValue()) { // Get the current value of the existing property. @@ -1444,8 +1443,8 @@ DefinePropertyIsRedundant(JSContext* cx, HandleNativeObject obj, HandleId id, return false; } - // The specification calls for SameValue here, but it seems to be a - // bug. See . + // Don't call SameValue here to ensure we properly update distinct + // NaN values. if (desc.value() != currentValue) return true; } @@ -1483,7 +1482,8 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, { desc_.assertValid(); - // Section numbers and step numbers below refer to ES2016. + // Section numbers and step numbers below refer to ES2018, draft rev + // 540b827fccf6122a984be99ab9af7be20e3b5562. // // 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 @@ -1520,8 +1520,8 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, // 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 - // property won't already exist; whereas the first time it is - // redefined, it will. + // 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(); } else if (JSID_IS_SYMBOL(id) && JSID_TO_SYMBOL(id) == cx->wellKnownSymbols().iterator) { @@ -1569,7 +1569,7 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, return result.succeed(); } - // Steps 3-4. (Step 3 is a special case of step 4.) We use shapeAttrs as a + // Step 3 and 7.a.i.3, 8.a.iii, 10 (partially). We use shapeAttrs as a // stand-in for shape in many places below, since shape might not be a // pointer to a real Shape (see IsImplicitDenseOrTypedArrayElement). unsigned shapeAttrs = GetPropertyAttributes(obj, prop); @@ -1596,7 +1596,7 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, obj->is() && !obj->getClass()->isDOMClass(); - // Step 5. + // Step 4. if (!IsConfigurable(shapeAttrs) && !skipRedefineChecks) { if (desc.hasConfigurable() && desc.configurable()) return result.fail(JSMSG_CANT_REDEFINE_PROP); @@ -1610,9 +1610,9 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, if (!desc.hasEnumerable()) desc.setEnumerable(IsEnumerable(shapeAttrs)); - // Steps 6-9. + // Steps 5-8. if (desc.isGenericDescriptor()) { - // Step 6. No further validation is required. + // Step 5. No further validation is required. // Fill in desc. A generic descriptor has none of these fields, so copy // everything from shape. @@ -1631,7 +1631,7 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, desc.setSetterObject(prop.shape()->setterObject()); } } else if (desc.isDataDescriptor() != IsDataDescriptor(shapeAttrs)) { - // Step 7. + // Step 6. if (!IsConfigurable(shapeAttrs) && !skipRedefineChecks) return result.fail(JSMSG_CANT_REDEFINE_PROP); @@ -1642,11 +1642,13 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, prop.setNativeProperty(obj->lookup(cx, id)); } - // Fill in desc fields with default values (steps 7.b.i and 7.c.i). + // Fill in desc fields with default values (steps 6.b.i and 6.c.i). CompletePropertyDescriptor(&desc); } else if (desc.isDataDescriptor()) { - // Step 8. + // Step 7. bool frozen = !IsConfigurable(shapeAttrs) && !IsWritable(shapeAttrs); + + // Step 7.a.i.1. if (frozen && desc.hasWritable() && desc.writable() && !skipRedefineChecks) return result.fail(JSMSG_CANT_REDEFINE_PROP); @@ -1666,7 +1668,7 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, // Fill in desc.[[Value]]. desc.setValue(currentValue); } else { - // Step 8.a.ii.1. + // Step 7.a.i.2. bool same; MOZ_ASSERT(!cx->helperThread()); if (!SameValue(cx, desc.value(), currentValue, &same)) @@ -1676,16 +1678,21 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, } } + // Step 7.a.i.3. + if (frozen && !skipRedefineChecks) + return result.succeed(); + if (!desc.hasWritable()) desc.setWritable(IsWritable(shapeAttrs)); } else { - // Step 9. + // Step 8. MOZ_ASSERT(prop.shape()->isAccessorDescriptor()); MOZ_ASSERT(desc.isAccessorDescriptor()); // The spec says to use SameValue, but since the values in // question are objects, we can just compare pointers. if (desc.hasSetterObject()) { + // Step 8.a.i. if (!IsConfigurable(shapeAttrs) && desc.setterObject() != prop.shape()->setterObject() && !skipRedefineChecks) @@ -1697,6 +1704,7 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, desc.setSetterObject(prop.shape()->setterObject()); } if (desc.hasGetterObject()) { + // Step 8.a.ii. if (!IsConfigurable(shapeAttrs) && desc.getterObject() != prop.shape()->getterObject() && !skipRedefineChecks) @@ -1707,11 +1715,15 @@ js::NativeDefineProperty(JSContext* cx, HandleNativeObject obj, HandleId id, // Fill in desc.[[Get]] from shape. desc.setGetterObject(prop.shape()->getterObject()); } + + // Step 8.a.iii (Omitted). } - // Step 10. + // Step 9. if (!AddOrChangeProperty(cx, obj, id, desc)) return false; + + // Step 10. return result.succeed(); }