JSEntryTrampoline: check for stack space before pushing arguments
Optimistically pushing a lot of arguments can run into the stack limit of the process, at least on operating systems where this limit is close to the limit that V8 sets for itself.
BUG=chromium:469768
LOG=y
Review URL: https://codereview.chromium.org/1056913003
Cr-Commit-Position: refs/heads/master@{#27614}
diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc
index 40531ca..e14570c 100644
--- a/src/arm/builtins-arm.cc
+++ b/src/arm/builtins-arm.cc
@@ -830,6 +830,39 @@
}
+enum IsTagged { kArgcIsSmiTagged, kArgcIsUntaggedInt };
+
+
+// Clobbers r2; preserves all other registers.
+static void Generate_CheckStackOverflow(MacroAssembler* masm,
+ const int calleeOffset, Register argc,
+ IsTagged argc_is_tagged) {
+ // Check the stack for overflow. We are not trying to catch
+ // interruptions (e.g. debug break and preemption) here, so the "real stack
+ // limit" is checked.
+ Label okay;
+ __ LoadRoot(r2, Heap::kRealStackLimitRootIndex);
+ // Make r2 the space we have left. The stack might already be overflowed
+ // here which will cause r2 to become negative.
+ __ sub(r2, sp, r2);
+ // Check if the arguments will overflow the stack.
+ if (argc_is_tagged == kArgcIsSmiTagged) {
+ __ cmp(r2, Operand::PointerOffsetFromSmiKey(argc));
+ } else {
+ DCHECK(argc_is_tagged == kArgcIsUntaggedInt);
+ __ cmp(r2, Operand(argc, LSL, kPointerSizeLog2));
+ }
+ __ b(gt, &okay); // Signed comparison.
+
+ // Out of stack space.
+ __ ldr(r1, MemOperand(fp, calleeOffset));
+ __ Push(r1, argc);
+ __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
+
+ __ bind(&okay);
+}
+
+
static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm,
bool is_construct) {
// Called from Generate_JS_Entry
@@ -857,6 +890,14 @@
__ push(r1);
__ push(r2);
+ // Check if we have enough stack space to push all arguments.
+ // The function is the first thing that was pushed above after entering
+ // the internal frame.
+ const int kFunctionOffset =
+ InternalFrameConstants::kCodeOffset - kPointerSize;
+ // Clobbers r2.
+ Generate_CheckStackOverflow(masm, kFunctionOffset, r3, kArgcIsUntaggedInt);
+
// Copy arguments to the stack in a loop.
// r1: function
// r3: argc
@@ -1336,29 +1377,6 @@
}
-static void Generate_CheckStackOverflow(MacroAssembler* masm,
- const int calleeOffset) {
- // Check the stack for overflow. We are not trying to catch
- // interruptions (e.g. debug break and preemption) here, so the "real stack
- // limit" is checked.
- Label okay;
- __ LoadRoot(r2, Heap::kRealStackLimitRootIndex);
- // Make r2 the space we have left. The stack might already be overflowed
- // here which will cause r2 to become negative.
- __ sub(r2, sp, r2);
- // Check if the arguments will overflow the stack.
- __ cmp(r2, Operand::PointerOffsetFromSmiKey(r0));
- __ b(gt, &okay); // Signed comparison.
-
- // Out of stack space.
- __ ldr(r1, MemOperand(fp, calleeOffset));
- __ Push(r1, r0);
- __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
-
- __ bind(&okay);
-}
-
-
static void Generate_PushAppliedArguments(MacroAssembler* masm,
const int argumentsOffset,
const int indexOffset,
@@ -1416,7 +1434,7 @@
__ InvokeBuiltin(Builtins::APPLY_PREPARE, CALL_FUNCTION);
}
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, r0, kArgcIsSmiTagged);
// Push current limit and index.
const int kIndexOffset =
@@ -1544,7 +1562,7 @@
__ push(r0);
__ InvokeBuiltin(Builtins::REFLECT_CONSTRUCT_PREPARE, CALL_FUNCTION);
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, r0, kArgcIsSmiTagged);
// Push current limit and index.
const int kIndexOffset =
diff --git a/src/arm64/builtins-arm64.cc b/src/arm64/builtins-arm64.cc
index dfb59c0..97ccf19 100644
--- a/src/arm64/builtins-arm64.cc
+++ b/src/arm64/builtins-arm64.cc
@@ -799,6 +799,46 @@
}
+enum IsTagged { kArgcIsSmiTagged, kArgcIsUntaggedInt };
+
+
+// Clobbers x10, x15; preserves all other registers.
+static void Generate_CheckStackOverflow(MacroAssembler* masm,
+ const int calleeOffset, Register argc,
+ IsTagged argc_is_tagged) {
+ Register function = x15;
+
+ // Check the stack for overflow.
+ // We are not trying to catch interruptions (e.g. debug break and
+ // preemption) here, so the "real stack limit" is checked.
+ Label enough_stack_space;
+ __ LoadRoot(x10, Heap::kRealStackLimitRootIndex);
+ __ Ldr(function, MemOperand(fp, calleeOffset));
+ // Make x10 the space we have left. The stack might already be overflowed
+ // here which will cause x10 to become negative.
+ // TODO(jbramley): Check that the stack usage here is safe.
+ __ Sub(x10, jssp, x10);
+ // Check if the arguments will overflow the stack.
+ if (argc_is_tagged == kArgcIsSmiTagged) {
+ __ Cmp(x10, Operand::UntagSmiAndScale(argc, kPointerSizeLog2));
+ } else {
+ DCHECK(argc_is_tagged == kArgcIsUntaggedInt);
+ __ Cmp(x10, Operand(argc, LSL, kPointerSizeLog2));
+ }
+ __ B(gt, &enough_stack_space);
+ // There is not enough stack space, so use a builtin to throw an appropriate
+ // error.
+ __ Push(function, argc);
+ __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
+ // We should never return from the APPLY_OVERFLOW builtin.
+ if (__ emit_debug_code()) {
+ __ Unreachable();
+ }
+
+ __ Bind(&enough_stack_space);
+}
+
+
// Input:
// x0: code entry.
// x1: function.
@@ -832,6 +872,15 @@
// Push the function and the receiver onto the stack.
__ Push(function, receiver);
+ // Check if we have enough stack space to push all arguments.
+ // The function is the first thing that was pushed above after entering
+ // the internal frame.
+ const int kFunctionOffset =
+ InternalFrameConstants::kCodeOffset - kPointerSize;
+ // Expects argument count in eax. Clobbers ecx, edx, edi.
+ Generate_CheckStackOverflow(masm, kFunctionOffset, argc,
+ kArgcIsUntaggedInt);
+
// Copy arguments to the stack in a loop, in reverse order.
// x3: argc.
// x4: argv.
@@ -1324,37 +1373,6 @@
}
-static void Generate_CheckStackOverflow(MacroAssembler* masm,
- const int calleeOffset) {
- Register argc = x0;
- Register function = x15;
-
- // Check the stack for overflow.
- // We are not trying to catch interruptions (e.g. debug break and
- // preemption) here, so the "real stack limit" is checked.
- Label enough_stack_space;
- __ LoadRoot(x10, Heap::kRealStackLimitRootIndex);
- __ Ldr(function, MemOperand(fp, calleeOffset));
- // Make x10 the space we have left. The stack might already be overflowed
- // here which will cause x10 to become negative.
- // TODO(jbramley): Check that the stack usage here is safe.
- __ Sub(x10, jssp, x10);
- // Check if the arguments will overflow the stack.
- __ Cmp(x10, Operand::UntagSmiAndScale(argc, kPointerSizeLog2));
- __ B(gt, &enough_stack_space);
- // There is not enough stack space, so use a builtin to throw an appropriate
- // error.
- __ Push(function, argc);
- __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
- // We should never return from the APPLY_OVERFLOW builtin.
- if (__ emit_debug_code()) {
- __ Unreachable();
- }
-
- __ Bind(&enough_stack_space);
-}
-
-
static void Generate_PushAppliedArguments(MacroAssembler* masm,
const int argumentsOffset,
const int indexOffset,
@@ -1422,7 +1440,7 @@
}
Register argc = x0;
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, argc, kArgcIsSmiTagged);
// Push current limit and index.
__ Mov(x1, 0); // Initial index.
@@ -1549,7 +1567,7 @@
__ InvokeBuiltin(Builtins::REFLECT_CONSTRUCT_PREPARE, CALL_FUNCTION);
Register argc = x0;
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, argc, kArgcIsSmiTagged);
// Push current limit and index, constructor & newTarget
__ Mov(x1, 0); // Initial index.
diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc
index ea9b8c9..b4966ba 100644
--- a/src/ia32/builtins-ia32.cc
+++ b/src/ia32/builtins-ia32.cc
@@ -574,6 +574,44 @@
}
+enum IsTagged { kEaxIsSmiTagged, kEaxIsUntaggedInt };
+
+
+// Clobbers ecx, edx, edi; preserves all other registers.
+static void Generate_CheckStackOverflow(MacroAssembler* masm,
+ const int calleeOffset,
+ IsTagged eax_is_tagged) {
+ // eax : the number of items to be pushed to the stack
+ //
+ // Check the stack for overflow. We are not trying to catch
+ // interruptions (e.g. debug break and preemption) here, so the "real stack
+ // limit" is checked.
+ Label okay;
+ ExternalReference real_stack_limit =
+ ExternalReference::address_of_real_stack_limit(masm->isolate());
+ __ mov(edi, Operand::StaticVariable(real_stack_limit));
+ // Make ecx the space we have left. The stack might already be overflowed
+ // here which will cause ecx to become negative.
+ __ mov(ecx, esp);
+ __ sub(ecx, edi);
+ // Make edx the space we need for the array when it is unrolled onto the
+ // stack.
+ __ mov(edx, eax);
+ int smi_tag = eax_is_tagged == kEaxIsSmiTagged ? kSmiTagSize : 0;
+ __ shl(edx, kPointerSizeLog2 - smi_tag);
+ // Check if the arguments will overflow the stack.
+ __ cmp(ecx, edx);
+ __ j(greater, &okay); // Signed comparison.
+
+ // Out of stack space.
+ __ push(Operand(ebp, calleeOffset)); // push this
+ __ push(eax);
+ __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
+
+ __ bind(&okay);
+}
+
+
static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm,
bool is_construct) {
ProfileEntryHookStub::MaybeCallEntryHook(masm);
@@ -599,6 +637,14 @@
__ mov(eax, Operand(ebx, EntryFrameConstants::kArgcOffset));
__ mov(ebx, Operand(ebx, EntryFrameConstants::kArgvOffset));
+ // Check if we have enough stack space to push all arguments.
+ // The function is the first thing that was pushed above after entering
+ // the internal frame.
+ const int kFunctionOffset =
+ InternalFrameConstants::kCodeOffset - kPointerSize;
+ // Expects argument count in eax. Clobbers ecx, edx, edi.
+ Generate_CheckStackOverflow(masm, kFunctionOffset, kEaxIsUntaggedInt);
+
// Copy arguments to the stack in a loop.
Label loop, entry;
__ Move(ecx, Immediate(0));
@@ -990,38 +1036,6 @@
}
-static void Generate_CheckStackOverflow(MacroAssembler* masm,
- const int calleeOffset) {
- // eax : the number of items to be pushed to the stack
- //
- // Check the stack for overflow. We are not trying to catch
- // interruptions (e.g. debug break and preemption) here, so the "real stack
- // limit" is checked.
- Label okay;
- ExternalReference real_stack_limit =
- ExternalReference::address_of_real_stack_limit(masm->isolate());
- __ mov(edi, Operand::StaticVariable(real_stack_limit));
- // Make ecx the space we have left. The stack might already be overflowed
- // here which will cause ecx to become negative.
- __ mov(ecx, esp);
- __ sub(ecx, edi);
- // Make edx the space we need for the array when it is unrolled onto the
- // stack.
- __ mov(edx, eax);
- __ shl(edx, kPointerSizeLog2 - kSmiTagSize);
- // Check if the arguments will overflow the stack.
- __ cmp(ecx, edx);
- __ j(greater, &okay); // Signed comparison.
-
- // Out of stack space.
- __ push(Operand(ebp, calleeOffset)); // push this
- __ push(eax);
- __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
-
- __ bind(&okay);
-}
-
-
static void Generate_PushAppliedArguments(MacroAssembler* masm,
const int argumentsOffset,
const int indexOffset,
@@ -1099,7 +1113,7 @@
__ InvokeBuiltin(Builtins::APPLY_PREPARE, CALL_FUNCTION);
}
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, kEaxIsSmiTagged);
// Push current index and limit.
const int kLimitOffset =
@@ -1229,7 +1243,7 @@
__ push(Operand(ebp, kNewTargetOffset));
__ InvokeBuiltin(Builtins::REFLECT_CONSTRUCT_PREPARE, CALL_FUNCTION);
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, kEaxIsSmiTagged);
// Push current index and limit.
const int kLimitOffset =
diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc
index 3141c17..dbf2dc4 100644
--- a/src/x64/builtins-x64.cc
+++ b/src/x64/builtins-x64.cc
@@ -573,6 +573,46 @@
}
+enum IsTagged { kRaxIsSmiTagged, kRaxIsUntaggedInt };
+
+
+// Clobbers rcx, rdx, kScratchRegister; preserves all other registers.
+static void Generate_CheckStackOverflow(MacroAssembler* masm,
+ const int calleeOffset,
+ IsTagged rax_is_tagged) {
+ // rax : the number of items to be pushed to the stack
+ //
+ // Check the stack for overflow. We are not trying to catch
+ // interruptions (e.g. debug break and preemption) here, so the "real stack
+ // limit" is checked.
+ Label okay;
+ __ LoadRoot(kScratchRegister, Heap::kRealStackLimitRootIndex);
+ __ movp(rcx, rsp);
+ // Make rcx the space we have left. The stack might already be overflowed
+ // here which will cause rcx to become negative.
+ __ subp(rcx, kScratchRegister);
+ // Make rdx the space we need for the array when it is unrolled onto the
+ // stack.
+ if (rax_is_tagged == kRaxIsSmiTagged) {
+ __ PositiveSmiTimesPowerOfTwoToInteger64(rdx, rax, kPointerSizeLog2);
+ } else {
+ DCHECK(rax_is_tagged == kRaxIsUntaggedInt);
+ __ movp(rdx, rax);
+ __ shlq(rdx, Immediate(kPointerSizeLog2));
+ }
+ // Check if the arguments will overflow the stack.
+ __ cmpp(rcx, rdx);
+ __ j(greater, &okay); // Signed comparison.
+
+ // Out of stack space.
+ __ Push(Operand(rbp, calleeOffset));
+ __ Push(rax);
+ __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
+
+ __ bind(&okay);
+}
+
+
static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm,
bool is_construct) {
ProfileEntryHookStub::MaybeCallEntryHook(masm);
@@ -655,6 +695,14 @@
// rsi : context
// rdi : function
+ // Check if we have enough stack space to push all arguments.
+ // The function is the first thing that was pushed above after entering
+ // the internal frame.
+ const int kFunctionOffset =
+ InternalFrameConstants::kCodeOffset - kRegisterSize;
+ // Expects argument count in rax. Clobbers rcx, rdx.
+ Generate_CheckStackOverflow(masm, kFunctionOffset, kRaxIsUntaggedInt);
+
// Copy arguments to the stack in a loop.
// Register rbx points to array of pointers to handle locations.
// Push the values of these handles.
@@ -1051,35 +1099,6 @@
}
-static void Generate_CheckStackOverflow(MacroAssembler* masm,
- const int calleeOffset) {
- // rax : the number of items to be pushed to the stack
- //
- // Check the stack for overflow. We are not trying to catch
- // interruptions (e.g. debug break and preemption) here, so the "real stack
- // limit" is checked.
- Label okay;
- __ LoadRoot(kScratchRegister, Heap::kRealStackLimitRootIndex);
- __ movp(rcx, rsp);
- // Make rcx the space we have left. The stack might already be overflowed
- // here which will cause rcx to become negative.
- __ subp(rcx, kScratchRegister);
- // Make rdx the space we need for the array when it is unrolled onto the
- // stack.
- __ PositiveSmiTimesPowerOfTwoToInteger64(rdx, rax, kPointerSizeLog2);
- // Check if the arguments will overflow the stack.
- __ cmpp(rcx, rdx);
- __ j(greater, &okay); // Signed comparison.
-
- // Out of stack space.
- __ Push(Operand(rbp, calleeOffset));
- __ Push(rax);
- __ InvokeBuiltin(Builtins::STACK_OVERFLOW, CALL_FUNCTION);
-
- __ bind(&okay);
-}
-
-
static void Generate_PushAppliedArguments(MacroAssembler* masm,
const int argumentsOffset,
const int indexOffset,
@@ -1157,7 +1176,7 @@
__ InvokeBuiltin(Builtins::APPLY_PREPARE, CALL_FUNCTION);
}
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, kRaxIsSmiTagged);
// Push current index and limit.
const int kLimitOffset =
@@ -1286,7 +1305,7 @@
__ Push(Operand(rbp, kNewTargetOffset));
__ InvokeBuiltin(Builtins::REFLECT_CONSTRUCT_PREPARE, CALL_FUNCTION);
- Generate_CheckStackOverflow(masm, kFunctionOffset);
+ Generate_CheckStackOverflow(masm, kFunctionOffset, kRaxIsSmiTagged);
// Push current index and limit.
const int kLimitOffset =
diff --git a/test/mjsunit/regress/regress-crbug-469768.js b/test/mjsunit/regress/regress-crbug-469768.js
new file mode 100644
index 0000000..e8a16b0
--- /dev/null
+++ b/test/mjsunit/regress/regress-crbug-469768.js
@@ -0,0 +1,33 @@
+// Copyright 2015 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.
+
+// Try several different argument counts to make sure none of them
+// sneak through the system of stack checks.
+
+try {
+ Array.prototype.concat.apply([], new Array(100000));
+} catch (e) {
+ // Throwing is fine, just don't crash.
+}
+
+
+try {
+ Array.prototype.concat.apply([], new Array(150000));
+} catch (e) {
+ // Throwing is fine, just don't crash.
+}
+
+
+try {
+ Array.prototype.concat.apply([], new Array(200000));
+} catch (e) {
+ // Throwing is fine, just don't crash.
+}
+
+
+try {
+ Array.prototype.concat.apply([], new Array(248000));
+} catch (e) {
+ // Throwing is fine, just don't crash.
+}
