Title: [232261] trunk
Revision
232261
Author
ca...@igalia.com
Date
2018-05-29 09:56:29 -0700 (Tue, 29 May 2018)

Log Message

[JSC] Fix Array.prototype.concat fast case when single argument is Proxy
https://bugs.webkit.org/show_bug.cgi?id=184267

Reviewed by Saam Barati.

JSTests:

* stress/array-concat-fast-spread-proxy.js: Copied from JSTests/stress/array-concat-spread-proxy.js.
(arrayEq):
(catch):
* stress/array-concat-spread-proxy.js:

Source/_javascript_Core:

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 prevented Proxy objects from being spread when
they were the only argument passed to A.prototype.concat(), violating ECMA-262.

* builtins/ArrayPrototype.js:
(concat):

Modified Paths

Added Paths

Diff

Modified: trunk/JSTests/ChangeLog (232260 => 232261)


--- trunk/JSTests/ChangeLog	2018-05-29 16:50:12 UTC (rev 232260)
+++ trunk/JSTests/ChangeLog	2018-05-29 16:56:29 UTC (rev 232261)
@@ -1,3 +1,15 @@
+2018-05-29  Caitlin Potter  <ca...@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 Saam Barati.
+
+        * 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  <ticaiol...@gmail.com>
 
         [ESNext][BigInt] Implement "+" and "-" unary operation

Copied: trunk/JSTests/stress/array-concat-fast-spread-proxy.js (from rev 232260, trunk/JSTests/stress/array-concat-spread-proxy.js) (0 => 232261)


--- trunk/JSTests/stress/array-concat-fast-spread-proxy.js	                        (rev 0)
+++ trunk/JSTests/stress/array-concat-fast-spread-proxy.js	2018-05-29 16:56:29 UTC (rev 232261)
@@ -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";
+}

Modified: trunk/JSTests/stress/array-concat-spread-proxy.js (232260 => 232261)


--- trunk/JSTests/stress/array-concat-spread-proxy.js	2018-05-29 16:50:12 UTC (rev 232260)
+++ trunk/JSTests/stress/array-concat-spread-proxy.js	2018-05-29 16:56:29 UTC (rev 232261)
@@ -35,5 +35,6 @@
         Array.prototype.concat.call(p,[]);
         passed = false;
     } catch (e) { }
-
+    if (!passed)
+        throw "failed to throw spreading revoked proxy";
 }

Modified: trunk/Source/_javascript_Core/ChangeLog (232260 => 232261)


--- trunk/Source/_javascript_Core/ChangeLog	2018-05-29 16:50:12 UTC (rev 232260)
+++ trunk/Source/_javascript_Core/ChangeLog	2018-05-29 16:56:29 UTC (rev 232261)
@@ -1,3 +1,19 @@
+2018-05-29  Caitlin Potter  <ca...@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 Saam Barati.
+
+        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 prevented Proxy objects from being spread when
+        they were the only argument passed to A.prototype.concat(), violating ECMA-262.
+
+        * builtins/ArrayPrototype.js:
+        (concat):
+
 2018-05-27  Yusuke Suzuki  <utatane....@gmail.com>
 
         [JSC] JSBigInt::digitDiv has undefined behavior which causes test failures

Modified: trunk/Source/_javascript_Core/builtins/ArrayPrototype.js (232260 => 232261)


--- trunk/Source/_javascript_Core/builtins/ArrayPrototype.js	2018-05-29 16:50:12 UTC (rev 232260)
+++ trunk/Source/_javascript_Core/builtins/ArrayPrototype.js	2018-05-29 16:56:29 UTC (rev 232261)
@@ -678,7 +678,7 @@
     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)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to