Changeset 267037 in webkit
- Timestamp:
- Sep 14, 2020 1:48:38 PM (4 years ago)
- Location:
- trunk
- Files:
-
- 4 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JSTests/ChangeLog
r267032 r267037 1 2020-09-14 Alexey Shvayka <shvaikalesh@gmail.com> 2 3 ArraySetLength should coerce [[Value]] before descriptor validation 4 https://bugs.webkit.org/show_bug.cgi?id=158791 5 6 Reviewed by Darin Adler. 7 8 * test262/expectations.yaml: Mark 4 test cases as passing. 9 1 10 2020-09-14 Saam Barati <sbarati@apple.com> 2 11 -
trunk/JSTests/test262/expectations.yaml
r267029 r267037 604 604 test/annexB/language/global-code/switch-dflt-global-skip-early-err.js: 605 605 default: "SyntaxError: Cannot declare a function that shadows a let/const/class/function variable 'f' in strict mode." 606 test/built-ins/Array/length/define-own-prop-length-coercion-order.js:607 default: 'Bad exit code: 11'608 strict mode: 'Bad exit code: 11'609 test/built-ins/Array/length/define-own-prop-length-overflow-order.js:610 default: 'Test262Error: Expected a RangeError but got a TypeError'611 strict mode: 'Test262Error: Expected a RangeError but got a TypeError'612 606 test/built-ins/ArrayBuffer/prototype/byteLength/detached-buffer.js: 613 607 default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all' -
trunk/Source/JavaScriptCore/ChangeLog
r267032 r267037 1 2020-09-14 Alexey Shvayka <shvaikalesh@gmail.com> 2 3 ArraySetLength should coerce [[Value]] before descriptor validation 4 https://bugs.webkit.org/show_bug.cgi?id=158791 5 6 Reviewed by Darin Adler. 7 8 This patch: 9 10 1. Moves [[Value]] coercion before descriptor validation as per spec [1], 11 which fixes ASSERT() failure and aligns JSC with V8 & SpiderMonkey. 12 13 2. Prevents JSArray::setLengthWithArrayStorage() from throwing if the length 14 is unchanged, even if it's read-only [2]. 15 16 3. Refactors JSArray::defineOwnProperty() leveraging #2 to always perform 17 setLength(), which greatly reduces the number of checks, branches, 18 and setLengthWritable() calls. 19 20 Following the ArraySetLength spec steps precisely [1] would result in 21 more difficult-to-follow code because descriptor validation [2] is inlined 22 and [[Delete]] failures are handled in setLength(). 23 24 This change is performance-neutral as it doesn't affect JSArray::put(), 25 which was vetted to be spec-correct and is covered by test262 suite. 26 27 [1]: https://tc39.es/ecma262/#sec-arraysetlength (steps 3-4) 28 [2]: https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor (step 7.a.ii) 29 30 * runtime/JSArray.cpp: 31 (JSC::JSArray::defineOwnProperty): 32 (JSC::JSArray::setLengthWithArrayStorage): 33 1 34 2020-09-14 Saam Barati <sbarati@apple.com> 2 35 -
trunk/Source/JavaScriptCore/runtime/JSArray.cpp
r266907 r267037 142 142 } 143 143 144 // Defined in ES5.1 15.4.5.1144 // https://tc39.es/ecma262/#sec-array-exotic-objects-defineownproperty-p-desc 145 145 bool JSArray::defineOwnProperty(JSObject* object, JSGlobalObject* globalObject, PropertyName propertyName, const PropertyDescriptor& descriptor, bool throwException) 146 146 { … … 150 150 JSArray* array = jsCast<JSArray*>(object); 151 151 152 // 3. If P is "length", then 152 // 2. If P is "length", then 153 // https://tc39.es/ecma262/#sec-arraysetlength 153 154 if (propertyName == vm.propertyNames->length) { 154 // All paths through length definition call the default [[DefineOwnProperty]], hence: 155 // from ES5.1 8.12.9 7.a. 155 // FIXME: Nothing prevents this from being called on a RuntimeArray, and the length function will always return 0 in that case. 156 unsigned newLength = array->length(); 157 if (descriptor.value()) { 158 newLength = descriptor.value().toUInt32(globalObject); 159 RETURN_IF_EXCEPTION(scope, false); 160 double valueAsNumber = descriptor.value().toNumber(globalObject); 161 RETURN_IF_EXCEPTION(scope, false); 162 if (valueAsNumber != static_cast<double>(newLength)) { 163 throwRangeError(globalObject, scope, "Invalid array length"_s); 164 return false; 165 } 166 } 167 168 // OrdinaryDefineOwnProperty (https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor) at steps 1.a, 11.a, and 15 is now performed: 169 // 4. If current.[[Configurable]] is false, then 170 // 4.a. If Desc.[[Configurable]] is present and its value is true, return false. 156 171 if (descriptor.configurablePresent() && descriptor.configurable()) 157 172 return typeError(globalObject, scope, throwException, UnconfigurablePropertyChangeConfigurabilityError); 158 // from ES5.1 8.12.9 7.b.173 // 4.b. If Desc.[[Enumerable]] is present and SameValue(Desc.[[Enumerable]], current.[[Enumerable]]) is false, return false. 159 174 if (descriptor.enumerablePresent() && descriptor.enumerable()) 160 175 return typeError(globalObject, scope, throwException, UnconfigurablePropertyChangeEnumerabilityError); 161 162 // a. If the [[Value]] field of Desc is absent, then 163 // a.i. Return the result of calling the default [[DefineOwnProperty]] internal method (8.12.9) on A passing "length", Desc, and Throw as arguments. 176 // 6. Else if SameValue(IsDataDescriptor(current), IsDataDescriptor(Desc)) is false, then 177 // 6.a. If current.[[Configurable]] is false, return false. 164 178 if (descriptor.isAccessorDescriptor()) 165 179 return typeError(globalObject, scope, throwException, UnconfigurablePropertyChangeAccessMechanismError); 166 // from ES5.1 8.12.9 10.a. 167 if (!array->isLengthWritable() && descriptor.writablePresent() && descriptor.writable()) 168 return typeError(globalObject, scope, throwException, UnconfigurablePropertyChangeWritabilityError); 169 // This descriptor is either just making length read-only, or changing nothing! 170 if (!descriptor.value()) { 171 if (descriptor.writablePresent()) 172 array->setLengthWritable(globalObject, descriptor.writable()); 173 return true; 174 } 175 176 // b. Let newLenDesc be a copy of Desc. 177 // c. Let newLen be ToUint32(Desc.[[Value]]). 178 unsigned newLen = descriptor.value().toUInt32(globalObject); 179 RETURN_IF_EXCEPTION(scope, false); 180 // d. If newLen is not equal to ToNumber( Desc.[[Value]]), throw a RangeError exception. 181 double valueAsNumber = descriptor.value().toNumber(globalObject); 182 RETURN_IF_EXCEPTION(scope, false); 183 if (newLen != valueAsNumber) { 184 JSC::throwException(globalObject, scope, createRangeError(globalObject, "Invalid array length"_s)); 185 return false; 186 } 187 188 // Based on SameValue check in 8.12.9, this is always okay. 189 // FIXME: Nothing prevents this from being called on a RuntimeArray, and the length function will always return 0 in that case. 190 if (newLen == array->length()) { 191 if (descriptor.writablePresent()) 192 array->setLengthWritable(globalObject, descriptor.writable()); 193 return true; 194 } 195 196 // e. Set newLenDesc.[[Value] to newLen. 197 // f. If newLen >= oldLen, then 198 // f.i. Return the result of calling the default [[DefineOwnProperty]] internal method (8.12.9) on A passing "length", newLenDesc, and Throw as arguments. 199 // g. Reject if oldLenDesc.[[Writable]] is false. 200 if (!array->isLengthWritable()) 201 return typeError(globalObject, scope, throwException, ReadonlyPropertyChangeError); 202 203 // h. If newLenDesc.[[Writable]] is absent or has the value true, let newWritable be true. 204 // i. Else, 205 // i.i. Need to defer setting the [[Writable]] attribute to false in case any elements cannot be deleted. 206 // i.ii. Let newWritable be false. 207 // i.iii. Set newLenDesc.[[Writable] to true. 208 // j. Let succeeded be the result of calling the default [[DefineOwnProperty]] internal method (8.12.9) on A passing "length", newLenDesc, and Throw as arguments. 209 // k. If succeeded is false, return false. 210 // l. While newLen < oldLen repeat, 211 // l.i. Set oldLen to oldLen – 1. 212 // l.ii. Let deleteSucceeded be the result of calling the [[Delete]] internal method of A passing ToString(oldLen) and false as arguments. 213 // l.iii. If deleteSucceeded is false, then 214 bool success = array->setLength(globalObject, newLen, throwException); 180 // 7. Else if IsDataDescriptor(current) and IsDataDescriptor(Desc) are both true, then 181 // 7.a. If current.[[Configurable]] is false and current.[[Writable]] is false, then 182 if (!array->isLengthWritable()) { 183 // 7.a.i. If Desc.[[Writable]] is present and Desc.[[Writable]] is true, return false. 184 // This check is unaffected by steps 13-14 of ArraySetLength as they change non-writable descriptors only. 185 if (descriptor.writablePresent() && descriptor.writable()) 186 return typeError(globalObject, scope, throwException, UnconfigurablePropertyChangeWritabilityError); 187 // 7.a.ii. If Desc.[[Value]] is present and SameValue(Desc.[[Value]], current.[[Value]]) is false, return false. 188 // This check also covers step 12 of ArraySetLength, which is only reachable if newLen < oldLen. 189 if (newLength != array->length()) 190 return typeError(globalObject, scope, throwException, ReadonlyPropertyChangeError); 191 } 192 193 // setLength() clears indices >= newLength and sets correct "length" value if [[Delete]] fails (step 17.b.i) 194 bool success = array->setLength(globalObject, newLength, throwException); 215 195 EXCEPTION_ASSERT(!scope.exception() || !success); 216 if (!success) {217 // 1. Set newLenDesc.[[Value] to oldLen+1.218 // 2. If newWritable is false, set newLenDesc.[[Writable] to false.219 // 3. Call the default [[DefineOwnProperty]] internal method (8.12.9) on A passing "length", newLenDesc, and false as arguments.220 // 4. Reject.221 if (descriptor.writablePresent())222 array->setLengthWritable(globalObject, descriptor.writable());223 return false;224 }225 226 // m. If newWritable is false, then227 // i. Call the default [[DefineOwnProperty]] internal method (8.12.9) on A passing "length",228 // Property Descriptor{[[Writable]]: false}, and false as arguments. This call will always229 // return true.230 196 if (descriptor.writablePresent()) 231 197 array->setLengthWritable(globalObject, descriptor.writable()); 232 // n. Return true. 233 return true; 198 return success; 234 199 } 235 200 … … 267 232 } 268 233 269 // ECMA 15.4.5.1234 // https://tc39.es/ecma262/#sec-array-exotic-objects-defineownproperty-p-desc 270 235 bool JSArray::put(JSCell* cell, JSGlobalObject* globalObject, PropertyName propertyName, JSValue value, PutPropertySlot& slot) 271 236 { … … 456 421 457 422 unsigned length = storage->length(); 423 if (newLength == length) 424 return true; 458 425 459 426 // If the length is read only then we enter sparse mode, so should enter the following 'if'.
Note: See TracChangeset
for help on using the changeset viewer.