R3 is used as the third argument register. R3 is also used as the addressTempRegister. JIT::callOperation() first setups up the arguments and then appends the call: ALWAYS_INLINE MacroAssembler::Call JIT::callOperation(C_JITOperation_E operation) { setupArgumentsExecState(); return appendCallWithExceptionCheck(operation); } appendCall calls updateTopCallFrame: ALWAYS_INLINE MacroAssembler::Call JIT::appendCallWithExceptionCheck(const FunctionPtr& function) { updateTopCallFrame(); MacroAssembler::Call call = appendCall(function); exceptionCheck(); return call; } updateTopCallFrame then does a storePtr which uses addressTempRegister and clobbers R3, corrupting the third argument. ALWAYS_INLINE void JIT::updateTopCallFrame() { ASSERT(static_cast<int>(m_bytecodeOffset) >= 0); #if USE(JSVALUE32_64) Instruction* instruction = m_codeBlock->instructions().begin() + m_bytecodeOffset + 1; uint32_t locationBits = CallFrame::Location::encodeAsBytecodeInstruction(instruction); #else uint32_t locationBits = CallFrame::Location::encodeAsBytecodeOffset(m_bytecodeOffset + 1); #endif store32(TrustedImm32(locationBits), intTagFor(JSStack::ArgumentCount)); storePtr(callFrameRegister, &m_vm->topCallFrame); } There is a comment describing this potential bug (now real) in GPRInfo.h: // FIXME: r3 is currently used be the MacroAssembler as a temporary - it seems // This could threoretically be a problem if this is used in code generation // between the arguments being set up, and the call being made. That said, // any change introducing a problem here is likely to be immediately apparent! static const GPRReg argumentGPR3 = ARMRegisters::r3; // FIXME! One potential fix would be to use something like the claimScratch() technique used in SH4Assembler.h or update topCallFrame before setting up the argument registers.
Hi, I have a fix for this, I'm just waiting that https://bugs.webkit.org/show_bug.cgi?id=123247 lands before
Created attachment 215380 [details] Fix 4th argument register trampling for ARM architecture. This patch solves a lot of crashes for ARM_TRADITIONAL. Could you test it for ARMv7 and give me your feedback please ?
Created attachment 215381 [details] Fix 4th argument register trampling for ARM architecture (with ChangeLog) Better with the ChangeLog
Comment on attachment 215381 [details] Fix 4th argument register trampling for ARM architecture (with ChangeLog) I've seen a regression in the RegExpJIT for ARM, and after reading the code, it must be fixed too. I'll submit a new patch soon
Created attachment 215403 [details] Fix 4th argument register trampling for ARM architecture (with RegExpJIT fix)
Comment on attachment 215403 [details] Fix 4th argument register trampling for ARM architecture (with RegExpJIT fix) r=me. Thanks for cleaning this up.
Comment on attachment 215403 [details] Fix 4th argument register trampling for ARM architecture (with RegExpJIT fix) Clearing flags on attachment: 215403 Committed r158208: <http://trac.webkit.org/changeset/158208>
All reviewed patches have been landed. Closing bug.