From 556b522ac62414c87d05f5d6bab33f6b6cab9b13 Mon Sep 17 00:00:00 2001 From: bmeurer Date: Wed, 23 Sep 2015 03:58:38 -0700 Subject: [PATCH] [runtime] Remove weird pushing of something on StackOverflow. We somehow try to push some stuff on the stack when we detect a stack overflow, that we don't need. Even worse we might access outside the valid stack bounds. Since we don't need this, it's gone. CQ_INCLUDE_TRYBOTS=tryserver.v8:v8_linux_layout_dbg,v8_linux_nosnap_dbg R=jarin@chromium.org BUG=chromium:534881 LOG=n Review URL: https://codereview.chromium.org/1360953003 Cr-Commit-Position: refs/heads/master@{#30883} --- src/arm/builtins-arm.cc | 18 ++++-------------- src/arm64/builtins-arm64.cc | 23 ++++------------------- src/ia32/builtins-ia32.cc | 16 +++------------- src/mips/builtins-mips.cc | 18 ++++-------------- src/mips64/builtins-mips64.cc | 18 ++++-------------- src/x64/builtins-x64.cc | 16 +++------------- 6 files changed, 22 insertions(+), 87 deletions(-) diff --git a/src/arm/builtins-arm.cc b/src/arm/builtins-arm.cc index 715118cea..5b278a1b1 100644 --- a/src/arm/builtins-arm.cc +++ b/src/arm/builtins-arm.cc @@ -702,8 +702,7 @@ enum IsTagged { kArgcIsSmiTagged, kArgcIsUntaggedInt }; // Clobbers r2; preserves all other registers. -static void Generate_CheckStackOverflow(MacroAssembler* masm, - const int calleeOffset, Register argc, +static void Generate_CheckStackOverflow(MacroAssembler* masm, 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 @@ -723,11 +722,6 @@ static void Generate_CheckStackOverflow(MacroAssembler* masm, __ b(gt, &okay); // Signed comparison. // Out of stack space. - __ ldr(r1, MemOperand(fp, calleeOffset)); - if (argc_is_tagged == kArgcIsUntaggedInt) { - __ SmiTag(argc); - } - __ Push(r1, argc); __ CallRuntime(Runtime::kThrowStackOverflow, 0); __ bind(&okay); @@ -764,12 +758,8 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm, __ Push(r1, 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); + Generate_CheckStackOverflow(masm, r3, kArgcIsUntaggedInt); // Remember new.target. __ mov(r5, r0); @@ -1338,7 +1328,7 @@ static void Generate_ApplyHelper(MacroAssembler* masm, bool targetIsArgument) { __ InvokeBuiltin(Context::APPLY_PREPARE_BUILTIN_INDEX, CALL_FUNCTION); } - Generate_CheckStackOverflow(masm, kFunctionOffset, r0, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, r0, kArgcIsSmiTagged); // Push current limit and index. const int kIndexOffset = kVectorOffset - (2 * kPointerSize); @@ -1399,7 +1389,7 @@ static void Generate_ConstructHelper(MacroAssembler* masm) { __ InvokeBuiltin(Context::REFLECT_CONSTRUCT_PREPARE_BUILTIN_INDEX, CALL_FUNCTION); - Generate_CheckStackOverflow(masm, kFunctionOffset, r0, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, r0, kArgcIsSmiTagged); // Push current limit and index. const int kIndexOffset = kVectorOffset - (2 * kPointerSize); diff --git a/src/arm64/builtins-arm64.cc b/src/arm64/builtins-arm64.cc index 53cf0eccb..afa3b077f 100644 --- a/src/arm64/builtins-arm64.cc +++ b/src/arm64/builtins-arm64.cc @@ -721,17 +721,13 @@ enum IsTagged { kArgcIsSmiTagged, kArgcIsUntaggedInt }; // Clobbers x10, x15; preserves all other registers. -static void Generate_CheckStackOverflow(MacroAssembler* masm, - const int calleeOffset, Register argc, +static void Generate_CheckStackOverflow(MacroAssembler* masm, 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. @@ -744,12 +740,6 @@ static void Generate_CheckStackOverflow(MacroAssembler* masm, __ 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. - if (argc_is_tagged == kArgcIsUntaggedInt) { - __ SmiTag(argc); - } - __ Push(function, argc); __ CallRuntime(Runtime::kThrowStackOverflow, 0); // We should never return from the APPLY_OVERFLOW builtin. if (__ emit_debug_code()) { @@ -798,13 +788,8 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm, __ 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); + Generate_CheckStackOverflow(masm, argc, kArgcIsUntaggedInt); // Copy arguments to the stack in a loop, in reverse order. // x3: argc. @@ -1395,7 +1380,7 @@ static void Generate_ApplyHelper(MacroAssembler* masm, bool targetIsArgument) { } Register argc = x0; - Generate_CheckStackOverflow(masm, kFunctionOffset, argc, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, argc, kArgcIsSmiTagged); // Push current limit, index and receiver. __ Mov(x1, 0); // Initial index. @@ -1466,7 +1451,7 @@ static void Generate_ConstructHelper(MacroAssembler* masm) { CALL_FUNCTION); Register argc = x0; - Generate_CheckStackOverflow(masm, kFunctionOffset, argc, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, argc, kArgcIsSmiTagged); // Push current limit and index & constructor function as callee. __ Mov(x1, 0); // Initial index. diff --git a/src/ia32/builtins-ia32.cc b/src/ia32/builtins-ia32.cc index 817b4252b..88e9841d5 100644 --- a/src/ia32/builtins-ia32.cc +++ b/src/ia32/builtins-ia32.cc @@ -449,7 +449,6 @@ 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 // @@ -474,11 +473,6 @@ static void Generate_CheckStackOverflow(MacroAssembler* masm, __ j(greater, &okay); // Signed comparison. // Out of stack space. - __ push(Operand(ebp, calleeOffset)); // push this - if (eax_is_tagged == kEaxIsUntaggedInt) { - __ SmiTag(eax); - } - __ push(eax); __ CallRuntime(Runtime::kThrowStackOverflow, 0); __ bind(&okay); @@ -512,12 +506,8 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm, __ 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); + Generate_CheckStackOverflow(masm, kEaxIsUntaggedInt); // Copy arguments to the stack in a loop. Label loop, entry; @@ -1042,7 +1032,7 @@ static void Generate_ApplyHelper(MacroAssembler* masm, bool targetIsArgument) { __ InvokeBuiltin(Context::APPLY_PREPARE_BUILTIN_INDEX, CALL_FUNCTION); } - Generate_CheckStackOverflow(masm, kFunctionOffset, kEaxIsSmiTagged); + Generate_CheckStackOverflow(masm, kEaxIsSmiTagged); // Push current index and limit. const int kLimitOffset = kVectorOffset - 1 * kPointerSize; @@ -1111,7 +1101,7 @@ static void Generate_ConstructHelper(MacroAssembler* masm) { __ InvokeBuiltin(Context::REFLECT_CONSTRUCT_PREPARE_BUILTIN_INDEX, CALL_FUNCTION); - Generate_CheckStackOverflow(masm, kFunctionOffset, kEaxIsSmiTagged); + Generate_CheckStackOverflow(masm, kEaxIsSmiTagged); // Push current index and limit. const int kLimitOffset = kVectorOffset - 1 * kPointerSize; diff --git a/src/mips/builtins-mips.cc b/src/mips/builtins-mips.cc index 600764ac4..4a2b611b7 100644 --- a/src/mips/builtins-mips.cc +++ b/src/mips/builtins-mips.cc @@ -700,8 +700,7 @@ enum IsTagged { kArgcIsSmiTagged, kArgcIsUntaggedInt }; // Clobbers a2; preserves all other registers. -static void Generate_CheckStackOverflow(MacroAssembler* masm, - const int calleeOffset, Register argc, +static void Generate_CheckStackOverflow(MacroAssembler* masm, 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 @@ -722,11 +721,6 @@ static void Generate_CheckStackOverflow(MacroAssembler* masm, __ Branch(&okay, gt, a2, Operand(t3)); // Out of stack space. - __ lw(a1, MemOperand(fp, calleeOffset)); - if (argc_is_tagged == kArgcIsUntaggedInt) { - __ SmiTag(argc); - } - __ Push(a1, argc); __ CallRuntime(Runtime::kThrowStackOverflow, 0); __ bind(&okay); @@ -763,12 +757,8 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm, __ Push(a1, a2); // 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 a2. - Generate_CheckStackOverflow(masm, kFunctionOffset, a3, kArgcIsUntaggedInt); + Generate_CheckStackOverflow(masm, a3, kArgcIsUntaggedInt); // Remember new.target. __ mov(t1, a0); @@ -1341,7 +1331,7 @@ static void Generate_ApplyHelper(MacroAssembler* masm, bool targetIsArgument) { } // Returns the result in v0. - Generate_CheckStackOverflow(masm, kFunctionOffset, v0, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, v0, kArgcIsSmiTagged); // Push current limit and index. const int kIndexOffset = kVectorOffset - (2 * kPointerSize); @@ -1405,7 +1395,7 @@ static void Generate_ConstructHelper(MacroAssembler* masm) { CALL_FUNCTION); // Returns result in v0. - Generate_CheckStackOverflow(masm, kFunctionOffset, v0, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, v0, kArgcIsSmiTagged); // Push current limit and index. const int kIndexOffset = kVectorOffset - (2 * kPointerSize); diff --git a/src/mips64/builtins-mips64.cc b/src/mips64/builtins-mips64.cc index 581fbe781..105d2e59f 100644 --- a/src/mips64/builtins-mips64.cc +++ b/src/mips64/builtins-mips64.cc @@ -700,8 +700,7 @@ enum IsTagged { kArgcIsSmiTagged, kArgcIsUntaggedInt }; // Clobbers a2; preserves all other registers. -static void Generate_CheckStackOverflow(MacroAssembler* masm, - const int calleeOffset, Register argc, +static void Generate_CheckStackOverflow(MacroAssembler* masm, 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 @@ -721,11 +720,6 @@ static void Generate_CheckStackOverflow(MacroAssembler* masm, __ Branch(&okay, gt, a2, Operand(a7)); // Signed comparison. // Out of stack space. - __ ld(a1, MemOperand(fp, calleeOffset)); - if (argc_is_tagged == kArgcIsUntaggedInt) { - __ SmiTag(argc); - } - __ Push(a1, argc); __ CallRuntime(Runtime::kThrowStackOverflow, 0); __ bind(&okay); @@ -761,12 +755,8 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm, __ Push(a1, a2); // 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 a2. - Generate_CheckStackOverflow(masm, kFunctionOffset, a3, kArgcIsUntaggedInt); + Generate_CheckStackOverflow(masm, a3, kArgcIsUntaggedInt); // Remember new.target. __ mov(a5, a0); @@ -1338,7 +1328,7 @@ static void Generate_ApplyHelper(MacroAssembler* masm, bool targetIsArgument) { } // Returns the result in v0. - Generate_CheckStackOverflow(masm, kFunctionOffset, v0, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, v0, kArgcIsSmiTagged); // Push current limit and index. const int kIndexOffset = kVectorOffset - (2 * kPointerSize); @@ -1402,7 +1392,7 @@ static void Generate_ConstructHelper(MacroAssembler* masm) { CALL_FUNCTION); // Returns result in v0. - Generate_CheckStackOverflow(masm, kFunctionOffset, v0, kArgcIsSmiTagged); + Generate_CheckStackOverflow(masm, v0, kArgcIsSmiTagged); // Push current limit and index. const int kIndexOffset = kVectorOffset - (2 * kPointerSize); diff --git a/src/x64/builtins-x64.cc b/src/x64/builtins-x64.cc index 41c224f8d..a056e4c2f 100644 --- a/src/x64/builtins-x64.cc +++ b/src/x64/builtins-x64.cc @@ -450,7 +450,6 @@ enum IsTagged { kRaxIsSmiTagged, kRaxIsUntaggedInt }; // Clobbers rcx, r11, 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 // @@ -477,11 +476,6 @@ static void Generate_CheckStackOverflow(MacroAssembler* masm, __ j(greater, &okay); // Signed comparison. // Out of stack space. - __ Push(Operand(rbp, calleeOffset)); - if (rax_is_tagged == kRaxIsUntaggedInt) { - __ Integer32ToSmi(rax, rax); - } - __ Push(rax); __ CallRuntime(Runtime::kThrowStackOverflow, 0); __ bind(&okay); @@ -588,12 +582,8 @@ static void Generate_JSEntryTrampolineHelper(MacroAssembler* masm, // rdx : new.target // 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, r11. - Generate_CheckStackOverflow(masm, kFunctionOffset, kRaxIsUntaggedInt); + Generate_CheckStackOverflow(masm, kRaxIsUntaggedInt); // Copy arguments to the stack in a loop. // Register rbx points to array of pointers to handle locations. @@ -1106,7 +1096,7 @@ static void Generate_ApplyHelper(MacroAssembler* masm, bool targetIsArgument) { __ InvokeBuiltin(Context::APPLY_PREPARE_BUILTIN_INDEX, CALL_FUNCTION); } - Generate_CheckStackOverflow(masm, kFunctionOffset, kRaxIsSmiTagged); + Generate_CheckStackOverflow(masm, kRaxIsSmiTagged); // Push current index and limit, and receiver. const int kLimitOffset = kVectorOffset - 1 * kPointerSize; @@ -1176,7 +1166,7 @@ static void Generate_ConstructHelper(MacroAssembler* masm) { __ InvokeBuiltin(Context::REFLECT_CONSTRUCT_PREPARE_BUILTIN_INDEX, CALL_FUNCTION); - Generate_CheckStackOverflow(masm, kFunctionOffset, kRaxIsSmiTagged); + Generate_CheckStackOverflow(masm, kRaxIsSmiTagged); // Push current index and limit. const int kLimitOffset = kVectorOffset - 1 * kPointerSize; -- 2.34.1