[JSC] Fix Array.prototype.concat fast case when single argument is Proxy
authorcaitp@igalia.com <caitp@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 May 2018 16:56:29 +0000 (16:56 +0000)
committercaitp@igalia.com <caitp@igalia.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 29 May 2018 16:56:29 +0000 (16:56 +0000)
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/JavaScriptCore:

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):

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

JSTests/ChangeLog
JSTests/stress/array-concat-fast-spread-proxy.js [new file with mode: 0644]
JSTests/stress/array-concat-spread-proxy.js
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ArrayPrototype.js

index d975758..68ef88b 100644 (file)
@@ -1,3 +1,15 @@
+2018-05-29  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 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  <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 (file)
index 0000000..eb6ee06
--- /dev/null
@@ -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";
+}
index e2cbd48..0726f62 100644 (file)
@@ -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";
 }
index 9101f8a..a92e107 100644 (file)
@@ -1,3 +1,19 @@
+2018-05-29  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 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.tea@gmail.com>
 
         [JSC] JSBigInt::digitDiv has undefined behavior which causes test failures
index 870e557..e6d4f10 100644 (file)
@@ -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)