Bug 1112159 part 3 - Align x86/x64 entry frame. r=bbouvier

js/src/jit/x64/Assembler-x64.h

@@ -179,6 +179,7 @@ static MOZ_CONSTEXPR_VAR Register PreBarrierReg = rdx;
 
 static const uint32_t ABIStackAlignment = 16;
 static const uint32_t CodeAlignment = 16;
+static const uint32_t JitStackAlignment = 16;
 
 // This boolean indicates whether we support SIMD instructions flavoured for
 // this architecture or not. Rather than a method in the LIRGenerator, it is
@@ -188,10 +189,14 @@ static const bool SupportsSimd = true;
 static const uint32_t SimdMemoryAlignment = 16;
 
 static_assert(CodeAlignment % SimdMemoryAlignment == 0,
-  "Code alignment should be larger than any of the alignment which are used for "
+  "Code alignment should be larger than any of the alignments which are used for "
   "the constant sections of the code buffer.  Thus it should be larger than the "
   "alignment for SIMD constants.");
 
+static_assert(JitStackAlignment % SimdMemoryAlignment == 0,
+  "Stack alignment should be larger than any of the alignments which are used for "
+  "spilled values.  Thus it should be larger than the alignment for SIMD accesses.");
+
 static const uint32_t AsmJSStackAlignment = SimdMemoryAlignment;
 
 static const Scale ScalePointer = TimesEight;

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

@@ -23,16 +23,14 @@ static const RegisterSet AllRegs =
   RegisterSet(GeneralRegisterSet(Registers::AllMask),
               FloatRegisterSet(FloatRegisters::AllMask));
 
