Title: [261430] trunk
Revision
261430
Author
shvaikal...@gmail.com
Date
2020-05-08 18:43:42 -0700 (Fri, 08 May 2020)

Log Message

Array.prototype.concat fast path checks should not be observable
https://bugs.webkit.org/show_bug.cgi?id=211643

Reviewed by Ross Kirsling.

JSTests:

While test262 has complete coverage for @@isConcatSpreadable getters in prototype
chain, Proxy cases are covered by stress/array-concat-fast-spread-proxy.js and
stress/array-concat-spread-proxy.js.

* test262/expectations.yaml: Mark 2 test cases as passing.

Source/_javascript_Core:

This change utilizes @tryGetByIdWithWellKnownSymbol intrinsic to make
off the spec Symbol.isConcatSpreadable lookups unobservable to userland code,
aligning JSC with V8 and SpiderMonkey.

Since @tryGetById uses PropertySlot::getPureResult(), which returns `null`
for Proxy [[Get]] traps and JS getters (covered by stress/try-get-by-id.js),
we can safely compare its result `undefined`. Also, this allows us to remove
@isProxyObject check as Proxy argument is never a fast path anyway.

This patch is neutral on microbenchmarks/concat-append-one.js.

* builtins/ArrayPrototype.js:
(concat):

Modified Paths

Diff

Modified: trunk/JSTests/ChangeLog (261429 => 261430)


--- trunk/JSTests/ChangeLog	2020-05-09 01:21:28 UTC (rev 261429)
+++ trunk/JSTests/ChangeLog	2020-05-09 01:43:42 UTC (rev 261430)
@@ -1,3 +1,16 @@
+2020-05-08  Alexey Shvayka  <shvaikal...@gmail.com>
+
+        Array.prototype.concat fast path checks should not be observable
+        https://bugs.webkit.org/show_bug.cgi?id=211643
+
+        Reviewed by Ross Kirsling.
+
+        While test262 has complete coverage for @@isConcatSpreadable getters in prototype
+        chain, Proxy cases are covered by stress/array-concat-fast-spread-proxy.js and
+        stress/array-concat-spread-proxy.js.
+
+        * test262/expectations.yaml: Mark 2 test cases as passing.
+
 2020-05-08  Pablo Saavedra  <psaave...@igalia.com>
 
         Unreviewed, reverting r261010.

Modified: trunk/JSTests/test262/expectations.yaml (261429 => 261430)


--- trunk/JSTests/test262/expectations.yaml	2020-05-09 01:21:28 UTC (rev 261429)
+++ trunk/JSTests/test262/expectations.yaml	2020-05-09 01:43:42 UTC (rev 261430)
@@ -639,9 +639,6 @@
 test/built-ins/Array/prototype/concat/arg-length-near-integer-limit.js:
   default: 'Test262Error: Expected a Test262Error but got a RangeError'
   strict mode: 'Test262Error: Expected a Test262Error but got a RangeError'
-test/built-ins/Array/prototype/concat/is-concat-spreadable-get-order.js:
-  default: 'Test262Error: Expected [isConcatSpreadable, constructor, isConcatSpreadable] and [constructor, isConcatSpreadable] to have the same contents. '
-  strict mode: 'Test262Error: Expected [isConcatSpreadable, constructor, isConcatSpreadable] and [constructor, isConcatSpreadable] to have the same contents. '
 test/built-ins/Array/prototype/splice/property-traps-order-with-species.js:
   default: 'Test262Error: Expected [defineProperty, defineProperty, set, getOwnPropertyDescriptor, defineProperty] and [defineProperty, defineProperty] to have the same contents. '
   strict mode: 'Test262Error: Expected [defineProperty, defineProperty, set, getOwnPropertyDescriptor, defineProperty] and [defineProperty, defineProperty] to have the same contents. '

Modified: trunk/Source/_javascript_Core/ChangeLog (261429 => 261430)


--- trunk/Source/_javascript_Core/ChangeLog	2020-05-09 01:21:28 UTC (rev 261429)
+++ trunk/Source/_javascript_Core/ChangeLog	2020-05-09 01:43:42 UTC (rev 261430)
@@ -1,3 +1,24 @@
+2020-05-08  Alexey Shvayka  <shvaikal...@gmail.com>
+
+        Array.prototype.concat fast path checks should not be observable
+        https://bugs.webkit.org/show_bug.cgi?id=211643
+
+        Reviewed by Ross Kirsling.
+
+        This change utilizes @tryGetByIdWithWellKnownSymbol intrinsic to make
+        off the spec Symbol.isConcatSpreadable lookups unobservable to userland code,
+        aligning JSC with V8 and SpiderMonkey.
+
+        Since @tryGetById uses PropertySlot::getPureResult(), which returns `null`
+        for Proxy [[Get]] traps and JS getters (covered by stress/try-get-by-id.js),
+        we can safely compare its result `undefined`. Also, this allows us to remove
+        @isProxyObject check as Proxy argument is never a fast path anyway.
+
+        This patch is neutral on microbenchmarks/concat-append-one.js.
+
+        * builtins/ArrayPrototype.js:
+        (concat):
+
 2020-05-07  Michael Catanzaro  <mcatanz...@gnome.org>
 
         Simplify preprocessor guards in GCMemoryOperations.h

Modified: trunk/Source/_javascript_Core/builtins/ArrayPrototype.js (261429 => 261430)


--- trunk/Source/_javascript_Core/builtins/ArrayPrototype.js	2020-05-09 01:21:28 UTC (rev 261429)
+++ trunk/Source/_javascript_Core/builtins/ArrayPrototype.js	2020-05-09 01:43:42 UTC (rev 261430)
@@ -588,8 +588,8 @@
 
     if (@argumentCount() === 1
         && @isJSArray(this)
-        && this.@@isConcatSpreadable === @undefined
-        && (!@isObject(first) || (!@isProxyObject(first) && first.@@isConcatSpreadable === @undefined))) {
+        && @tryGetByIdWithWellKnownSymbol(this, "isConcatSpreadable") === @undefined
+        && (!@isObject(first) || @tryGetByIdWithWellKnownSymbol(first, "isConcatSpreadable") === @undefined)) {
 
         var 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