TypeArrays should not store properties that are canonical numeric indices
authortzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Apr 2019 22:25:09 +0000 (22:25 +0000)
committertzagallo@apple.com <tzagallo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 30 Apr 2019 22:25:09 +0000 (22:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=197228
<rdar://problem/49557381>

Reviewed by Darin Adler.

JSTests:

* stress/typed-array-canonical-numeric-index-string.js: Added.
(makeTest.assert):
(makeTest):
(const.testInvalidIndices.makeTest.set assert):
(const.testInvalidIndices.makeTest):
(const.testValidIndices.makeTest.set assert):
(const.testValidIndices.makeTest):

Source/JavaScriptCore:

According to the spec[1], TypedArrays should not perform an ordinary GetOwnProperty/SetOwnProperty
if the index is a CanonicalNumericIndexString, but invalid according toIntegerIndexedElementGet
and similar functions. I.e., there are a few properties that should not be set in a TypedArray,
like NaN, Infinity and -0.

[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/JSTypedArrays.cpp:
* runtime/PropertyName.h:
(JSC::canonicalNumericIndexString):

LayoutTests:

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

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244806 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChangeLog
JSTests/stress/typed-array-canonical-numeric-index-string.js [new file with mode: 0644]
LayoutTests/ChangeLog
LayoutTests/fast/canvas/canvas-ImageData-behaviour-expected.txt
LayoutTests/fast/canvas/canvas-ImageData-behaviour.js
Source/JavaScriptCore/CMakeLists.txt
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
Source/JavaScriptCore/runtime/JSTypedArrays.cpp
Source/JavaScriptCore/runtime/PropertyName.h

index aafd973..4b09e48 100644 (file)
@@ -1,3 +1,19 @@
+2019-04-30  Tadeu Zagallo  <tzagallo@apple.com>
+
+        TypeArrays should not store properties that are canonical numeric indices
+        https://bugs.webkit.org/show_bug.cgi?id=197228
+        <rdar://problem/49557381>
+
+        Reviewed by Darin Adler.
+
+        * stress/typed-array-canonical-numeric-index-string.js: Added.
+        (makeTest.assert):
+        (makeTest):
+        (const.testInvalidIndices.makeTest.set assert):
+        (const.testInvalidIndices.makeTest):
+        (const.testValidIndices.makeTest.set assert):
+        (const.testValidIndices.makeTest):
+
 2019-04-29  Yusuke Suzuki  <ysuzuki@apple.com>
 
         normalizeMapKey should normalize NaN to one PureNaN bit pattern to make MapHash same
diff --git a/JSTests/stress/typed-array-canonical-numeric-index-string.js b/JSTests/stress/typed-array-canonical-numeric-index-string.js
new file mode 100644 (file)
index 0000000..7e03df0
--- /dev/null
@@ -0,0 +1,88 @@
+//@ requireOptions("--forceEagerCompilation=true", "--osrExitCountForReoptimization=10", "--useConcurrentJIT=0")
+
+const typedArrays = [
+    Uint8ClampedArray,
+    Uint8Array,
+    Uint16Array,
+    Uint32Array,
+    Int8Array,
+    Int16Array,
+    Int32Array,
+    Float32Array,
+    Float64Array,
+];
+
+const failures = new Set();
+
+function makeTest(test) {
+    noInline(test);
+
+    function assert(typedArray, condition, message) {
+        if (!condition)
+            failures.add(`${typedArray.name}: ${message}`);
+    }
+
+    function testFor(typedArray, key) {
+        key = JSON.stringify(key);
+        return new Function('test', 'assert', `
+            const value = 42;
+            const u8 = new ${typedArray.name}(1);
+            u8[${key}] = value;
+            test(u8, ${key}, value, assert.bind(undefined, ${typedArray.name}));
+        `).bind(undefined, test, assert);
+    };
+
+    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} should not be set`);
+    assert(!(key in array), `${key} 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`);
+});
+
+testInvalidIndices([
+    '-0',
+    '-1',
+    -1,
+    1,
+    'Infinity',
+    '-Infinity',
+    'NaN',
+    '0.1',
+    '4294967294',
+    '4294967295',
+    '4294967296',
+]);
+
+const testValidIndices = makeTest((array, key, value, assert) => {
+    assert(array[key] === value, `${key} should be set to ${value}`);
+    assert(key in array, `should contain key ${key}`);
+});
+
+testValidIndices([
+    '01',
+    '0.10',
+    '+Infinity',
+    '-NaN',
+    '-0.0',
+    '0',
+    0,
+]);
+
+if (failures.size)
+    throw new Error(`Subtests failed:\n${Array.from(failures).join('\n')}`);
index c59c091..46d511b 100644 (file)
@@ -1,3 +1,14 @@
+2019-04-30  Tadeu Zagallo  <tzagallo@apple.com>
+
+        TypeArrays should not store properties that are canonical numeric indices
+        https://bugs.webkit.org/show_bug.cgi?id=197228
+        <rdar://problem/49557381>
+
+        Reviewed by Darin Adler.
+
+        * fast/canvas/canvas-ImageData-behaviour-expected.txt:
+        * fast/canvas/canvas-ImageData-behaviour.js:
+
 2019-04-30  Commit Queue  <commit-queue@webkit.org>
 
         Unreviewed, rolling out r244774.
index dbddf8f..cea50d8 100644 (file)
@@ -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
 
index acfc2fb..dacd943 100644 (file)
@@ -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");
index a12c074..fb6210c 100644 (file)
@@ -857,6 +857,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
index 9dfd7d1..b5d539f 100644 (file)
@@ -1,3 +1,30 @@
+2019-04-30  Tadeu Zagallo  <tzagallo@apple.com>
+
+        TypeArrays should not store properties that are canonical numeric indices
+        https://bugs.webkit.org/show_bug.cgi?id=197228
+        <rdar://problem/49557381>
+
+        Reviewed by Darin Adler.
+
+        According to the spec[1], TypedArrays should not perform an ordinary GetOwnProperty/SetOwnProperty
+        if the index is a CanonicalNumericIndexString, but invalid according toIntegerIndexedElementGet
+        and similar functions. I.e., there are a few properties that should not be set in a TypedArray,
+        like NaN, Infinity and -0.
+
+        [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/JSTypedArrays.cpp:
+        * runtime/PropertyName.h:
+        (JSC::canonicalNumericIndexString):
+
 2019-04-30  Brian Burg  <bburg@apple.com>
 
         Web Automation: use a more informative key to indicate automation availability
index d8b2aab..e7dc887 100644 (file)
                BC18C52E0E16FCE100B34460 /* Lexer.lut.h in Headers */ = {isa = PBXBuildFile; fileRef = BC18C52D0E16FCE100B34460 /* Lexer.lut.h */; };
                BC3046070E1F497F003232CF /* Error.h in Headers */ = {isa = PBXBuildFile; fileRef = BC3046060E1F497F003232CF /* Error.h */; settings = {ATTRIBUTES = (Private, ); }; };
                BC6AAAE50E1F426500AD87D8 /* ClassInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = BC6AAAE40E1F426500AD87D8 /* ClassInfo.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               BC756FC90E2031B200DE7D12 /* JSGlobalObjectFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = BC756FC70E2031B200DE7D12 /* JSGlobalObjectFunctions.h */; };
+               BC756FC90E2031B200DE7D12 /* JSGlobalObjectFunctions.h in Headers */ = {isa = PBXBuildFile; fileRef = BC756FC70E2031B200DE7D12 /* JSGlobalObjectFunctions.h */; settings = {ATTRIBUTES = (Private, ); }; };
                BC87CDB910712AD4000614CF /* JSONObject.lut.h in Headers */ = {isa = PBXBuildFile; fileRef = BC87CDB810712ACA000614CF /* JSONObject.lut.h */; };
                BC9041480EB9250900FE26FA /* StructureTransitionTable.h in Headers */ = {isa = PBXBuildFile; fileRef = BC9041470EB9250900FE26FA /* StructureTransitionTable.h */; settings = {ATTRIBUTES = (Private, ); }; };
                BC95437D0EBA70FD0072B6D3 /* PropertyMapHashTable.h in Headers */ = {isa = PBXBuildFile; fileRef = BC95437C0EBA70FD0072B6D3 /* PropertyMapHashTable.h */; settings = {ATTRIBUTES = (Private, ); }; };
index 1c5d573..c10ee93 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2013-2018 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2019 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -359,7 +359,10 @@ bool JSGenericTypedArrayView<Adaptor>::getOwnPropertySlot(
         slot.setValue(thisObject, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, jsUndefined());
         return false;
     }
-    
+
+    if (canonicalNumericIndexString(propertyName))
+        return false;
+
     return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
 }
 
@@ -375,7 +378,10 @@ bool JSGenericTypedArrayView<Adaptor>::put(
     // 9.4.5.5-2-b-i Return ? IntegerIndexedElementSet(O, numericIndex, V).
     if (Optional<uint32_t> index = parseIndex(propertyName))
         return putByIndex(thisObject, exec, index.value(), value, slot.isStrictMode());
-    
+
+    if (canonicalNumericIndexString(propertyName))
+        return false;
+
     return Base::put(thisObject, exec, propertyName, value, slot);
 }
 
@@ -410,7 +416,10 @@ bool JSGenericTypedArrayView<Adaptor>::defineOwnProperty(
         }
         return true;
     }
-    
+
+    if (canonicalNumericIndexString(propertyName))
+        return false;
+
     RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, exec, propertyName, descriptor, shouldThrow));
 }
 
@@ -433,7 +442,7 @@ bool JSGenericTypedArrayView<Adaptor>::deleteProperty(
 
 template<typename Adaptor>
 bool JSGenericTypedArrayView<Adaptor>::getOwnPropertySlotByIndex(
-    JSObject* object, ExecState* exec, unsigned propertyName, PropertySlot& slot)
+    JSObject* object, ExecState*, unsigned propertyName, PropertySlot& slot)
 {
     JSGenericTypedArrayView* thisObject = jsCast<JSGenericTypedArrayView*>(object);
 
@@ -442,10 +451,8 @@ bool JSGenericTypedArrayView<Adaptor>::getOwnPropertySlotByIndex(
         return true;
     }
 
-    if (propertyName > MAX_ARRAY_INDEX) {
-        return thisObject->methodTable(exec->vm())->getOwnPropertySlot(
-            thisObject, exec, Identifier::from(exec, propertyName), slot);
-    }
+    if (propertyName > MAX_ARRAY_INDEX)
+        return false;
     
     if (!thisObject->canGetIndexQuickly(propertyName))
         return false;
@@ -456,14 +463,12 @@ bool JSGenericTypedArrayView<Adaptor>::getOwnPropertySlotByIndex(
 
 template<typename Adaptor>
 bool JSGenericTypedArrayView<Adaptor>::putByIndex(
-    JSCell* cell, ExecState* exec, unsigned propertyName, JSValue value, bool shouldThrow)
+    JSCell* cell, ExecState* exec, unsigned propertyName, JSValue value, bool)
 {
     JSGenericTypedArrayView* thisObject = jsCast<JSGenericTypedArrayView*>(cell);
 
-    if (propertyName > MAX_ARRAY_INDEX) {
-        PutPropertySlot slot(JSValue(thisObject), shouldThrow);
-        return thisObject->methodTable(exec->vm())->put(thisObject, exec, Identifier::from(exec, propertyName), value, slot);
-    }
+    if (propertyName > MAX_ARRAY_INDEX)
+        return false;
     
     return thisObject->setIndex(exec, propertyName, value);
 }
index 09189d2..e3ad6b9 100644 (file)
@@ -55,6 +55,5 @@ JSUint8Array* createUint8TypedArray(ExecState* exec, Structure* structure, RefPt
     return JSUint8Array::create(exec, structure, WTFMove(buffer), byteOffset, length);
 }
 
-
 } // namespace JSC
 
index c035fa6..8b62d4f 100644 (file)
 #pragma once
 
 #include "Identifier.h"
+#include "JSGlobalObjectFunctions.h"
 #include "PrivateName.h"
 #include <wtf/Optional.h>
+#include <wtf/dtoa.h>
 
 namespace JSC {
 
@@ -130,4 +132,18 @@ ALWAYS_INLINE Optional<uint32_t> parseIndex(PropertyName propertyName)
     return parseIndex(*uid);
 }
 
+// https://www.ecma-international.org/ecma-262/9.0/index.html#sec-canonicalnumericindexstring
+ALWAYS_INLINE Optional<double> canonicalNumericIndexString(const PropertyName& propertyName)
+{
+    StringImpl* property = propertyName.uid();
+    if (equal(property, "-0"))
+        return { -0.0 };
+    double index = jsToNumber(property);
+    NumberToStringBuffer buffer;
+    const char* indexString = WTF::numberToString(index, buffer);
+    if (!equal(property, indexString))
+        return WTF::nullopt;
+    return { index };
+}
+
 } // namespace JSC