From 7d6baa84d770282bd96dcbc99eecae446f8a2aed Mon Sep 17 00:00:00 2001 From: Bill McCloskey Date: Fri, 27 Jan 2012 17:01:25 -0800 Subject: [PATCH] Bug 708303 - Use pinReg/unpinReg more in write barriers (r=dmandelin) --- js/src/jsanalyze.h | 4 ++++ js/src/methodjit/Compiler.cpp | 8 +++++++- js/src/methodjit/FastOps.cpp | 21 +++++++++++++++------ js/src/methodjit/FrameState-inl.h | 3 +-- 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/js/src/jsanalyze.h b/js/src/jsanalyze.h index 8eadd0a4e14..d85a01f737a 100644 --- a/js/src/jsanalyze.h +++ b/js/src/jsanalyze.h @@ -1154,6 +1154,10 @@ class ScriptAnalysis /* Decompose the slot above. */ bool arg; uint32_t index; + + const Value **basePointer() const { + return arg ? &nesting->argArray : &nesting->varArray; + } }; NameAccess resolveNameAccess(JSContext *cx, jsid id, bool addDependency = false); diff --git a/js/src/methodjit/Compiler.cpp b/js/src/methodjit/Compiler.cpp index 8ebf5bf0390..b95a86c8ccf 100644 --- a/js/src/methodjit/Compiler.cpp +++ b/js/src/methodjit/Compiler.cpp @@ -5552,7 +5552,13 @@ mjit::Compiler::jsop_setprop(PropertyName *name, bool popGuaranteed) if (cx->compartment->needsBarrier()) { stubcc.linkExit(masm.jump(), Uses(0)); stubcc.leave(); - stubcc.masm.addPtr(Imm32(address.offset), address.base, Registers::ArgReg1); + + /* sync() may have overwritten nameReg, so we reload its data. */ + JS_ASSERT(address.base == nameReg); + stubcc.masm.move(ImmPtr(access.basePointer()), nameReg); + stubcc.masm.loadPtr(Address(nameReg), nameReg); + stubcc.masm.addPtr(Imm32(address.offset), nameReg, Registers::ArgReg1); + OOL_STUBCALL(stubs::WriteBarrier, REJOIN_NONE); stubcc.rejoin(Changes(0)); } diff --git a/js/src/methodjit/FastOps.cpp b/js/src/methodjit/FastOps.cpp index 373dbb2f8dc..5bf65a7185f 100644 --- a/js/src/methodjit/FastOps.cpp +++ b/js/src/methodjit/FastOps.cpp @@ -1188,17 +1188,26 @@ mjit::Compiler::jsop_setelem_dense() /* * The sync call below can potentially clobber key.reg() and slotsReg. - * So we save and restore them. Additionally, the WriteBarrier stub can - * clobber both registers. The rejoin call will restore key.reg() but - * not slotsReg. So we restore it again after the stub call. + * We pin key.reg() to avoid it being clobbered. If |hoisted| is true, + * we can also pin slotsReg. If not, then slotsReg is owned by the + * compiler and we save in manually to VMFrame::scratch. + * + * Additionally, the WriteBarrier stub can clobber both registers. The + * rejoin call will restore key.reg() but not slotsReg. So we save + * slotsReg in the frame and restore it after the stub call. */ stubcc.masm.storePtr(slotsReg, FrameAddress(offsetof(VMFrame, scratch))); + if (hoisted) + frame.pinReg(slotsReg); if (!key.isConstant()) - stubcc.masm.push(key.reg()); + frame.pinReg(key.reg()); frame.sync(stubcc.masm, Uses(3)); if (!key.isConstant()) - stubcc.masm.pop(key.reg()); - stubcc.masm.loadPtr(FrameAddress(offsetof(VMFrame, scratch)), slotsReg); + frame.unpinReg(key.reg()); + if (hoisted) + frame.unpinReg(slotsReg); + else + stubcc.masm.loadPtr(FrameAddress(offsetof(VMFrame, scratch)), slotsReg); if (key.isConstant()) stubcc.masm.lea(Address(slotsReg, key.index() * sizeof(Value)), Registers::ArgReg1); diff --git a/js/src/methodjit/FrameState-inl.h b/js/src/methodjit/FrameState-inl.h index 6bfb1270acf..196fbbc38a8 100644 --- a/js/src/methodjit/FrameState-inl.h +++ b/js/src/methodjit/FrameState-inl.h @@ -919,8 +919,7 @@ FrameState::loadNameAddress(const analyze::ScriptAnalysis::NameAccess &access, R { JS_ASSERT(access.script && access.nesting); - const Value **pbase = access.arg ? &access.nesting->argArray : &access.nesting->varArray; - masm.move(ImmPtr(pbase), reg); + masm.move(ImmPtr(access.basePointer()), reg); masm.loadPtr(Address(reg), reg); return Address(reg, access.index * sizeof(Value));