Attachment 368245 Details for Bug 197228 – Patch

[patch] Patch

bug-197228-20190425192708.patch (text/plain), 12.71 KB, created by Tadeu Zagallo on 2019-04-25 10:27:37 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2019-04-25 10:27:37 PDT

Size: 12.71 KB

patch

obsolete

>Subversion Revision: 244643
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index faaa5b93536c99265d128514fac9ea6069dabecb..4b198ae6652bdc299232b95da3c78d219bb9ca4e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-04-25  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 NOBODY (OOPS!).
>+
>+        According to the spec, 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.
>+
>+        * runtime/JSGenericTypedArrayViewInlines.h:
>+        (JSC::JSGenericTypedArrayView<Adaptor>::getOwnPropertySlot):
>+        (JSC::JSGenericTypedArrayView<Adaptor>::put):
>+        (JSC::JSGenericTypedArrayView<Adaptor>::defineOwnProperty):
>+        (JSC::JSGenericTypedArrayView<Adaptor>::deleteProperty):
>+        (JSC::JSGenericTypedArrayView<Adaptor>::getOwnPropertySlotByIndex):
>+        * runtime/JSTypedArrays.cpp:
>+        (JSC::canonicalNumericIndexString):
>+        (JSC::isValidIndex):
>+        * runtime/JSTypedArrays.h:
>+
> 2019-04-24  Saam Barati  <sbarati@apple.com>
> 
>         Add SPI callbacks for before and after module execution
>diff --git a/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h b/Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h
>index 1c5d5732a8be57f3390fa2f039e2b3e70dc873df..0d624c386389a9a2314895a2a1645580738370f2 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
>@@ -31,6 +31,7 @@
> #include "ExceptionHelpers.h"
> #include "JSArrayBuffer.h"
> #include "JSGenericTypedArrayView.h"
>+#include "JSTypedArrays.h"
> #include "TypeError.h"
> #include "TypedArrays.h"
> #include <wtf/text/StringConcatenateNumbers.h>
>@@ -359,7 +360,12 @@ bool JSGenericTypedArrayView<Adaptor>::getOwnPropertySlot(
>         slot.setValue(thisObject, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly, jsUndefined());
>         return false;
>     }
>-    
>+
>+    if (Optional<double> index = canonicalNumericIndexString(propertyName)) {
>+        ASSERT_UNUSED(index, !isValidIndex(index.value()));
>+        return false;
>+    }
>+
>     return Base::getOwnPropertySlot(thisObject, exec, propertyName, slot);
> }
> 
>@@ -375,7 +381,12 @@ 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 (Optional<double> index = canonicalNumericIndexString(propertyName)) {
>+        ASSERT_UNUSED(index, !isValidIndex(index.value()));
>+        return false;
>+    }
>+
>     return Base::put(thisObject, exec, propertyName, value, slot);
> }
> 
>@@ -410,7 +421,12 @@ bool JSGenericTypedArrayView<Adaptor>::defineOwnProperty(
>         }
>         return true;
>     }
>-    
>+
>+    if (Optional<double> index = canonicalNumericIndexString(propertyName)) {
>+        ASSERT_UNUSED(index, !isValidIndex(index.value()));
>+        return false;
>+    }
>+
>     RELEASE_AND_RETURN(scope, Base::defineOwnProperty(thisObject, exec, propertyName, descriptor, shouldThrow));
> }
> 
>@@ -427,7 +443,7 @@ bool JSGenericTypedArrayView<Adaptor>::deleteProperty(
> 
>     if (parseIndex(propertyName))
>         return false;
>-    
>+
>     return Base::deleteProperty(thisObject, exec, propertyName);
> }
> 
>@@ -446,10 +462,10 @@ bool JSGenericTypedArrayView<Adaptor>::getOwnPropertySlotByIndex(
>         return thisObject->methodTable(exec->vm())->getOwnPropertySlot(
>             thisObject, exec, Identifier::from(exec, propertyName), slot);
>     }
>-    
>+
>     if (!thisObject->canGetIndexQuickly(propertyName))
>         return false;
>-    
>+
>     slot.setValue(thisObject, static_cast<unsigned>(PropertyAttribute::DontDelete), thisObject->getIndexQuickly(propertyName));
>     return true;
> }
>diff --git a/Source/JavaScriptCore/runtime/JSTypedArrays.cpp b/Source/JavaScriptCore/runtime/JSTypedArrays.cpp
>index 09189d2c5ca5b535da4333146934becfd303f5c8..669628e81dcd34807369b867d05d023d42641b87 100644
>--- a/Source/JavaScriptCore/runtime/JSTypedArrays.cpp
>+++ b/Source/JavaScriptCore/runtime/JSTypedArrays.cpp
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013 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
>@@ -27,8 +27,10 @@
> #include "JSTypedArrays.h"
> 
> #include "GenericTypedArrayViewInlines.h"
>-#include "JSGenericTypedArrayViewInlines.h"
> #include "JSCInlines.h"
>+#include "JSGenericTypedArrayViewInlines.h"
>+#include "JSGlobalObjectFunctions.h"
>+#include <wtf/dtoa.h>
> 
> namespace JSC {
> 
>@@ -55,6 +57,34 @@ JSUint8Array* createUint8TypedArray(ExecState* exec, Structure* structure, RefPt
>     return JSUint8Array::create(exec, structure, WTFMove(buffer), byteOffset, length);
> }
> 
>+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 };
>+}
>+
>+#if !ASSERT_DISABLED
>+bool isValidIndex(double index)
>+{
>+    if (!std::isfinite(index))
>+        return false;
>+    double absIndex = std::abs(index);
>+    if (std::floor(absIndex) != absIndex)
>+        return false;
>+    if (index == 0.0 && std::signbit(index))
>+        return false;
>+    if (index < 0)
>+        return false;
>+    return true;
>+}
>+#endif
> 
> } // namespace JSC
> 
>diff --git a/Source/JavaScriptCore/runtime/JSTypedArrays.h b/Source/JavaScriptCore/runtime/JSTypedArrays.h
>index b3c8b7f28e381f6861f1b19985d0bf895d749d1f..b53c46a5639ede8dfd9ab7e55231395e22c6606e 100644
>--- a/Source/JavaScriptCore/runtime/JSTypedArrays.h
>+++ b/Source/JavaScriptCore/runtime/JSTypedArrays.h
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2013 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
>@@ -42,4 +42,10 @@ typedef JSGenericTypedArrayView<Float64Adaptor> JSFloat64Array;
> 
> JS_EXPORT_PRIVATE JSUint8Array* createUint8TypedArray(ExecState*, Structure*, RefPtr<ArrayBuffer>&&, unsigned byteOffset, unsigned length);
> 
>+JS_EXPORT_PRIVATE Optional<double> canonicalNumericIndexString(const PropertyName&);
>+
>+#if !ASSERT_DISABLED
>+JS_EXPORT_PRIVATE bool isValidIndex(double);
>+#endif
>+
> } // namespace JSC
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ac5e47c14f8ff50df3d0042085daff8c7fb4fc44..fad4f0355dec0f10e1c1948eba6aaf50100c019f 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-04-25  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 NOBODY (OOPS!).
>+
>+        * fast/canvas/canvas-ImageData-behaviour-expected.txt:
>+        * fast/canvas/canvas-ImageData-behaviour.js:
>+
> 2019-04-25  Antti Koivisto  <antti@apple.com>
> 
>         Visited link hash should be computed only once
>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..d3d7ce997046f901a7645d1ac65c446901ef0398 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-04-25  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 NOBODY (OOPS!).
>+
>+        * stress/typed-array-canonical-numeric-index-string.js: Added.
>+        (makeTest.assert):
>+        (makeTest):
>+        (const.testInvalidIndices.makeTest.set const):
>+        (set assert):
>+        (const.testInvalidIndices.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..06f2fd345495aa935be43dfff08319cb1ce8e77a
>--- /dev/null
>+++ b/JSTests/stress/typed-array-canonical-numeric-index-string.js
>@@ -0,0 +1,80 @@
>+//@ 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 runTest(typedArray, key) {
>+        const value = 42;
>+        const u8 = new typedArray(1);
>+        u8[key] = value;
>+        test(u8, key, value, assert.bind(undefined, typedArray));
>+    };
>+
>+    return function(keys) {
>+        for (let typedArray of typedArrays) {
>+            for (let key of keys) {
>+                for (let i = 0; i < 100; i++) {
>+                    runTest(typedArray, key);
>+                }
>+            }
>+        }
>+    }
>+}
>+
>+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',
>+]);
>+
>+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,
>+]);
>+
>+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