From 49e7425e4f3713a80127fcf2244de51963eb198e Mon Sep 17 00:00:00 2001 From: "Nicolas B. Pierron" Date: Mon, 19 Jan 2015 14:30:13 +0100 Subject: [PATCH] Bug 1112159 part 3 - Align x86/x64 entry frame. r=bbouvier --- js/src/jit/x64/Assembler-x64.h | 7 ++++++- js/src/jit/x64/Trampoline-x64.cpp | 35 +++++++++++++++++++------------ js/src/jit/x86/Assembler-x86.h | 7 ++++++- js/src/jit/x86/Trampoline-x86.cpp | 35 ++++++++++++++++++------------- 4 files changed, 55 insertions(+), 29 deletions(-) diff --git a/js/src/jit/x64/Assembler-x64.h b/js/src/jit/x64/Assembler-x64.h index 2d79080e4105..9f9874d9790c 100644 --- a/js/src/jit/x64/Assembler-x64.h +++ b/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; diff --git a/js/src/jit/x64/Trampoline-x64.cpp b/js/src/jit/x64/Trampoline-x64.cpp index a9e0a143a73a..f0b375baec72 100644 --- a/js/src/jit/x64/Trampoline-x64.cpp +++ b/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); diff --git a/js/src/jit/x86/Assembler-x86.h b/js/src/jit/x86/Assembler-x86.h index a7776750ae14..8aeb92022bd0 100644 --- a/js/src/jit/x86/Assembler-x86.h +++ b/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 diff --git a/js/src/jit/x86/Trampoline-x86.cpp b/js/src/jit/x86/Trampoline-x86.cpp index 46f7135a4e26..598f05e117f7 100644 --- a/js/src/jit/x86/Trampoline-x86.cpp +++ b/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