Bug 1840745: Standardize return address pushing in tailCallVM on link-register platforms r=jandem

This rewrites all the other platforms to match arm64.

Differential Revision: https://phabricator.services.mozilla.com/D182433

js/src/jit/arm/SharedICHelpers-arm-inl.h

@@ -32,12 +32,14 @@ inline void EmitBaselineTailCallVM(TrampolinePtr target, MacroAssembler& masm,
 #endif
 
   // Push frame descriptor and perform the tail call.
-  // ICTailCallReg (lr) already contains the return address (as we keep
-  // it there through the stub calls), but the VMWrapper code being called
-  // expects the return address to also be pushed on the stack.
-  static_assert(ICTailCallReg == lr);
   masm.pushFrameDescriptor(FrameType::BaselineJS);
-  masm.push(lr);
+
+  static_assert(ICTailCallReg == lr);
+  // The return address will be pushed by the VM wrapper, for compatibility
+  // with direct calls. Refer to the top of generateVMWrapper().
+  // ICTailCallReg (lr) already contains the return address (as we keep
+  // it there through the stub calls).
+
   masm.jump(target);
 }
 

js/src/jit/arm/Trampoline-arm.cpp

@@ -264,7 +264,7 @@ void JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm) {
 
     masm.push(r0);  // jitcode
 
-    using Fn = bool (*)(BaselineFrame * frame, InterpreterFrame * interpFrame,
+    using Fn = bool (*)(BaselineFrame* frame, InterpreterFrame* interpFrame,
                         uint32_t numStackValues);
     masm.setupUnalignedABICall(scratch);
     masm.passABIArg(framePtrScratch);  // BaselineFrame
@@ -385,8 +385,7 @@ void JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail) {
   // setupAlignedABICall.
   masm.reserveStack(sizeof(void*) * 2);
   masm.mov(sp, r1);
-  using Fn =
-      bool (*)(InvalidationBailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(InvalidationBailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(r0);
   masm.passABIArg(r1);
@@ -561,7 +560,7 @@ static void GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail) {
   // Make space for Bailout's bailoutInfo outparam.
   masm.reserveStack(sizeof(void*));
   masm.mov(sp, r1);
-  using Fn = bool (*)(BailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(BailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
 
   masm.passABIArg(r0);
@@ -604,16 +603,13 @@ bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
   Register cxreg = r0;
   regs.take(cxreg);
 
-  // Stack is:
-  //    ... frame ...
-  //  +8  [args] + argPadding
-  //  +0  ExitFrame
-  //
-  // If it isn't a tail call, then the return address needs to be saved.
+  // On link-register platforms, it is the responsibility of the VM *callee* to
+  // push the return address, while the caller must ensure that the address
+  // is stored in lr on entry. This allows the VM wrapper to work with both
+  // direct calls and tail calls.
+  masm.pushReturnAddress();
+
   // Push the frame pointer to finish the exit frame, then link it up.
-  if (f.expectTailCall == NonTailCall) {
-    masm.pushReturnAddress();
-  }
   masm.Push(FramePointer);
   masm.moveStackPtrTo(FramePointer);
   masm.loadJSContext(cxreg);

js/src/jit/arm64/Trampoline-arm64.cpp

@@ -226,7 +226,7 @@ void JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm) {
     masm.push(reg_code);
 
     // Initialize the frame, including filling in the slots.
-    using Fn = bool (*)(BaselineFrame * frame, InterpreterFrame * interpFrame,
+    using Fn = bool (*)(BaselineFrame* frame, InterpreterFrame* interpFrame,
                         uint32_t numStackValues);
     masm.setupUnalignedABICall(r19);
     masm.passABIArg(framePtrScratch);  // BaselineFrame.
@@ -389,8 +389,7 @@ void JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail) {
   masm.Sub(x1, masm.GetStackPointer64(), Operand(sizeof(void*)));
   masm.moveToStackPtr(r1);
 
-  using Fn =
-      bool (*)(InvalidationBailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(InvalidationBailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupUnalignedABICall(r10);
   masm.passABIArg(r0);
   masm.passABIArg(r1);
@@ -551,7 +550,7 @@ static void GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail) {
   masm.reserveStack(sizeof(void*));
   masm.moveStackPtrTo(r1);
 
-  using Fn = bool (*)(BailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(BailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupUnalignedABICall(r2);
   masm.passABIArg(r0);
   masm.passABIArg(r1);
@@ -592,11 +591,11 @@ bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
       (Register::Codes::VolatileMask & ~Register::Codes::WrapperMask) == 0,
       "Wrapper register set must be a superset of the Volatile register set.");
 
-  // Unlike on other platforms, it is the responsibility of the VM *callee* to
+  // On link-register platforms, it is the responsibility of the VM *callee* to
   // push the return address, while the caller must ensure that the address
   // is stored in lr on entry. This allows the VM wrapper to work with both
   // direct calls and tail calls.
-  masm.push(lr);
+  masm.pushReturnAddress();
 
   // First argument is the JSContext.
   Register reg_cx = IntArgReg0;

js/src/jit/loong64/SharedICHelpers-loong64-inl.h

@@ -33,12 +33,13 @@ inline void EmitBaselineTailCallVM(TrampolinePtr target, MacroAssembler& masm,
 #endif
 
   // Push frame descriptor and perform the tail call.
-  // ICTailCallReg (ra) already contains the return address (as we
-  // keep it there through the stub calls), but the VMWrapper code being
-  // called expects the return address to also be pushed on the stack.
-  MOZ_ASSERT(ICTailCallReg == ra);
   masm.pushFrameDescriptor(FrameType::BaselineJS);
-  masm.push(ra);
+
+  MOZ_ASSERT(ICTailCallReg == ra);
+  // The return address will be pushed by the VM wrapper, for compatibility
+  // with direct calls. Refer to the top of generateVMWrapper().
+  // ICTailCallReg (ra) already contains the return address (as we keep
+  // it there through the stub calls).
 
   masm.jump(target);
 }

js/src/jit/loong64/Trampoline-loong64.cpp

@@ -242,7 +242,7 @@ void JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm) {
                   Address(StackPointer, sizeof(uintptr_t)));  // BaselineFrame
     masm.storePtr(reg_code, Address(StackPointer, 0));        // jitcode
 
-    using Fn = bool (*)(BaselineFrame * frame, InterpreterFrame * interpFrame,
+    using Fn = bool (*)(BaselineFrame* frame, InterpreterFrame* interpFrame,
                         uint32_t numStackValues);
     masm.setupUnalignedABICall(scratch);
     masm.passABIArg(framePtrScratch);  // BaselineFrame
@@ -344,8 +344,7 @@ void JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail) {
   // Pass pointer to BailoutInfo
   masm.movePtr(StackPointer, a1);
 
-  using Fn =
-      bool (*)(InvalidationBailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(InvalidationBailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -572,7 +571,7 @@ static void GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail) {
   masm.movePtr(StackPointer, a1);
 
   // Call the bailout function.
-  using Fn = bool (*)(BailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(BailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupUnalignedABICall(a2);
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -617,10 +616,11 @@ bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
   Register cxreg = a0;
   regs.take(cxreg);
 
-  // If it isn't a tail call, then the return address needs to be saved
-  if (f.expectTailCall == NonTailCall) {
-    masm.pushReturnAddress();
-  }
+  // On link-register platforms, it is the responsibility of the VM *callee* to
+  // push the return address, while the caller must ensure that the address
+  // is stored in ra on entry. This allows the VM wrapper to work with both
+  // direct calls and tail calls.
+  masm.pushReturnAddress();
 
   // Push the frame pointer to finish the exit frame, then link it up.
   masm.Push(FramePointer);

js/src/jit/mips-shared/SharedICHelpers-mips-shared-inl.h

@@ -33,12 +33,14 @@ inline void EmitBaselineTailCallVM(TrampolinePtr target, MacroAssembler& masm,
 #endif
 
   // Push frame descriptor and perform the tail call.
-  // ICTailCallReg (ra) already contains the return address (as we
-  // keep it there through the stub calls), but the VMWrapper code being
-  // called expects the return address to also be pushed on the stack.
-  MOZ_ASSERT(ICTailCallReg == ra);
   masm.pushFrameDescriptor(FrameType::BaselineJS);
-  masm.push(ra);
+
+  // The return address will be pushed by the VM wrapper, for compatibility
+  // with direct calls. Refer to the top of generateVMWrapper().
+  // ICTailCallReg (lr) already contains the return address (as we keep
+  // it there through the stub calls).
+  MOZ_ASSERT(ICTailCallReg == ra);
+
   masm.jump(target);
 }
 

js/src/jit/mips32/Trampoline-mips32.cpp

@@ -262,7 +262,7 @@ void JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm) {
                   Address(StackPointer, sizeof(uintptr_t)));  // BaselineFrame
     masm.storePtr(reg_code, Address(StackPointer, 0));        // jitcode
 
-    using Fn = bool (*)(BaselineFrame * frame, InterpreterFrame * interpFrame,
+    using Fn = bool (*)(BaselineFrame* frame, InterpreterFrame* interpFrame,
                         uint32_t numStackValues);
     masm.setupUnalignedABICall(scratch);
     masm.passABIArg(FramePointer);  // BaselineFrame
@@ -394,8 +394,8 @@ void JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail) {
   // Pass pointer to BailoutInfo
   masm.movePtr(StackPointer, a2);
 
-  using Fn = bool (*)(InvalidationBailoutStack * sp, size_t * frameSizeOut,
-                      BaselineBailoutInfo * *info);
+  using Fn = bool (*)(InvalidationBailoutStack* sp, size_t* frameSizeOut,
+                      BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -625,7 +625,7 @@ static void GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail) {
   masm.storePtr(ImmPtr(nullptr), Address(StackPointer, 0));
   masm.movePtr(StackPointer, a1);
 
-  using Fn = bool (*)(BailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(BailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -697,10 +697,11 @@ bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
   Register cxreg = a0;
   regs.take(cxreg);
 
-  // If it isn't a tail call, then the return address needs to be saved
-  if (f.expectTailCall == NonTailCall) {
-    masm.pushReturnAddress();
-  }
+  // On link-register platforms, it is the responsibility of the VM *callee* to
+  // push the return address, while the caller must ensure that the address
+  // is stored in ra on entry. This allows the VM wrapper to work with both
+  // direct calls and tail calls.
+  masm.pushReturnAddress();
 
   // We're aligned to an exit frame, so link it up.
   masm.loadJSContext(cxreg);

js/src/jit/mips64/Trampoline-mips64.cpp

@@ -278,7 +278,7 @@ void JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm) {
                   Address(StackPointer, sizeof(uintptr_t)));  // BaselineFrame
     masm.storePtr(reg_code, Address(StackPointer, 0));        // jitcode
 
-    using Fn = bool (*)(BaselineFrame * frame, InterpreterFrame * interpFrame,
+    using Fn = bool (*)(BaselineFrame* frame, InterpreterFrame* interpFrame,
                         uint32_t numStackValues);
     masm.setupUnalignedABICall(scratch);
     masm.passABIArg(framePtrScratch);  // BaselineFrame
@@ -380,8 +380,7 @@ void JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail) {
   // Pass pointer to BailoutInfo
   masm.movePtr(StackPointer, a1);
 
-  using Fn =
-      bool (*)(InvalidationBailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(InvalidationBailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -609,7 +608,7 @@ static void GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail) {
   masm.subPtr(Imm32(sizeOfBailoutInfo), StackPointer);
   masm.movePtr(StackPointer, a1);
 
-  using Fn = bool (*)(BailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(BailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -654,10 +653,11 @@ bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
   Register cxreg = a0;
   regs.take(cxreg);
 
-  // If it isn't a tail call, then the return address needs to be saved
-  if (f.expectTailCall == NonTailCall) {
-    masm.pushReturnAddress();
-  }
+  // On link-register platforms, it is the responsibility of the VM *callee* to
+  // push the return address, while the caller must ensure that the address
+  // is stored in ra on entry. This allows the VM wrapper to work with both
+  // direct calls and tail calls.
+  masm.pushReturnAddress();
 
   // Push the frame pointer to finish the exit frame, then link it up.
   masm.Push(FramePointer);

js/src/jit/riscv64/SharedICHelpers-riscv64-inl.h

@@ -30,12 +30,13 @@ inline void EmitBaselineTailCallVM(TrampolinePtr target, MacroAssembler& masm,
 #endif
 
   // Push frame descriptor and perform the tail call.
-  // ICTailCallReg (ra) already contains the return address (as we
-  // keep it there through the stub calls), but the VMWrapper code being
-  // called expects the return address to also be pushed on the stack.
-  MOZ_ASSERT(ICTailCallReg == ra);
   masm.pushFrameDescriptor(FrameType::BaselineJS);
-  masm.push(ra);
+
+  MOZ_ASSERT(ICTailCallReg == ra);
+  // The return address will be pushed by the VM wrapper, for compatibility
+  // with direct calls. Refer to the top of generateVMWrapper().
+  // ICTailCallReg (ra) already contains the return address (as we keep
+  // it there through the stub calls).
 
   masm.jump(target);
 }

js/src/jit/riscv64/Trampoline-riscv64.cpp

@@ -161,7 +161,7 @@ static void GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail) {
   masm.movePtr(StackPointer, a1);
 
   // Call the bailout function.
-  using Fn = bool (*)(BailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(BailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupUnalignedABICall(a2);
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -311,7 +311,7 @@ void JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm) {
                   Address(StackPointer, sizeof(uintptr_t)));  // BaselineFrame
     masm.storePtr(reg_code, Address(StackPointer, 0));        // jitcode
 
-    using Fn = bool (*)(BaselineFrame * frame, InterpreterFrame * interpFrame,
+    using Fn = bool (*)(BaselineFrame* frame, InterpreterFrame* interpFrame,
                         uint32_t numStackValues);
     masm.setupUnalignedABICall(scratch);
     masm.passABIArg(framePtrScratch);  // BaselineFrame
@@ -410,8 +410,7 @@ void JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail) {
   // Pass pointer to BailoutInfo
   masm.movePtr(StackPointer, a1);
 
-  using Fn =
-      bool (*)(InvalidationBailoutStack * sp, BaselineBailoutInfo * *info);
+  using Fn = bool (*)(InvalidationBailoutStack* sp, BaselineBailoutInfo** info);
   masm.setupAlignedABICall();
   masm.passABIArg(a0);
   masm.passABIArg(a1);
@@ -696,10 +695,11 @@ bool JitRuntime::generateVMWrapper(JSContext* cx, MacroAssembler& masm,
   Register cxreg = a0;
   regs.take(cxreg);
 
-  // If it isn't a tail call, then the return address needs to be saved
-  if (f.expectTailCall == NonTailCall) {
-    masm.pushReturnAddress();
-  }
+  // On link-register platforms, it is the responsibility of the VM *callee* to
+  // push the return address, while the caller must ensure that the address
+  // is stored in ra on entry. This allows the VM wrapper to work with both
+  // direct calls and tail calls.
+  masm.pushReturnAddress();
 
   // Push the frame pointer to finish the exit frame, then link it up.
   masm.Push(FramePointer);