Merge r244950 - TypedArrays should not store properties that are cano…

…nical numeric indices

https://bugs.webkit.org/show_bug.cgi?id=197228
<rdar://problem/49557381>

Reviewed by Saam Barati.

JSTests:

* stress/array-species-config-array-constructor.js:
(test):
* stress/put-direct-index-broken-2.js:
* stress/typed-array-canonical-numeric-index-string.js: Added.
(makeTest.assert):
(makeTest):
(const.testInvalidIndices.makeTest.set assert):
(const.testInvalidIndices.makeTest):
(const.makeTestValidIndex.configurable.set assert):
(const.makeTestValidIndex.configurable):
* stress/typedarray-access-monomorphic-neutered.js:
(checkNoException):
(testNoException):
(testFTLNoException):
* stress/typedarray-access-neutered.js:
(testNoException):
* stress/typedarray-getownproperty-not-configurable.js:
(foo):
* test262/expectations.yaml:

Source/JavaScriptCore:

According to the spec[1]:
- TypedArrays should not perform an ordinary GetOwnProperty/SetOwnProperty if the index is a
CanonicalNumericIndexString, but invalid according to IntegerIndexedElementGet and similar
functions. I.e., there are a few properties that should not be set in a TypedArray, like NaN,
Infinity and -0.
- On DefineOwnProperty, the out-of-bounds check should be performed before validating the property
descriptor.
- On GetOwnProperty, the returned descriptor for numeric properties should have writable set to true.

[1]: https://www.ecma-international.org/ecma-262/9.0/index.html#sec-integer-indexed-exotic-objects-defineownproperty-p-desc