-/* This method generates a trampoline on x64 for a c++ function with
- * the following signature:
- *   bool blah(void *code, int argc, Value *argv, JSObject *scopeChain,
- *               Value *vp)
- *   ...using standard x64 fastcall calling convention
- */
+// Generates a trampoline for calling Jit compiled code from a C++ function.
+// The trampoline use the EnterJitCode signature, with the standard x64 fastcall
+// calling convention.
 JitCode *
 JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
 {
     MacroAssembler masm(cx);
+    masm.assertStackAlignment(ABIStackAlignment, -int32_t(sizeof(uintptr_t)) /* return address */);
 
     const Register reg_code  = IntArgReg0;
     const Register reg_argc  = IntArgReg1;
@@ -89,16 +87,23 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
 
     // Remember number of bytes occupied by argument vector
     masm.mov(reg_argc, r13);
-    masm.shll(Imm32(3), r13);
+    masm.shll(Imm32(3), r13);   // r13 = argc * sizeof(Value)
+    static_assert(sizeof(Value) == 1 << 3, "Constant is baked in assembly code");
 
-    // Guarantee 16-byte alignment.
-    // We push argc, callee token, frame size, and return address.
-    // The latter two are 16 bytes together, so we only consider argc and the
-    // token.
+    // Guarantee stack alignment of Jit frames.
+    //
+    // This code compensates for the offset created by the copy of the vector of
+    // arguments, such that the jit frame will be aligned once the return
+    // address is pushed on the stack.
+    //
+    // In the computation of the offset, we omit the size of the JitFrameLayout
+    // which is pushed on the stack, as the JitFrameLayout size is a multiple of
+    // the JitStackAlignment.
     masm.mov(rsp, r12);
     masm.subq(r13, r12);
-    masm.subq(Imm32(8), r12);
-    masm.andl(Imm32(0xf), r12);
+    static_assert(sizeof(JitFrameLayout) % JitStackAlignment == 0,
+      "No need to consider the JitFrameLayout for aligning the stack");
+    masm.andl(Imm32(JitStackAlignment - 1), r12);
     masm.subq(r12, rsp);
 
     /***************************************************************
@@ -258,6 +263,10 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
         masm.movq(scopeChain, R1.scratchReg());
     }
 
+    // The call will push the return address on the stack, thus we check that
+    // the stack would be aligned once the call is complete.
+    masm.assertStackAlignment(JitStackAlignment, sizeof(uintptr_t));
+
     // Call function.
     masm.call(reg_code);
 

js/src/jit/x86/Assembler-x86.h

@@ -109,6 +109,7 @@ static const uint32_t ABIStackAlignment = 16;
 static const uint32_t ABIStackAlignment = 4;
 #endif
 static const uint32_t CodeAlignment = 16;
+static const uint32_t JitStackAlignment = 16;
 
 // This boolean indicates whether we support SIMD instructions flavoured for
 // this architecture or not. Rather than a method in the LIRGenerator, it is
@@ -118,10 +119,14 @@ static const bool SupportsSimd = true;
 static const uint32_t SimdMemoryAlignment = 16;
 
 static_assert(CodeAlignment % SimdMemoryAlignment == 0,
-  "Code alignment should be larger than any of the alignment which are used for "
+  "Code alignment should be larger than any of the alignments which are used for "
   "the constant sections of the code buffer.  Thus it should be larger than the "
   "alignment for SIMD constants.");
 
+static_assert(JitStackAlignment % SimdMemoryAlignment == 0,
+  "Stack alignment should be larger than any of the alignments which are used for "
+  "spilled values.  Thus it should be larger than the alignment for SIMD accesses.");
+
 static const uint32_t AsmJSStackAlignment = SimdMemoryAlignment;
 
 struct ImmTag : public Imm32

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

@@ -40,14 +40,15 @@ enum EnterJitEbpArgumentOffset {
     ARG_RESULT          = 9 * sizeof(void *)
 };
 
-/*
- * Generates a trampoline for a C++ function with the EnterJitCode signature,
- * using the standard cdecl calling convention.
- */
+
+// Generates a trampoline for calling Jit compiled code from a C++ function.
+// The trampoline use the EnterJitCode signature, with the standard cdecl
+// calling convention.
 JitCode *
 JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
 {
     MacroAssembler masm(cx);
+    masm.assertStackAlignment(ABIStackAlignment, -int32_t(sizeof(uintptr_t)) /* return address */);
 
     // Save old stack frame pointer, set new stack frame pointer.
     masm.push(ebp);
@@ -68,20 +69,22 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
     masm.loadPtr(Address(ebp, ARG_ARGC), eax);
     masm.shll(Imm32(3), eax);
 
-    // We need to ensure that the stack is aligned on a 12-byte boundary, so
-    // inside the JIT function the stack is 16-byte aligned. Our stack right
-    // now might not be aligned on some platforms (win32, gcc) so we factor
-    // this possibility in, and simulate what the new stack address would be.
-    //   +argc * 8 for arguments
-    //   +4 for pushing alignment
-    //   +4 for pushing the callee token
-    //   +4 for pushing the return address
+    // Guarantee stack alignment of Jit frames.
+    //
+    // This code compensates for the offset created by the copy of the vector of
+    // arguments, such that the jit frame will be aligned once the return
+    // address is pushed on the stack.
+    //
+    // In the computation of the offset, we omit the size of the JitFrameLayout
+    // which is pushed on the stack, as the JitFrameLayout size is a multiple of
+    // the JitStackAlignment.
     masm.movl(esp, ecx);
     masm.subl(eax, ecx);
-    masm.subl(Imm32(4 * 3), ecx);
+    static_assert(sizeof(JitFrameLayout) % JitStackAlignment == 0,
+      "No need to consider the JitFrameLayout for aligning the stack");
 
     // ecx = ecx & 15, holds alignment.
-    masm.andl(Imm32(15), ecx);
+    masm.andl(Imm32(JitStackAlignment - 1), ecx);
     masm.subl(ecx, esp);
 
     /***************************************************************
@@ -249,6 +252,10 @@ JitRuntime::generateEnterJIT(JSContext *cx, EnterJitType type)
         masm.loadPtr(Address(ebp, ARG_SCOPECHAIN), R1.scratchReg());
     }
 
+    // The call will push the return address on the stack, thus we check that
+    // the stack would be aligned once the call is complete.
+    masm.assertStackAlignment(JitStackAlignment, sizeof(uintptr_t));
+
     /***************************************************************
         Call passed-in code, get return value and fill in the
         passed in return value pointer