Changeset 261987 in webkit

Timestamp:

May 21, 2020 1:41:21 AM (4 years ago)

Author:

Alexey Shvayka

Message:

Array.prototype.concat is incorrect with objects whose "length" exceeds 2 32 - 1
​https://bugs.webkit.org/show_bug.cgi?id=212167

Reviewed by Saam Barati.

JSTests:

• stress/array-prototype-concat-of-long-spliced-arrays.js:
• stress/array-prototype-concat-of-long-spliced-arrays2.js:
• test262/expectations.yaml: Mark 4 test cases as passing.

Source/JavaScriptCore:

This patch increases "length" limit of Array.prototype.concat result to @MAX_SAFE_INTEGER
and changes thrown error to TypeError, aligning JSC with the spec [1], V8, and SpiderMonkey.

Also, adds missing @MAX_SAFE_INTEGER overflow check in Array.from [2] (we implement similar
checks in other methods). SunSpider and microbenchmarks/concat-append-one.js are both neutral.

[1]: ​https://tc39.es/ecma262/#sec-array.prototype.concat (steps 5.c.iii, 5.d.ii)
[2]: ​https://tc39.es/ecma262/#sec-array.from (step 5.e.i)

• builtins/ArrayConstructor.js:

(from):

• builtins/ArrayPrototype.js:

(globalPrivate.concatSlowPath):

Location:

trunk

Files:

• JSTests/ChangeLog (modified) (1 diff)
• JSTests/stress/array-prototype-concat-of-long-spliced-arrays.js (modified) (1 diff)
• JSTests/stress/array-prototype-concat-of-long-spliced-arrays2.js (modified) (1 diff)
• JSTests/test262/expectations.yaml (modified) (1 diff)
• Source/JavaScriptCore/ChangeLog (modified) (1 diff)
• Source/JavaScriptCore/builtins/ArrayConstructor.js (modified) (1 diff)
• Source/JavaScriptCore/builtins/ArrayPrototype.js (modified) (2 diffs)

trunk/JSTests/ChangeLog

@@ +1 @@
+2020-05-21  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        Array.prototype.concat is incorrect with objects whose "length" exceeds 2 ** 32 - 1
+        https://bugs.webkit.org/show_bug.cgi?id=212167
+
+        Reviewed by Saam Barati.
+
+        * stress/array-prototype-concat-of-long-spliced-arrays.js:
+        * stress/array-prototype-concat-of-long-spliced-arrays2.js:
+        * test262/expectations.yaml: Mark 4 test cases as passing.
+
 2020-05-19  Michael Catanzaro  <mcatanzaro@gnome.org>
 

trunk/JSTests/stress/array-prototype-concat-of-long-spliced-arrays.js

@@ -31 +31 @@
     }
 
-    shouldEqual(exception, "RangeError: Length exceeded the maximum array length");
+    shouldEqual(exception, "RangeError: Invalid array length");
 }
 

trunk/JSTests/stress/array-prototype-concat-of-long-spliced-arrays2.js

@@ -21 +21 @@
         exception = e;
     }
-    shouldEqual(exception, "RangeError: Length exceeded the maximum array length");
+    shouldEqual(exception, "RangeError: Invalid array length");
 }
 

trunk/JSTests/test262/expectations.yaml

@@ -655 +655 @@
   default: 'Test262Error: Expected SameValue(«undefined», «[object Function]») to be true'
   strict mode: 'Test262Error: Expected SameValue(«undefined», «[object Function]») to be true'
-test/built-ins/Array/prototype/concat/arg-length-exceeding-integer-limit.js:
-  default: 'Test262Error: Expected a TypeError but got a RangeError'
-  strict mode: 'Test262Error: Expected a TypeError but got a RangeError'
-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/splice/property-traps-order-with-species.js:
   default: 'Test262Error: Expected [defineProperty, defineProperty, set, getOwnPropertyDescriptor, defineProperty] and [defineProperty, defineProperty] to have the same contents. '

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2020-05-21  Alexey Shvayka  <shvaikalesh@gmail.com>
+
+        Array.prototype.concat is incorrect with objects whose "length" exceeds 2 ** 32 - 1
+        https://bugs.webkit.org/show_bug.cgi?id=212167
+
+        Reviewed by Saam Barati.
+
+        This patch increases "length" limit of Array.prototype.concat result to @MAX_SAFE_INTEGER
+        and changes thrown error to TypeError, aligning JSC with the spec [1], V8, and SpiderMonkey.
+
+        Also, adds missing @MAX_SAFE_INTEGER overflow check in Array.from [2] (we implement similar
+        checks in other methods). SunSpider and microbenchmarks/concat-append-one.js are both neutral.
+
+        [1]: https://tc39.es/ecma262/#sec-array.prototype.concat (steps 5.c.iii, 5.d.ii)
+        [2]: https://tc39.es/ecma262/#sec-array.from (step 5.e.i)
+
+        * builtins/ArrayConstructor.js:
+        (from):
+        * builtins/ArrayPrototype.js:
+        (globalPrivate.concatSlowPath):
+
 2020-05-20  Michael Saboff  <msaboff@apple.com>
 

trunk/Source/JavaScriptCore/builtins/ArrayConstructor.js

@@ -70 +70 @@
 
         for (var value of wrapper) {
+            if (k >= @MAX_SAFE_INTEGER)
+                @throwTypeError("Length exceeded the maximum array length");
             if (mapFn)
                 @putByValDirect(result, k, thisArg === @undefined ? mapFn(value, k) : mapFn.@call(thisArg, value, k));

trunk/Source/JavaScriptCore/builtins/ArrayPrototype.js

@@ -559 +559 @@
         if ((spreadable === @undefined && @isArray(currentElement)) || spreadable) {
             var length = @toLength(currentElement.length);
-            if (length + resultIndex > @MAX_ARRAY_INDEX)
-                @throwRangeError("Length exceeded the maximum array length");
-            if (resultIsArray && @isJSArray(currentElement)) {
+            if (length + resultIndex > @MAX_SAFE_INTEGER)
+                @throwTypeError("Length exceeded the maximum array length");
+            if (resultIsArray && @isJSArray(currentElement) && length + resultIndex <= @MAX_ARRAY_INDEX) {
                 @appendMemcpy(result, currentElement, resultIndex);
                 resultIndex += length;
@@ -572 +572 @@
             }
         } else {
-            if (resultIndex >= @MAX_ARRAY_INDEX)
-                @throwRangeError("Length exceeded the maximum array length");
+            if (resultIndex >= @MAX_SAFE_INTEGER)
+                @throwTypeError("Length exceeded the maximum array length");
             @putByValDirect(result, resultIndex++, currentElement);
         }