[builtin] Fix CallOrConstructForwardVarargs to handle reversed JS stack

Change-Id: Idc204cffce49b564d134a93114a03939c3e75f20
Bug: v8:10201
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2307313
Commit-Queue: Victor Gomes <victorgomes@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69497}

src/builtins/arm/builtins-arm.cc

@@ -2105,31 +2105,69 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   __ sub(r5, r5, r2, SetCC);
   __ b(le, &stack_done);
   {
+    // ----------- S t a t e -------------
+    //  -- r0 : the number of arguments already in the stack (not including the
+    //  receiver)
+    //  -- r1 : the target to call (can be any Object)
+    //  -- r2 : start index (to support rest parameters)
+    //  -- r3 : the new.target (for [[Construct]] calls)
+    //  -- r4 : point to the caller stack frame
+    //  -- r5 : number of arguments to copy, i.e. arguments count - start index
+    // -----------------------------------
+
     // Check for stack overflow.
-    Generate_StackOverflowCheck(masm, r5, r2, &stack_overflow);
+    Generate_StackOverflowCheck(masm, r5, scratch, &stack_overflow);
 
     // Forward the arguments from the caller frame.
+#ifdef V8_REVERSE_JSARGS
+    // Point to the first argument to copy (skipping the receiver).
+    __ add(r4, r4,
+           Operand(CommonFrameConstants::kFixedFrameSizeAboveFp +
+                   kSystemPointerSize));
+    __ add(r4, r4, Operand(r2, LSL, kSystemPointerSizeLog2));
+
+    // Move the arguments already in the stack,
+    // including the receiver and the return address.
+    {
+      Label copy, check;
+      Register num = r8, src = r9,
+               dest = r2;  // r7 and r10 are context and root.
+      __ mov(src, sp);
+      // Update stack pointer.
+      __ lsl(scratch, r5, Operand(kSystemPointerSizeLog2));
+      __ AllocateStackSpace(scratch);
+      __ mov(dest, sp);
+      __ mov(num, r0);
+      __ b(&check);
+      __ bind(&copy);
+      __ ldr(scratch, MemOperand(src, kSystemPointerSize, PostIndex));
+      __ str(scratch, MemOperand(dest, kSystemPointerSize, PostIndex));
+      __ sub(num, num, Operand(1), SetCC);
+      __ bind(&check);
+      __ b(ge, &copy);
+    }
+#endif
+    // Copy arguments from the caller frame.
+    // TODO(victorgomes): Consider using forward order as potentially more cache
+    // friendly.
     {
       Label loop;
-#ifdef V8_REVERSE_JSARGS
-      // Skips frame pointer and old receiver.
-      __ add(r4, r4, Operand(2 * kPointerSize));
-      __ pop(r8);  // Save new receiver.
-#else
+#ifndef V8_REVERSE_JSARGS
       // Skips frame pointer.
-      __ add(r4, r4, Operand(kPointerSize));
+      __ add(r4, r4, Operand(CommonFrameConstants::kFixedFrameSizeAboveFp));
 #endif
       __ add(r0, r0, r5);
       __ bind(&loop);
       {
-        __ ldr(scratch, MemOperand(r4, r5, LSL, kPointerSizeLog2));
-        __ push(scratch);
         __ sub(r5, r5, Operand(1), SetCC);
-        __ b(ne, &loop);
-      }
+        __ ldr(scratch, MemOperand(r4, r5, LSL, kSystemPointerSizeLog2));
 #ifdef V8_REVERSE_JSARGS
-      __ push(r8);  // Recover new receiver.
+        __ str(scratch, MemOperand(r2, r5, LSL, kSystemPointerSizeLog2));
+#else
+        __ push(scratch);
 #endif
+        __ b(ne, &loop);
+      }
     }
   }
   __ b(&stack_done);

src/builtins/arm64/builtins-arm64.cc

