Fix buggy interactions between IC patching and invalidation (bug 793165 part 2, r=jandem).

This commit is contained in:
David Anderson 2012-10-02 13:43:47 -07:00
parent 50cb4ce866
commit 2c47e5f574
8 changed files with 125 additions and 37 deletions

View File

@ -3141,6 +3141,13 @@ CodeGenerator::visitCache(LInstruction *ins)
if (!addOutOfLineCode(ool))
return false;
#if 0
// NOTE: This is currently disabled. OSI and IC interaction are protected
// through other means in ICs, since the nops incur significant overhead.
//
// ensureOsiSpace();
#endif
CodeOffsetJump jump = masm.jumpWithPatch(ool->repatchEntry());
CodeOffsetLabel label = masm.labelForPatch();
masm.bind(ool->rejoin());
@ -3945,6 +3952,10 @@ CodeGenerator::emitInstanceOf(LInstruction *ins, Register rhs)
if (!addOutOfLineCode(ool))
return false;
// If the IC code wants to patch, make sure there is enough space to that
// the patching does not overwrite an invalidation marker.
ensureOsiSpace();
CodeOffsetJump jump = masm.jumpWithPatch(ool->repatchEntry());
CodeOffsetLabel label = masm.labelForPatch();
masm.bind(ool->rejoin());

View File

