Changeset 273750 in webkit

Timestamp:

Mar 2, 2021 12:00:31 PM (17 months ago)

Author:

Alexey Shvayka

Message:

TypedArray's DefineOwnProperty? should throw in case of failure
​https://bugs.webkit.org/show_bug.cgi?id=220492

Reviewed by Darin Adler.

JSTests:

• stress/array-species-config-array-constructor.js:
• stress/put-direct-index-broken-2.js:

Update assertions, which are most certainly incorrect, partly aligning JSC with V8.

• stress/typedarray-access-monomorphic-neutered.js:
• stress/typedarray-access-neutered.js:
• stress/typedarray-defineOwnProperty-error.js: Added.
• test262/expectations.yaml:

Incorrect tests are being updated at ​https://github.com/tc39/test262/pull/2958.

Source/JavaScriptCore:

While web reality [1] requires failures of the most TypeArray internal methods to be
silent, DefineOwnProperty? can fail loudly if called via Object.defineProperty.

With this patch, TypeError is thrown for detached buffer, out-of-bounds index, and
non-index canonical numeric string key. Aligns JSC with the spec [2], V8, and SM.

[1]: ​https://github.com/tc39/ecma262/pull/2164
[2]: ​https://tc39.es/ecma262/#sec-integer-indexed-exotic-objects-defineownproperty-p-desc (step 3.b)

• runtime/JSGenericTypedArrayViewInlines.h:

(JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/array-species-config-array-constructor.js (modified) (1 diff)
• JSTests/stress/put-direct-index-broken-2.js (modified) (1 diff)
• JSTests/stress/typedarray-access-monomorphic-neutered.js (modified) (2 diffs)
• JSTests/stress/typedarray-access-neutered.js (modified) (1 diff)
• JSTests/stress/typedarray-defineOwnProperty-error.js (added)
• JSTests/test262/expectations.yaml (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h (modified) (3 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2021-03-02  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        TypedArray's [[DefineOwnProperty]] should throw in case of failure
+        https://bugs.webkit.org/show_bug.cgi?id=220492
+
+        Reviewed by Darin Adler.
+
+        * stress/array-species-config-array-constructor.js:
+        * stress/put-direct-index-broken-2.js:
+        Update assertions, which are most certainly incorrect, partly aligning JSC with V8.
+
+        * stress/typedarray-access-monomorphic-neutered.js:
+        * stress/typedarray-access-neutered.js:
+        * stress/typedarray-defineOwnProperty-error.js: Added.
+        * test262/expectations.yaml:
+        Incorrect tests are being updated at https://github.com/tc39/test262/pull/2958.
+
 2021-03-01  Yusuke Suzuki  <ysuzuki@apple.com>
 

trunk/JSTests/stress/array-species-config-array-constructor.js

@@ -32 +32 @@
 
 function test() {
-    foo.concat([1]);
+    shouldThrow(() => {
+        foo.concat([1]);
+    }, "TypeError: Attempting to store out-of-bounds property on a typed array at index: 0");
     foo = [1,2,3,4];
     foo.slice(0);

trunk/JSTests/stress/put-direct-index-broken-2.js

@@ -57 +57 @@
         err = e;
     }
-    assert(!err);
+    assert(err.toString() == "TypeError: Attempting to store out-of-bounds property on a typed array at index: 0");
 });
 

trunk/JSTests/stress/typedarray-access-monomorphic-neutered.js

@@ -21 +21 @@
     testNoException("array[0] = 1", array);
     testNoException("array[i] = 1", array);
-    testNoException("Object.defineProperty(array, 0, { value: 1, writable: true, configurable: true, enumerable: true })", array);
 }
 
@@ -41 +40 @@
     testFTLNoException("array[0] = 1", array, failArray);
     testFTLNoException("array[i] = 1", array, failArray);
-    testFTLNoException("Object.defineProperty(array, 0, { value: 1, writable: true, configurable: true, enumerable: true })", array, failArray);
 }

