Commit 78abedb9 authored by littledan's avatar littledan Committed by Commit bot

Revert of Check that array length stays a safe integer in Array.prototype.push...

Revert of Check that array length stays a safe integer in Array.prototype.push (patchset #7 id:120001 of https://codereview.chromium.org/1428483002/ )

Reason for revert:
Caused for-in-opt test to fail

Original issue's description:
> Check that array length stays a safe integer in Array.prototype.push
>
> This patch adds a check in Array.prototype.push to assert that the new
> length does not become greater than 2**53-1. Such a length would be
> dangerous because integer arithmetic becomes imprecise after the
> boundary. The check is also required by a test262 test.
>
> R=adamk
> LOG=Y
> BUG=v8:3087
>
> Committed: https://crrev.com/e68adf4548dd101dc08fcbff14444152fb1b7fe7
> Cr-Commit-Position: refs/heads/master@{#31588}

TBR=adamk@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:3087

Review URL: https://codereview.chromium.org/1418093007

Cr-Commit-Position: refs/heads/master@{#31590}
parent 113b78e9
...@@ -517,15 +517,6 @@ function ArrayPush() { ...@@ -517,15 +517,6 @@ function ArrayPush() {
var n = TO_LENGTH_OR_UINT32(array.length); var n = TO_LENGTH_OR_UINT32(array.length);
var m = %_ArgumentsLength(); var m = %_ArgumentsLength();
// It appears that there is no enforced, absolute limit on the number of
// arguments, but it would surely blow the stack to use 2**30 or more.
// To avoid integer overflow, do the comparison to the max safe integer
// after subtracting 2**30 from both sides. (2**31 would seem like a
// natural value, but it is negative in JS, and 2**32 is 1.)
if (m > (1 << 30) || (n - (1 << 30)) + m > kMaxSafeInteger - (1 << 30)) {
throw MakeTypeError(kPushPastSafeLength, m, n);
}
for (var i = 0; i < m; i++) { for (var i = 0; i < m; i++) {
array[i+n] = %_Arguments(i); array[i+n] = %_Arguments(i);
} }
......
...@@ -326,9 +326,6 @@ class CallSite { ...@@ -326,9 +326,6 @@ class CallSite {
T(NoCatchOrFinally, "Missing catch or finally after try") \ T(NoCatchOrFinally, "Missing catch or finally after try") \
T(NotIsvar, "builtin %%IS_VAR: not a variable") \ T(NotIsvar, "builtin %%IS_VAR: not a variable") \
T(ParamAfterRest, "Rest parameter must be last formal parameter") \ T(ParamAfterRest, "Rest parameter must be last formal parameter") \
T(PushPastSafeLength, \
"Pushing % elements on an array-like of length % " \
"is disallowed, as the total surpasses 2**53-1") \
T(BadSetterRestParameter, \ T(BadSetterRestParameter, \
"Setter function argument must not be a rest parameter") \ "Setter function argument must not be a rest parameter") \
T(ParamDupe, "Duplicate parameter name not allowed in this context") \ T(ParamDupe, "Duplicate parameter name not allowed in this context") \
......
...@@ -33,7 +33,13 @@ assertEquals(1, o.length); ...@@ -33,7 +33,13 @@ assertEquals(1, o.length);
assertEquals(1, o[0]); assertEquals(1, o[0]);
var o = { length: Number.MAX_VALUE }; var o = { length: Number.MAX_VALUE };
assertThrows(() => Array.prototype.push.call(o, 1), TypeError); Array.prototype.push.call(o, 1);
assertEquals(o.length, Number.MAX_SAFE_INTEGER + 1);
assertEquals(1, o[Number.MAX_SAFE_INTEGER]);
Array.prototype.push.call(o, 2);
assertEquals(o.length, Number.MAX_SAFE_INTEGER + 1);
assertEquals(2, o[Number.MAX_SAFE_INTEGER]);
// ArrayPop // ArrayPop
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment