Skip to content

Commit

Permalink
Bug 1030446 - Store and use exit-fp instead of exit-sp for asm.js sta…
Browse files Browse the repository at this point in the history 
…ck unwinding (r=dougc)

--HG--
extra : rebase_source : 1c72f7049869064c465dc711f0cab8332ef7dc12
  • Loading branch information
Luke Wagner committed Jun 25, 2014
1 parent a43a101 commit 0028103
Show file tree
Hide file tree
Showing 13 changed files with 90 additions and 173 deletions.
98 changes: 51 additions & 47 deletions js/src/jit/AsmJS.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6217,11 +6217,15 @@ GenerateFFIInterpreterExit(ModuleCompiler &m, const ModuleCompiler::ExitDescript
// See AsmJSFrameSize comment in Assembler-*.h.
#if defined(JS_CODEGEN_ARM)
masm.push(lr);
#endif
#if defined(JS_CODEGEN_MIPS)
#elif defined(JS_CODEGEN_MIPS)
masm.push(ra);
#endif

// Store the frame pointer in AsmJSActivation::exitFP for stack unwinding.
Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
LoadAsmJSActivationIntoRegister(masm, activation);
masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfExitFP()));

MIRType typeArray[] = { MIRType_Pointer, // cx
MIRType_Pointer, // exitDatum
MIRType_Int32, // argc
Expand All @@ -6240,16 +6244,11 @@ GenerateFFIInterpreterExit(ModuleCompiler &m, const ModuleCompiler::ExitDescript

// Fill the argument array.
unsigned offsetToCallerStackArgs = AsmJSFrameSize + masm.framePushed();
Register scratch = ABIArgGenerator::NonArgReturnVolatileReg0;
Register scratch = ABIArgGenerator::NonArgReturnVolatileReg1;
FillArgumentArray(m, exit.sig().args(), offsetToArgv, offsetToCallerStackArgs, scratch);

// Prepare the arguments for the call to InvokeFromAsmJS_*.
ABIArgMIRTypeIter i(invokeArgTypes);
Register activation = ABIArgGenerator::NonArgReturnVolatileReg1;
LoadAsmJSActivationIntoRegister(masm, activation);

// Record sp in the AsmJSActivation for stack-walking.
masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfExitSP()));

// argument 0: cx
if (i->kind() == ABIArg::GPR) {
Expand Down Expand Up @@ -6290,16 +6289,16 @@ GenerateFFIInterpreterExit(ModuleCompiler &m, const ModuleCompiler::ExitDescript
AssertStackAlignment(masm);
switch (exit.sig().retType().which()) {
case RetType::Void:
masm.callExit(AsmJSImmPtr(AsmJSImm_InvokeFromAsmJS_Ignore), i.stackBytesConsumedSoFar());
masm.call(AsmJSImmPtr(AsmJSImm_InvokeFromAsmJS_Ignore));
masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel);
break;
case RetType::Signed:
masm.callExit(AsmJSImmPtr(AsmJSImm_InvokeFromAsmJS_ToInt32), i.stackBytesConsumedSoFar());
masm.call(AsmJSImmPtr(AsmJSImm_InvokeFromAsmJS_ToInt32));
masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel);
masm.unboxInt32(argv, ReturnReg);
break;
case RetType::Double:
masm.callExit(AsmJSImmPtr(AsmJSImm_InvokeFromAsmJS_ToNumber), i.stackBytesConsumedSoFar());
masm.call(AsmJSImmPtr(AsmJSImm_InvokeFromAsmJS_ToNumber));
masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel);
masm.loadDouble(argv, ReturnDoubleReg);
break;
Expand All @@ -6311,6 +6310,11 @@ GenerateFFIInterpreterExit(ModuleCompiler &m, const ModuleCompiler::ExitDescript
// Note: the caller is IonMonkey code which means there are no non-volatile
// registers to restore.
masm.freeStack(stackDec);

// Clear exitFP before the frame is destroyed.
LoadAsmJSActivationIntoRegister(masm, activation);
masm.storePtr(ImmWord(0), Address(activation, AsmJSActivation::offsetOfExitFP()));

masm.ret();
}

Expand All @@ -6335,9 +6339,6 @@ GenerateOOLConvert(ModuleCompiler &m, RetType retType, Label *throwLabel)
Register activation = ABIArgGenerator::NonArgReturnVolatileReg1;
LoadAsmJSActivationIntoRegister(masm, activation);

// Record sp in the AsmJSActivation for stack-walking.
masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfExitSP()));

// Store real arguments
ABIArgMIRTypeIter i(callArgTypes);

