diff --git a/hphp/runtime/vm/translator/hopt/codegen.cpp b/hphp/runtime/vm/translator/hopt/codegen.cpp index 3d856e834..6cc4e4adb 100644 --- a/hphp/runtime/vm/translator/hopt/codegen.cpp +++ b/hphp/runtime/vm/translator/hopt/codegen.cpp @@ -2425,9 +2425,9 @@ void CodeGenerator::cgExitTrace(IRInstruction* inst) { emitMovRegReg(a, fp->getReg(), rVmFp); // Get the SrcKey for the dest - SrcKey destSK(func->getValFunc(), pc->getValInt()); + SrcKey destSK(func->getValFunc(), pc->getValInt()); - switch(exitType) { + switch (exitType) { case TraceExitType::NormalCc: if (toSmash) { TCA smashAddr = toSmash->getTCA(); @@ -2510,10 +2510,12 @@ void CodeGenerator::cgExitTrace(IRInstruction* inst) { } } break; - case TraceExitType::GuardFailure: + + case TraceExitType::GuardFailure: { SrcRec* destSR = m_tx64->getSrcRec(destSK); m_tx64->emitFallbackUncondJmp(a, *destSR); break; + } } } diff --git a/hphp/runtime/vm/translator/hopt/ir.cpp b/hphp/runtime/vm/translator/hopt/ir.cpp index b07316f76..e90faa8d3 100644 --- a/hphp/runtime/vm/translator/hopt/ir.cpp +++ b/hphp/runtime/vm/translator/hopt/ir.cpp @@ -578,7 +578,7 @@ bool cmpOpTypesMayReenter(Opcode op, Type t0, Type t1) { TraceExitType::ExitType getExitType(Opcode opc) { assert(opc >= ExitTrace && opc <= ExitGuardFailure); - return (TraceExitType::ExitType)(opc - ExitTrace); + return TraceExitType::ExitType(opc - ExitTrace); } Opcode getExitOpcode(TraceExitType::ExitType type) { diff --git a/hphp/runtime/vm/translator/hopt/ir.h b/hphp/runtime/vm/translator/hopt/ir.h index fdf846b18..2f502693f 100644 --- a/hphp/runtime/vm/translator/hopt/ir.h +++ b/hphp/runtime/vm/translator/hopt/ir.h @@ -975,12 +975,13 @@ enum ExitType { NormalCc, Slow, SlowNoProgress, - GuardFailure + GuardFailure, }; } -extern TraceExitType::ExitType getExitType(Opcode opc); -extern Opcode getExitOpcode(TraceExitType::ExitType); +TraceExitType::ExitType getExitType(Opcode opc); +Opcode getExitOpcode(TraceExitType::ExitType); + inline bool isExitSlow(TraceExitType::ExitType t) { return t == TraceExitType::Slow || t == TraceExitType::SlowNoProgress; } diff --git a/hphp/runtime/vm/translator/srcdb.cpp b/hphp/runtime/vm/translator/srcdb.cpp index f7f3434d0..b47374363 100644 --- a/hphp/runtime/vm/translator/srcdb.cpp +++ b/hphp/runtime/vm/translator/srcdb.cpp @@ -48,35 +48,37 @@ TCA SrcRec::getFallbackTranslation() const { return m_anchorTranslation; } -void SrcRec::chainFrom(Asm& a, IncomingBranch br) { +void SrcRec::chainFrom(IncomingBranch br) { TCA destAddr = getTopTranslation(); m_incomingBranches.push_back(br); - TRACE(1, "SrcRec(%p)::chainFrom %p -> %p; %zd incoming branches\n", + TRACE(1, "SrcRec(%p)::chainFrom %p -> %p (type %d); %zd incoming branches\n", this, - a.code.frontier, destAddr, m_incomingBranches.size()); - patch(&a, br, destAddr); + br.toSmash(), destAddr, br.type(), m_incomingBranches.size()); + patch(br, destAddr); } -void SrcRec::emitFallbackJump(Asm &a, TCA from, int cc /* = -1 */) { +void SrcRec::emitFallbackJump(TCA from, int cc /* = -1 */) { TCA destAddr = getFallbackTranslation(); - IncomingBranch incoming(cc < 0 ? IncomingBranch::JMP : IncomingBranch::JCC, - from); + auto incoming = cc < 0 ? IncomingBranch::jmpFrom(from) + : IncomingBranch::jccFrom(from); + auto& a = tx64->getAsmFor(from); + // emit dummy jump to be smashed via patch() if (cc < 0) { a.jmp(a.code.frontier); } else { - assert(incoming.m_type == IncomingBranch::JCC); + assert(incoming.type() == IncomingBranch::JCC); a.jcc((ConditionCode)cc, a.code.frontier); } - patch(&a, incoming, destAddr); + patch(incoming, destAddr); // We'll need to know the location of this jump later so we can // patch it to new translations added to the chain. m_inProgressTailJumps.push_back(incoming); } -void SrcRec::newTranslation(Asm& a, Asm &astubs, TCA newStart) { +void SrcRec::newTranslation(TCA newStart) { // When translation punts due to hitting limit, will generate one // more translation that will call the interpreter. assert(m_translations.size() <= kMaxTranslations); @@ -86,7 +88,7 @@ void SrcRec::newTranslation(Asm& a, Asm &astubs, TCA newStart) { m_translations.push_back(newStart); if (!m_topTranslation) { atomic_release_store(&m_topTranslation, newStart); - patchIncomingBranches(a, astubs, newStart); + patchIncomingBranches(newStart); } /* @@ -104,8 +106,7 @@ void SrcRec::newTranslation(Asm& a, Asm &astubs, TCA newStart) { * translation possibly for this same situation.) */ for (size_t i = 0; i < m_tailFallbackJumps.size(); ++i) { - auto& as = asmChoose(m_tailFallbackJumps[i].m_src, a, astubs); - patch(&as, m_tailFallbackJumps[i], newStart); + patch(m_tailFallbackJumps[i], newStart); } // This is the new tail translation, so store the fallback jump list @@ -114,15 +115,14 @@ void SrcRec::newTranslation(Asm& a, Asm &astubs, TCA newStart) { m_inProgressTailJumps.clear(); } -void SrcRec::addDebuggerGuard(Asm& a, Asm &astubs, TCA dbgGuard, - TCA dbgBranchGuardSrc) { +void SrcRec::addDebuggerGuard(TCA dbgGuard, TCA dbgBranchGuardSrc) { assert(!m_dbgBranchGuardSrc); TRACE(1, "SrcRec(%p)::addDebuggerGuard @%p, " "%zd incoming branches to rechain\n", this, dbgGuard, m_incomingBranches.size()); - patchIncomingBranches(a, astubs, dbgGuard); + patchIncomingBranches(dbgGuard); // Set m_dbgBranchGuardSrc after patching, so we don't try to patch // the debug guard. @@ -130,12 +130,14 @@ void SrcRec::addDebuggerGuard(Asm& a, Asm &astubs, TCA dbgGuard, atomic_release_store(&m_topTranslation, dbgGuard); } -void SrcRec::patchIncomingBranches(Asm& a, Asm &astubs, TCA newStart) { +void SrcRec::patchIncomingBranches(TCA newStart) { if (hasDebuggerGuard()) { // We have a debugger guard, so all jumps to us funnel through // this. Just smash m_dbgBranchGuardSrc. TRACE(1, "smashing m_dbgBranchGuardSrc @%p\n", m_dbgBranchGuardSrc); - TranslatorX64::smashJmp(a, m_dbgBranchGuardSrc, newStart); + TranslatorX64::smashJmp(tx64->getAsmFor(m_dbgBranchGuardSrc), + m_dbgBranchGuardSrc, + newStart); return; } @@ -144,46 +146,62 @@ void SrcRec::patchIncomingBranches(Asm& a, Asm &astubs, TCA newStart) { vector& change = m_incomingBranches; for (unsigned i = 0; i < change.size(); ++i) { TRACE(1, "SrcRec(%p)::newTranslation rechaining @%p -> %p\n", - this, change[i].m_src, newStart); - Asm *as = change[i].m_type == IncomingBranch::ADDR ? - nullptr : &asmChoose(change[i].m_src, a, astubs); - patch(as, change[i], newStart); + this, change[i].toSmash(), newStart); + patch(change[i], newStart); } } -void SrcRec::replaceOldTranslations(Asm& a, Asm& astubs) { +void SrcRec::replaceOldTranslations() { // Everyone needs to give up on old translations; send them to the anchor, - // which is a REQ_RETRANSLATE + // which is a REQ_RETRANSLATE. m_translations.clear(); m_tailFallbackJumps.clear(); atomic_release_store(&m_topTranslation, static_cast(0)); - patchIncomingBranches(a, astubs, m_anchorTranslation); + + /* + * It may seem a little weird that we're about to point every + * incoming branch at the anchor, since that's going to just + * unconditionally retranslate this SrcKey and never patch the + * incoming branch to do something else. + * + * The reason this is ok is this mechanism is only used in + * non-RepoAuthoritative mode, and the granularity of code + * invalidation there is such that we'll only have incoming branches + * like this basically within the same file since we don't have + * whole program analysis. + * + * This means all these incoming branches are about to go away + * anyway ... + * + * If we ever change that we'll have to change this to patch to + * some sort of rebind requests. + */ + assert(!RuntimeOption::RepoAuthoritative); + patchIncomingBranches(m_anchorTranslation); } -void SrcRec::patch(Asm* a, IncomingBranch branch, TCA dest) { - if (branch.m_type == IncomingBranch::ADDR) { - // Note that this effectively ignores a - atomic_release_store(branch.m_addr, dest); - return; +void SrcRec::patch(IncomingBranch branch, TCA dest) { + switch (branch.type()) { + case IncomingBranch::JMP: { + auto& a = tx64->getAsmFor(branch.toSmash()); + CodeCursor cg(a, branch.toSmash()); + TranslatorX64::smashJmp(a, branch.toSmash(), dest); + break; } - // modifying reachable code - switch(branch.m_type) { - case IncomingBranch::JMP: { - CodeCursor cg(*a, branch.m_src); - TranslatorX64::smashJmp(*a, branch.m_src, dest); - break; - } - case IncomingBranch::JCC: { - // patch destination, but preserve the condition code - int32_t delta = safe_cast((dest - branch.m_src) - - TranslatorX64::kJmpccLen); - int32_t* addr = (int32_t*)(branch.m_src + TranslatorX64::kJmpccLen - 4); - atomic_release_store(addr, delta); - break; - } - default: - not_implemented(); + case IncomingBranch::JCC: { + // patch destination, but preserve the condition code + int32_t delta = safe_cast((dest - branch.toSmash()) - + TranslatorX64::kJmpccLen); + int32_t* addr = (int32_t*)(branch.toSmash() + + TranslatorX64::kJmpccLen - 4); + atomic_release_store(addr, delta); + break; + } + + case IncomingBranch::ADDR: + // Note that this effectively ignores a + atomic_release_store(reinterpret_cast(branch.toSmash()), dest); } } diff --git a/hphp/runtime/vm/translator/srcdb.h b/hphp/runtime/vm/translator/srcdb.h index 8fc012c68..09131152c 100644 --- a/hphp/runtime/vm/translator/srcdb.h +++ b/hphp/runtime/vm/translator/srcdb.h @@ -28,31 +28,49 @@ namespace HPHP { namespace VM { namespace Transl { +/* + * Incoming branches between different translations are tracked using + * this structure. + * + * This allows us to smash them later to point to different things. + * We handle conditional and unconditional jumps, as well as pointers + * to code (via IncomingBranch::ADDR, used for example in a switch + * table). + * + * We don't need to track which condition code a conditional jump used + * because we take care to smash only the address and leave the code + * intact. + */ struct IncomingBranch { enum BranchType { JMP, JCC, ADDR, }; - IncomingBranch(BranchType t, TCA src) - : m_type(t), m_src(src) { } - IncomingBranch(TCA src) - : m_type(JMP), m_src(src) { } - IncomingBranch(TCA* addr) - : m_type(ADDR), m_addr(addr) { } + + static IncomingBranch jmpFrom(TCA from) { return IncomingBranch(JMP, from); } + static IncomingBranch jccFrom(TCA from) { return IncomingBranch(JCC, from); } + static IncomingBranch addr(TCA* from) { + return IncomingBranch(ADDR, TCA(from)); + } + + BranchType type() const { return m_type; } + TCA toSmash() const { return m_toSmash; } + +private: + explicit IncomingBranch(BranchType type, TCA toSmash) + : m_type(type) + , m_toSmash(toSmash) + {} BranchType m_type; - union { - TCA m_src; - TCA* m_addr; - }; + TCA m_toSmash; }; /* * SrcRec: record of translator output for a given source location. */ struct SrcRec { - typedef X64Assembler Asm; static const unsigned int kMaxTranslations = 12; SrcRec() @@ -79,12 +97,11 @@ struct SrcRec { * when holding the translator write lease. */ void setFuncInfo(const Func* f); - void chainFrom(Asm& a, IncomingBranch br); - void emitFallbackJump(Asm &a, TCA from, int cc = -1); - void newTranslation(Asm& a, Asm &astubs, TCA newStart); - void replaceOldTranslations(Asm& a, Asm& astubs); - void addDebuggerGuard(Asm& a, Asm &astubs, TCA dbgGuard, - TCA m_dbgBranchGuardSrc); + void chainFrom(IncomingBranch br); + void emitFallbackJump(TCA from, int cc = -1); + void newTranslation(TCA newStart); + void replaceOldTranslations(); + void addDebuggerGuard(TCA dbgGuard, TCA m_dbgBranchGuardSrc); bool hasDebuggerGuard() const { return m_dbgBranchGuardSrc != nullptr; } const MD5& unitMd5() const { return m_unitMd5; } @@ -92,6 +109,10 @@ struct SrcRec { return m_translations; } + /* + * The anchor translation is a retranslate request for the current + * SrcKey that will continue the tracelet chain. + */ void setAnchorTranslation(TCA anc) { assert(!m_anchorTranslation); assert(m_tailFallbackJumps.empty()); @@ -108,8 +129,8 @@ struct SrcRec { private: TCA getFallbackTranslation() const; - void patch(Asm* a, IncomingBranch branch, TCA dest); - void patchIncomingBranches(Asm& a, Asm& astubs, TCA newStart); + void patch(IncomingBranch branch, TCA dest); + void patchIncomingBranches(TCA newStart); private: // This either points to the most recent translation in the diff --git a/hphp/runtime/vm/translator/translator-x64.cpp b/hphp/runtime/vm/translator/translator-x64.cpp index 59b61db4d..86c2748ca 100644 --- a/hphp/runtime/vm/translator/translator-x64.cpp +++ b/hphp/runtime/vm/translator/translator-x64.cpp @@ -1223,10 +1223,7 @@ asm_label(a, release); size_t(a.code.frontier - m_dtorGenericStub)); } -/* - * Translation call targets. It is a lot easier, and a bit more - * portable, to use C linkage from assembly. - */ + TCA TranslatorX64::retranslate(SrcKey sk, bool align, bool allowIR) { if (isDebuggerAttachedProcess() && isSrcKeyInBL(curUnit(), sk)) { // We are about to translate something known to be blacklisted by @@ -2560,12 +2557,11 @@ TranslatorX64::bindJmp(TCA toSmash, SrcKey destSk, smashed = true; SrcRec* sr = getSrcRec(destSk); if (req == REQ_BIND_ADDR) { - sr->chainFrom(a, IncomingBranch((TCA*)toSmash)); + sr->chainFrom(IncomingBranch::addr(reinterpret_cast(toSmash))); } else if (req == REQ_BIND_JCC) { - sr->chainFrom(getAsmFor(toSmash), - IncomingBranch(IncomingBranch::JCC, toSmash)); + sr->chainFrom(IncomingBranch::jccFrom(toSmash)); } else { - sr->chainFrom(getAsmFor(toSmash), IncomingBranch(toSmash)); + sr->chainFrom(IncomingBranch::jmpFrom(toSmash)); } return tDest; } @@ -2616,7 +2612,7 @@ TranslatorX64::bindJmpccFirst(TCA toSmash, emitServiceReq(SRFlags::SRNone, REQ_BIND_JMPCC_SECOND, 3, toSmash, uint64_t(offWillDefer), uint64_t(cc)); - Asm &as = getAsmFor(toSmash); + Asm& as = getAsmFor(toSmash); // Its not clear where chainFrom should go to if as is astubs assert(&as != &astubs); @@ -2647,7 +2643,7 @@ TranslatorX64::bindJmpccFirst(TCA toSmash, */ CodeCursor cg(as, toSmash); as.jcc(cc, stub); - getSrcRec(dest)->chainFrom(as, IncomingBranch(as.code.frontier)); + getSrcRec(dest)->chainFrom(IncomingBranch::jmpFrom(as.code.frontier)); TRACE(5, "bindJmpccFirst: overwrote with cc%02x taken %d\n", cc, taken); return tDest; } @@ -2663,8 +2659,7 @@ TranslatorX64::bindJmpccSecond(TCA toSmash, const Offset off, if (branch && writer.acquire()) { smashed = true; SrcRec* destRec = getSrcRec(dest); - destRec->chainFrom(getAsmFor(toSmash), - IncomingBranch(IncomingBranch::JCC, toSmash)); + destRec->chainFrom(IncomingBranch::jccFrom(toSmash)); } return branch; } @@ -2824,19 +2819,19 @@ void TranslatorX64::emitFallbackJmp(Asm& as, SrcRec& dest, ConditionCode cc /* = CC_NZ */) { prepareForSmash(as, kJmpccLen); - dest.emitFallbackJump(as, as.code.frontier, cc); + dest.emitFallbackJump(as.code.frontier, cc); } void TranslatorX64::emitFallbackUncondJmp(Asm& as, SrcRec& dest) { prepareForSmash(as, kJmpLen); - dest.emitFallbackJump(as, as.code.frontier); + dest.emitFallbackJump(as.code.frontier); } void TranslatorX64::emitFallbackCondJmp(Asm& as, SrcRec& dest, ConditionCode cc) { prepareForSmash(as, kJmpccLen); - dest.emitFallbackJump(as, as.code.frontier, cc); + dest.emitFallbackJump(as.code.frontier, cc); } void TranslatorX64::emitReqRetransNoIR(Asm& as, SrcKey& sk) { @@ -3257,7 +3252,8 @@ bool TranslatorX64::handleServiceRequest(TReqInfo& info, case REQ_BIND_JMP: case REQ_BIND_JCC: case REQ_BIND_JMP_NO_IR: - case REQ_BIND_ADDR: { + case REQ_BIND_ADDR: + { TCA toSmash = (TCA)args[0]; Offset off = args[1]; sk = SrcKey(curFunc(), off); @@ -3296,7 +3292,7 @@ bool TranslatorX64::handleServiceRequest(TReqInfo& info, if (writer) { smashed = true; SrcRec* sr = getSrcRec(sk); - sr->chainFrom(a, IncomingBranch(&rlsa->m_pseudoMain)); + sr->chainFrom(IncomingBranch::addr(&rlsa->m_pseudoMain)); } } } break; @@ -11406,7 +11402,7 @@ TranslatorX64::translateTracelet(SrcKey sk, bool considerHHIR/*=true*/, // metadata is not yet visible. TRACE(1, "newTranslation: %p sk: (func %d, bcOff %d)\n", start, sk.getFuncId(), sk.m_offset); - srcRec.newTranslation(a, astubs, start); + srcRec.newTranslation(start); TRACE(1, "tx64: %zd-byte tracelet\n", a.code.frontier - start); if (Trace::moduleEnabledRelease(Trace::tcspace, 1)) { Trace::traceRelease(getUsage().c_str()); @@ -12177,7 +12173,7 @@ void TranslatorX64::addDbgGuardImpl(SrcKey sk, SrcRec& srcRec) { TCA dbgBranchGuardSrc = a.code.frontier; a. jmp(realCode); // Add it to srcRec - srcRec.addDebuggerGuard(a, astubs, dbgGuard, dbgBranchGuardSrc); + srcRec.addDebuggerGuard(dbgGuard, dbgBranchGuardSrc); } bool TranslatorX64::dumpTCCode(const char* filename) { @@ -12367,7 +12363,7 @@ void TranslatorX64::invalidateSrcKey(SrcKey sk) { * just created some garbage in the TC. We currently have no mechanism * to reclaim this. */ - sr->replaceOldTranslations(a, astubs); + sr->replaceOldTranslations(); } void TranslatorX64::invalidateFileWork(Eval::PhpFile* f) {