Skip to content

Commit

Permalink
[builtin] Fix CallOrConstructForwardVarargs to handle reversed JS stack
Browse files Browse the repository at this point in the history 
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}
  • Loading branch information
@victorgomes
victorgomes authored and Commit Bot committed Aug 20, 2020
1 parent 5665d83 commit db15c5e
Show file tree
Hide file tree
Showing 5 changed files with 209 additions and 58 deletions.
62 changes: 50 additions & 12 deletions src/builtins/arm/builtins-arm.cc
View file
Expand Up @@ -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);
Expand Down
21 changes: 18 additions & 3 deletions src/builtins/arm64/builtins-arm64.cc
View file
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
103 changes: 78 additions & 25 deletions src/builtins/ia32/builtins-ia32.cc
View file
Expand Up @@ -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);

Expand All @@ -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);
}
Expand Down
75 changes: 60 additions & 15 deletions src/builtins/x64/builtins-x64.cc
View file
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/codegen/interface-descriptors.h
View file
Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit db15c5e

Please sign in to comment.