Bug 1498047 - Fix generateInvalidator(). r=nbp
authorSean Stangl <sstangl@mozilla.com>
Wed, 10 Oct 2018 15:41:00 -0400
changeset 490820 f4d8966d0af45c469399f46b93afaf635c09c9b0
parent 490819 926f4d264ac77e5e2d75a0f98259c647eb8b8874
child 490821 7884434cdddeaa4c763168db3524cd785486b10d
push id247
push userfmarier@mozilla.com
push dateSat, 27 Oct 2018 01:06:44 +0000
reviewersnbp
bugs1498047
milestone65.0a1
Bug 1498047 - Fix generateInvalidator(). r=nbp
js/src/jit/arm64/CodeGenerator-arm64.cpp
js/src/jit/arm64/Trampoline-arm64.cpp
--- a/js/src/jit/arm64/CodeGenerator-arm64.cpp
+++ b/js/src/jit/arm64/CodeGenerator-arm64.cpp
@@ -1035,16 +1035,19 @@ CodeGeneratorARM64::generateInvalidateEp
     // Ensure that there is enough space in the buffer for the OsiPoint patching
     // to occur. Otherwise, we could overwrite the invalidation epilogue.
     for (size_t i = 0; i < sizeof(void*); i += Assembler::NopSize()) {
         masm.nop();
     }
 
     masm.bind(&invalidate_);
 
+    // Push the return address of the point that we bailout out onto the stack.
+    masm.push(lr);
+
     // Push the Ion script onto the stack (when we determine what that pointer is).
     invalidateEpilogueData_ = masm.pushWithPatch(ImmWord(uintptr_t(-1)));
 
     TrampolinePtr thunk = gen->jitRuntime()->getInvalidationThunk();
     masm.call(thunk);
 
     // We should never reach this point in JIT code -- the invalidation thunk
     // should pop the invalidated JS frame and return directly to its caller.
--- a/js/src/jit/arm64/Trampoline-arm64.cpp
+++ b/js/src/jit/arm64/Trampoline-arm64.cpp
@@ -15,24 +15,16 @@
 #include "jit/VMFunctions.h"
 
 #include "jit/MacroAssembler-inl.h"
 #include "jit/SharedICHelpers-inl.h"
 
 using namespace js;
 using namespace js::jit;
 
