Attachment 341460 Details for Bug 184267 – Patch

[patch] Patch

bug-184267-20180528183452.patch (text/plain), 4.85 KB, created by Caitlin Potter (:caitp) on 2018-05-28 15:34:53 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Caitlin Potter (:caitp)

Created: 2018-05-28 15:34:53 PDT

Size: 4.85 KB

patch

obsolete

>Subversion Revision: 232249
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index c64ee950f9ab052972591fcb2879e1da56ffe3b5..4e5c6de10c1a42036fd503998d877dcad2adbad8 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-05-28  Caitlin Potter  <caitp@igalia.com>
>+
>+        [JSC] Fix Array.prototype.concat fast case when single argument is Proxy
>+        https://bugs.webkit.org/show_bug.cgi?id=184267
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Before this patch, the fast case for Array.prototype.concat was taken if
>+        there was a single argument passed to the function, which is either a
>+        non-JSCell, or an ObjectType JSCell not marked as concat-spreadable.
>+        This incorrectly treated prevented Proxy objects from being spread when
>+        they were the only argument passed to A.p.concat(), violating ECMA-262.
>+
>+        * builtins/ArrayPrototype.js:
>+        (concat):
>+
> 2018-05-27  Yusuke Suzuki  <utatane.tea@gmail.com>
> 
>         [WTF] Add clz32 / clz64 for MSVC
>diff --git a/Source/JavaScriptCore/builtins/ArrayPrototype.js b/Source/JavaScriptCore/builtins/ArrayPrototype.js
>index 870e557deb85c2740dec69466ac78f1ad3409d30..e6d4f104a2a55b52e46602768cfcf50276a3718f 100644
>--- a/Source/JavaScriptCore/builtins/ArrayPrototype.js
>+++ b/Source/JavaScriptCore/builtins/ArrayPrototype.js
>@@ -678,7 +678,7 @@ function concat(first)
>     if (@argumentCount() === 1
>         && @isJSArray(this)
>         && this.@isConcatSpreadableSymbol === @undefined
>-        && (!@isObject(first) || first.@isConcatSpreadableSymbol === @undefined)) {
>+        && (!@isObject(first) || (!@isProxyObject(first) && first.@isConcatSpreadableSymbol === @undefined))) {
> 
>         let result = @concatMemcpy(this, first);
>         if (result !== null)
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index d9757587d901ba6d880cfe9e0e60677bbc348d94..8350f5229ebe28f1305ee2c43172f374c7c6ac5a 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2018-05-28  Caitlin Potter  <caitp@igalia.com>
>+
>+        [JSC] Fix Array.prototype.concat fast case when single argument is Proxy
>+        https://bugs.webkit.org/show_bug.cgi?id=184267
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Before this patch, the fast case for Array.prototype.concat was taken if
>+        there was a single argument passed to the function, which is either a
>+        non-JSCell, or an ObjectType JSCell not marked as concat-spreadable.
>+        This incorrectly treated prevented Proxy objects from being spread when
>+        they were the only argument passed to A.p.concat(), violating ECMA-262.
>+
>+        * stress/array-concat-fast-spread-proxy.js: Copied from JSTests/stress/array-concat-spread-proxy.js.
>+        (arrayEq):
>+        (catch):
>+        * stress/array-concat-spread-proxy.js:
>+
> 2018-05-27  Caio Lima  <ticaiolima@gmail.com>
> 
>         [ESNext][BigInt] Implement "+" and "-" unary operation
>diff --git a/JSTests/stress/array-concat-fast-spread-proxy.js b/JSTests/stress/array-concat-fast-spread-proxy.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..eb6ee0665ec51dde1f4c1a7ae5498924fac14ebe
>--- /dev/null
>+++ b/JSTests/stress/array-concat-fast-spread-proxy.js
>@@ -0,0 +1,41 @@
>+// This file tests is concat spreadable when taking the fast path
>+// (single argument, JSArray receiver)
>+
>+function arrayEq(a, b) {
>+    if (a.length !== b.length)
>+        return false;
>+    for (let i = 0; i < a.length; i++) {
>+        if (a[i] !== b[i])
>+            return false;
>+    }
>+    return true;
>+}
>+
>+
>+{
>+    let array = [1,2,3];
>+    let {proxy:p, revoke} = Proxy.revocable([4, 5], {});
>+
>+    // Test it works with proxies by default
>+    for (let i = 0; i < 10000; i++) {
>+        if (!arrayEq(Array.prototype.concat.call(array, p), [1,2,3,4,5]))
>+            throw "failed normally with a proxy"
>+    }
>+
>+    // Test it works with spreadable false.
>+    p[Symbol.isConcatSpreadable] = false;
>+    for (let i = 0; i < 10000; i++) {
>+        if (!arrayEq(Array.prototype.concat.call(array,p), [1,2,3,p]))
>+            throw "failed with no spread"
>+    }
>+
>+    p[Symbol.isConcatSpreadable] = undefined;
>+    revoke();
>+    passed = true;
>+    try {
>+        Array.prototype.concat.call(array,p);
>+        passed = false;
>+    } catch (e) { }
>+    if (!passed)
>+        throw "failed to throw spreading revoked proxy";
>+}
>diff --git a/JSTests/stress/array-concat-spread-proxy.js b/JSTests/stress/array-concat-spread-proxy.js
>index e2cbd48ec2963550e1b118dc36d452d877a2d3f6..0726f62aeedbe246ca10e59fffd0799eae11aaea 100644
>--- a/JSTests/stress/array-concat-spread-proxy.js
>+++ b/JSTests/stress/array-concat-spread-proxy.js
>@@ -35,5 +35,6 @@ function arrayEq(a, b) {
>         Array.prototype.concat.call(p,[]);
>         passed = false;
>     } catch (e) { }
>-
>+    if (!passed)
>+        throw "failed to throw spreading revoked proxy";
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 184267: 341460 | 341489 | 341520 | 341538