From d8df647b38a0ec60a08752ab8bb33e5d03313d04 Mon Sep 17 00:00:00 2001 From: Paul Bissonnette Date: Thu, 6 Jun 2013 16:08:23 -0700 Subject: [PATCH] Fixed cleanup/exception handling for vector operations. Vector statements ending with a SetProp now properly free memory when run with JIT enabled. Additionally, in both JIT and interpreted mode memory will still be freed if exceptions are thrown during evaluation of vector operations. --- hphp/runtime/base/execution_context.h | 10 +- hphp/runtime/vm/bytecode.cpp | 48 +++++----- hphp/runtime/vm/jit/codegen.cpp | 2 +- hphp/runtime/vm/jit/hhbctranslator.h | 35 ++++++- hphp/runtime/vm/jit/vectortranslator.cpp | 74 +++++++-------- hphp/test/quick/vector-unwind-decref.php | 94 +++++++++++++++++++ .../quick/vector-unwind-decref.php.expect | 26 +++++ 7 files changed, 224 insertions(+), 65 deletions(-) create mode 100644 hphp/test/quick/vector-unwind-decref.php create mode 100644 hphp/test/quick/vector-unwind-decref.php.expect diff --git a/hphp/runtime/base/execution_context.h b/hphp/runtime/base/execution_context.h index 1cc53c0b7..360dcbccb 100644 --- a/hphp/runtime/base/execution_context.h +++ b/hphp/runtime/base/execution_context.h @@ -473,12 +473,12 @@ private: MemberCode& mcode, TypedValue*& curMember); template void getHelperPost(unsigned ndiscard, TypedValue*& tvRet, - TypedValue& tvScratch, TypedValue& tvRef, - TypedValue& tvRef2); + TypedValue& tvScratch, Variant& tvRef, + Variant& tvRef2); void getHelper(PC& pc, unsigned& ndiscard, TypedValue*& tvRet, TypedValue*& base, TypedValue& tvScratch, TypedValue& tvLiteral, - TypedValue& tvRef, TypedValue& tvRef2, + Variant& tvRef, Variant& tvRef2, MemberCode& mcode, TypedValue*& curMember); template - void setHelperPost(unsigned ndiscard, TypedValue& tvRef, - TypedValue& tvRef2); + void setHelperPost(unsigned ndiscard, Variant& tvRef, + Variant& tvRef2); bool cellInstanceOf(TypedValue* c, const HPHP::NamedEntity* s); bool initIterator(PC& pc, PC& origPc, Iter* it, Offset offset, Cell* c1); diff --git a/hphp/runtime/vm/bytecode.cpp b/hphp/runtime/vm/bytecode.cpp index e11c3d77d..c482c5bd6 100644 --- a/hphp/runtime/vm/bytecode.cpp +++ b/hphp/runtime/vm/bytecode.cpp @@ -3275,8 +3275,8 @@ static inline void ratchetRefs(TypedValue*& result, TypedValue& tvRef, TypedValue* base; \ TypedValue tvScratch; \ TypedValue tvLiteral; \ - TypedValue tvRef; \ - TypedValue tvRef2; \ + Variant tvRef; \ + Variant tvRef2; \ MemberCode mcode = MEL; \ TypedValue* curMember = 0; #define DECLARE_SETHELPER_ARGS DECLARE_MEMBERHELPER_ARGS @@ -3285,7 +3285,11 @@ static inline void ratchetRefs(TypedValue*& result, TypedValue& tvRef, TypedValue* tvRet; #define MEMBERHELPERPRE_ARGS \ - pc, ndiscard, base, tvScratch, tvLiteral, \ + pc, ndiscard, base, tvScratch, tvLiteral, \ + *tvRef.asTypedValue(), *tvRef2.asTypedValue(), mcode, curMember + +#define MEMBERHELPERPRE_OUT \ + pc, ndiscard, base, tvScratch, tvLiteral, \ tvRef, tvRef2, mcode, curMember // The following arguments are outputs: @@ -3317,14 +3321,14 @@ inline void OPTBLD_INLINE VMExecutionContext::getHelperPre( MemberCode& mcode, TypedValue*& curMember) { memberHelperPre(MEMBERHELPERPRE_ARGS); + false, 0, mleave, saveResult>(MEMBERHELPERPRE_OUT); } #define GETHELPERPOST_ARGS ndiscard, tvRet, tvScratch, tvRef, tvRef2 template inline void OPTBLD_INLINE VMExecutionContext::getHelperPost( unsigned ndiscard, TypedValue*& tvRet, TypedValue& tvScratch, - TypedValue& tvRef, TypedValue& tvRef2) { + Variant& tvRef, Variant& tvRef2) { // Clean up all ndiscard elements on the stack. Actually discard // only ndiscard - 1, and overwrite the last cell with the result, // or if ndiscard is zero we actually need to allocate a cell. @@ -3339,8 +3343,6 @@ inline void OPTBLD_INLINE VMExecutionContext::getHelperPost( m_stack.ndiscard(ndiscard - 1); tvRet = m_stack.topTV(); } - tvRefcountedDecRef(&tvRef); - tvRefcountedDecRef(&tvRef2); if (saveResult) { // If tvRef wasn't just allocated, we've already decref'd it in @@ -3359,8 +3361,8 @@ VMExecutionContext::getHelper(PC& pc, TypedValue*& base, TypedValue& tvScratch, TypedValue& tvLiteral, - TypedValue& tvRef, - TypedValue& tvRef2, + Variant& tvRef, + Variant& tvRef2, MemberCode& mcode, TypedValue*& curMember) { getHelperPre(MEMBERHELPERPRE_ARGS); @@ -3647,13 +3649,13 @@ inline bool OPTBLD_INLINE VMExecutionContext::setHelperPre( TypedValue& tvRef, TypedValue& tvRef2, MemberCode& mcode, TypedValue*& curMember) { return memberHelperPre(MEMBERHELPERPRE_ARGS); + reffy, mdepth, mleave, false>(MEMBERHELPERPRE_OUT); } #define SETHELPERPOST_ARGS ndiscard, tvRef, tvRef2 template inline void OPTBLD_INLINE VMExecutionContext::setHelperPost( - unsigned ndiscard, TypedValue& tvRef, TypedValue& tvRef2) { + unsigned ndiscard, Variant& tvRef, Variant& tvRef2) { // Clean up the stack. Decref all the elements for the vector, but // leave the first mdepth (they are not part of the vector data). for (unsigned depth = mdepth; depth-mdepth < ndiscard; ++depth) { @@ -3678,8 +3680,6 @@ inline void OPTBLD_INLINE VMExecutionContext::setHelperPost( } m_stack.ndiscard(ndiscard); - tvRefcountedDecRef(&tvRef); - tvRefcountedDecRef(&tvRef2); } inline void OPTBLD_INLINE VMExecutionContext::iopLowInvalid(PC& pc) { @@ -4928,7 +4928,8 @@ inline void OPTBLD_INLINE VMExecutionContext::iopIssetM(PC& pc) { case MEC: case MET: case MEI: { - issetResult = IssetEmptyElem(tvScratch, tvRef, base, curMember); + issetResult = IssetEmptyElem(tvScratch, *tvRef.asTypedValue(), + base, curMember); break; } case MPL: @@ -5057,7 +5058,8 @@ inline void OPTBLD_INLINE VMExecutionContext::iopEmptyM(PC& pc) { case MEC: case MET: case MEI: { - emptyResult = IssetEmptyElem(tvScratch, tvRef, base, curMember); + emptyResult = IssetEmptyElem(tvScratch, *tvRef.asTypedValue(), + base, curMember); break; } case MPL: @@ -5302,20 +5304,22 @@ inline void OPTBLD_INLINE VMExecutionContext::iopSetOpM(PC& pc) { Cell* rhs = m_stack.topC(); if (mcode == MW) { - result = SetOpNewElem(tvScratch, tvRef, op, base, rhs); + result = SetOpNewElem(tvScratch, *tvRef.asTypedValue(), op, base, rhs); } else { switch (mcode) { case MEL: case MEC: case MET: case MEI: - result = SetOpElem(tvScratch, tvRef, op, base, curMember, rhs); + result = SetOpElem(tvScratch, *tvRef.asTypedValue(), op, base, + curMember, rhs); break; case MPL: case MPC: case MPT: { Class *ctx = arGetContextClass(m_fp); - result = SetOpProp(tvScratch, tvRef, ctx, op, base, curMember, rhs); + result = SetOpProp(tvScratch, *tvRef.asTypedValue(), ctx, op, base, + curMember, rhs); break; } default: @@ -5390,20 +5394,22 @@ inline void OPTBLD_INLINE VMExecutionContext::iopIncDecM(PC& pc) { if (!setHelperPre(MEMBERHELPERPRE_ARGS)) { if (mcode == MW) { - IncDecNewElem(tvScratch, tvRef, op, base, to); + IncDecNewElem(tvScratch, *tvRef.asTypedValue(), op, base, to); } else { switch (mcode) { case MEL: case MEC: case MET: case MEI: - IncDecElem(tvScratch, tvRef, op, base, curMember, to); + IncDecElem(tvScratch, *tvRef.asTypedValue(), op, base, + curMember, to); break; case MPL: case MPC: case MPT: { Class* ctx = arGetContextClass(m_fp); - IncDecProp(tvScratch, tvRef, ctx, op, base, curMember, to); + IncDecProp(tvScratch, *tvRef.asTypedValue(), ctx, op, base, + curMember, to); break; } default: assert(false); diff --git a/hphp/runtime/vm/jit/codegen.cpp b/hphp/runtime/vm/jit/codegen.cpp index cea9a0787..d5ee87a24 100644 --- a/hphp/runtime/vm/jit/codegen.cpp +++ b/hphp/runtime/vm/jit/codegen.cpp @@ -820,7 +820,7 @@ void CodeGenerator::cgBeginCatch(IRInstruction* inst) { // successfully, so skip over any stack arguments and pop any // saved registers. if (info.rspOffset) { - m_as.subq(info.rspOffset, rsp); + m_as.addq(info.rspOffset, rsp); } PhysRegSaverParity::emitPops(m_as, info.savedRegs); } diff --git a/hphp/runtime/vm/jit/hhbctranslator.h b/hphp/runtime/vm/jit/hhbctranslator.h index a9a7ecb2f..b151d0b56 100644 --- a/hphp/runtime/vm/jit/hhbctranslator.h +++ b/hphp/runtime/vm/jit/hhbctranslator.h @@ -440,6 +440,33 @@ private: void emitMapGet(SSATmp* key); void emitMapIsset(); + // Generate a catch trace that does not perform any final DecRef operations + // on scratch space + IRTrace* getEmptyCatchTrace() { + return m_ht.getCatchTrace(); + } + + // Generate a catch trace that will contain DecRef instructions for tvRef + // and/or tvRef2 as required + IRTrace* getCatchTrace() { + IRTrace* t = getEmptyCatchTrace(); + m_failedTraceVec.push_back(t); + return t; + } + + // Generate a catch trace that will free any scratch space used and perform + // a side-exit from a failed set operation + IRTrace* getCatchSetTrace() { + assert(!m_failedSetTrace); + return m_failedSetTrace = getCatchTrace(); + } + + void prependToTraces(IRInstruction* inst) { + for (auto t : m_failedTraceVec) { + t->front()->prepend(inst->clone(&m_irf)); + } + } + // Misc Helpers void numberStackInputs(); void setNoMIState() { m_needMIS = false; } @@ -531,7 +558,13 @@ private: * unexpected type, in which case they'll throw an InvalidSetMException. To * handle this, emitMPost adds code to the catch trace to fish the correct * value out of the exception and side exit. */ - IRTrace* m_failedSetTrace; + IRTrace* m_failedSetTrace; + + /* Contains a list of all catch traces created in building the vector. + * Each trace must be appended with cleanup tasks (generally just DecRef) + * to be performed if an exception occurs during the course of the vector + * operation */ + std::vector m_failedTraceVec; }; private: // tracebuilder forwarding utilities diff --git a/hphp/runtime/vm/jit/vectortranslator.cpp b/hphp/runtime/vm/jit/vectortranslator.cpp index b39bbe128..645ac568d 100644 --- a/hphp/runtime/vm/jit/vectortranslator.cpp +++ b/hphp/runtime/vm/jit/vectortranslator.cpp @@ -553,7 +553,7 @@ void HhbcTranslator::VectorTranslator::emitBaseLCR() { // Check for Uninit and warn/promote to InitNull as appropriate if (baseType.subtypeOf(Type::Uninit)) { if (mia & MIA_warn) { - gen(RaiseUninitLoc, m_ht.getCatchTrace(), + gen(RaiseUninitLoc, getEmptyCatchTrace(), LocalId(base.location.offset)); } if (mia & MIA_define) { @@ -716,7 +716,7 @@ void HhbcTranslator::VectorTranslator::emitBaseG() { OpFunc opFunc = opFuncs[mia & MIA_base]; SSATmp* gblName = getBase(); m_base = gen(BaseG, - m_ht.getCatchTrace(), + getEmptyCatchTrace(), cns(reinterpret_cast(opFunc)), gblName, genMisPtr()); @@ -820,10 +820,10 @@ void HhbcTranslator::VectorTranslator::emitPropGeneric() { SSATmp* key = getKey(); BUILD_OPTAB(mia, m_base->isA(Type::Obj)); if (mia & Define) { - m_base = genStk(PropDX, m_ht.getCatchTrace(), cns((TCA)opFunc), CTX(), + m_base = genStk(PropDX, getCatchTrace(), cns((TCA)opFunc), CTX(), m_base, key, genMisPtr()); } else { - m_base = gen(PropX, m_ht.getCatchTrace(), + m_base = gen(PropX, getCatchTrace(), cns((TCA)opFunc), CTX(), m_base, key, genMisPtr()); } } @@ -1043,10 +1043,10 @@ void HhbcTranslator::VectorTranslator::emitElem() { typedef TypedValue* (*OpFunc)(TypedValue*, TypedValue, MInstrState*); BUILD_OPTAB_HOT(getKeyTypeIS(key), mia); if (define || unset) { - m_base = genStk(define ? ElemDX : ElemUX, m_ht.getCatchTrace(), + m_base = genStk(define ? ElemDX : ElemUX, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr()); } else { - m_base = gen(ElemX, m_ht.getCatchTrace(), + m_base = gen(ElemX, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr()); } } @@ -1176,7 +1176,7 @@ void HhbcTranslator::VectorTranslator::emitCGetProp() { typedef TypedValue (*OpFunc)(Class*, TypedValue*, TypedValue, MInstrState*); SSATmp* key = getKey(); BUILD_OPTAB_HOT(getKeyTypeS(key), m_base->isA(Type::Obj)); - m_result = gen(CGetProp, m_ht.getCatchTrace(), + m_result = gen(CGetProp, getCatchTrace(), cns((TCA)opFunc), CTX(), m_base, key, genMisPtr()); } #undef HELPER_TABLE @@ -1218,7 +1218,7 @@ void HhbcTranslator::VectorTranslator::emitVGetProp() { SSATmp* key = getKey(); typedef RefData* (*OpFunc)(Class*, TypedValue*, TypedValue, MInstrState*); BUILD_OPTAB_HOT(getKeyTypeS(key), m_base->isA(Type::Obj)); - m_result = genStk(VGetProp, m_ht.getCatchTrace(), cns((TCA)opFunc), CTX(), + m_result = genStk(VGetProp, getCatchTrace(), cns((TCA)opFunc), CTX(), m_base, key, genMisPtr()); } #undef HELPER_TABLE @@ -1250,7 +1250,7 @@ void HhbcTranslator::VectorTranslator::emitIssetEmptyProp(bool isEmpty) { SSATmp* key = getKey(); typedef uint64_t (*OpFunc)(Class*, TypedValue*, TypedValue); BUILD_OPTAB(isEmpty, m_base->isA(Type::Obj)); - m_result = gen(isEmpty ? EmptyProp : IssetProp, m_ht.getCatchTrace(), + m_result = gen(isEmpty ? EmptyProp : IssetProp, getCatchTrace(), cns((TCA)opFunc), CTX(), m_base, key); } #undef HELPER_TABLE @@ -1306,8 +1306,7 @@ void HhbcTranslator::VectorTranslator::emitSetProp() { typedef void (*OpFunc)(Class*, TypedValue*, TypedValue, Cell); SSATmp* key = getKey(); BUILD_OPTAB(m_base->isA(Type::Obj)); - m_failedSetTrace = m_ht.getCatchTrace(); - genStk(SetProp, m_failedSetTrace, cns((TCA)opFunc), CTX(), + genStk(SetProp, getCatchSetTrace(), cns((TCA)opFunc), CTX(), m_base, key, value); m_result = value; } @@ -1348,7 +1347,7 @@ void HhbcTranslator::VectorTranslator::emitSetOpProp() { BUILD_OPTAB(m_base->isA(Type::Obj)); m_tb.gen(StRaw, m_misBase, cns(RawMemSlot::MisCtx), CTX()); m_result = - genStk(SetOpProp, m_ht.getCatchTrace(), cns((TCA)opFunc), + genStk(SetOpProp, getCatchTrace(), cns((TCA)opFunc), m_base, key, value, genMisPtr(), cns(op)); } #undef HELPER_TABLE @@ -1388,7 +1387,7 @@ void HhbcTranslator::VectorTranslator::emitIncDecProp() { BUILD_OPTAB(m_base->isA(Type::Obj)); m_tb.gen(StRaw, m_misBase, cns(RawMemSlot::MisCtx), CTX()); m_result = - genStk(IncDecProp, m_ht.getCatchTrace(), cns((TCA)opFunc), + genStk(IncDecProp, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr(), cns(op)); } #undef HELPER_TABLE @@ -1424,7 +1423,7 @@ void HhbcTranslator::VectorTranslator::emitBindProp() { typedef void (*OpFunc)(Class*, TypedValue*, TypedValue, RefData*, MInstrState*); BUILD_OPTAB(m_base->isA(Type::Obj)); - genStk(BindProp, m_ht.getCatchTrace(), cns((TCA)opFunc), CTX(), + genStk(BindProp, getCatchTrace(), cns((TCA)opFunc), CTX(), m_base, key, box, genMisPtr()); m_result = box; } @@ -1519,7 +1518,7 @@ inline TypedValue vectorGet(c_Vector* vec, int64_t key) { } void HhbcTranslator::VectorTranslator::emitVectorGet(SSATmp* key) { - SSATmp* value = gen(VectorGet, m_ht.getCatchTrace(), + SSATmp* value = gen(VectorGet, getCatchTrace(), cns((TCA)VectorHelpers::vectorGet), m_base, key); m_result = gen(IncRef, value); } @@ -1552,7 +1551,7 @@ void HhbcTranslator::VectorTranslator::emitMapGet(SSATmp* key) { typedef TypedValue (*OpFunc)(c_Map*, TypedValue*); BUILD_OPTAB_HOT(keyType); - SSATmp* value = gen(MapGet, m_ht.getCatchTrace(), + SSATmp* value = gen(MapGet, getCatchTrace(), cns((TCA)opFunc), m_base, key); m_result = gen(IncRef, value); } @@ -1605,7 +1604,7 @@ void HhbcTranslator::VectorTranslator::emitCGetElem() { default: typedef TypedValue (*OpFunc)(TypedValue*, TypedValue, MInstrState*); BUILD_OPTAB_HOT(getKeyTypeIS(key)); - m_result = gen(CGetElem, m_ht.getCatchTrace(), cns((TCA)opFunc), + m_result = gen(CGetElem, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr()); break; } @@ -1646,7 +1645,7 @@ void HhbcTranslator::VectorTranslator::emitVGetElem() { SSATmp* key = getKey(); typedef RefData* (*OpFunc)(TypedValue*, TypedValue, MInstrState*); BUILD_OPTAB(getKeyTypeIS(key)); - m_result = genStk(VGetElem, m_ht.getCatchTrace(), cns((TCA)opFunc), + m_result = genStk(VGetElem, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr()); } #undef HELPER_TABLE @@ -1686,7 +1685,7 @@ void HhbcTranslator::VectorTranslator::emitIssetEmptyElem(bool isEmpty) { typedef uint64_t (*OpFunc)(TypedValue*, TypedValue, MInstrState*); BUILD_OPTAB_HOT(getKeyTypeIS(key), isEmpty); - m_result = gen(isEmpty ? EmptyElem : IssetElem, m_ht.getCatchTrace(), + m_result = gen(isEmpty ? EmptyElem : IssetElem, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr()); } #undef HELPER_TABLE @@ -1734,7 +1733,7 @@ void HhbcTranslator::VectorTranslator::emitArrayIsset() { typedef uint64_t (*OpFunc)(ArrayData*, TypedValue*); BUILD_OPTAB(keyType, checkForInt); assert(m_base->isA(Type::Arr)); - m_result = gen(ArrayIsset, m_ht.getCatchTrace(), + m_result = gen(ArrayIsset, getCatchTrace(), cns((TCA)opFunc), m_base, key); } #undef HELPER_TABLE @@ -1930,7 +1929,7 @@ void HhbcTranslator::VectorTranslator::emitSetWithRefLElem() { emitSetElem(); assert(m_strTestResult == nullptr); } else { - genStk(SetWithRefElem, m_ht.getCatchTrace(), + genStk(SetWithRefElem, getCatchTrace(), cns((TCA)VectorHelpers::setWithRefElemC), m_base, key, locAddr, genMisPtr()); } @@ -1954,7 +1953,7 @@ void HhbcTranslator::VectorTranslator::emitSetWithRefNewElem() { getValue()->type().notBoxed()) { emitSetNewElem(); } else { - genStk(SetWithRefNewElem, m_ht.getCatchTrace(), + genStk(SetWithRefNewElem, getCatchTrace(), cns((TCA)VectorHelpers::setWithRefNewElem), m_base, getValAddr(), genMisPtr()); } @@ -1971,7 +1970,7 @@ static inline void vectorSet(c_Vector* vec, void HhbcTranslator::VectorTranslator::emitVectorSet( SSATmp* key, SSATmp* value) { - gen(VectorSet, m_ht.getCatchTrace(), + gen(VectorSet, getCatchTrace(), cns((TCA)VectorHelpers::vectorSet), m_base, key, value); m_result = value; } @@ -2006,7 +2005,7 @@ void HhbcTranslator::VectorTranslator::emitMapSet( typedef TypedValue (*OpFunc)(c_Map*, TypedValue*, TypedValue*); BUILD_OPTAB_HOT(keyType); - gen(MapSet, m_ht.getCatchTrace(), + gen(MapSet, getCatchTrace(), cns((TCA)opFunc), m_base, key, value); m_result = value; } @@ -2054,7 +2053,7 @@ void HhbcTranslator::VectorTranslator::emitSetElem() { // Emit the appropriate helper call. typedef StringData* (*OpFunc)(TypedValue*, TypedValue, Cell); BUILD_OPTAB_HOT(getKeyTypeIS(key)); - m_failedSetTrace = m_ht.getCatchTrace(); + m_failedSetTrace = getCatchSetTrace(); SSATmp* result = genStk(SetElem, m_failedSetTrace, cns((TCA)opFunc), m_base, key, value); auto t = result->type(); @@ -2114,7 +2113,7 @@ void HhbcTranslator::VectorTranslator::emitSetOpElem() { BUILD_OPTAB_ARG(SETOP_OPS, op); # undef SETOP_OP m_result = - genStk(SetOpElem, m_ht.getCatchTrace(), cns((TCA)opFunc), + genStk(SetOpElem, getCatchTrace(), cns((TCA)opFunc), m_base, key, getValue(), genMisPtr()); } #undef HELPER_TABLE @@ -2148,7 +2147,7 @@ void HhbcTranslator::VectorTranslator::emitIncDecElem() { # define INCDEC_OP(op) HELPER_TABLE(FILL_ROW, op) BUILD_OPTAB_ARG(INCDEC_OPS, op); # undef INCDEC_OP - m_result = genStk(IncDecElem, m_ht.getCatchTrace(), cns((TCA)opFunc), + m_result = genStk(IncDecElem, getCatchTrace(), cns((TCA)opFunc), m_base, key, genMisPtr()); } #undef HELPER_TABLE @@ -2166,7 +2165,7 @@ void bindElemC(TypedValue* base, TypedValue keyVal, RefData* val, void HhbcTranslator::VectorTranslator::emitBindElem() { SSATmp* key = getKey(); SSATmp* box = getValue(); - genStk(BindElem, m_ht.getCatchTrace(), cns((TCA)VectorHelpers::bindElemC), + genStk(BindElem, getCatchTrace(), cns((TCA)VectorHelpers::bindElemC), m_base, key, box, genMisPtr()); m_result = box; } @@ -2210,7 +2209,7 @@ void HhbcTranslator::VectorTranslator::emitUnsetElem() { typedef void (*OpFunc)(TypedValue*, TypedValue); BUILD_OPTAB_HOT(getKeyTypeIS(key)); - genStk(UnsetElem, m_ht.getCatchTrace(), cns((TCA)opFunc), m_base, key); + genStk(UnsetElem, getCatchTrace(), cns((TCA)opFunc), m_base, key); } #undef HELPER_TABLE @@ -2224,8 +2223,7 @@ void HhbcTranslator::VectorTranslator::emitVGetNewElem() { void HhbcTranslator::VectorTranslator::emitSetNewElem() { SSATmp* value = getValue(); - m_failedSetTrace = m_ht.getCatchTrace(); - gen(SetNewElem, m_failedSetTrace, m_base, value); + gen(SetNewElem, getCatchSetTrace(), m_base, value); m_result = value; } @@ -2239,7 +2237,7 @@ void HhbcTranslator::VectorTranslator::emitIncDecNewElem() { void HhbcTranslator::VectorTranslator::emitBindNewElem() { SSATmp* box = getValue(); - genStk(BindNewElem, m_ht.getCatchTrace(), m_base, box, genMisPtr()); + genStk(BindNewElem, getCatchTrace(), m_base, box, genMisPtr()); m_result = box; } @@ -2299,15 +2297,17 @@ void HhbcTranslator::VectorTranslator::emitMPost() { m_ni.mInstrOp() == OpSetWithRefRM); } - // Clean up tvRef(2) + // Clean up tvRef(2): during exception handling any objects required only + // during vector expansion need to be DecRef'd. There may be either one + // or two such scratch objects, in the case of a Set the first of which will + // always be tvRef2, in all other cases if only one scratch value is present + // it will be stored in tvRef. static const size_t refOffs[] = { HHIR_MISOFF(tvRef), HHIR_MISOFF(tvRef2) }; for (unsigned i = 0; i < std::min(nLogicalRatchets(), 2U); ++i) { IRInstruction* inst = m_irf.gen(DecRefMem, Type::Gen, m_misBase, - cns(refOffs[i])); + cns(refOffs[m_failedSetTrace ? 1 - i : i])); m_tb.add(inst); - if (m_failedSetTrace) { - m_failedSetTrace->back()->push_back(inst->clone(&m_irf)); - } + prependToTraces(inst); } emitSideExits(catchSp, nStack); diff --git a/hphp/test/quick/vector-unwind-decref.php b/hphp/test/quick/vector-unwind-decref.php new file mode 100644 index 000000000..0e32b64ae --- /dev/null +++ b/hphp/test/quick/vector-unwind-decref.php @@ -0,0 +1,94 @@ +prop->blah = 'something'; + + + echo "calling c.__get() and logger.__get()\n"; + $x = $o->porp->halb; + echo "got value " . $x . "\n"; + + echo "creating new d\n"; + $b = new d(); + echo "calling d.__get() and logger2.__set()\n"; + $b->fake->anotherfake = 'ello'; + # exception! +} + +try { + echo "calling main\n"; + main(); +} catch (Exception $e) { + echo "Caught exception\n"; +} + +echo "last line\n"; diff --git a/hphp/test/quick/vector-unwind-decref.php.expect b/hphp/test/quick/vector-unwind-decref.php.expect new file mode 100644 index 000000000..710b4b03b --- /dev/null +++ b/hphp/test/quick/vector-unwind-decref.php.expect @@ -0,0 +1,26 @@ +calling main +in main +creating new c +c constructing +calling c.__get() and logger.__set() +returning new logger +logger constructing +set was called +logger destructing +calling c.__get() and logger.__get() +returning new logger +logger constructing +get was called +logger destructing +got value 10 +creating new d +d constructing +calling d.__get() and logger2.__set() +returning new logger2 +logger2 constructing +set was called, throwing an exception +logger2 destructing +d destructing +c destructing +Caught exception +last line