Skip to content

Commit

Permalink
[runtime] Reduce spread/apply call max arguments
Browse files Browse the repository at this point in the history
Bug: chromium:906043
Change-Id: I308b29af0644c318d73926b27e65a94913c760c7
Reviewed-on: https://chromium-review.googlesource.com/c/1346115
Commit-Queue: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57731}
  • Loading branch information
psmarshall authored and Commit Bot committed Nov 22, 2018
1 parent 38cd61d commit 4e3a17d
Show file tree
Hide file tree
Showing 17 changed files with 108 additions and 26 deletions.
23 changes: 22 additions & 1 deletion src/builtins/builtins-call-gen.cc
Expand Up @@ -184,6 +184,8 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructWithArrayLike(
Goto(&if_done);
}

Label too_many_args(this, Label::kDeferred);

// Tail call to the appropriate builtin (depending on whether we have
// a {new_target} passed).
BIND(&if_done);
Expand All @@ -194,6 +196,8 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructWithArrayLike(
TNode<Int32T> length = var_length.value();
{
Label normalize_done(this);
GotoIf(Int32GreaterThan(length, Int32Constant(Code::kMaxArguments)),
&too_many_args);
GotoIfNot(Word32Equal(length, Int32Constant(0)), &normalize_done);
// Make sure we don't accidentally pass along the
// empty_fixed_double_array since the tailed-called stubs cannot handle
Expand Down Expand Up @@ -228,6 +232,9 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructWithArrayLike(
Int32Constant(HOLEY_DOUBLE_ELEMENTS));
}
}

BIND(&too_many_args);
ThrowRangeError(context, MessageTemplate::kTooManyArguments);
}

// Takes a FixedArray of doubles and creates a new FixedArray with those doubles
Expand All @@ -239,6 +246,11 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructDoubleVarargs(
TNode<Int32T> args_count, TNode<Context> context, TNode<Int32T> kind) {
const ElementsKind new_kind = PACKED_ELEMENTS;
const WriteBarrierMode barrier_mode = UPDATE_WRITE_BARRIER;

Label too_many_args(this, Label::kDeferred);
GotoIf(Int32GreaterThan(length, Int32Constant(Code::kMaxArguments)),
&too_many_args);

TNode<IntPtrT> intptr_length = ChangeInt32ToIntPtr(length);
CSA_ASSERT(this, WordNotEqual(intptr_length, IntPtrConstant(0)));

Expand All @@ -258,13 +270,16 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructDoubleVarargs(
TailCallStub(callable, context, target, new_target, args_count, length,
new_elements);
}

BIND(&too_many_args);
ThrowRangeError(context, MessageTemplate::kTooManyArguments);
}

