Bug 163417 - Array methods are incorrect with objects whose "length" exceeds 2 ** 32 - 1
Summary: Array methods are incorrect with objects whose "length" exceeds 2 ** 32 - 1
Status: RESOLVED DUPLICATE of bug 211205
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Minor
Assignee: Alexey Shvayka
URL:
Keywords:
: 185625 187777 199663 208639 (view as bug list)
Depends on: 209449
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-13 16:13 PDT by Mark Lam
Modified: 2020-05-20 14:14 PDT (History)
16 users (show)

See Also:


Attachments
archiving test case for step 8 of the spec. (769 bytes, application/x-javascript)
2016-10-13 16:20 PDT, Mark Lam
no flags Details
WIP Patch (5.12 KB, patch)
2020-04-03 08:58 PDT, Alexey Shvayka
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2016-10-13 16:13:59 PDT
Step 8 of the Array.prototype.splice spec (https://tc39.github.io/ecma262/#sec-array.prototype.splice) states that we should throw a TypeError if the spliced array's length will exceed 2^53-1.  This is gated on the JSArray getLength() convenience function being updated to return a length value that is greater than 32-bits.  This issue is only a compliance issue in a contrived example, e.g.

    var maxLength = (2 ** 53) - 1;
    Array.prototype.splice.call({ length: maxLength }, maxLength, 0, 1, 2, 3);

In real world code, the JSC runtime prevents us from creating arrays with a length that exceeds UINT_MAX.  We're also bounded on the number of args that we pass to Array.prototype.splice.  As a result, we can never splice together an array that gets anywhere near 2^53-1.  We'll get an OutOfMemoryError long before then.
Comment 1 Mark Lam 2016-10-13 16:20:40 PDT
Created attachment 291531 [details]
archiving test case for step 8 of the spec.
Comment 2 Alexey Shvayka 2020-03-24 09:35:06 PDT
Apart from Array.prototype.splice, other Array methods (mostly ones that are implemented in C++) are affected.
https://github.com/tc39/test262/pull/2509 completes test coverage for those, allowing them to be fixed with a single patch.
Comment 3 Alexey Shvayka 2020-03-24 09:40:32 PDT
*** Bug 208639 has been marked as a duplicate of this bug. ***
Comment 4 Alexey Shvayka 2020-03-24 09:42:09 PDT
*** Bug 187777 has been marked as a duplicate of this bug. ***
Comment 5 Alexey Shvayka 2020-04-03 08:58:49 PDT
Created attachment 395381 [details]
WIP Patch

WIP
Comment 6 Alexey Shvayka 2020-04-03 09:23:09 PDT
The following Array.prototype.* methods are affected:
  indexOf (slow path)
  lastIndexOf (slow path)
  pop (slow path)
  push
  reverse (slow path)
  slice (slow path)
  splice
  unshift

Given that fast paths are affected, we can't just wrap indices in Identifier::from().
Instead, we need to add support for indices > MAX_ARRAY_INDEX in:
  get
  hasProperty
  putDirectIndex
  deletePropertyByIndex
(apart from few ArrayPrototype.cpp helpers)

1. Should we create new overloads like we already have for get():
  ALWAYS_INLINE JSValue JSValue::get(JSGlobalObject* globalObject, uint64_t propertyName) const
or promote `unsigned` parameter types of existing declarations?

2. Should we use `double` (toLength() return value, Identifier::from() argument type) or `uint64_t`
(argument type of speciesConstructArray() and slowJoin())?
Comment 7 Alexey Shvayka 2020-04-03 09:46:29 PDT
*** Bug 185625 has been marked as a duplicate of this bug. ***
Comment 8 Alexey Shvayka 2020-04-18 19:59:24 PDT
*** Bug 199663 has been marked as a duplicate of this bug. ***
Comment 9 Alexey Shvayka 2020-04-30 15:23:50 PDT

*** This bug has been marked as a duplicate of bug 211205 ***