@@ -2308,6 +2308,7 @@ void LeaveArgumentsAdaptorFrame(MacroAssembler* masm) {
 // one slot up or one slot down, as needed.
 void Generate_PrepareForCopyingVarargs(MacroAssembler* masm, Register argc,
                                        Register len) {
+  Label exit;
 #ifdef V8_REVERSE_JSARGS
   Label even;
   Register slots_to_copy = x10;
@@ -2330,6 +2331,7 @@ void Generate_PrepareForCopyingVarargs(MacroAssembler* masm, Register argc,
   }
 
   __ Bind(&even);
+  __ Cbz(slots_to_claim, &exit);
   __ Claim(slots_to_claim);
 
   // Move the arguments already in the stack including the receiver.
@@ -2341,7 +2343,7 @@ void Generate_PrepareForCopyingVarargs(MacroAssembler* masm, Register argc,
     __ CopyDoubleWords(dst, src, slots_to_copy);
   }
 #else   // !V8_REVERSE_JSARGS
-  Label len_odd, exit;
+  Label len_odd;
   Register slots_to_copy = x10;  // If needed.
   __ Add(slots_to_copy, argc, 1);
   __ Add(argc, argc, len);
@@ -2393,8 +2395,8 @@ void Generate_PrepareForCopyingVarargs(MacroAssembler* masm, Register argc,
             Operand(scratch, LSL, kSystemPointerSizeLog2));  // Store padding.
   }
 
-  __ Bind(&exit);
 #endif  // !V8_REVERSE_JSARGS
+  __ Bind(&exit);
 }
 
 }  // namespace
@@ -2562,8 +2564,21 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   // Push varargs.
   {
     Register dst = x13;
-    __ Add(args_fp, args_fp, 2 * kSystemPointerSize);
+#ifdef V8_REVERSE_JSARGS
+    // Point to the fist argument to copy from (skipping receiver).
+    __ Add(args_fp, args_fp,
+           CommonFrameConstants::kFixedFrameSizeAboveFp + kSystemPointerSize);
+    __ lsl(start_index, start_index, kSystemPointerSizeLog2);
+    __ Add(args_fp, args_fp, start_index);
+    // Point to the position to copy to.
+    __ Add(x10, argc, 1);
+    __ SlotAddress(dst, x10);
+    // Update total number of arguments.
+    __ Add(argc, argc, len);
+#else
+    __ Add(args_fp, args_fp, CommonFrameConstants::kFixedFrameSizeAboveFp);
     __ SlotAddress(dst, 0);
+#endif
     __ CopyDoubleWords(dst, args_fp, len);
   }
   __ B(&stack_done);

src/builtins/ia32/builtins-ia32.cc