Expand Down Expand Up @@ -6365,12 +6366,12 @@ GenerateOOLConvert(ModuleCompiler &m, RetType retType, Label *throwLabel)
AssertStackAlignment(masm);
switch (retType.which()) {
case RetType::Signed:
masm.callExit(AsmJSImmPtr(AsmJSImm_CoerceInPlace_ToInt32), i.stackBytesConsumedSoFar());
masm.call(AsmJSImmPtr(AsmJSImm_CoerceInPlace_ToInt32));
masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel);
masm.unboxInt32(Address(StackPointer, offsetToArgv), ReturnReg);
break;
case RetType::Double:
masm.callExit(AsmJSImmPtr(AsmJSImm_CoerceInPlace_ToNumber), i.stackBytesConsumedSoFar());
masm.call(AsmJSImmPtr(AsmJSImm_CoerceInPlace_ToNumber));
masm.branchTest32(Assembler::Zero, ReturnReg, ReturnReg, throwLabel);
masm.loadDouble(Address(StackPointer, offsetToArgv), ReturnDoubleReg);
break;
Expand All @@ -6389,18 +6390,21 @@ GenerateFFIIonExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit
masm.setFramePushed(0);

// See AsmJSFrameSize comment in Assembler-*.h.
#if defined(JS_CODEGEN_X64)
masm.Push(HeapReg);
#elif defined(JS_CODEGEN_ARM)
#if defined(JS_CODEGEN_ARM)
masm.push(lr);

// The GlobalReg (r10) and HeapReg (r11) also need to be restored before
// returning to asm.js code.
// The NANReg also needs to be restored, but is a constant and is reloaded before
// returning to asm.js code.
masm.PushRegsInMask(GeneralRegisterSet((1<<GlobalReg.code()) | (1<<HeapReg.code())));
#elif defined(JS_CODEGEN_MIPS)
masm.push(ra);
#endif

// Store the frame pointer in AsmJSActivation::exitFP for stack unwinding.
Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
LoadAsmJSActivationIntoRegister(masm, activation);
masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfExitFP()));

// Ion does not preserve nonvolatile registers, so we have to preserve them.
#if defined(JS_CODEGEN_X64)
masm.Push(HeapReg);
#elif defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS)
masm.PushRegsInMask(GeneralRegisterSet((1<<GlobalReg.code()) | (1<<HeapReg.code())));
#endif

Expand Down Expand Up @@ -6498,19 +6502,6 @@ GenerateFFIIonExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit
Register reg2 = AsmJSIonExitRegE2;
Register reg3 = AsmJSIonExitRegE3;

LoadAsmJSActivationIntoRegister(masm, reg0);

// Record sp in the AsmJSActivation for stack-walking.
#if defined(JS_CODEGEN_MIPS)
// Add a flag to indicate to AsmJSFrameIterator that we are calling
// into Ion, since the offset from SP to the return address is
// different when calling Ion vs. the native ABI.
masm.ma_or(reg1, StackPointer, Imm32(0x1));
masm.storePtr(reg1, Address(reg0, AsmJSActivation::offsetOfExitSP()));
#else
masm.storePtr(StackPointer, Address(reg0, AsmJSActivation::offsetOfExitSP()));
#endif

// The following is inlined:
// JSContext *cx = activation->cx();
// Activation *act = cx->mainThread().activation();
Expand All @@ -6524,6 +6515,7 @@ GenerateFFIIonExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit
size_t offsetOfJitTop = offsetof(JSRuntime, mainThread) + offsetof(PerThreadData, jitTop);
size_t offsetOfJitJSContext = offsetof(JSRuntime, mainThread) +
offsetof(PerThreadData, jitJSContext);
LoadAsmJSActivationIntoRegister(masm, reg0);
masm.loadPtr(Address(reg0, AsmJSActivation::offsetOfContext()), reg3);
masm.loadPtr(Address(reg3, JSContext::offsetOfRuntime()), reg0);
masm.loadPtr(Address(reg0, offsetOfActivation), reg1);
Expand Down Expand Up @@ -6595,13 +6587,19 @@ GenerateFFIIonExit(ModuleCompiler &m, const ModuleCompiler::ExitDescriptor &exit

masm.bind(&done);
masm.freeStack(stackDec);

// Restore non-volatile registers saved in the prologue.
#if defined(JS_CODEGEN_X64)
masm.Pop(HeapReg);
#endif
#if defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS)
#elif defined(JS_CODEGEN_ARM) || defined(JS_CODEGEN_MIPS)
masm.loadConstantDouble(GenericNaN(), NANReg);
masm.PopRegsInMask(GeneralRegisterSet((1<<GlobalReg.code()) | (1<<HeapReg.code())));
#endif

// Clear exitFP before the frame is destroyed.
LoadAsmJSActivationIntoRegister(masm, activation);
masm.storePtr(ImmWord(0), Address(activation, AsmJSActivation::offsetOfExitFP()));

masm.ret();
JS_ASSERT(masm.framePushed() == 0);

Expand Down Expand Up @@ -6642,18 +6640,20 @@ GenerateStackOverflowExit(ModuleCompiler &m, Label *throwLabel)
masm.align(CodeAlignment);
masm.bind(&m.stackOverflowLabel());

// The stack-overflow is checked before bumping the stack.
masm.setFramePushed(0);

// Store the frame pointer in AsmJSActivation::exitFP for stack unwinding.
Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
LoadAsmJSActivationIntoRegister(masm, activation);
masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfExitFP()));

