1354974 - Array.slice(2**31) is always an empty array

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[bevan]

Attachment: Array.slice(2147483648) test case copy.js

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/57.0.2987.133 Safari/537.36

Steps to reproduce:

1. Create an array
2. Set a value at index 2147483648 (2 ** 31)
3. Call Array.prototype.slice on the array so as to return a new array including the value set in the previous step


Actual results:

The returned array excludes values from 2147483648.


Expected results:

The value should be present in the new array.

Comment 1

[bevan]

OS: Mac OS X 10.11.6, possibly other OSes
Architecture: x64, possibly other architectures

A test case is attached.

For comparison;

- Firefox has the same bug, but fails at 2 ** 32 - 1, suggesting the same error but with signed integers instead of unsigned integers.
- Safari however freezes/crashes when trying to slice.

Here is the equivalent bug for V8; https://bugs.chromium.org/p/v8/issues/detail?id=6235

Comment 2

[bevan]

Attachment: Array.slice(2147483648) test case.js

Comment 3

[Jim Michaels]

hmm. when I looked at the last spec I looked at for internal data types for js, INT32 was one of them. theoretically it should work, right? try subtracting 1. that value is an overflow error for that data type in C/C++, as the upper bound is (2**31)-1 (are you using fortran?)
(Math.pow(2,31)-1)  see MDN Math.pow

<script>
function newarray() {
    var a=new Array();
    a[(Math.pow(2,31)-1)]=1;
    return a;
}
newarray()[0];
newarray()[(Math.pow(2,31)-1)];
</script>
//undefined
//1

this is with v53 ff x64 in win7

Comment 4

[André Bargull [:anba]]

http://searchfox.org/mozilla-central/rev/7aa21f3b531ddee90a353215bd86e97d6974e25b/js/src/jsarray.cpp#2826-2832 needs to be fixed, JSID_IS_INT only accepts jsids which are int32_t.

Comment 5

[André Bargull [:anba]]

Attachment: bug1354974.patch

Simple patch to use IdIsIndex instead of JSID_IS_INT/JSID_TO_INT when collecting array indices in GetIndexedPropertiesInRange() to ensure indices larger than INT32_MAX are accepted.

Comment 6

[Jan de Mooij [:jandem]]

Comment on attachment 8862039 [details] [diff] [review]
bug1354974.patch

Review of attachment 8862039 [details] [diff] [review]:
-----------------------------------------------------------------

Nice, thanks!

Comment 7

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=27f3061caa7cc2c908cd282cd77eee795c037fde

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/270383723cd0
Handle array indices greater than 2^31-1 when collecting indices for Array.prototype.slice. r=jandem

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Backed out bug 1358246, bug 1354974 and bug 1317383 for failing various tests in different test suites:

Bug 1317383
https://hg.mozilla.org/integration/mozilla-inbound/rev/edaf81997a7bd28d3fd6c1955ef52b837b183ff5
https://hg.mozilla.org/integration/mozilla-inbound/rev/bc2539cd72c91ae71633c8aea460577e1120647c
https://hg.mozilla.org/integration/mozilla-inbound/rev/4cb0cfe2b32ef6c3ad3096b2cbfca879d375e5da
https://hg.mozilla.org/integration/mozilla-inbound/rev/12a29d618d6eb7ed27c0e8f3b1ce551fa8ef0880

Bug 1354974
https://hg.mozilla.org/integration/mozilla-inbound/rev/dc19ec159be24735c18f3afdee00c6b9c881b39f

Bug 1358246
https://hg.mozilla.org/integration/mozilla-inbound/rev/2baf4e5a516aa6a8eb7ca7080dc7d8c8f41e5c25

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=999318b53733c5ccef06fcf87025cf00d515b0af&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable

Most of the failures look related to bug 1317383, but not sure about these failures in wpt's /_mozilla/wasm/jsapi.js.html:

https://treeherder.mozilla.org/logviewer.html#?job_id=95206554&repo=mozilla-inbound

[task 2017-04-28T13:53:40.253238Z] 13:53:40     INFO - TEST-PASS | /_mozilla/wasm/jsapi.js.html | unexpected success in assertInstantiateError 
[task 2017-04-28T13:53:40.253621Z] 13:53:40     INFO - TEST-PASS | /_mozilla/wasm/jsapi.js.html | unexpected success in assertInstantiateError 
[task 2017-04-28T13:53:40.253707Z] 13:53:40     INFO - TEST-UNEXPECTED-FAIL | /_mozilla/wasm/jsapi.js.html | unexpected success in assertInstantiateError - assert_equals: expected true but got false
[task 2017-04-28T13:53:40.254089Z] 13:53:40     INFO - assertInstantiateError/</<@http://web-platform.test:8000/_mozilla/wasm/js/jsapi.js:725:21
[task 2017-04-28T13:53:40.254159Z] 13:53:40     INFO - Async*assertInstantiateError/<@http://web-platform.test:8000/_mozilla/wasm/js/jsapi.js:719:20
[task 2017-04-28T13:53:40.254558Z] 13:53:40     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1409:20
[task 2017-04-28T13:53:40.254628Z] 13:53:40     INFO - promise callback*promise_test@http://web-platform.test:8000/resources/testharness.js:529:31
[task 2017-04-28T13:53:40.254728Z] 13:53:40     INFO - assertInstantiateError@http://web-platform.test:8000/_mozilla/wasm/js/jsapi.js:718:9
[task 2017-04-28T13:53:40.254790Z] 13:53:40     INFO - @http://web-platform.test:8000/_mozilla/wasm/js/jsapi.js:744:5
[task 2017-04-28T13:53:40.255032Z] 13:53:40     INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1409:20
[task 2017-04-28T13:53:40.255709Z] 13:53:40     INFO - test@http://web-platform.test:8000/resources/testharness.js:501:9
[task 2017-04-28T13:53:40.255779Z] 13:53:40     INFO - testJSAPI@http://web-platform.test:8000/_mozilla/wasm/js/jsapi.js:711:1
[task 2017-04-28T13:53:40.255880Z] 13:53:40     INFO - @http://web-platform.test:8000/_mozilla/wasm/js/jsapi.js:17:11
[task 2017-04-28T13:53:40.256403Z] 13:53:40     INFO - 

Looks like the stack changed: https://dxr.mozilla.org/mozilla-central/rev/2cca333f546f38860f84940d4c72d7470a3410f4/testing/web-platform/mozilla/tests/wasm/js/jsapi.js#725

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7f3a44bd3ef9
Handle array indices greater than 2^31-1 when collecting indices for Array.prototype.slice. r=jandem

Comment 11

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Mystery solved by bug 1317383 comment 18.

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/7f3a44bd3ef9