trunk/JSTests/stress/typedarray-access-neutered.js

@@ -17 +17 @@
     testNoException((array) => Object.getOwnPropertyDescriptor(array, 0), i);
     testNoException((array) => array[0] = 1, i);
-    testNoException((array) => Object.defineProperty(array, 0, { value: 1, writable: true, configurable: true, enumerable: true }), i)
 }

trunk/JSTests/test262/expectations.yaml

@@ -827 +827 @@
   default: 'SyntaxError: Invalid regular expression: number too large in {} quantifier'
   strict mode: 'SyntaxError: Invalid regular expression: number too large in {} quantifier'
+test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/BigInt/tonumber-value-detached-buffer.js:
+  default: 'Test262Error: Reflect.defineProperty(ta, 0, {value: {valueOf() {$DETACHBUFFER(ta.buffer); return 42n;}}}) must return false Expected SameValue(«true», «false») to be true (Testing with BigInt64Array.)'
+  strict mode: 'Test262Error: Reflect.defineProperty(ta, 0, {value: {valueOf() {$DETACHBUFFER(ta.buffer); return 42n;}}}) must return false Expected SameValue(«true», «false») to be true (Testing with BigInt64Array.)'
+test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/tonumber-value-detached-buffer.js:
+  default: 'Test262Error: Reflect.defineProperty(ta, 0, {value: {valueOf() {$DETACHBUFFER(ta.buffer); return 42;}}} ) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
+  strict mode: 'Test262Error: Reflect.defineProperty(ta, 0, {value: {valueOf() {$DETACHBUFFER(ta.buffer); return 42;}}} ) must return false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
 test/intl402/DateTimeFormat/prototype/formatRange/en-US.js:
   default: 'Test262Error: Expected SameValue(«1/3/2019 – 1/5/2019», «1/3/2019 – 1/5/2019») to be true'

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2021-03-02  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        TypedArray's [[DefineOwnProperty]] should throw in case of failure
+        https://bugs.webkit.org/show_bug.cgi?id=220492
+
+        Reviewed by Darin Adler.
+
+        While web reality [1] requires failures of the most TypeArray internal methods to be
+        silent, [[DefineOwnProperty]] can fail loudly if called via Object.defineProperty.
+
+        With this patch, TypeError is thrown for detached buffer, out-of-bounds index, and
+        non-index canonical numeric string key. Aligns JSC with the spec [2], V8, and SM.
+
+        [1]: https://github.com/tc39/ecma262/pull/2164
+        [2]: https://tc39.es/ecma262/#sec-integer-indexed-exotic-objects-defineownproperty-p-desc (step 3.b)
+
+        * runtime/JSGenericTypedArrayViewInlines.h:
+        (JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
+
 2021-03-01  Keith Miller  <keith_miller@apple.com>
 

trunk/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h

@@ -404 +404 @@
         };
 
-        if (index.value() >= thisObject->m_length)
-            return false;
+        if (thisObject->isDetached())
+            return typeError(globalObject, scope, shouldThrow, typedArrayBufferHasBeenDetachedErrorMessage);
+
+        if (!thisObject->inBounds(index.value()))
+            return throwTypeErrorIfNeeded("Attempting to store out-of-bounds property on a typed array at index: ");
 
         if (descriptor.isAccessorDescriptor())
@@ -419 +422 @@
             return throwTypeErrorIfNeeded("Attempting to store non-writable property on a typed array at index: ");
 
+        scope.release();
         if (descriptor.value())
-            RELEASE_AND_RETURN(scope, thisObject->setIndex(globalObject, index.value(), descriptor.value()));
+            thisObject->setIndex(globalObject, index.value(), descriptor.value());
 
         return true;
@@ -426 +430 @@
 
     if (isCanonicalNumericIndexString(propertyName))
-        return false;
+        return typeError(globalObject, scope, shouldThrow, "Attempting to store canonical numeric string property on a typed array"_s);
 
     RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, globalObject, propertyName, descriptor, shouldThrow));