[Interpreter] Adds stackcheck in InterpreterPushArgsAndCall/Construct builtins.

src/builtins/ia32/builtins-ia32.cc

 // Copyright 2012 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.
 
 #if V8_TARGET_ARCH_IA32
 
 #include "src/code-factory.h"
 #include "src/codegen.h"
 #include "src/deoptimizer.h"
 #include "src/full-codegen/full-codegen.h"
@@ skipping 685 matching lines @@
     // Push function as argument and compile for baseline.
     __ push(edi);
     __ CallRuntime(Runtime::kCompileBaseline);
 
     // Restore return value.
     __ pop(eax);
   }
   __ ret(0);
 }
 
+static void Generate_StackOverflowCheck(MacroAssembler* masm, Register num_args,
+                                        Register scratch1, Register scratch2,
+                                        Label* stack_overflow) {
+  // 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.
+  ExternalReference real_stack_limit =
+      ExternalReference::address_of_real_stack_limit(masm->isolate());
+  __ mov(scratch1, Operand::StaticVariable(real_stack_limit));
+  // Make scratch2 the space we have left. The stack might already be overflowed
+  // here which will cause scratch2 to become negative.
+  __ mov(scratch2, esp);
+  __ sub(scratch2, scratch1);
+  // Make scratch1 the space we need for the array when it is unrolled onto the
+  // stack.
+  __ mov(scratch1, num_args);
+  __ shl(scratch1, kPointerSizeLog2);
+  // Check if the arguments will overflow the stack.
+  __ cmp(scratch2, scratch1);
+  __ j(less_equal, stack_overflow);  // Signed comparison.
+}
+
 static void Generate_InterpreterPushArgs(MacroAssembler* masm,
                                          Register array_limit,
                                          Register start_address) {
   // ----------- S t a t e -------------
   //  -- start_address : Pointer to the last argument in the args array.
   //  -- array_limit : Pointer to one before the first argument in the
   //                   args array.
   // -----------------------------------
   Label loop_header, loop_check;
   __ jmp(&loop_check);
   __ bind(&loop_header);
   __ Push(Operand(start_address, 0));
   __ sub(start_address, Immediate(kPointerSize));
   __ bind(&loop_check);
   __ cmp(start_address, array_limit);
   __ j(greater, &loop_header, Label::kNear);
 }
 
 // static
 void Builtins::Generate_InterpreterPushArgsAndCallImpl(
     MacroAssembler* masm, TailCallMode tail_call_mode,
     CallableType function_type) {
   // ----------- S t a t e -------------
   //  -- eax : the number of arguments (not including the receiver)
   //  -- ebx : the address of the first argument to be pushed. Subsequent
   //           arguments should be consecutive above this, in the same order as
   //           they are to be pushed onto the stack.
   //  -- edi : the target to call (can be any Object).
   // -----------------------------------
+  Label stack_overflow;
+  // Compute the expected number of arguments.
+  __ mov(ecx, eax);
+  __ add(ecx, Immediate(1));  // Add one for receiver.
+
+  // Add a stack check before pushing the arguments. We need an extra register
+  // to perform a stack check. So push it onto the stack temporarily. This
+  // might cause stack overflow, but it will be detected by the check.
+  __ Push(edi);
+  Generate_StackOverflowCheck(masm, ecx, edx, edi, &stack_overflow);
+  __ Pop(edi);
 
   // Pop return address to allow tail-call after pushing arguments.
   __ Pop(edx);
 
   // Find the address of the last argument.
-  __ mov(ecx, eax);
-  __ add(ecx, Immediate(1));  // Add one for receiver.
   __ shl(ecx, kPointerSizeLog2);
   __ neg(ecx);
   __ add(ecx, ebx);
-
-  // TODO(mythria): Add a stack check before pushing the arguments.
   Generate_InterpreterPushArgs(masm, ecx, ebx);
 
   // Call the target.
   __ Push(edx);  // Re-push return address.
 
   if (function_type == CallableType::kJSFunction) {
     __ Jump(masm->isolate()->builtins()->CallFunction(ConvertReceiverMode::kAny,
                                                       tail_call_mode),
             RelocInfo::CODE_TARGET);
   } else {
     DCHECK_EQ(function_type, CallableType::kAny);
     __ Jump(masm->isolate()->builtins()->Call(ConvertReceiverMode::kAny,
                                               tail_call_mode),
             RelocInfo::CODE_TARGET);
   }
+
+  __ bind(&stack_overflow);
+  {
+    // Pop the temporary registers, so that return address is on top of stack.
+    __ Pop(edi);

[rmcilroy, 2016/09/12 14:43:17]

Not really keen on this pop. Could you not just use esi instead of edi above to
avoid the extra push?

[mythria, 2016/09/13 09:53:33]

As discussed offline, we can't use esi, because it holds context. So we can't
avoid this pop.

+
+    __ TailCallRuntime(Runtime::kThrowStackOverflow);
+
+    __ int3();
+  }
 }
 
 namespace {
 
 // This function modified start_addr, and only reads the contents of num_args
 // register. scratch1 and scratch2 are used as temporary registers. Their
 // original values are restored after the use.
 void Generate_InterpreterPushArgsAndReturnAddress(
     MacroAssembler* masm, Register num_args, Register start_addr,
-    Register scratch1, Register scratch2, bool receiver_in_args) {
+    Register scratch1, Register scratch2, bool receiver_in_args,
+    Label* stack_overflow) {
   // Store scratch2, scratch1 onto the stack. We need to restore the original
-  // values
-  // so store scratch2, scratch1 temporarily on stack.
+  // values so store scratch2, scratch1 temporarily on stack.
   __ Push(scratch2);
   __ Push(scratch1);
 
   // We have to pop return address and the two temporary registers before we
   // can push arguments onto the stack. we do not have any free registers so
   // update the stack and copy them into the correct places on the stack.
   //  current stack    =====>    required stack layout
   // |             |            | scratch1      | (2) <-- esp(1)
   // |             |            | scratch2      | (3)
   // |             |            | return addr   | (4)
   // |             |            | arg N         | (5)
   // | scratch1    | <-- esp    | ....          |
   // | scratch2    |            | arg 0         |
   // | return addr |            | receiver slot |
 
+  // Check for stack overflow before we push arguments.
+  // 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.
+  ExternalReference real_stack_limit =
+      ExternalReference::address_of_real_stack_limit(masm->isolate());
+  __ mov(scratch1, Operand::StaticVariable(real_stack_limit));
+  // Make scratch2 the space we have left. The stack might already be overflowed
+  // here which will cause scratch2 to become negative.
+  __ mov(scratch2, esp);
+  __ sub(scratch2, scratch1);
+
+  // Compute expected number of arguments.
+  __ mov(scratch1, num_args);
+  __ add(scratch1, Immediate(1));  // Add one for receiver.
+  __ shl(scratch1, kPointerSizeLog2);
+
+  __ cmp(scratch2, scratch1);
+  __ j(less_equal, stack_overflow);  // Signed comparison.
+
   // First increment the stack pointer to the correct location.
   // we need additional slots for arguments and the receiver.
   // Step 1 - compute the required increment to the stack.
-  __ mov(scratch1, num_args);
-  __ shl(scratch1, kPointerSizeLog2);
-  __ add(scratch1, Immediate(kPointerSize));
 
 #ifdef _MSC_VER
   // TODO(mythria): Move it to macro assembler.
   // In windows, we cannot increment the stack size by more than one page
   // (mimimum page size is 4KB) without accessing at least one byte on the
   // page. Check this:
   // https://msdn.microsoft.com/en-us/library/aa227153(v=vs.60).aspx.
   const int page_size = 4 * 1024;
   Label check_offset, update_stack_pointer;
   __ bind(&check_offset);
   __ cmp(scratch1, page_size);
   __ j(less, &update_stack_pointer);
   __ sub(esp, Immediate(page_size));
   // Just to touch the page, before we increment further.
   __ mov(Operand(esp, 0), Immediate(0));
   __ sub(scratch1, Immediate(page_size));
   __ jmp(&check_offset);
   __ bind(&update_stack_pointer);
 #endif
 
-  // TODO(mythria): Add a stack check before updating the stack pointer.
 
   // Step 1 - Update the stack pointer.
   __ sub(esp, scratch1);
 
   // Step 2 move scratch1 to the correct location. Move scratch1 first otherwise
   // we may overwrite when num_args = 0 or 1, basically when the source and
   // destination overlap. We at least need one extra slot for receiver,
   // so no extra checks are required to avoid copy.
   __ mov(scratch1,
          Operand(esp, num_args, times_pointer_size, 1 * kPointerSize));
@@ skipping 45 matching lines @@
     MacroAssembler* masm, CallableType construct_type) {
   // ----------- S t a t e -------------
   //  -- eax : the number of arguments (not including the receiver)
   //  -- edx : the new target
   //  -- edi : the constructor
   //  -- ebx : allocation site feedback (if available or undefined)
   //  -- ecx : the address of the first argument to be pushed. Subsequent
   //           arguments should be consecutive above this, in the same order as
   //           they are to be pushed onto the stack.
   // -----------------------------------
+  Label stack_overflow;
 
   // Push arguments and move return address to the top of stack.
   // The eax register is readonly. The ecx register will be modified. The edx
   // and edi registers will be modified but restored to their original values.
-  Generate_InterpreterPushArgsAndReturnAddress(masm, eax, ecx, edx, edi, false);
+  Generate_InterpreterPushArgsAndReturnAddress(masm, eax, ecx, edx, edi, false,
+                                               &stack_overflow);
 
   __ AssertUndefinedOrAllocationSite(ebx);
   if (construct_type == CallableType::kJSFunction) {
     // Tail call to the function-specific construct stub (still in the caller
     // context at this point).
     __ AssertFunction(edi);
 
     __ mov(ecx, FieldOperand(edi, JSFunction::kSharedFunctionInfoOffset));
     __ mov(ecx, FieldOperand(ecx, SharedFunctionInfo::kConstructStubOffset));
     __ lea(ecx, FieldOperand(ecx, Code::kHeaderSize));
     __ jmp(ecx);
   } else {
     DCHECK_EQ(construct_type, CallableType::kAny);
 
     // Call the constructor with unmodified eax, edi, edx values.
     __ Jump(masm->isolate()->builtins()->Construct(), RelocInfo::CODE_TARGET);
   }
+
+  __ bind(&stack_overflow);
+  {
+    // Pop the temporary registers, so that return address is on top of stack.
+    __ Pop(edx);
+    __ Pop(edi);

[rmcilroy, 2016/09/12 14:43:17]

Really don't like this - it's impossible to see that these have been pushed
since they are pushed as scratch2 / scratch 1. Can you rework to avoid using
extra registers, or at least move the pushes to the same function as they are
poped

[mythria, 2016/09/13 09:53:33]

I reworked the code. I don't think we can avoid using registers, but I reworked
it so that Pushes and Pops happen in this function. We do not have any free
registers. Since we have to pop the return address before we push anything onto
the stack, we need some free registers. Two is the minimum we need. In the new
code, I use three to make the InterpreterPushArgsCopyArguments loop simpler. I
can revert back to the older version of using only two temporary registers.

+
+    __ TailCallRuntime(Runtime::kThrowStackOverflow);
+
+    __ int3();
+  }
 }
 
 // static
 void Builtins::Generate_InterpreterPushArgsAndConstructArray(
     MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- eax : the number of arguments (not including the receiver)
   //  -- edx : the target to call checked to be Array function.
   //  -- ebx : the allocation site feedback
   //  -- ecx : the address of the first argument to be pushed. Subsequent
   //           arguments should be consecutive above this, in the same order as
   //           they are to be pushed onto the stack.
   // -----------------------------------
+  Label stack_overflow;
 
   // Push arguments and move return address to the top of stack.
   // The eax register is readonly. The ecx register will be modified. The edx
   // and edi registers will be modified but restored to their original values.
-  Generate_InterpreterPushArgsAndReturnAddress(masm, eax, ecx, edx, ebx, true);
+  Generate_InterpreterPushArgsAndReturnAddress(masm, eax, ecx, edx, ebx, true,
+                                               &stack_overflow);
 
   // Array constructor expects constructor in edi. It is same as edx here.
   __ Move(edi, edx);
 
   ArrayConstructorStub stub(masm->isolate());
   __ TailCallStub(&stub);
+
+  __ bind(&stack_overflow);
+  {
+    // Pop the temporary registers, so that return address is on top of stack.
+    __ Pop(edx);
+    __ Pop(edi);
+
+    __ TailCallRuntime(Runtime::kThrowStackOverflow);
+
+    __ int3();
+  }
 }
 
 void Builtins::Generate_InterpreterEnterBytecodeDispatch(MacroAssembler* masm) {
   // Set the return address to the correct point in the interpreter entry
   // trampoline.
   Smi* interpreter_entry_return_pc_offset(
       masm->isolate()->heap()->interpreter_entry_return_pc_offset());
   DCHECK_NE(interpreter_entry_return_pc_offset, Smi::FromInt(0));
   __ LoadHeapObject(ebx,
                     masm->isolate()->builtins()->InterpreterEntryTrampoline());
@@ skipping 1205 matching lines @@
   __ bind(&drop_frame_and_ret);
   {
     // Drop all arguments including the receiver.
     __ PopReturnAddressTo(ecx);
     __ lea(esp, Operand(esp, ebx, times_pointer_size, kPointerSize));
     __ PushReturnAddressFrom(ecx);
     __ Ret();
   }
 }
 
-static void ArgumentsAdaptorStackCheck(MacroAssembler* masm,
-                                       Label* stack_overflow) {
-  // ----------- S t a t e -------------
-  //  -- eax : actual number of arguments
-  //  -- ebx : expected number of arguments
-  //  -- edx : new target (passed through to callee)
-  // -----------------------------------
-  // 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.
-  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 edi the space we need for the array when it is unrolled onto the
-  // stack.
-  __ mov(edi, ebx);
-  __ shl(edi, kPointerSizeLog2);
-  // Check if the arguments will overflow the stack.
-  __ cmp(ecx, edi);
-  __ j(less_equal, stack_overflow);  // Signed comparison.
-}
-
 static void EnterArgumentsAdaptorFrame(MacroAssembler* masm) {
   __ push(ebp);
   __ mov(ebp, esp);
 
   // Store the arguments adaptor context sentinel.
   __ push(Immediate(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR)));
 
   // Push the function on the stack.
   __ push(edi);
 
@@ skipping 726 matching lines @@
 
   Label enough, too_few;
   __ cmp(eax, ebx);
   __ j(less, &too_few);
   __ cmp(ebx, SharedFunctionInfo::kDontAdaptArgumentsSentinel);
   __ j(equal, &dont_adapt_arguments);
 
   {  // Enough parameters: Actual >= expected.
     __ bind(&enough);
     EnterArgumentsAdaptorFrame(masm);
-    ArgumentsAdaptorStackCheck(masm, &stack_overflow);
+    // edi is used as a scratch register. It should be restored from the frame
+    // when needed.
+    Generate_StackOverflowCheck(masm, ebx, ecx, edi, &stack_overflow);
 
     // Copy receiver and all expected arguments.
     const int offset = StandardFrameConstants::kCallerSPOffset;
     __ lea(edi, Operand(ebp, eax, times_4, offset));
     __ mov(eax, -1);  // account for receiver
 
     Label copy;
     __ bind(&copy);
     __ inc(eax);
     __ push(Operand(edi, 0));
     __ sub(edi, Immediate(kPointerSize));
     __ cmp(eax, ebx);
     __ j(less, &copy);
     // eax now contains the expected number of arguments.
     __ jmp(&invoke);
   }
 
   {  // Too few parameters: Actual < expected.
     __ bind(&too_few);
     EnterArgumentsAdaptorFrame(masm);
-    ArgumentsAdaptorStackCheck(masm, &stack_overflow);
+    // edi is used as a scratch register. It should be restored from the frame
+    // when needed.
+    Generate_StackOverflowCheck(masm, ebx, ecx, edi, &stack_overflow);
 
     // Remember expected arguments in ecx.
     __ mov(ecx, ebx);
 
     // Copy receiver and all actual arguments.
     const int offset = StandardFrameConstants::kCallerSPOffset;
     __ lea(edi, Operand(ebp, eax, times_4, offset));
     // ebx = expected - actual.
     __ sub(ebx, eax);
     // eax = -actual - 1
@@ skipping 214 matching lines @@
 
 void Builtins::Generate_InterpreterOnStackReplacement(MacroAssembler* masm) {
   Generate_OnStackReplacementHelper(masm, true);
 }
 
 #undef __
 }  // namespace internal
 }  // namespace v8
 
 #endif  // V8_TARGET_ARCH_IA32