* CMakeLists.txt:
* JavaScriptCore.xcodeproj/project.pbxproj:
* runtime/JSGenericTypedArrayViewInlines.h:
(JSC::JSGenericTypedArrayView<Adaptor>::getOwnPropertySlot):
(JSC::JSGenericTypedArrayView<Adaptor>::put):
(JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
(JSC::JSGenericTypedArrayView<Adaptor>::getOwnPropertySlotByIndex):
(JSC::JSGenericTypedArrayView<Adaptor>::putByIndex):
* runtime/PropertyName.h:
(JSC::isCanonicalNumericIndexString):

LayoutTests:

* fast/canvas/canvas-ImageData-behaviour-expected.txt:
* fast/canvas/canvas-ImageData-behaviour.js:

JSTests/ChangeLog

@@ -1,3 +1,31 @@
+2019-05-04  Tadeu Zagallo  <tzagallo@apple.com>
+
+        TypedArrays should not store properties that are canonical numeric indices
+        https://bugs.webkit.org/show_bug.cgi?id=197228
+        <rdar://problem/49557381>
+
+        Reviewed by Saam Barati.
+
+        * stress/array-species-config-array-constructor.js:
+        (test):
+        * stress/put-direct-index-broken-2.js:
+        * stress/typed-array-canonical-numeric-index-string.js: Added.
+        (makeTest.assert):
+        (makeTest):
+        (const.testInvalidIndices.makeTest.set assert):
+        (const.testInvalidIndices.makeTest):
+        (const.makeTestValidIndex.configurable.set assert):
+        (const.makeTestValidIndex.configurable):
+        * stress/typedarray-access-monomorphic-neutered.js:
+        (checkNoException):
+        (testNoException):
+        (testFTLNoException):
+        * stress/typedarray-access-neutered.js:
+        (testNoException):
+        * stress/typedarray-getownproperty-not-configurable.js:
+        (foo):
+        * test262/expectations.yaml:
+
 2019-04-15  Saam barati  <sbarati@apple.com>
 
         SafeToExecute for GetByOffset/GetGetterByOffset/PutByOffset is using the wrong child for the base

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

@@ -32,7 +32,7 @@ function shouldThrow(f, m) {
 
 function test() {
     const message = "TypeError: Attempting to configure non-configurable property on a typed array at index: 0";
-    shouldThrow(() => foo.concat([1]), message);
+    foo.concat([1]);
     foo = [1,2,3,4];
     shouldThrow(() => foo.slice(0), message);
     foo = [1,2,3,4];

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

@@ -57,7 +57,7 @@ test(function() {
     } catch(e) {
         err = e;
     }
-    assert(err.toString() ===  "TypeError: Attempting to configure non-configurable property on a typed array at index: 0");
+    assert(!err);
 });
 
 test(function() {

JSTests/stress/typed-array-canonical-numeric-index-string.js

@@ -0,0 +1,107 @@
+//@ requireOptions("--forceEagerCompilation=true", "--osrExitCountForReoptimization=10", "--useConcurrentJIT=0")
+
+const typedArrays = [
+    Uint8ClampedArray,
+    Uint8Array,
+    Uint16Array,
+    Uint32Array,
+    Int8Array,
+    Int16Array,
+    Int32Array,
+    Float32Array,
+    Float64Array,
+];
+
+const failures = new Set();
+
+let value = 0;
+function makeTest(test) {
+    noInline(test);
+
+    function assert(typedArray, condition, message) {
+        if (!condition)
+            failures.add(`${typedArray.name}: ${message}`);
+    }
+
+    function testFor(typedArray, key) {
+        return new Function('key', 'typedArray', 'test', 'assert', `
+            const value = ${value++} % 128;
+            const u8 = new typedArray(1);
+            u8[key] = value;
+            test(u8, key, value, assert);
+        `).bind(undefined, key, typedArray, test, assert.bind(undefined, typedArray));
+    };
+
+    return function(keys) {
+        for (let typedArray of typedArrays) {
+            for (let key of keys) {
+                const runTest = testFor(typedArray, key);
+                noInline(runTest);
+                for (let i = 0; i < 10; i++) {
+                    runTest();
+                }
+            }
+        }
+    }
+}
+
+const testInvalidIndices = makeTest((array, key, value, assert) => {
+    assert(array[key] === undefined, `${key.toString()} should not be set`);
+    assert(!(key in array), `${key.toString()} should not be in array`);
+
+    const keys = Object.keys(array);
+    assert(keys.length === 1, `no new keys should be added`);
+    assert(keys[0] === '0', `'0' should be the only key`);
+    assert(array[0] === 0, `offset 0 should not have been modified`);
+
+    assert(array.hasOwnProperty(key) === false, `hasOwnProperty(${key.toString()}) should be false`);
+    assert(Object.getOwnPropertyDescriptor(array, key) === undefined, `Object.getOwnPropertyDescriptor(${key.toString()}) should be undefined`);
+});
+
+testInvalidIndices([
+    '-0',
+    '-1',
+    -1,
+    1,
+    'Infinity',
+    '-Infinity',
+    'NaN',
+    '0.1',
+    '4294967294',
+    '4294967295',
+    '4294967296',
+]);
+
+const makeTestValidIndex = (configurable) =>
+    (array, key, value, assert) => {
+        assert(array[key] === value, `${key.toString()} should be set to ${value}`);
+        assert(key in array, `should contain key ${key.toString()}`);
+
+        assert(array.hasOwnProperty(key) === true, `hasOwnProperty(${key.toString()}) should be true`);
+        const descriptor = Object.getOwnPropertyDescriptor(array, key);
+        assert(typeof descriptor === 'object', `Object.getOwnPropertyDescriptor(${key.toString()}) return an object`);
+        assert(descriptor.value === value, `descriptor.value should be ${value}`);
+        assert(descriptor.writable === true, `descriptor.writable should be true`);
+        assert(descriptor.enumerable === true, `descriptor.enumerable should be true`);
+        assert(descriptor.configurable === configurable, `descriptor.configurable should be ${configurable}`);
+    };
+
+const testValidConfigurableIndices = makeTest(makeTestValidIndex(true));
+
+testValidConfigurableIndices([
+    '01',
+    '0.10',
+    '+Infinity',
+    '-NaN',
+    '-0.0',
+    Symbol('1'),
+]);
+
+testValidNonConfigurableIndices = makeTest(makeTestValidIndex(false))
+testValidNonConfigurableIndices([
+    '0',
+    0,
+]);
+
+if (failures.size)
+    throw new Error(`Subtests failed:\n${Array.from(failures).join('\n')}`);

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

@@ -28,7 +28,6 @@ for (let constructor of typedArrays) {
     test("array[0]", array);
     test("delete array[0]", array);
     test("Object.getOwnPropertyDescriptor(array, 0)", array);
-    test("Object.defineProperty(array, 0, { value: 1, writable: true, configurable: false, enumerable: true })", array);
     test("array[0] = 1", array);
     test("array[i] = 1", array);
 }
@@ -48,7 +47,39 @@ for (let constructor of typedArrays) {
     testFTL("array[0]", array, failArray);
     testFTL("delete array[0]", array, failArray);
     testFTL("Object.getOwnPropertyDescriptor(array, 0)", array, failArray);
-    testFTL("Object.defineProperty(array, 0, { value: 1, writable: true, configurable: false, enumerable: true })", array, failArray);
     testFTL("array[0] = 1", array, failArray);
     testFTL("array[i] = 1", array, failArray);
 }
+
+
+function checkNoException(array, thunk, count) {
+    thunk(array, count);
+}
+noInline(check);
+
+function testNoException(thunk, array) {
+    let fn = Function("array", "i", thunk);
+    noInline(fn);
+    for (let i = 0; i < 10000; i++)
+        checkNoException(array, fn, i);
+}
+
+for (let constructor of typedArrays) {
+    let array = new constructor(10);
+    transferArrayBuffer(array.buffer);
+    testNoException("Object.defineProperty(array, 0, { value: 1, writable: true, configurable: false, enumerable: true })", array);
+}
+
+function testFTLNoException(thunk, array, failArray) {
+    let fn = Function("array", "i", thunk);
+    noInline(fn);
+    for (let i = 0; i < 10000; i++)
+        fn(array, i)
+    checkNoException(failArray, fn, 10000);
+}
+for (let constructor of typedArrays) {
+    let array = new constructor(10);
+    let failArray = new constructor(10);
+    transferArrayBuffer(failArray.buffer);
+    testFTLNoException("Object.defineProperty(array, 0, { value: 1, writable: true, configurable: false, enumerable: true })", array, failArray);
+}

JSTests/stress/typedarray-access-neutered.js

@@ -25,6 +25,20 @@ for (let i = 0; i < 10000; i++) {
     test((array) => array[0], i);
     test((array) => delete array[0], i);
     test((array) => Object.getOwnPropertyDescriptor(array, 0), i);
-    test((array) => Object.defineProperty(array, 0, { value: 1, writable: true, configurable: false, enumerable: true }), i)
     test((array) => array[0] = 1, i);
 }
+
+function checkNoException(thunk, count) {
+    let array = new constructor(10);
+    transferArrayBuffer(array.buffer);
+    thunk(array);
+}
+
+function testNoException(thunk, count) {
+    for (constructor of typedArrays)
+        checkNoException(thunk, count);
+}
+
+for (let i = 0; i < 10000; i++) {
+    testNoException((array) => Object.defineProperty(array, 0, { value: 1, writable: true, configurable: false, enumerable: true }), i)
+}

JSTests/stress/typedarray-getownproperty-not-configurable.js

@@ -10,7 +10,7 @@ function foo() {
         let a = new constructor(10);
         let b = Object.getOwnPropertyDescriptor(a, 0);
         assert(b.value === 0);
-        assert(b.writable === false);
+        assert(b.writable === true);
         assert(b.enumerable === true);
         assert(b.configurable === false);
     }

JSTests/test262/expectations.yaml

@@ -1394,33 +1394,18 @@ test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/conversion-ope
 test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/desc-value-throws.js:
   default: 'Test262Error: Expected a Test262Error but got a TypeError (Testing with Float64Array.)'
   strict mode: 'Test262Error: Expected a Test262Error but got a TypeError (Testing with Float64Array.)'
+test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/detached-buffer-realm.js:
+  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
+  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/detached-buffer.js:
-  default: 'Test262Error: Return false before Detached Buffer check when value is a negative number Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: Return false before Detached Buffer check when value is a negative number Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/key-is-lower-than-zero.js:
-  default: 'Test262Error: -1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: -1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/key-is-minus-zero.js:
-  default: 'Test262Error: defineProperty returns false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: defineProperty returns false Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/key-is-not-integer.js:
-  default: 'Test262Error: 0.1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: 0.1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/key-is-numericindex.js:
-  default: 'Test262Error: property is writable Expected SameValue(«false», «true») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: property is writable Expected SameValue(«false», «true») to be true (Testing with Float64Array.)'
+  default: 'Test262Error: Throws TypeError on valid numeric index if instance has a detached buffer Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
+  strict mode: 'Test262Error: Throws TypeError on valid numeric index if instance has a detached buffer Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/set-value.js:
   default: 'Test262Error: set value for sample[0] returns true Expected SameValue(«false», «true») to be true (Testing with Float64Array.)'
   strict mode: 'Test262Error: set value for sample[0] returns true Expected SameValue(«false», «true») to be true (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/DefineOwnProperty/tonumber-value-detached-buffer.js:
   default: 'Test262Error: detaching a ArrayBuffer during defining an element of a typed array viewing it should throw Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
   strict mode: 'Test262Error: detaching a ArrayBuffer during defining an element of a typed array viewing it should throw Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/Get/detached-buffer.js:
-  default: 'Test262Error: detach buffer runs before checking for 1.1 Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-  strict mode: 'Test262Error: detach buffer runs before checking for 1.1 Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/Get/infinity-detached-buffer.js:
-  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-  strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/Get/key-is-not-integer.js:
   default: 'Test262Error: OrdinaryGet was called! Ref: 9.1.8.1 3.c (Testing with Float64Array.)'
   strict mode: 'Test262Error: OrdinaryGet was called! Ref: 9.1.8.1 3.c (Testing with Float64Array.)'
@@ -1433,9 +1418,6 @@ test/built-ins/TypedArrayConstructors/internals/Get/key-is-out-of-bounds.js:
 test/built-ins/TypedArrayConstructors/internals/GetOwnProperty/enumerate-detached-buffer.js:
   default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
   strict mode: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/GetOwnProperty/index-prop-desc.js:
-  default: 'Test262Error: index descriptor is writable [0] Expected SameValue(«false», «true») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: index descriptor is writable [0] Expected SameValue(«false», «true») to be true (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/HasProperty/abrupt-from-ordinary-has-parent-hasproperty.js:
   default: 'Test262Error:  (Testing with Float64Array.)'
   strict mode: 'Test262Error:  (Testing with Float64Array.)'
@@ -1445,8 +1427,6 @@ test/built-ins/TypedArrayConstructors/internals/HasProperty/detached-buffer-real
 test/built-ins/TypedArrayConstructors/internals/HasProperty/detached-buffer.js:
   default: 'Test262Error: 0 Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
   strict mode: 'Test262Error: 0 Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/HasProperty/infinity-with-detached-buffer.js:
-  default: 'Test262Error: Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/HasProperty/inherited-property.js:
   default: 'Test262Error: 42 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
   strict mode: 'Test262Error: 42 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
@@ -1462,18 +1442,6 @@ test/built-ins/TypedArrayConstructors/internals/HasProperty/key-is-minus-zero.js
 test/built-ins/TypedArrayConstructors/internals/HasProperty/key-is-not-integer.js:
   default: 'Test262Error: 1.1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
   strict mode: 'Test262Error: 1.1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/Set/detached-buffer.js:
-  default: 'Test262Error: detach buffer runs before checking for 1.1 Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-  strict mode: 'Test262Error: detach buffer runs before checking for 1.1 Expected a TypeError to be thrown but no exception was thrown at all (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/Set/key-is-minus-zero.js:
-  default: 'Test262Error: -0 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: -0 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/Set/key-is-not-integer.js:
-  default: 'Test262Error: 1.1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: 1.1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds.js:
-  default: 'Test262Error: -1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
-  strict mode: 'Test262Error: -1 Expected SameValue(«true», «false») to be true (Testing with Float64Array.)'
 test/built-ins/TypedArrayConstructors/internals/Set/tonumber-value-throws.js:
   default: 'Test262Error: ToNumber runs before ToInteger(index) Expected a Test262Error to be thrown but no exception was thrown at all (Testing with Float64Array.)'
   strict mode: 'Test262Error: ToNumber runs before ToInteger(index) Expected a Test262Error to be thrown but no exception was thrown at all (Testing with Float64Array.)'

LayoutTests/ChangeLog

@@ -1,3 +1,14 @@
+2019-05-04  Tadeu Zagallo  <tzagallo@apple.com>
+
+        TypedArrays should not store properties that are canonical numeric indices
+        https://bugs.webkit.org/show_bug.cgi?id=197228
+        <rdar://problem/49557381>
+
+        Reviewed by Saam Barati.
+
+        * fast/canvas/canvas-ImageData-behaviour-expected.txt:
+        * fast/canvas/canvas-ImageData-behaviour.js:
+
 2019-05-02  Chris Dumez  <cdumez@apple.com>
 
         Setting a frame's src to a javascript URL should not run it synchronously

LayoutTests/fast/canvas/canvas-ImageData-behaviour-expected.txt

@@ -43,7 +43,7 @@ PASS imageData.data[0] = 256, imageData.data[0] is 255
 PASS imageData.data[0] = null, imageData.data[0] is 0
 PASS imageData.data[0] = undefined, imageData.data[0] is 0
 PASS imageData.data['foo']='garbage',imageData.data['foo'] is 'garbage'
-PASS imageData.data[-1]='garbage',imageData.data[-1] is 'garbage'
+PASS imageData.data[-1]='garbage',imageData.data[-1] is undefined
 PASS imageData.data[17]='garbage',imageData.data[17] is undefined
 PASS successfullyParsed is true
 

LayoutTests/fast/canvas/canvas-ImageData-behaviour.js

@@ -21,5 +21,5 @@ for (var i = 0; i < testValues.length; i++) {
 }
 
 shouldBe("imageData.data['foo']='garbage',imageData.data['foo']", "'garbage'");
-shouldBe("imageData.data[-1]='garbage',imageData.data[-1]", "'garbage'");
+shouldBe("imageData.data[-1]='garbage',imageData.data[-1]", "undefined");
 shouldBe("imageData.data[17]='garbage',imageData.data[17]", "undefined");

Source/JavaScriptCore/CMakeLists.txt

@@ -847,6 +847,7 @@ set(JavaScriptCore_PRIVATE_FRAMEWORK_HEADERS
     runtime/JSGenericTypedArrayViewPrototypeInlines.h
     runtime/JSGlobalLexicalEnvironment.h
     runtime/JSGlobalObject.h
+    runtime/JSGlobalObjectFunctions.h
     runtime/JSGlobalObjectInlines.h
     runtime/JSImmutableButterfly.h
     runtime/JSInternalPromise.h