void CallOrConstructBuiltinsAssembler::CallOrConstructWithSpread(
TNode<Object> target, TNode<Object> new_target, TNode<Object> spread,
TNode<Int32T> args_count, TNode<Context> context) {
Label if_smiorobject(this), if_double(this),
if_generic(this, Label::kDeferred);
if_generic(this, Label::kDeferred), too_many_args(this, Label::kDeferred);

TVARIABLE(Int32T, var_length);
TVARIABLE(FixedArrayBase, var_elements);
Expand Down Expand Up @@ -330,6 +345,9 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructWithSpread(
TNode<FixedArrayBase> elements = var_elements.value();
TNode<Int32T> length = var_length.value();

GotoIf(Int32GreaterThan(length, Int32Constant(Code::kMaxArguments)),
&too_many_args);

if (new_target == nullptr) {
Callable callable = CodeFactory::CallVarargs(isolate());
TailCallStub(callable, context, target, args_count, length, elements);
Expand All @@ -347,6 +365,9 @@ void CallOrConstructBuiltinsAssembler::CallOrConstructWithSpread(
var_length.value(), args_count, context,
var_elements_kind.value());
}

BIND(&too_many_args);
ThrowRangeError(context, MessageTemplate::kTooManyArguments);
}

TF_BUILTIN(CallWithArrayLike, CallOrConstructBuiltinsAssembler) {
Expand Down
2 changes: 1 addition & 1 deletion src/message-template.h
Expand Up @@ -458,7 +458,7 @@ namespace internal {
T(AwaitExpressionFormalParameter, \
"Illegal await-expression in formal parameters of async function") \
T(TooManyArguments, \
"Too many arguments in function call (only 65535 allowed)") \
"Too many arguments in function call (only 65534 allowed)") \
T(TooManyParameters, \
"Too many parameters in function definition (only 65534 allowed)") \
T(TooManySpreads, \
Expand Down
10 changes: 8 additions & 2 deletions test/mjsunit/apply.js
Expand Up @@ -122,7 +122,10 @@ for (var j = 1; j < 0x400000; j <<= 1) {
a[j - 1] = 42;
assertEquals(42 + j, al.apply(345, a));
} catch (e) {
assertTrue(e.toString().indexOf("Maximum call stack size exceeded") != -1);
assertTrue(
e.toString().indexOf('Maximum call stack size exceeded') != -1 ||
e.toString().indexOf(
'Too many arguments in function call (only 65534 allowed)') != -1);
for (; j < 0x400000; j <<= 1) {
var caught = false;
try {
Expand All @@ -133,7 +136,10 @@ for (var j = 1; j < 0x400000; j <<= 1) {
assertUnreachable("Apply of array with length " + a.length +
" should have thrown");
} catch (e) {
assertTrue(e.toString().indexOf("Maximum call stack size exceeded") != -1);
assertTrue(
e.toString().indexOf('Maximum call stack size exceeded') != -1 ||
e.toString().indexOf(
'Too many arguments in function call (only 65534 allowed)') != -1);
caught = true;
}
assertTrue(caught, "exception not caught");
Expand Down
6 changes: 3 additions & 3 deletions test/mjsunit/regress/regress-3027.js
Expand Up @@ -30,13 +30,13 @@

function boom() {
var args = [];
for (var i = 0; i < 125000; i++) {
for (var i = 0; i < 65534; i++) {
args.push(i);
}
return Array.apply(Array, args);
}

var array = boom();

assertEquals(125000, array.length);
assertEquals(124999, array[124999]);
assertEquals(65534, array.length);
assertEquals(65533, array[65533]);
2 changes: 1 addition & 1 deletion test/mjsunit/regress/regress-331444.js
Expand Up @@ -29,7 +29,7 @@

function boom() {
var args = [];
for (var i = 0; i < 125000; i++)
for (var i = 0; i < 65534; i++)
args.push(i);
return Array.apply(Array, args);
}
Expand Down
2 changes: 1 addition & 1 deletion test/mjsunit/regress/regress-358090.js
Expand Up @@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

var x = Array(100000);
var x = Array(65534);
y = Array.apply(Array, x);
y.unshift(4);
y.shift();
2 changes: 1 addition & 1 deletion test/mjsunit/regress/regress-732836.js
Expand Up @@ -4,7 +4,7 @@

function boom() {
var args = [];
for (var i = 0; i < 125000; i++)
for (var i = 0; i < 65534; i++)
args.push(1.1);
return Array.apply(Array, args);
}
Expand Down
4 changes: 2 additions & 2 deletions test/mjsunit/regress/regress-803750.js
Expand Up @@ -3,5 +3,5 @@
// found in the LICENSE file.

// Verify that very large arrays can be constructed.
assertEquals(Array.isArray(Array.of.apply(Array, Array(65536))), true);
assertEquals(Array.isArray(Array.of.apply(null, Array(65536))), true);
assertEquals(Array.isArray(Array.of.apply(Array, Array(65534))), true);
assertEquals(Array.isArray(Array.of.apply(null, Array(65534))), true);
2 changes: 1 addition & 1 deletion test/mjsunit/regress/regress-869735.js
Expand Up @@ -10,5 +10,5 @@ function f() {

var a = [];
%OptimizeFunctionOnNextCall(f);
a.length = 81832;
a.length = 65534;
f(...a);
5 changes: 1 addition & 4 deletions test/mjsunit/regress/regress-crbug-614727.js
Expand Up @@ -7,10 +7,7 @@
function f(a, b, c) { return arguments }
function g(...args) { return args }

// On 64-bit machine this produces a 768K array which is sufficiently small to
// not cause a stack overflow, but big enough to move the allocated arguments
// object into large object space (kMaxRegularHeapObjectSize == 600K).
var length = Math.pow(2, 15) * 3;
var length = 65534;
var args = new Array(length);
assertEquals(length, f.apply(null, args).length);
assertEquals(length, g.apply(null, args).length);
Expand Down
2 changes: 1 addition & 1 deletion test/mjsunit/regress/regress-crbug-813450.js
Expand Up @@ -4,7 +4,7 @@

// Flags: --allow-natives-syntax

var constructorArgs = new Array(0x10100);
var constructorArgs = new Array(65534);
var constructor = function() {};
var target = new Proxy(constructor, {
construct: function() {
Expand Down
54 changes: 54 additions & 0 deletions test/mjsunit/regress/regress-crbug-906043.js
@@ -0,0 +1,54 @@
// Copyright 2018 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

// Flags: --allow-natives-syntax

function fun(arg) {
let x = arguments.length;
a1 = new Array(0x10);
a1[0] = 1.1;
a2 = new Array(0x10);
a2[0] = 1.1;
a1[(x >> 16) * 21] = 1.39064994160909e-309; // 0xffff00000000
a1[(x >> 16) * 41] = 8.91238232205e-313; // 0x2a00000000
}

var a1, a2;
var a3 = [1.1,2.2];
a3.length = 0x11000;
a3.fill(3.3);

var a4 = [1.1];

for (let i = 0; i < 3; i++) fun(...a4);
%OptimizeFunctionOnNextCall(fun);
fun(...a4);

assertThrows(() => fun(...a3), RangeError);
assertThrows(() => fun.apply(null, a3), RangeError);

const kMaxArguments = 65534;
let big_array = [];
for (let i = 0; i < kMaxArguments + 1; i++) big_array.push(i);
assertThrows(() => fun(...big_array), RangeError);
assertThrows(() => new fun(...big_array), RangeError);
assertThrows(() => fun.apply(null, big_array), RangeError);
assertThrows(() => Reflect.construct(fun, big_array), RangeError);
assertThrows(() => Reflect.apply(fun, undefined, big_array), RangeError);

big_array = [];
for (let i = 0; i < kMaxArguments + 1; i++) big_array.push(i + 0.1);
assertThrows(() => fun(...big_array), RangeError);
assertThrows(() => new fun(...big_array), RangeError);
assertThrows(() => fun.apply(null, big_array), RangeError);
assertThrows(() => Reflect.construct(fun, big_array), RangeError);
assertThrows(() => Reflect.apply(fun, undefined, big_array), RangeError);

big_array = [];
for (let i = 0; i < kMaxArguments + 1; i++) big_array.push({i: i});
assertThrows(() => fun(...big_array), RangeError);
assertThrows(() => new fun(...big_array), RangeError);
assertThrows(() => fun.apply(null, big_array), RangeError);
assertThrows(() => Reflect.construct(fun, big_array), RangeError);
assertThrows(() => Reflect.apply(fun, undefined, big_array), RangeError);
2 changes: 1 addition & 1 deletion test/mjsunit/regress/regress-v8-6716.js
Expand Up @@ -3,5 +3,5 @@
// found in the LICENSE file.

function f() {}
var a = Array(2 ** 16); // Elements in large-object-space.
var a = Array(65534);
f.bind(...a);
2 changes: 1 addition & 1 deletion test/mjsunit/string-indexof-1.js
Expand Up @@ -133,7 +133,7 @@ assertEquals(-1, asciiString.indexOf("\x2061"));

// Search in string containing many non-ASCII chars.
var allCodePoints = [];
for (var i = 0; i < 65536; i++) allCodePoints[i] = i;
for (var i = 0; i < 65534; i++) allCodePoints[i] = i;
var allCharsString = String.fromCharCode.apply(String, allCodePoints);
// Search for string long enough to trigger complex search with ASCII pattern
// and UC16 subject.
Expand Down
3 changes: 3 additions & 0 deletions test/mozilla/mozilla.status
Expand Up @@ -460,6 +460,9 @@
'js1_5/Regress/regress-313967-02': [FAIL_OK],
'js1_5/extensions/regress-459606': [FAIL_OK],

# We restrict the number of apply arguments.
'js1_5/Array/regress-350256-01': [SKIP],

# This fails because we don't have stack space for Function.prototype.apply
# with very large numbers of arguments. The test uses 2^24 arguments.
'js1_5/Array/regress-350256-03': [FAIL_OK],
Expand Down
7 changes: 4 additions & 3 deletions test/webkit/fast/js/function-apply-expected.txt
Expand Up @@ -54,13 +54,14 @@ PASS arrayApplyChangeLength2() is 2
PASS arrayApplyChangeLength3() is 2
PASS arrayApplyChangeLength4() is 0
PASS var a = []; a.length = 0xFFFE; [].constructor.apply('', a).length is 0xFFFE
PASS var a = []; a.length = 0xFFFF; [].constructor.apply('', a).length is 0xFFFF
PASS var a = []; a.length = 0x10000; [].constructor.apply('', a).length is 0x10000
PASS var a = []; a.length = 0x10001; [].constructor.apply('', a).length is 0x10001
PASS var a = []; a.length = 0xFFFF; [].constructor.apply('', a).length threw exception RangeError: Too many arguments in function call (only 65534 allowed).
PASS var a = []; a.length = 0x10000; [].constructor.apply('', a).length threw exception RangeError: Too many arguments in function call (only 65534 allowed).
PASS var a = []; a.length = 0x10001; [].constructor.apply('', a).length threw exception RangeError: Too many arguments in function call (only 65534 allowed).
PASS var a = []; a.length = 0xFFFFFFFE; [].constructor.apply('', a).length threw exception RangeError: Invalid array length.
PASS var a = []; a.length = 0xFFFFFFFF; [].constructor.apply('', a).length threw exception RangeError: Invalid array length.
PASS (function(a,b,c,d){ return d ? -1 : (a+b+c); }).apply(undefined, {length:3, 0:100, 1:20, 2:3}) is 123
PASS successfullyParsed is true


TEST COMPLETE

6 changes: 3 additions & 3 deletions test/webkit/fast/js/function-apply.js
Expand Up @@ -308,9 +308,9 @@ shouldBe("arrayApplyChangeLength3()", "2");
shouldBe("arrayApplyChangeLength4()", "0");

shouldBe("var a = []; a.length = 0xFFFE; [].constructor.apply('', a).length", "0xFFFE");
shouldBe("var a = []; a.length = 0xFFFF; [].constructor.apply('', a).length", "0xFFFF");
shouldBe("var a = []; a.length = 0x10000; [].constructor.apply('', a).length", "0x10000");
shouldBe("var a = []; a.length = 0x10001; [].constructor.apply('', a).length", "0x10001");
shouldThrow("var a = []; a.length = 0xFFFF; [].constructor.apply('', a).length");
shouldThrow("var a = []; a.length = 0x10000; [].constructor.apply('', a).length");
shouldThrow("var a = []; a.length = 0x10001; [].constructor.apply('', a).length");
shouldThrow("var a = []; a.length = 0xFFFFFFFE; [].constructor.apply('', a).length");
shouldThrow("var a = []; a.length = 0xFFFFFFFF; [].constructor.apply('', a).length");

Expand Down

0 comments on commit 4e3a17d

Please sign in to comment.