@@ -2266,38 +2266,88 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   __ sub(edx, ecx);
   __ j(less_equal, &stack_done);
   {
-    Generate_StackOverflowCheck(masm, edx, ecx, &stack_overflow);
+    // ----------- S t a t e -------------
+    //  -- eax : the number of arguments already in the stack (not including the
+    //  receiver)
+    //  -- ecx : start index (to support rest parameters)
+    //  -- edx : number of arguments to copy, i.e. arguments count - start index
+    //  -- edi : the target to call (can be any Object)
+    //  -- esi : point to the caller stack frame
+    //  -- xmm0 : context for the Call / Construct builtin
+    //  -- xmm1 : the new target (for [[Construct]] calls)
+    // -----------------------------------
 
     // Forward the arguments from the caller frame.
+#ifdef V8_REVERSE_JSARGS
+    __ movd(xmm2, edi);  // Preserve the target to call.
+    Generate_StackOverflowCheck(masm, edx, edi, &stack_overflow);
+    __ movd(xmm3, ebx);  // Preserve root register.
+
+    Register scratch = ebx;
+
+    // Point to the first argument to copy (skipping receiver).
+    __ lea(ecx, Operand(ecx, times_system_pointer_size,
+                        CommonFrameConstants::kFixedFrameSizeAboveFp +
+                            kSystemPointerSize));
+    __ add(esi, ecx);
+
+    // Move the arguments already in the stack,
+    // including the receiver and the return address.
+    {
+      Label copy, check;
+      Register src = ecx, current = edi;
+      // Update stack pointer.
+      __ mov(src, esp);
+      __ lea(scratch, Operand(edx, times_system_pointer_size, 0));
+      __ AllocateStackSpace(scratch);
+      // Include return address and receiver.
+      __ add(eax, Immediate(2));
+      __ Set(current, 0);
+      __ jmp(&check);
+      // Loop.
+      __ bind(&copy);
+      __ mov(scratch, Operand(src, current, times_system_pointer_size, 0));
+      __ mov(Operand(esp, current, times_system_pointer_size, 0), scratch);
+      __ inc(current);
+      __ bind(&check);
+      __ cmp(current, eax);
+      __ j(less, &copy);
+      __ lea(ecx, Operand(esp, eax, times_system_pointer_size, 0));
+    }
+
+    // Update total number of arguments.
+    __ sub(eax, Immediate(2));
+    __ add(eax, edx);
+
+    // Copy the additional caller arguments onto the stack.
+    // TODO(victorgomes): Consider using forward order as potentially more cache
+    // friendly.
     {
+      Register src = esi, dest = ecx, num = edx;
       Label loop;
-      __ add(eax, edx);
-      __ PopReturnAddressTo(ecx);
-#ifdef V8_REVERSE_JSARGS
-      // TODO(victor): When we remove the arguments adaptor machinery above,
-      // we can free the scratch register and avoid this move.
-      __ movd(xmm2, ebx);  // Save root register.
-      __ Pop(ebx);         // Save new receiver.
-#endif
       __ bind(&loop);
-      {
-        __ dec(edx);
-#ifdef V8_REVERSE_JSARGS
-        // Skips old receiver.
-        __ Push(Operand(scratch, edx, times_system_pointer_size,
-                        kFPOnStackSize + kPCOnStackSize + kSystemPointerSize));
+      __ dec(num);
+      __ mov(scratch, Operand(src, num, times_system_pointer_size, 0));
+      __ mov(Operand(dest, num, times_system_pointer_size, 0), scratch);
+      __ j(not_zero, &loop);
+    }
+
+    __ movd(ebx, xmm3);  // Restore root register.
+    __ movd(edi, xmm2);  // Restore the target to call.
 #else
-        __ Push(Operand(scratch, edx, times_system_pointer_size,
-                        kFPOnStackSize + kPCOnStackSize));
-#endif
-        __ j(not_zero, &loop);
-      }
-#ifdef V8_REVERSE_JSARGS
-      __ Push(ebx);        // Push new receiver.
-      __ movd(ebx, xmm2);  // Recover root register.
-#endif
-      __ PushReturnAddressFrom(ecx);
+    Generate_StackOverflowCheck(masm, edx, ecx, &stack_overflow);
+    Label loop;
+    __ add(eax, edx);
+    __ PopReturnAddressTo(ecx);
+    __ bind(&loop);
+    {
+      __ dec(edx);
+      __ Push(Operand(scratch, edx, times_system_pointer_size,
+                      kFPOnStackSize + kPCOnStackSize));
+      __ j(not_zero, &loop);
     }
+    __ PushReturnAddressFrom(ecx);
+#endif
   }
   __ bind(&stack_done);
 
@@ -2308,6 +2358,9 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   __ Jump(code, RelocInfo::CODE_TARGET);
 
   __ bind(&stack_overflow);
+#ifdef V8_REVERSE_JSARGS
+  __ movd(edi, xmm2);  // Restore the target to call.
+#endif
   __ movd(esi, xmm0);  // Restore the context.
   __ TailCallRuntime(Runtime::kThrowStackOverflow);
 }

src/builtins/x64/builtins-x64.cc

