Attachment 368615 Details for Bug 197228 – Patch for landing

[patch] Patch for landing

bug-197228-20190430233429.patch (text/plain), 14.93 KB, created by Tadeu Zagallo on 2019-04-30 14:34:32 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-04-30 14:34:32 PDT

Size: 14.93 KB

patch

obsolete

>Subversion Revision: 244706
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index 6114aa50feaf3826d4e386dcbcbe476a8d24d40d..97a6a6c4701c62fa8eb70a47aa88ce0e25d1ec1e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-04-29  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-26  Don Olmstead  <don.olmstead@sony.com>
> 
>         Add WTF::findIgnoringASCIICaseWithoutLength to replace strcasestr
>diff --git a/Source/JavaScriptCore/CMakeLists.txt b/Source/JavaScriptCore/CMakeLists.txt
>index ab69001d00519539a9134923a1f79165fc6a845e..fcd27067ec9afaf40240a1583125c6bf0e9d1391 100644
>--- a/Source/JavaScriptCore/CMakeLists.txt
>+++ b/Source/JavaScriptCore/CMakeLists.txt
>@@ -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
>diff --git a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>index d8b2aab3477dd928798c932573ec5a856113c61c..e7dc887b9aaf815bb3b80a7d42e8a07f894fdd03 100644
>--- a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>+++ b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>@@ -1669,7 +1669,7 @@
> 		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, ); }; };
>diff --git a/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h b/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
>index 1c5d5732a8be57f3390fa2f039e2b3e70dc873df..c10ee9347d16619df630c0077f3915e5b3ee5ca2 100644
>--- a/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
>+++ b/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
>@@ -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);
> }
>diff --git a/Source/JavaScriptCore/runtime/JSTypedArrays.cpp b/Source/JavaScriptCore/runtime/JSTypedArrays.cpp
>index 09189d2c5ca5b535da4333146934becfd303f5c8..e3ad6b95c6f8e9be98213c623492573fb0ac4f60 100644
>--- a/Source/JavaScriptCore/runtime/JSTypedArrays.cpp
>+++ b/Source/JavaScriptCore/runtime/JSTypedArrays.cpp
>@@ -55,6 +55,5 @@ JSUint8Array* createUint8TypedArray(ExecState* exec, Structure* structure, RefPt
>     return JSUint8Array::create(exec, structure, WTFMove(buffer), byteOffset, length);
> }
> 
>-
> } // namespace JSC
> 
>diff --git a/Source/JavaScriptCore/runtime/PropertyName.h b/Source/JavaScriptCore/runtime/PropertyName.h
>index c035fa68a18d1be04c5cc8d29566f5337adc541e..8b62d4f0dce7a1f580203df1f37034c5ca3cc0c8 100644
>--- a/Source/JavaScriptCore/runtime/PropertyName.h
>+++ b/Source/JavaScriptCore/runtime/PropertyName.h
>@@ -26,8 +26,10 @@
> #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
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 52b7b788d46ddc52093166a779abaf47d92a7e4c..d4e0a21e17ec17d95191d1c4545937714043e74c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-04-29  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-26  Youenn Fablet  <youenn@apple.com>
> 
>         Use normal loading path for ping loads
>diff --git a/LayoutTests/fast/canvas/canvas-ImageData-behaviour-expected.txt b/LayoutTests/fast/canvas/canvas-ImageData-behaviour-expected.txt
>index dbddf8f58a432c70e897283aebdf51d3b1a11efd..cea50d8321827e586b05295d3374328424b7585b 100644
>--- a/LayoutTests/fast/canvas/canvas-ImageData-behaviour-expected.txt
>+++ b/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
> 
>diff --git a/LayoutTests/fast/canvas/canvas-ImageData-behaviour.js b/LayoutTests/fast/canvas/canvas-ImageData-behaviour.js
>index acfc2fb89b02236f89e729642caec0b1f803efb5..dacd9436a10e136e0d7c4b8676b626aa0bf9a185 100644
>--- a/LayoutTests/fast/canvas/canvas-ImageData-behaviour.js
>+++ b/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");
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 315ecee71258ce3c3e14d46e1030b53c323ee8b4..510269a19d2d475461fc2b0b1af199e47a698062 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-04-29  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-23  Saam Barati  <sbarati@apple.com>
> 
>         LICM incorrectly assumes it'll never insert a node which provably OSR exits
>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
>index 0000000000000000000000000000000000000000..7e03df026738ec12d76ae2225085797ae84b4525
>--- /dev/null
>+++ b/JSTests/stress/typed-array-canonical-numeric-index-string.js
>@@ -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')}`);

Actions: View | Formatted Diff | Diff

Attachments on bug 197228: 368115 | 368126 | 368140 | 368153 | 368156 | 368157 | 368160 | 368163 | 368245 | 368262 | 368404 | 368444 | 368615 | 368687 | 368696 | 368929 | 368959 | 368977 | 368999 | 369064 | 369072