From b63a29a3084581614cafd0bab1b08723615af4a4 Mon Sep 17 00:00:00 2001 From: Jacob Bramley Date: Fri, 20 Aug 2010 13:21:46 -0700 Subject: [PATCH] [JAEGER] Bug 587833 reland part 2: remove VMFrame::scriptedReturn ARM fixes --- js/src/methodjit/BaseAssembler.h | 20 +++---- js/src/methodjit/Compiler.cpp | 14 +---- js/src/methodjit/MethodJIT.cpp | 71 ++++++++++++++----------- js/src/methodjit/TrampolineCompiler.cpp | 6 +-- 4 files changed, 53 insertions(+), 58 deletions(-) diff --git a/js/src/methodjit/BaseAssembler.h b/js/src/methodjit/BaseAssembler.h index 5970ee950c65..4811f0e37c09 100644 --- a/js/src/methodjit/BaseAssembler.h +++ b/js/src/methodjit/BaseAssembler.h @@ -313,17 +313,17 @@ static const JSC::MacroAssembler::RegisterID JSReturnReg_Data = JSC::ARMRegister return MacroAssembler::call(reg); } -#if defined(JS_CPU_ARM) - void ret() { - /* The return sequence emitted by the ARM macro-assembler assumes that the return address - * is in LR. We could load it into LR before calling it, but it's probably better to simply - * pop into the PC. (Note that JaegerTrampoline expects the return sequence to pop this - * single word from the stack, so the stack will be unaligned on return from JIT code. - * JaegerTrampoline fixes this up.) */ - MacroAssembler::pop(JSC::ARMRegisters::pc); - } - // #else fall back to the inherited implementation in MacroAssembler::ret(). + void restoreReturnAddress() + { +#ifndef JS_CPU_ARM + /* X86 and X64's "ret" instruction expects a return address on the stack. */ + push(Address(JSFrameReg, offsetof(JSStackFrame, ncode))); +#else + /* ARM returns either using its link register (LR) or directly from the stack, but masm.ret() + * always emits a return to LR. */ + load32(Address(JSFrameReg, offsetof(JSStackFrame, ncode)), JSC::ARMRegisters::lr); #endif + } void finalize(uint8 *ncode) { JSC::JITCode jc(ncode, size()); diff --git a/js/src/methodjit/Compiler.cpp b/js/src/methodjit/Compiler.cpp index 8de7f9ac4485..c8fb9dd3d894 100644 --- a/js/src/methodjit/Compiler.cpp +++ b/js/src/methodjit/Compiler.cpp @@ -1561,16 +1561,6 @@ mjit::Compiler::jsop_getglobal(uint32 index) frame.push(address); } -void -mjit::Compiler::restoreReturnAddress(Assembler &masm) -{ -#ifndef JS_CPU_ARM - masm.push(Address(JSFrameReg, offsetof(JSStackFrame, ncode))); -#else - masm.move(Address(JSFrameReg, offsetof(JSStackFrame, ncode)), JSC::ARMRegisters::lr); -#endif -} - void mjit::Compiler::emitReturn() { @@ -1582,7 +1572,7 @@ mjit::Compiler::emitReturn() FrameAddress(offsetof(VMFrame, entryFp)), JSFrameReg); stubcc.linkExit(noInlineCalls, Uses(frame.frameDepth())); - restoreReturnAddress(stubcc.masm); + stubcc.masm.restoreReturnAddress(); stubcc.masm.ret(); JS_ASSERT_IF(!fun, JSOp(*PC) == JSOP_STOP); @@ -1643,7 +1633,7 @@ mjit::Compiler::emitReturn() Address rval(JSFrameReg, JSStackFrame::offsetReturnValue()); masm.loadPayload(rval, JSReturnReg_Data); masm.loadTypeTag(rval, JSReturnReg_Type); - restoreReturnAddress(masm); + masm.restoreReturnAddress(); masm.move(Registers::ReturnReg, JSFrameReg); #ifdef DEBUG masm.storePtr(ImmPtr(JSStackFrame::sInvalidPC), diff --git a/js/src/methodjit/MethodJIT.cpp b/js/src/methodjit/MethodJIT.cpp index 4763bd8869ad..73b46133dd53 100644 --- a/js/src/methodjit/MethodJIT.cpp +++ b/js/src/methodjit/MethodJIT.cpp @@ -56,7 +56,7 @@ using namespace js::mjit; * creates a VMFrame on the machine stack and calls into JIT'd code. The JIT'd * code will eventually return to the VMFrame. * - * - Called from C++ functions JaegerShot and JaegerBomb. + * - Called from C++ function EnterMethodJIT. * - Parameters: cx, fp, code, stackLimit, safePoint * - Notes: safePoint is used in combination with SafePointTrampoline, * explained further down. @@ -73,10 +73,9 @@ using namespace js::mjit; * - js_InternalThrow may return 0, which means the place to continue, if any, * is above this JaegerShot activation, so we just return, in the same way * the trampoline does. - * - Otherwise, js_InternalThrow returns a jit-code address to continue - * execution - * at. Because the jit-code ABI conditions are satisfied, we can just jump - * to that point. + * - Otherwise, js_InternalThrow returns a jit-code address to continue execution + * at. Because the jit-code ABI conditions are satisfied, we can just jump to + * that point. * * * SafePointTrampoline - Inline script calls link their return addresses through @@ -85,9 +84,9 @@ using namespace js::mjit; * enter a method JIT'd function at an arbitrary safe point. Safe points * do not have the return address linking code that the method prologue has. * SafePointTrampoline is a thunk which correctly links the initial return - * address. It is used in JaegerBomb, and passed as the "script code" - * parameter. Using the "safePoint" parameter to JaegerTrampoline, it correctly - * jumps to the intended point in the method. + * address. It is used in JaegerShotAtSafePoint, and passed as the "script + * code" parameter. Using the "safePoint" parameter to JaegerTrampoline, it + * correctly jumps to the intended point in the method. * * - Used by JaegerTrampoline() * @@ -216,8 +215,7 @@ SYMBOL_STRING(JaegerTrampoline) ":" "\n" "call " SYMBOL_STRING_RELOC(PushActiveVMFrame) "\n" /* - * Jump into into the JIT'd code. The call implicitly fills in - * the precious f.scriptedReturn member of VMFrame. + * Jump into into the JIT'd code. */ "call *0(%rsp)" "\n" "movq %rsp, %rdi" "\n" @@ -413,9 +411,7 @@ JS_STATIC_ASSERT(offsetof(VMFrame, cx) == (4*8)); JS_STATIC_ASSERT(offsetof(VMFrame, fp) == (4*7)); JS_STATIC_ASSERT(offsetof(VMFrame, oldRegs) == (4*4)); JS_STATIC_ASSERT(offsetof(VMFrame, previous) == (4*3)); -JS_STATIC_ASSERT(offsetof(VMFrame, scriptedReturn) == (4*0)); JS_STATIC_ASSERT(offsetof(JSStackFrame, ncode) == 60); -JS_STATIC_ASSERT(offsetof(JSStackFrame, rval) == 40); asm volatile ( ".text\n" @@ -433,9 +429,17 @@ asm volatile ( ".text\n" ".globl " SYMBOL_STRING(SafePointTrampoline) "\n" SYMBOL_STRING(SafePointTrampoline) ":" - "str lr, [r11, #60]" "\n" - /* This should load the fifth parameter from JaegerTrampoline and jump to it. */ - "" "\n" + /* + * On entry to SafePointTrampoline: + * r11 = fp + * sp[80] = safePoint + */ + "ldr ip, [sp, #80]" "\n" + /* Save the return address (in JaegerTrampoline) to fp->ncode. */ + "str lr, [r11, #60]" "\n" + /* Jump to 'safePoint' via 'ip' because a load into the PC from an address on + * the stack looks like a return, and may upset return stack prediction. */ + "bx ip" "\n" ); asm volatile ( @@ -444,10 +448,11 @@ asm volatile ( SYMBOL_STRING(JaegerTrampoline) ":" "\n" /* * On entry to JaegerTrampoline: - * r0 = cx - * r1 = fp - * r2 = code - * r3 = inlineCallCount + * r0 = cx + * r1 = fp + * r2 = code + * r3 = stackLimit + * sp[0] = safePoint * * The VMFrame for ARM looks like this: * [ lr ] \ @@ -467,14 +472,12 @@ SYMBOL_STRING(JaegerTrampoline) ":" "\n" * [ regs.pc ] * [ oldRegs ] * [ previous ] - * [ args.ptr ] + * [ args.ptr3 ] * [ args.ptr2 ] - * [ srpt. ret ] } Scripted return. + * [ args.ptr ] */ - /* Push callee-saved registers. TODO: Do we actually need to push all of them? If the - * compiled JavaScript function is EABI-compliant, we only need to push what we use in - * JaegerTrampoline. */ + /* Push callee-saved registers. */ " push {r4-r11,lr}" "\n" /* Push interesting VMFrame content. */ " push {r1}" "\n" /* entryFp */ @@ -484,8 +487,12 @@ SYMBOL_STRING(JaegerTrampoline) ":" "\n" /* Remaining fields are set elsewhere, but we need to leave space for them. */ " sub sp, sp, #(4*7)" "\n" + /* Preserve 'code' (r2) in an arbitrary callee-saved register. */ +" mov r4, r2" "\n" + /* Preserve 'fp' (r1) in r11 (JSFrameReg) for SafePointTrampoline. */ +" mov r11, r1" "\n" + " mov r0, sp" "\n" -" mov r4, r2" "\n" /* Preserve r2 ('code') in a callee-saved register. */ " bl " SYMBOL_STRING_RELOC(SetVMFrameRegs) "\n" " mov r0, sp" "\n" " bl " SYMBOL_STRING_RELOC(PushActiveVMFrame)"\n" @@ -511,20 +518,22 @@ asm volatile ( ".text\n" ".globl " SYMBOL_STRING(JaegerThrowpoline) "\n" SYMBOL_STRING(JaegerThrowpoline) ":" "\n" - /* Restore 'f', as it will have been clobbered. */ + /* Find the VMFrame pointer for js_InternalThrow. */ " mov r0, sp" "\n" /* Call the utility function that sets up the internal throw routine. */ " bl " SYMBOL_STRING_RELOC(js_InternalThrow) "\n" - /* If 0 was returned, just bail out as normal. Otherwise, we have a 'catch' or 'finally' clause - * to execute. */ + /* If js_InternalThrow found a scripted handler, jump to it. Otherwise, tidy + * up and return. */ " cmp r0, #0" "\n" " bxne r0" "\n" - /* Skip past the parameters we pushed (such as cx and the like). */ -" add sp, sp, #(4*7 + 4*4)" "\n" - + /* Tidy up, then return '0' to represent an unhandled exception. */ +" mov r0, sp" "\n" +" bl " SYMBOL_STRING_RELOC(PopActiveVMFrame) "\n" +" add sp, sp, #(4*7 + 4*4)" "\n" +" mov r0, #0" "\n" " pop {r4-r11,pc}" "\n" ); diff --git a/js/src/methodjit/TrampolineCompiler.cpp b/js/src/methodjit/TrampolineCompiler.cpp index 55c4c8f79477..89e084211718 100644 --- a/js/src/methodjit/TrampolineCompiler.cpp +++ b/js/src/methodjit/TrampolineCompiler.cpp @@ -139,11 +139,7 @@ TrampolineCompiler::generateForceReturn(Assembler &masm) masm.loadPayload(rval, JSReturnReg_Data); masm.loadTypeTag(rval, JSReturnReg_Type); -#ifndef JS_CPU_ARM - masm.push(Address(JSFrameReg, offsetof(JSStackFrame, ncode))); -#else - masm.move(Address(JSFrameReg, offsetof(JSStackFrame, ncode)), JSC::ARMRegisters::lr); -#endif + masm.restoreReturnAddress(); masm.move(Registers::ReturnReg, JSFrameReg); #ifdef DEBUG