@@ -2371,37 +2371,82 @@ void Builtins::Generate_CallOrConstructForwardVarargs(MacroAssembler* masm,
   __ subl(r8, rcx);
   __ j(less_equal, &stack_done);
   {
+    // ----------- S t a t e -------------
+    //  -- rax : the number of arguments already in the stack (not including the
+    //  receiver)
+    //  -- rbx : point to the caller stack frame
+    //  -- rcx : start index (to support rest parameters)
+    //  -- rdx : the new target (for [[Construct]] calls)
+    //  -- rdi : the target to call (can be any Object)
+    //  -- r8  : number of arguments to copy, i.e. arguments count - start index
+    // -----------------------------------
+
     // Check for stack overflow.
-    Generate_StackOverflowCheck(masm, r8, rcx, &stack_overflow, Label::kNear);
+    Generate_StackOverflowCheck(masm, r8, r12, &stack_overflow, Label::kNear);
 
     // Forward the arguments from the caller frame.
+#ifdef V8_REVERSE_JSARGS
+    // Move the arguments already in the stack,
+    // including the receiver and the return address.
+    {
+      Label copy, check;
+      Register src = r9, dest = rsp, num = r12, current = r11;
+      __ movq(src, rsp);
+      __ leaq(kScratchRegister, Operand(r8, times_system_pointer_size, 0));
+      __ AllocateStackSpace(kScratchRegister);
+      __ leaq(num, Operand(rax, 2));  // Number of words to copy.
+                                      // +2 for receiver and return address.
+      __ Set(current, 0);
+      __ jmp(&check);
+      __ bind(&copy);
+      __ movq(kScratchRegister,
+              Operand(src, current, times_system_pointer_size, 0));
+      __ movq(Operand(dest, current, times_system_pointer_size, 0),
+              kScratchRegister);
+      __ incq(current);
+      __ bind(&check);
+      __ cmpq(current, num);
+      __ j(less, &copy);
+      __ leaq(r9, Operand(rsp, num, times_system_pointer_size, 0));
+    }
+
+    __ addl(rax, r8);  // Update total number of arguments.
+
+    // Point to the first argument to copy (skipping receiver).
+    __ leaq(rcx, Operand(rcx, times_system_pointer_size,
+                         CommonFrameConstants::kFixedFrameSizeAboveFp +
+                             kSystemPointerSize));
+    __ addq(rbx, rcx);
+
+    // Copy the additional caller arguments onto the stack.
+    // TODO(victorgomes): Consider using forward order as potentially more cache
+    // friendly.
+    {
+      Register src = rbx, dest = r9, num = r8;
+      Label loop;
+      __ bind(&loop);
+      __ decq(num);
+      __ movq(kScratchRegister,
+              Operand(src, num, times_system_pointer_size, 0));
+      __ movq(Operand(dest, num, times_system_pointer_size, 0),
+              kScratchRegister);
+      __ j(not_zero, &loop);
+    }
+#else
     {
       Label loop;
       __ addl(rax, r8);
       __ PopReturnAddressTo(rcx);
-#ifdef V8_REVERSE_JSARGS
-      // The new receiver is already on the stack. Save it to push it later.
-      __ Pop(kScratchRegister);
-#endif
       __ bind(&loop);
       {
         __ decl(r8);
-#ifdef V8_REVERSE_JSARGS
-        // Skips the old receiver.
-        __ Push(Operand(rbx, r8, times_system_pointer_size,
-                        kFPOnStackSize + kPCOnStackSize + kSystemPointerSize));
-#else
         __ Push(Operand(rbx, r8, times_system_pointer_size,
                         kFPOnStackSize + kPCOnStackSize));
-#endif
         __ j(not_zero, &loop);
       }
-#ifdef V8_REVERSE_JSARGS
-      // Recover the new receiver.
-      __ Push(kScratchRegister);
-#endif
       __ PushReturnAddressFrom(rcx);
     }
+#endif
   }
   __ jmp(&stack_done, Label::kNear);
   __ bind(&stack_overflow);

src/codegen/interface-descriptors.h

@@ -927,8 +927,8 @@ class CallTrampolineDescriptor : public CallInterfaceDescriptor {
 
 class CallVarargsDescriptor : public CallInterfaceDescriptor {
  public:
-  DEFINE_PARAMETERS(kTarget, kActualArgumentsCount, kArgumentsLength,
-                    kArgumentsList)
+  DEFINE_PARAMETERS_VARARGS(kTarget, kActualArgumentsCount, kArgumentsLength,
+                            kArgumentsList)
   DEFINE_PARAMETER_TYPES(MachineType::AnyTagged(),  // kTarget
                          MachineType::Int32(),      // kActualArgumentsCount
                          MachineType::Int32(),      // kArgumentsLength
@@ -938,7 +938,7 @@ class CallVarargsDescriptor : public CallInterfaceDescriptor {
 
 class CallForwardVarargsDescriptor : public CallInterfaceDescriptor {
  public:
-  DEFINE_PARAMETERS(kTarget, kActualArgumentsCount, kStartIndex)
+  DEFINE_PARAMETERS_VARARGS(kTarget, kActualArgumentsCount, kStartIndex)
   DEFINE_PARAMETER_TYPES(MachineType::AnyTagged(),  // kTarget
                          MachineType::Int32(),      // kActualArgumentsCount
                          MachineType::Int32())      // kStartIndex