-// All registers to save and restore. This includes the stack pointer, since we
-// use the ability to reference register values on the stack by index.
-// TODO: This is almost certainly incorrect.
-// TODO: All uses need auditing.
-static const LiveRegisterSet AllRegs =
-    LiveRegisterSet(GeneralRegisterSet(Registers::AllMask & ~(1 << 31 | 1 << 30 | 1 << 29| 1 << 28)),
-                    FloatRegisterSet(FloatRegisters::AllMask));
-
 /* This method generates a trampoline on ARM64 for a c++ function with
  * the following signature:
  *   bool blah(void* code, int argc, Value* argv, JSObject* scopeChain, Value* vp)
  *   ...using standard AArch64 calling convention
  */
 void
 JitRuntime::generateEnterJIT(JSContext* cx, MacroAssembler& masm)
 {
@@ -274,24 +266,57 @@ JitRuntime::generateEnterJIT(JSContext* 
 
     // Return using the value popped into x30.
     masm.abiret();
 
     // Reset stack pointer.
     masm.SetStackPointer64(PseudoStackPointer64);
 }
 
+static void
+PushRegisterDump(MacroAssembler& masm)
+{
+    const LiveRegisterSet First28GeneralRegisters =
+        LiveRegisterSet(GeneralRegisterSet(
+                            Registers::AllMask & ~(1 << 31 | 1 << 30 | 1 << 29 | 1 << 28)),
+                        FloatRegisterSet(FloatRegisters::NoneMask));
+
+    const LiveRegisterSet AllFloatRegisters =
+        LiveRegisterSet(GeneralRegisterSet(Registers::NoneMask),
+                        FloatRegisterSet(FloatRegisters::AllMask));
+
+    // Push all general-purpose registers.
+    //
+    // The ARM64 ABI does not treat SP as a normal register that can
+    // be pushed. So pushing happens in two phases.
+    //
+    // Registers are pushed in reverse order of code.
+
+    // First, push the last four registers, passing zero for sp.
+    // Zero is pushed for x28 and x31: the pseudo-SP and SP, respectively.
+    masm.asVIXL().Push(xzr, x30, x29, xzr);
+
+    // Second, push the first 28 registers that serve no special purpose.
+    masm.PushRegsInMask(First28GeneralRegisters);
+
+    // Finally, push all floating-point registers, completing the RegisterDump.
+    masm.PushRegsInMask(AllFloatRegisters);
+}
+
 void
 JitRuntime::generateInvalidator(MacroAssembler& masm, Label* bailoutTail)
 {
     invalidatorOffset_ = startTrampolineCode(masm);
 
-    masm.push(r0, r1, r2, r3);
-
-    masm.PushRegsInMask(AllRegs);
+    // The InvalidationBailoutStack saved in r0 must be:
+    // - osiPointReturnAddress_
+    // - ionScript_  (pushed by CodeGeneratorARM64::generateInvalidateEpilogue())
+    // - regs_  (pushed here)
+    // - fpregs_  (pushed here) [=r0]
+    PushRegisterDump(masm);
     masm.moveStackPtrTo(r0);
 
     masm.Sub(x1, masm.GetStackPointer64(), Operand(sizeof(size_t)));
     masm.Sub(x2, masm.GetStackPointer64(), Operand(sizeof(size_t) + sizeof(void*)));
     masm.moveToStackPtr(r2);
 
     masm.setupUnalignedABICall(r10);
     masm.passABIArg(r0);
@@ -414,48 +439,22 @@ JitRuntime::generateArgumentsRectifier(M
 
     // Pop the return address from earlier and branch.
     masm.ret();
 }
 
 static void
 PushBailoutFrame(MacroAssembler& masm, Register spArg)
 {
-    const LiveRegisterSet First28GeneralRegisters =
-        LiveRegisterSet(GeneralRegisterSet(
-                            Registers::AllMask & ~(1 << 31 | 1 << 30 | 1 << 29 | 1 << 28)),
-                        FloatRegisterSet(FloatRegisters::NoneMask));
-
-    const LiveRegisterSet AllFloatRegisters =
-        LiveRegisterSet(GeneralRegisterSet(Registers::NoneMask),
-                        FloatRegisterSet(FloatRegisters::AllMask));
-
-    // Push all general-purpose registers.
-    //
-    // The bailout frame expects all registers to be present, including SP.
-    // But the ARM64 ABI does not treat SP as a normal register that can
-    // be pushed. So pushing happens in two phases.
-    //
-    // Registers are pushed in reverse order of code.
-
-    // First, push the last four registers, passing zero for sp.
-    // Zero is pushed for x28 and x31: the pseudo-SP and SP, respectively.
-    masm.asVIXL().Push(xzr, x30, x29, xzr);
-
-    // Second, push the first 28 registers that serve no special purpose.
-    masm.PushRegsInMask(First28GeneralRegisters);
-
-    // Finally, push all floating-point registers, completing the BailoutStack.
-    masm.PushRegsInMask(AllFloatRegisters);
-
     // The stack saved in spArg must be (higher entries have higher memory addresses):
     // - snapshotOffset_
     // - frameSize_
     // - regs_
     // - fpregs_ (spArg + 0)
+    PushRegisterDump(masm);
     masm.moveStackPtrTo(spArg);
 }
 
 static void
 GenerateBailoutThunk(MacroAssembler& masm, Label* bailoutTail)
 {
     PushBailoutFrame(masm, r0);