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

@@ -473,12 +473,12 @@ private:
                     MemberCode& mcode, TypedValue*& curMember);
   template <bool saveResult>
   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 <bool warn, bool define, bool unset, bool reffy, unsigned mdepth,
@@ -489,8 +489,8 @@ private:
                     TypedValue& tvRef, TypedValue& tvRef2,
                     MemberCode& mcode, TypedValue*& curMember);
   template <unsigned mdepth>
-  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);

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<false, warn, false, false,
-    false, 0, mleave, saveResult>(MEMBERHELPERPRE_ARGS);
+    false, 0, mleave, saveResult>(MEMBERHELPERPRE_OUT);
 }
 
 #define GETHELPERPOST_ARGS ndiscard, tvRet, tvScratch, tvRef, tvRef2
 template <bool saveResult>
 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<true, true, ConsumeAll>(MEMBERHELPERPRE_ARGS);
@@ -3647,13 +3649,13 @@ inline bool OPTBLD_INLINE VMExecutionContext::setHelperPre(
     TypedValue& tvRef, TypedValue& tvRef2,
     MemberCode& mcode, TypedValue*& curMember) {
   return memberHelperPre<true, warn, define, unset,
-    reffy, mdepth, mleave, false>(MEMBERHELPERPRE_ARGS);
+    reffy, mdepth, mleave, false>(MEMBERHELPERPRE_OUT);
 }
 
 #define SETHELPERPOST_ARGS ndiscard, tvRef, tvRef2
 template <unsigned mdepth>
 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<false>(tvScratch, tvRef, base, curMember);
+    issetResult = IssetEmptyElem<false>(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<true>(tvScratch, tvRef, base, curMember);
+    emptyResult = IssetEmptyElem<true>(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<MoreWarnings, true, false, false, 0,
       LeaveLast>(MEMBERHELPERPRE_ARGS)) {
     if (mcode == MW) {
-      IncDecNewElem<true>(tvScratch, tvRef, op, base, to);
+      IncDecNewElem<true>(tvScratch, *tvRef.asTypedValue(), op, base, to);
     } else {
       switch (mcode) {
       case MEL:
       case MEC:
       case MET:
       case MEI:
-        IncDecElem<true>(tvScratch, tvRef, op, base, curMember, to);
+        IncDecElem<true>(tvScratch, *tvRef.asTypedValue(), op, base,
+            curMember, to);
         break;
       case MPL:
       case MPC:
       case MPT: {
         Class* ctx = arGetContextClass(m_fp);
-        IncDecProp<true>(tvScratch, tvRef, ctx, op, base, curMember, to);
+        IncDecProp<true>(tvScratch, *tvRef.asTypedValue(), ctx, op, base,
+            curMember, to);
         break;
       }
       default: assert(false);

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);
 }

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<IRTrace*> m_failedTraceVec;
   };
 
 private: // tracebuilder forwarding utilities

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<TCA>(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);

hphp/test/quick/vector-unwind-decref.php

@@ -0,0 +1,94 @@
+<?php
+// Copyright 2004-present Facebook. All Rights Reserved.
+
+class logger2 {
+  public function __construct() {
+    echo "logger2 constructing\n";
+  }
+
+  public function __destruct() {
+    echo "logger2 destructing\n";
+  }
+
+  public function __set($name, $value) {
+    echo "set was called, throwing an exception\n";
+    throw new Exception('nope');
+  }
+}
+
+class logger {
+  public function __construct() {
+    echo "logger constructing\n";
+  }
+
+  public function __destruct() {
+    echo "logger destructing\n";
+  }
+
+  public function __set($name, $value) {
+    echo "set was called\n";
+  }
+
+  public function __get($name) {
+    echo "get was called\n";
+    return 10;
+  }
+}
+
+class c {
+  public function __construct() {
+    echo "c constructing\n";
+  }
+
+  public function __destruct() {
+    echo "c destructing\n";
+  }
+
+  public function __get($name) {
+    echo "returning new logger\n";
+    return new logger;
+  }
+}
+
+class d {
+  public function __construct() {
+    echo "d constructing\n";
+  }
+
+  public function __destruct() {
+    echo "d destructing\n";
+  }
+
+  public function __get($name) {
+    echo "returning new logger2\n";
+    return new logger2;
+  }
+}
+
+function main() {
+  echo "in main\n";
+  echo "creating new c\n";
+  $o = new c();
+  echo "calling c.__get() and logger.__set()\n";
+  $o->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";

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