Added: trunk/JSTests/stress/array-prototype-methods-set-length.js (0 => 289164)
--- trunk/JSTests/stress/array-prototype-methods-set-length.js (rev 0)
+++ trunk/JSTests/stress/array-prototype-methods-set-length.js 2022-02-06 00:14:49 UTC (rev 289164)
@@ -0,0 +1,144 @@
+function shouldBe(actual, expected) {
+ if (actual !== expected)
+ throw new Error(`Bad value: ${actual}!\ncreateTestObject:\n${createTestObject}\nmakeLengthReadOnly: ${makeLengthReadOnly}`);
+};
+
+function shouldThrow(func, reExpectedError) {
+ let errorThrown = false;
+ try {
+ func();
+ } catch (error) {
+ errorThrown = true;
+ if (!reExpectedError.test(error.toString()))
+ throw new Error(`Bad error: ${error}!\ncreateTestObject:\n${createTestObject}\nmakeLengthReadOnly: ${makeLengthReadOnly}`);
+ }
+ if (!errorThrown)
+ throw new Error(`Didn't throw!\ncreateTestObject: ${createTestObject}\nmakeLengthReadOnly: ${makeLengthReadOnly}`);
+};
+
+var createTestObject;
+const createTestObjectFunctions = [
+ len => new Array(len),
+ len => new Proxy(new Array(len), {}),
+ len => { const obj = Object.create(Array.prototype); obj.length = len; return obj; },
+];
+
+var makeLengthReadOnly;
+const makeLengthReadOnlyFunctions = [
+ arr => { Object.freeze(arr); },
+ arr => { Object.defineProperty(arr, "length", { writable: false }); },
+];
+
+var testObject;
+const expectedTypeError = /^TypeError:.+/;
+
+for (createTestObject of createTestObjectFunctions) {
+for (makeLengthReadOnly of makeLengthReadOnlyFunctions) {
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { "use strict"; testObject.length = 0; }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.pop(); }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(1);
+ testObject[0] = 1;
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.pop(); }, expectedTypeError);
+ shouldBe(testObject.length, 1);
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.push(); }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.push(1); }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.shift(); }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(1);
+ testObject[0] = 1;
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.shift(); }, expectedTypeError);
+ shouldBe(testObject.length, 1);
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.unshift(); }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(0);
+ makeLengthReadOnly(testObject);
+ shouldThrow(() => { testObject.unshift(1); }, expectedTypeError);
+ shouldBe(testObject.length, 0);
+
+}}
+
+// let's have a bad time?
+
+for (createTestObject of createTestObjectFunctions) {
+for (makeLengthReadOnly of makeLengthReadOnlyFunctions) {
+
+ let arrayPrototypeGet0Calls = 0;
+ let arrayPrototypeSet0Calls = 0;
+
+ Object.defineProperty(Array.prototype, "0", {
+ get() {
+ Object.defineProperty(testObject, "length", { writable: false });
+ arrayPrototypeGet0Calls++;
+ },
+ set(_val) {
+ Object.defineProperty(testObject, "length", { writable: false });
+ arrayPrototypeSet0Calls++;
+ },
+ configurable: true,
+ });
+
+ testObject = createTestObject(1);
+ shouldThrow(() => { testObject.pop(); }, expectedTypeError);
+ shouldBe(arrayPrototypeGet0Calls, 1);
+ shouldBe(testObject.hasOwnProperty(0), false);
+ shouldBe(testObject.length, 1);
+
+ testObject = createTestObject(1);
+ shouldThrow(() => { testObject.shift(); }, expectedTypeError);
+ shouldBe(arrayPrototypeGet0Calls, 2);
+ shouldBe(testObject.hasOwnProperty(0), false);
+ shouldBe(testObject.length, 1);
+
+ testObject = createTestObject(0);
+ shouldThrow(() => { testObject.push(1); }, expectedTypeError);
+ shouldBe(arrayPrototypeSet0Calls, 1);
+ shouldBe(testObject.hasOwnProperty(0), false);
+ shouldBe(testObject.length, 0);
+
+ testObject = createTestObject(0);
+ shouldThrow(() => { testObject.unshift(1); }, expectedTypeError);
+ shouldBe(arrayPrototypeSet0Calls, 2);
+ shouldBe(testObject.hasOwnProperty(0), false);
+ shouldBe(testObject.length, 0);
+
+}}
+
+// [[DefineOwnProperty]] shouldn't fail
+
+for (createTestObject of createTestObjectFunctions) {
+for (makeLengthReadOnly of makeLengthReadOnlyFunctions) {
+
+ testObject = createTestObject(42);
+ makeLengthReadOnly(testObject);
+ Object.defineProperty(testObject, "length", { value: 42 }); // doesn't throw
+ shouldBe(Reflect.defineProperty(testObject, "length", { value: 42 }), true);
+ shouldBe(testObject.length, 42);
+
+}}
Modified: trunk/Source/_javascript_Core/ChangeLog (289163 => 289164)
--- trunk/Source/_javascript_Core/ChangeLog 2022-02-05 23:42:27 UTC (rev 289163)
+++ trunk/Source/_javascript_Core/ChangeLog 2022-02-06 00:14:49 UTC (rev 289164)
@@ -1,3 +1,35 @@
+2022-02-05 Alexey Shvayka <ashva...@apple.com>
+
+ Attempting to [[Set]] JSArray's read-only "length" should throw even with current [[Value]]
+ https://bugs.webkit.org/show_bug.cgi?id=221177
+
+ Reviewed by Saam Barati.
+
+ As per OrdinarySet algorithm [1]. To achieve that, while ensuring no error is thrown
+ if read-only "length" isn't actually changed via [[DefineOwnProperty]] [2], this patch
+ moves `newLength == oldLength` check to JSArray::defineOwnProperty().
+
+ That is guaranteed to be correct because:
+ a) it's the only caller of setLengthWithArrayStorage() that performs [[DefineOwnProperty]],
+ while others implement [[Set]];
+ b) there can't possibly be array indices that JSArray::defineOwnProperty() has to remove,
+ and even the spec a shortcut here [3].
+
+ All code paths in pop() / shift() / push() / unshift() are covered by the newly added test,
+ as well as JSArray's [[DefineOwnProperty]], while slice() / splice() / etc were vetted to
+ [[Set]] "length" according to the spec.
+
+ Aligns JSC with SpiderMonkey and partly with V8, which is correct for Object.freeze()
+ but not for `Object.defineProperty(array, "length", { writable: false })`.
+
+ [1]: https://tc39.es/ecma262/#sec-ordinarysetwithowndescriptor (step 2.a)
+ [2]: https://tc39.es/ecma262/#sec-validateandapplypropertydescriptor (step 5 and 7)
+ [3]: https://tc39.es/ecma262/#sec-arraysetlength (step 11)
+
+ * runtime/JSArray.cpp:
+ (JSC::JSArray::defineOwnProperty):
+ (JSC::JSArray::setLengthWithArrayStorage):
+
2022-02-05 Yusuke Suzuki <ysuz...@apple.com>
Thread suspend and resume should take a global lock to avoid deadlock
Modified: trunk/Source/_javascript_Core/runtime/JSArray.cpp (289163 => 289164)
--- trunk/Source/_javascript_Core/runtime/JSArray.cpp 2022-02-05 23:42:27 UTC (rev 289163)
+++ trunk/Source/_javascript_Core/runtime/JSArray.cpp 2022-02-06 00:14:49 UTC (rev 289164)
@@ -191,8 +191,11 @@
}
// setLength() clears indices >= newLength and sets correct "length" value if [[Delete]] fails (step 17.b.i)
- bool success = array->setLength(globalObject, newLength, throwException);
- EXCEPTION_ASSERT(!scope.exception() || !success);
+ bool success = true;
+ if (newLength != array->length()) {
+ success = array->setLength(globalObject, newLength, throwException);
+ EXCEPTION_ASSERT(!scope.exception() || !success);
+ }
if (descriptor.writablePresent())
array->setLengthWritable(globalObject, descriptor.writable());
return success;
@@ -415,8 +418,6 @@
auto scope = DECLARE_THROW_SCOPE(vm);
unsigned length = storage->length();
- if (newLength == length)
- return true;
// If the length is read only then we enter sparse mode, so should enter the following 'if'.
ASSERT(isLengthWritable() || storage->m_sparseMap);