MIRTypeVector argTypes(m.cx());
argTypes.infallibleAppend(MIRType_Pointer); // cx

unsigned stackDec = StackDecrementForCall(masm, argTypes, MaybeRetAddr);
masm.reserveStack(stackDec);

Register activation = ABIArgGenerator::NonArgReturnVolatileReg0;
LoadAsmJSActivationIntoRegister(masm, activation);

// Record sp in the AsmJSActivation for stack-walking.
masm.storePtr(StackPointer, Address(activation, AsmJSActivation::offsetOfExitSP()));

ABIArgMIRTypeIter i(argTypes);

// argument 0: cx
Expand All @@ -6668,7 +6668,11 @@ GenerateStackOverflowExit(ModuleCompiler &m, Label *throwLabel)
JS_ASSERT(i.done());

AssertStackAlignment(masm);
masm.callExit(AsmJSImmPtr(AsmJSImm_ReportOverRecursed), i.stackBytesConsumedSoFar());
masm.call(AsmJSImmPtr(AsmJSImm_ReportOverRecursed));

// Clear exitFP before the frame is destroyed.
LoadAsmJSActivationIntoRegister(masm, activation);
masm.storePtr(ImmWord(0), Address(activation, AsmJSActivation::offsetOfExitFP()));

// Don't worry about restoring the stack; throwLabel will pop everything.
masm.jump(throwLabel);
Expand Down
70 changes: 16 additions & 54 deletions js/src/jit/AsmJSFrameIterator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,81 +13,43 @@ using namespace js;
using namespace js::jit;

static uint8_t *
ReturnAddressForExitCall(uint8_t **psp)
ReturnAddressFromFP(uint8_t *fp)
{
uint8_t *sp = *psp;
#if defined(JS_CODEGEN_X86) || defined(JS_CODEGEN_X64)
// For calls to Ion/C++ on x86/x64, the exitSP is the SP right before the call
// to C++. Since the call instruction pushes the return address, we know
// that the return address is 1 word below exitSP.
return *(uint8_t**)(sp - sizeof(void*));
#elif defined(JS_CODEGEN_ARM)
// For calls to Ion/C++ on ARM, the *caller* pushes the return address on
// the stack. For Ion, this is just part of the ABI. For C++, the return
// address is explicitly pushed before the call since we cannot expect the
// callee to immediately push lr. This means that exitSP points to the
// return address.
return *(uint8_t**)sp;
#elif defined(JS_CODEGEN_MIPS)
// On MIPS we have two cases: an exit to C++ will store the return address
// at ShadowStackSpace above sp; an exit to Ion will store the return
// address at sp. To distinguish the two cases, the low bit of sp (which is
// aligned and therefore zero) is set for Ion exits.
if (uintptr_t(sp) & 0x1) {
sp = *psp -= 0x1; // Clear the low bit
return *(uint8_t**)sp;
}
return *(uint8_t**)(sp + ShadowStackSpace);
#else
# error "Unknown architecture!"
#endif
}

static uint8_t *
ReturnAddressForJitCall(uint8_t *sp)
{
// Once inside JIT code, sp always points to the word before the return
// address.
return *(uint8_t**)(sp - sizeof(void*));
// In asm.js code, the "frame" consists of a single word: the saved
// return address of the caller.
static_assert(AsmJSFrameSize == sizeof(void*), "Frame size mismatch");
return *(uint8_t**)fp;
}