@ -726,6 +726,14 @@ IonScript::toggleBarriers(bool enabled)
void
IonScript::purgeCaches(JSCompartment *c)
{
// Don't reset any ICs if we're invalidated, otherwise, repointing the
// inline jump could overwrite an invalidation marker. These ICs can
// no longer run, however, the IC slow paths may be active on the stack.
// ICs therefore are required to check for invalidation before patching,
// to ensure the same invariant.
if (invalidated())
return;
// This is necessary because AutoFlushCache::updateTop()
// looks up the current flusher in the IonContext. Without one
// it cannot work.
@ -1481,6 +1489,13 @@ InvalidateActivation(FreeOp *fop, uint8 *ionTop, bool invalidateAll)
if (!invalidateAll && !script->ion->invalidated())
continue;
IonScript *ionScript = script->ion;
// Purge ICs before we mark this script as invalidated. This will
// prevent lastJump_ from appearing to be a bogus pointer, just
// in case anyone tries to read it.
ionScript->purgeCaches(script->compartment());
// This frame needs to be invalidated. We do the following:
//
// 1. Increment the reference counter to keep the ionScript alive
@ -1502,7 +1517,6 @@ InvalidateActivation(FreeOp *fop, uint8 *ionTop, bool invalidateAll)
// instructions after the call) in to capture an appropriate
// snapshot after the call occurs.
IonScript *ionScript = script->ion;
ionScript->incref();
const SafepointIndex *si = ionScript->getSafepointIndex(it.returnAddressToFp());

View File

@ -199,7 +199,8 @@ struct GetNativePropertyStub
};
bool
IonCacheGetProperty::attachNative(JSContext *cx, JSObject *obj, JSObject *holder, const Shape *shape)
IonCacheGetProperty::attachNative(JSContext *cx, IonScript *ion, JSObject *obj, JSObject *holder,
const Shape *shape)
{
MacroAssembler masm;
RepatchLabel failures;
@ -215,6 +216,9 @@ IonCacheGetProperty::attachNative(JSContext *cx, JSObject *obj, JSObject *holder
getprop.rejoinOffset.fixup(&masm);
getprop.exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, getprop.rejoinOffset);
CodeLocationJump exitJump(code, getprop.exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -256,7 +260,8 @@ IsCacheableGetProp(JSObject *obj, JSObject *holder, const Shape *shape)
}
static bool
TryAttachNativeStub(JSContext *cx, IonCacheGetProperty &cache, HandleObject obj,
TryAttachNativeStub(JSContext *cx, IonScript *ion,
IonCacheGetProperty &cache, HandleObject obj,
HandlePropertyName name, bool *isCacheableNative)
{
JS_ASSERT(!*isCacheableNative);
@ -295,7 +300,7 @@ TryAttachNativeStub(JSContext *cx, IonCacheGetProperty &cache, HandleObject obj,
if (cache.stubCount() < MAX_STUBS) {
cache.incrementStubCount();
if (!cache.attachNative(cx, obj, holder, shape))
if (!cache.attachNative(cx, ion, obj, holder, shape))
return false;
}
@ -327,7 +332,7 @@ js::ion::GetPropertyCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Mu
// limit. Once we can make calls from within generated stubs, a new call
// stub will be generated instead and the previous stubs unlinked.
bool isCacheableNative = false;
if (!TryAttachNativeStub(cx, cache, obj, name, &isCacheableNative))
if (!TryAttachNativeStub(cx, ion, cache, obj, name, &isCacheableNative))
return false;
if (cache.idempotent() && !isCacheableNative) {
@ -395,7 +400,7 @@ IonCache::reset()
}
bool
IonCacheSetProperty::attachNativeExisting(JSContext *cx, JSObject *obj, const Shape *shape)
IonCacheSetProperty::attachNativeExisting(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *shape)
{
MacroAssembler masm;
@ -438,6 +443,9 @@ IonCacheSetProperty::attachNativeExisting(JSContext *cx, JSObject *obj, const Sh
rejoinOffset.fixup(&masm);
exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, rejoinOffset);
CodeLocationJump exitJump(code, exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -452,7 +460,8 @@ IonCacheSetProperty::attachNativeExisting(JSContext *cx, JSObject *obj, const Sh
}
bool
IonCacheSetProperty::attachNativeAdding(JSContext *cx, JSObject *obj, const Shape *oldShape, const Shape *newShape,
IonCacheSetProperty::attachNativeAdding(JSContext *cx, IonScript *ion, JSObject *obj,
const Shape *oldShape, const Shape *newShape,
const Shape *propShape)
{
MacroAssembler masm;
@ -525,6 +534,9 @@ IonCacheSetProperty::attachNativeAdding(JSContext *cx, JSObject *obj, const Shap
rejoinOffset.fixup(&masm);
exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, rejoinOffset);
CodeLocationJump exitJump(code, exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -639,7 +651,7 @@ js::ion::SetPropertyCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Ha
bool inlinable = IsPropertyInlineable(obj, cache);
if (inlinable && IsPropertySetInlineable(cx, obj, name, &id, &shape)) {
cache.incrementStubCount();
if (!cache.attachNativeExisting(cx, obj, shape))
if (!cache.attachNativeExisting(cx, ion, obj, shape))
return false;
}
@ -655,7 +667,7 @@ js::ion::SetPropertyCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Ha
if (inlinable && IsPropertyAddInlineable(cx, obj, id, oldSlots, &shape)) {
const Shape *newShape = obj->lastProperty();
cache.incrementStubCount();
if (!cache.attachNativeAdding(cx, obj, oldShape, newShape, shape))
if (!cache.attachNativeAdding(cx, ion, obj, oldShape, newShape, shape))
return false;
}
@ -663,7 +675,8 @@ js::ion::SetPropertyCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Ha
}
bool
IonCacheGetElement::attachGetProp(JSContext *cx, HandleObject obj, const Value &idval, PropertyName *name)
IonCacheGetElement::attachGetProp(JSContext *cx, IonScript *ion, HandleObject obj,
const Value &idval, PropertyName *name)
{
RootedObject holder(cx);
RootedShape shape(cx);
@ -696,6 +709,9 @@ IonCacheGetElement::attachGetProp(JSContext *cx, HandleObject obj, const Value &
getprop.rejoinOffset.fixup(&masm);
getprop.exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, getprop.rejoinOffset);
CodeLocationJump exitJump(code, getprop.exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -721,7 +737,7 @@ GetDenseArrayShape(JSContext *cx, JSObject *globalObj)
}
bool
IonCacheGetElement::attachDenseArray(JSContext *cx, JSObject *obj, const Value &idval)
IonCacheGetElement::attachDenseArray(JSContext *cx, IonScript *ion, JSObject *obj, const Value &idval)
{
JS_ASSERT(obj->isDenseArray());
JS_ASSERT(idval.isInt32());
@ -782,6 +798,9 @@ IonCacheGetElement::attachDenseArray(JSContext *cx, JSObject *obj, const Value &
rejoinOffset.fixup(&masm);
exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, rejoinOffset);
CodeLocationJump exitJump(code, exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -819,14 +838,14 @@ js::ion::GetElementCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Han
uint32_t dummy;
if (idval.isString() && JSID_IS_ATOM(id) && !JSID_TO_ATOM(id)->isIndex(&dummy)) {
if (!cache.attachGetProp(cx, obj, idval, JSID_TO_ATOM(id)->asPropertyName()))
if (!cache.attachGetProp(cx, ion, obj, idval, JSID_TO_ATOM(id)->asPropertyName()))
return false;
}
} else if (!cache.hasDenseArrayStub() && obj->isDenseArray() && idval.isInt32()) {
// Generate at most one dense array stub.
cache.incrementStubCount();
if (!cache.attachDenseArray(cx, obj, idval))
if (!cache.attachDenseArray(cx, ion, obj, idval))
return false;
}
}
@ -844,7 +863,7 @@ js::ion::GetElementCache(JSContext *cx, size_t cacheIndex, HandleObject obj, Han
}
bool
IonCacheBindName::attachGlobal(JSContext *cx, JSObject *scopeChain)
IonCacheBindName::attachGlobal(JSContext *cx, IonScript *ion, JSObject *scopeChain)
{
JS_ASSERT(scopeChain->isGlobal());
@ -869,6 +888,9 @@ IonCacheBindName::attachGlobal(JSContext *cx, JSObject *scopeChain)
rejoinOffset.fixup(&masm);
exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, rejoinOffset);
CodeLocationJump exitJump(code, exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -930,7 +952,7 @@ GenerateScopeChainGuards(MacroAssembler &masm, JSObject *scopeChain, JSObject *h
}
bool
IonCacheBindName::attachNonGlobal(JSContext *cx, JSObject *scopeChain, JSObject *holder)
IonCacheBindName::attachNonGlobal(JSContext *cx, IonScript *ion, JSObject *scopeChain, JSObject *holder)
{
JS_ASSERT(IsCacheableNonGlobalScope(scopeChain));
@ -977,6 +999,9 @@ IonCacheBindName::attachNonGlobal(JSContext *cx, JSObject *scopeChain, JSObject
rejoinOffset.fixup(&masm);
exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, rejoinOffset);
CodeLocationJump exitJump(code, exitOffset);
CodeLocationJump lastJump_ = lastJump();
@ -1035,10 +1060,10 @@ js::ion::BindNameCache(JSContext *cx, size_t cacheIndex, HandleObject scopeChain
cache.incrementStubCount();
if (scopeChain->isGlobal()) {
if (!cache.attachGlobal(cx, scopeChain))
if (!cache.attachGlobal(cx, ion, scopeChain))
return NULL;
} else if (IsCacheableScopeChain(scopeChain, holder)) {
if (!cache.attachNonGlobal(cx, scopeChain, holder))
if (!cache.attachNonGlobal(cx, ion, scopeChain, holder))
return NULL;
} else {
IonSpew(IonSpew_InlineCaches, "BINDNAME uncacheable scope chain");
@ -1049,7 +1074,7 @@ js::ion::BindNameCache(JSContext *cx, size_t cacheIndex, HandleObject scopeChain
}
bool
IonCacheName::attach(JSContext *cx, HandleObject scopeChain, HandleObject holder, Shape *shape)
IonCacheName::attach(JSContext *cx, IonScript *ion, HandleObject scopeChain, HandleObject holder, Shape *shape)
{
MacroAssembler masm;
Label failures;
@ -1093,6 +1118,9 @@ IonCacheName::attach(JSContext *cx, HandleObject scopeChain, HandleObject holder
if (failures.bound())
exitOffset.fixup(&masm);
if (ion->invalidated())
return true;
CodeLocationJump rejoinJump(code, rejoinOffset);
CodeLocationJump lastJump_ = lastJump();
PatchJump(lastJump_, CodeLocationLabel(code));
@ -1168,7 +1196,7 @@ js::ion::GetNameCache(JSContext *cx, size_t cacheIndex, HandleObject scopeChain,
if (cache.stubCount() < MAX_STUBS &&
IsCacheableName(cx, scopeChain, obj, holder, shape))
{
if (!cache.attach(cx, scopeChain, obj, shape))
if (!cache.attach(cx, ion, scopeChain, obj, shape))
return false;
cache.incrementStubCount();
}

View File

@ -268,7 +268,8 @@ class IonCacheGetProperty : public IonCache
PropertyName *name() const { return u.getprop.name; }
TypedOrValueRegister output() const { return u.getprop.output.data(); }
bool attachNative(JSContext *cx, JSObject *obj, JSObject *holder, const Shape *shape);
bool attachNative(JSContext *cx, IonScript *ion, JSObject *obj, JSObject *holder,
const Shape *shape);
};
class IonCacheSetProperty : public IonCache
@ -294,9 +295,9 @@ class IonCacheSetProperty : public IonCache
ConstantOrRegister value() const { return u.setprop.value.data(); }
bool strict() const { return u.setprop.strict; }
bool attachNativeExisting(JSContext *cx, JSObject *obj, const Shape *shape);
bool attachNativeAdding(JSContext *cx, JSObject *obj, const Shape *oldshape, const Shape *newshape,
const Shape *propshape);
bool attachNativeExisting(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *shape);
bool attachNativeAdding(JSContext *cx, IonScript *ion, JSObject *obj, const Shape *oldshape,
const Shape *newshape, const Shape *propshape);
};
class IonCacheGetElement : public IonCache
@ -337,8 +338,8 @@ class IonCacheGetElement : public IonCache
u.getelem.hasDenseArrayStub = true;
}
bool attachGetProp(JSContext *cx, HandleObject obj, const Value &idval, PropertyName *name);
bool attachDenseArray(JSContext *cx, JSObject *obj, const Value &idval);
bool attachGetProp(JSContext *cx, IonScript *ion, HandleObject obj, const Value &idval, PropertyName *name);
bool attachDenseArray(JSContext *cx, IonScript *ion, JSObject *obj, const Value &idval);
};
class IonCacheBindName : public IonCache
@ -367,8 +368,8 @@ class IonCacheBindName : public IonCache
return u.bindname.output;
}
bool attachGlobal(JSContext *cx, JSObject *scopeChain);
bool attachNonGlobal(JSContext *cx, JSObject *scopeChain, JSObject *holder);
bool attachGlobal(JSContext *cx, IonScript *ion, JSObject *scopeChain);
bool attachNonGlobal(JSContext *cx, IonScript *ion, JSObject *scopeChain, JSObject *holder);
};
class IonCacheName : public IonCache
@ -401,7 +402,8 @@ class IonCacheName : public IonCache
return kind_ == NameTypeOf;
}
bool attach(JSContext *cx, HandleObject scopeChain, HandleObject obj, Shape *shape);
bool attach(JSContext *cx, IonScript *ion, HandleObject scopeChain, HandleObject obj,
Shape *shape);
};
bool

View File

@ -308,15 +308,24 @@ CodeGeneratorShared::markSafepointAt(uint32 offset, LInstruction *ins)
return safepointIndices_.append(SafepointIndex(offset, ins->safepoint()));
}
bool
CodeGeneratorShared::markOsiPoint(LOsiPoint *ins, uint32 *callPointOffset)
void
CodeGeneratorShared::ensureOsiSpace()
{
if (!encode(ins->snapshot()))
return false;
// We need to ensure that two OSI point patches don't overlap with
// each other, so we check that there's enough space for the near
// call between any two OSI points.
// For a refresher, an invalidation point is of the form:
// 1: call <target>
// 2: ...
// 3: <osipoint>
//
// The four bytes *before* instruction 2 are overwritten with an offset.
// Callers must ensure that the instruction itself has enough bytes to
// support this.
//
// The bytes *at* instruction 3 are overwritten with an invalidation jump.
// jump. These bytes may be in a completely different IR sequence, but
// represent the join point of the call out of the function.
//
// At points where we want to ensure that invalidation won't corrupt an
// important instruction, we make sure to pad with nops.
if (masm.currentOffset() - lastOsiPointOffset_ < Assembler::patchWrite_NearCallSize()) {
int32 paddingSize = Assembler::patchWrite_NearCallSize();
paddingSize -= masm.currentOffset() - lastOsiPointOffset_;
@ -325,6 +334,15 @@ CodeGeneratorShared::markOsiPoint(LOsiPoint *ins, uint32 *callPointOffset)
}
JS_ASSERT(masm.currentOffset() - lastOsiPointOffset_ >= Assembler::patchWrite_NearCallSize());
lastOsiPointOffset_ = masm.currentOffset();
}
bool
CodeGeneratorShared::markOsiPoint(LOsiPoint *ins, uint32 *callPointOffset)
{
if (!encode(ins->snapshot()))
return false;
ensureOsiSpace();
*callPointOffset = masm.currentOffset();
SnapshotOffset so = ins->snapshot()->snapshotOffset();

View File

@ -190,6 +190,13 @@ class CodeGeneratorShared : public LInstructionVisitor
// |returnPointOffset|.
bool markOsiPoint(LOsiPoint *ins, uint32 *returnPointOffset);
// Ensure that there is enough room between the last OSI point and the
// current instruction, such that:
// (1) Invalidation will not overwrite the current instruction, and
// (2) Overwriting the current instruction will not overwrite
// an invalidation marker.
void ensureOsiSpace();
bool emitTruncateDouble(const FloatRegister &src, const Register &dest);
void emitPreBarrier(Register base, const LAllocation *index, MIRType type);

View File

@ -182,6 +182,14 @@ namespace ion {
static inline void
PatchJump(CodeLocationJump jump, CodeLocationLabel label)
{
#ifdef DEBUG
// Assert that we're overwriting a jump instruction, either:
// 0F 80+cc <imm32>, or
// E9 <imm32>
unsigned char *x = (unsigned char *)jump.raw() - 5;
JS_ASSERT(((*x >= 0x80 && *x <= 0x8F) && *(x - 1) == 0x0F) ||
(*x == 0xE9));
#endif
JSC::X86Assembler::setRel32(jump.raw(), label.raw());
}

View File

@ -710,7 +710,7 @@ class CallCompiler : public BaseCompiler
masm.loadPtr(Address(t0, ion::IonCode::offsetOfCode()), t0);
masm.call(t0);
#elif defined(JS_CPU_ARM)
masm.loadPtr(Address(t0, ion::IonCode::OffsetOfCode()), JSC::ARMRegisters::ip);
masm.loadPtr(Address(t0, ion::IonCode::offsetOfCode()), JSC::ARMRegisters::ip);
masm.callAddress(JS_FUNC_TO_DATA_PTR(void *, IonVeneer));
#endif