AsmJSFrameIterator::AsmJSFrameIterator(const AsmJSActivation *activation)
: module_(nullptr)
AsmJSFrameIterator::AsmJSFrameIterator(const AsmJSActivation &activation)
: module_(&activation.module()),
fp_(activation.exitFP())
{
if (!activation || activation->isInterruptedSP())
if (!fp_)
return;

module_ = &activation->module();
sp_ = activation->exitSP();

settle(ReturnAddressForExitCall(&sp_));
settle(ReturnAddressFromFP(fp_));
}

void
AsmJSFrameIterator::operator++()
{
settle(ReturnAddressForJitCall(sp_));
JS_ASSERT(!done());
fp_ += callsite_->stackDepth();
settle(ReturnAddressFromFP(fp_));
}

void
AsmJSFrameIterator::settle(uint8_t *returnAddress)
{
callsite_ = module_->lookupCallSite(returnAddress);
if (!callsite_ || callsite_->isEntry()) {
module_ = nullptr;
return;
}
JS_ASSERT(callsite_);

if (callsite_->isEntry()) {
module_ = nullptr;
fp_ = nullptr;
JS_ASSERT(done());
return;
}

sp_ += callsite_->stackDepth();

if (callsite_->isExit())
return settle(ReturnAddressForJitCall(sp_));

JS_ASSERT(callsite_->isNormal());
}

Expand Down
7 changes: 4 additions & 3 deletions js/src/jit/AsmJSFrameIterator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ class AsmJSFrameIterator
{
const AsmJSModule *module_;
const jit::CallSite *callsite_;
uint8_t *sp_;
uint8_t *fp_;

void settle(uint8_t *returnAddress);

public:
explicit AsmJSFrameIterator(const AsmJSActivation *activation);
explicit AsmJSFrameIterator() : module_(nullptr) {}
explicit AsmJSFrameIterator(const AsmJSActivation &activation);
void operator++();
bool done() const { return !module_; }
bool done() const { return !fp_; }
JSAtom *functionDisplayAtom() const;
unsigned computeLine(uint32_t *column) const;
};
Expand Down
8 changes: 4 additions & 4 deletions js/src/jit/AsmJSSignalHandlers.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ HandleSimulatorInterrupt(JSRuntime *rt, AsmJSActivation *activation, void *fault
if (module.containsPC((void *)rt->mainThread.simulator()->get_pc()) &&
module.containsPC(faultingAddress))
{
activation->setInterrupted(nullptr);
activation->setResumePC(nullptr);
int32_t nextpc = int32_t(module.interruptExit());
rt->mainThread.simulator()->set_resume_pc(nextpc);
return true;
Expand Down Expand Up @@ -465,7 +465,7 @@ HandleException(PEXCEPTION_POINTERS exception)
// The trampoline will jump to activation->resumePC if execution isn't
// interrupted.
if (module.containsPC(faultingAddress)) {
activation->setInterrupted(pc);
activation->setResumePC(pc);
*ppc = module.interruptExit();

JSRuntime::AutoLockForInterrupt lock(rt);
Expand Down Expand Up @@ -668,7 +668,7 @@ HandleMachException(JSRuntime *rt, const ExceptionRequest &request)
// The trampoline will jump to activation->resumePC if execution isn't
// interrupted.
if (module.containsPC(faultingAddress)) {
activation->setInterrupted(pc);
activation->setResumePC(pc);
*ppc = module.interruptExit();

JSRuntime::AutoLockForInterrupt lock(rt);
Expand Down Expand Up @@ -918,7 +918,7 @@ HandleSignal(int signum, siginfo_t *info, void *ctx)
// The trampoline will jump to activation->resumePC if execution isn't
// interrupted.
if (module.containsPC(faultingAddress)) {
activation->setInterrupted(pc);
activation->setResumePC(pc);
*ppc = module.interruptExit();

JSRuntime::AutoLockForInterrupt lock(rt);
Expand Down
13 changes: 0 additions & 13 deletions js/src/jit/arm/MacroAssembler-arm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3626,19 +3626,6 @@ MacroAssemblerARM::ma_call(ImmPtr dest)
as_blx(CallReg);
}

void
MacroAssemblerARM::ma_callAndStoreRet(const Register r, uint32_t stackArgBytes)
{
// Note: this function stores the return address to sp[0]. The caller must
// anticipate this by pushing additional space on the stack. The ABI does
// not provide space for a return address so this function may only be
// called if no argument are passed.
JS_ASSERT(stackArgBytes == 0);
AutoForbidPools afp(this);
as_dtr(IsStore, 32, Offset, pc, DTRAddr(sp, DtrOffImm(0)));
as_blx(r);
}

void
MacroAssemblerARMCompat::breakpoint()
{
Expand Down
Loading

0 comments on commit 0028103

Please sign in to comment.