Make stack walking logic from Stack::toString reusable

I need to walk a live eval stack, handling FPI regions and
generators, and duplicating that logic in yet another place seems
wrong.  This makes the Stack::toString stuff use a reusable one---at
some point I'll see if the unwinder can use it as well, but it works
also for my use case.  Also, Stack::toString was broken for mutable
array iterators---fixes that.  And use a union now for struct Iter.
And don't include bytecode.h from func.h.

hphp/compiler/analysis/emitter.cpp

@@ -13,8 +13,8 @@
    | license@php.net so we can mail you a copy immediately.               |
    +----------------------------------------------------------------------+
 */
-
 #include "hphp/compiler/analysis/emitter.h"
+
 #include "hphp/compiler/builtin_symbols.h"
 #include "hphp/compiler/analysis/class_scope.h"
 #include "hphp/compiler/analysis/file_scope.h"
@@ -6448,17 +6448,25 @@ EmitterVisitor::emitBreakHandler(Emitter& e,
   }
 }
 
+namespace {
+
 class ForeachIterGuard {
   EmitterVisitor& m_ev;
  public:
-  ForeachIterGuard(EmitterVisitor& ev, Id iterId, bool itRef) : m_ev(ev) {
-    m_ev.pushIterScope(iterId, itRef);
+  ForeachIterGuard(EmitterVisitor& ev,
+                   Id iterId,
+                   EmitterVisitor::IterKind kind)
+    : m_ev(ev)
+  {
+    m_ev.pushIterScope(iterId, kind);
   }
   ~ForeachIterGuard() {
     m_ev.popIterScope();
   }
 };
 
+}
+
 void EmitterVisitor::emitForeach(Emitter& e, ForEachStatementPtr fe) {
   ExpressionPtr ae(fe->getArrayExp());
   ExpressionPtr val(fe->getValueExp());
@@ -6474,7 +6482,7 @@ void EmitterVisitor::emitForeach(Emitter& e, ForEachStatementPtr fe) {
   Label start;
   Offset bIterStart;
   Id itId = m_curFunc->allocIterator();
-  ForeachIterGuard fig(*this, itId, strong);
+  ForeachIterGuard fig(*this, itId, strong ? KindOfMIter : KindOfIter);
   bool simpleCase = (!key || isNormalLocalVariable(key)) &&
                     isNormalLocalVariable(val);
 
@@ -6612,7 +6620,7 @@ void EmitterVisitor::emitForeach(Emitter& e, ForEachStatementPtr fe) {
     }
   }
   newFaultRegion(bIterStart, m_ue.bcPos(), new IterFreeThunklet(itId, strong),
-                 itId);
+                 { itId, strong ? KindOfMIter : KindOfIter });
   if (needBreakHandler) {
     e.Jmp(exit);
     IterKind itKind = strong ? KindOfMIter : KindOfIter;
@@ -6686,6 +6694,7 @@ void EmitterVisitor::emitMakeUnitFatal(Emitter& e, const std::string& msg) {
 void EmitterVisitor::addFunclet(Thunklet* body, Label* entry) {
   m_funclets.push_back(Funclet(body, entry));
 }
+
 void EmitterVisitor::emitFunclets(Emitter& e) {
   while (!m_funclets.empty()) {
     Funclet& f = m_funclets.front();
@@ -6697,9 +6706,11 @@ void EmitterVisitor::emitFunclets(Emitter& e) {
   m_funclets.clear();
 }
 
-void EmitterVisitor::newFaultRegion(Offset start, Offset end, Thunklet* t,
-                                    Id iterId) {
-  FaultRegion* r = new FaultRegion(start, end, iterId);
+void EmitterVisitor::newFaultRegion(Offset start,
+                                    Offset end,
+                                    Thunklet* t,
+                                    FaultIterInfo iter) {
+  auto r = new FaultRegion(start, end, iter.iterId, iter.kind);
   m_faultRegions.push_back(r);
   addFunclet(t, &r->m_func);
 }
@@ -6727,15 +6738,16 @@ void EmitterVisitor::copyOverExnHandlers(FuncEmitter* fe) {
     delete *it;
   }
   m_exnHandlers.clear();
-  for (std::deque<FaultRegion*>::iterator it = m_faultRegions.begin();
-      it != m_faultRegions.end(); ++it) {
+
+  for (auto& fr : m_faultRegions) {
     EHEnt& e = fe->addEHEnt();
     e.m_ehtype = EHEnt::EHType_Fault;
-    e.m_base = (*it)->m_start;
-    e.m_past = (*it)->m_end;
-    e.m_iterId = (*it)->m_iterId;
-    e.m_fault = (*it)->m_func.getAbsoluteOffset();
-    delete *it;
+    e.m_base = fr->m_start;
+    e.m_past = fr->m_end;
+    e.m_iterId = fr->m_iterId;
+    e.m_itRef = fr->m_iterKind == KindOfMIter;
+    e.m_fault = fr->m_func.getAbsoluteOffset();
+    delete fr;
   }
   m_faultRegions.clear();
 }

hphp/compiler/analysis/emitter.h

@@ -379,15 +379,19 @@ public:
     KindOfMIter = 1
   };
 
-  void pushIterScope(Id id, bool itRef = false) {
-    IterKind itKind = itRef ? KindOfMIter : KindOfIter;
-    m_pendingIters.push_back(std::pair<Id,IterKind>(id,itKind));
+  void pushIterScope(Id id, IterKind kind) {
+    m_pendingIters.emplace_back(id, kind);
   }
   void popIterScope() { m_pendingIters.pop_back(); }
+
 private:
   typedef std::pair<StringData*, bool> ClosureUseVar;  // (name, byRef)
   typedef std::vector<ClosureUseVar> ClosureUseVarVec;
   typedef std::vector<std::pair<Id,IterKind> > PendingIterVec;
+  typedef std::pair<StringData*, ExpressionPtr> NonScalarPair;
+  typedef std::vector<NonScalarPair> NonScalarVec;
+  typedef std::pair<Id, int> StrCase;
+
   class PostponedMeth {
   public:
     PostponedMeth(MethodStatementPtr m, FuncEmitter* fe, bool top,
@@ -398,6 +402,7 @@ private:
     bool m_top;
     ClosureUseVarVec* m_closureUseVars;
   };
+
   class PostponedCtor {
   public:
     PostponedCtor(InterfaceStatementPtr is, FuncEmitter* fe)
@@ -405,8 +410,7 @@ private:
     InterfaceStatementPtr m_is;
     FuncEmitter* m_fe;
   };
-  typedef std::pair<StringData*, ExpressionPtr> NonScalarPair;
-  typedef std::vector<NonScalarPair> NonScalarVec;
+
   class PostponedNonScalars {
   public:
     PostponedNonScalars(InterfaceStatementPtr is, FuncEmitter* fe,
@@ -419,6 +423,7 @@ private:
     FuncEmitter* m_fe;
     NonScalarVec* m_vec;
   };
+
   class PostponedClosureCtor {
   public:
     PostponedClosureCtor(ClosureUseVarVec& v, ClosureExpressionPtr e,
@@ -428,6 +433,7 @@ private:
     ClosureExpressionPtr m_expr;
     FuncEmitter* m_fe;
   };
+
   class ControlTargets {
   public:
     ControlTargets(Id itId, bool itRef, Label& brkTarg, Label& cntTarg,
@@ -441,6 +447,7 @@ private:
     Label& m_brkHand;  // Push N and jump here for "break N;"
     Label& m_cntHand;  // Push N and jump here for "continue N;"
   };
+
   class ControlTargetPusher {
   public:
     ControlTargetPusher(EmitterVisitor* e, Id itId, bool itRef, Label& brkTarg,
@@ -454,6 +461,7 @@ private:
   private:
     EmitterVisitor* m_e;
   };
+
   class ExnHandlerRegion {
   public:
     ExnHandlerRegion(Offset start, Offset end) : m_start(start),
@@ -469,15 +477,23 @@ private:
     std::set<StringData*, string_data_lt> m_names;
     std::vector<std::pair<StringData*, Label*> > m_catchLabels;
   };
+
   class FaultRegion {
   public:
-    FaultRegion(Offset start, Offset end, Id iterId)
-     : m_start(start), m_end(end), m_iterId(iterId) {}
+    FaultRegion(Offset start, Offset end, Id iterId, IterKind kind)
+      : m_start(start)
+      , m_end(end)
+      , m_iterId(iterId)
+      , m_iterKind(kind)
+    {}
+
     Offset m_start;
     Offset m_end;
     Id m_iterId;
-    Label m_func;
+    IterKind m_iterKind;
+    Label m_func; // note: a pointer to this is handed out to the Funclet
   };
+
   class FPIRegion {
     public:
       FPIRegion(Offset start, Offset end, Offset fpOff)
@@ -486,7 +502,7 @@ private:
       Offset m_end;
       Offset m_fpOff;
   };
-  typedef std::pair<Id, int> StrCase;
+
   struct SwitchState : private boost::noncopyable {
     SwitchState() : nonZeroI(-1), defI(-1) {}
     std::map<int64_t, int> cases; // a map from int (or litstr id) to case index
@@ -650,7 +666,14 @@ public:
 
   void addFunclet(Thunklet* body, Label* entry);
   void emitFunclets(Emitter& e);
-  void newFaultRegion(Offset start, Offset end, Thunklet* t, Id iter = -1);
+
+  struct FaultIterInfo {
+    Id iterId;
+    IterKind kind;
+  };
+  void newFaultRegion(Offset start, Offset end, Thunklet* t,
+                      FaultIterInfo = FaultIterInfo { -1, KindOfIter });
+
   void newFPIRegion(Offset start, Offset end, Offset fpOff);
   void copyOverExnHandlers(FuncEmitter* fe);
   void copyOverFPIRegions(FuncEmitter* fe);

hphp/runtime/base/array/array_iterator.h

@@ -469,13 +469,12 @@ class CufIter {
 };
 
 struct Iter {
-  ArrayIter& arr() {
-    return *(ArrayIter*)m_u;
-  }
-  MArrayIter& marr() {
-    return *(MArrayIter*)m_u;
-  }
-  CufIter& cuf() { return *(CufIter*)m_u; }
+  const ArrayIter&   arr() const { return m_u.aiter; }
+  const MArrayIter& marr() const { return m_u.maiter; }
+  const CufIter&     cuf() const { return m_u.cufiter; }
+        ArrayIter&   arr()       { return m_u.aiter; }
+        MArrayIter& marr()       { return m_u.maiter; }
+        CufIter&     cuf()       { return m_u.cufiter; }
 
   bool init(TypedValue* c1);
   bool minit(TypedValue* v1);
@@ -484,10 +483,14 @@ struct Iter {
   void free();
   void mfree();
   void cfree();
-  private:
-  // C++ won't let you have union members with constructors. So we get to
-  // implement unions by hand.
-  char m_u[MAX(MAX(sizeof(ArrayIter), sizeof(MArrayIter)), sizeof(CufIter))];
+
+private:
+  union Data {
+    Data() {}
+    ArrayIter aiter;
+    MArrayIter maiter;
+    CufIter cufiter;
+  } m_u;
 } __attribute__ ((aligned(16)));
 
 bool interp_init_iterator(Iter* it, TypedValue* c1);

hphp/runtime/vm/bytecode.cpp

@@ -13,8 +13,15 @@
    | license@php.net so we can mail you a copy immediately.               |
    +----------------------------------------------------------------------+
 */
-
 #include "hphp/runtime/vm/bytecode.h"
+
+#include <algorithm>
+#include <string>
+#include <vector>
+#include <sstream>
+
+#include "folly/String.h"
+
 #include "hphp/compiler/builtin_symbols.h"
 #include "hphp/runtime/vm/event_hook.h"
 #include "hphp/runtime/vm/jit/translator-x64.h"
@@ -654,27 +661,28 @@ void flush_evaluation_stack() {
   }
 }
 
-void Stack::toStringElm(std::ostream& os, TypedValue* tv, const ActRec* fp)
-  const {
+static std::string toStringElm(const TypedValue* tv) {
+  std::ostringstream os;
+
   if (tv->m_type < MinDataType || tv->m_type > MaxNumDataTypes) {
     os << " ??? type " << tv->m_type << "\n";
-    return;
+    return os.str();
   }
+
   assert(tv->m_type >= MinDataType && tv->m_type < MaxNumDataTypes);
   if (IS_REFCOUNTED_TYPE(tv->m_type) && tv->m_data.pref->_count <= 0) {
     // OK in the invoking frame when running a destructor.
     os << " ??? inner_count " << tv->m_data.pref->_count << " ";
-    return;
+    return os.str();
   }
+
   switch (tv->m_type) {
   case KindOfRef:
     os << "V:(";
     os << "@" << tv->m_data.pref;
-    tv = tv->m_data.pref->tv();  // Unbox so contents get printed below
-    assert(tv->m_type != KindOfRef);
-    toStringElm(os, tv, fp);
+    os << toStringElm(tv->m_data.pref->tv());
     os << ")";
-    return;
+    return os.str();
   case KindOfClass:
     os << "A:";
     break;
@@ -682,161 +690,83 @@ void Stack::toStringElm(std::ostream& os, TypedValue* tv, const ActRec* fp)
     os << "C:";
     break;
   }
+
   switch (tv->m_type) {
-  case KindOfUninit: {
-    os << "Undefined";
+  case KindOfUninit:
+    os << "Uninit";
     break;
-  }
-  case KindOfNull: {
+  case KindOfNull:
     os << "Null";
     break;
-  }
-  case KindOfBoolean: {
+  case KindOfBoolean:
     os << (tv->m_data.num ? "True" : "False");
     break;
-  }
-  case KindOfInt64: {
+  case KindOfInt64:
     os << "0x" << std::hex << tv->m_data.num << std::dec;
     break;
-  }
-  case KindOfDouble: {
+  case KindOfDouble:
     os << tv->m_data.dbl;
     break;
-  }
   case KindOfStaticString:
-  case KindOfString: {
-    int len = tv->m_data.pstr->size();
-    bool truncated = false;
-    if (len > 128) {
-      len = 128;
-      truncated = true;
+  case KindOfString:
+    {
+      int len = tv->m_data.pstr->size();
+      bool truncated = false;
+      if (len > 128) {
+        len = 128;
+        truncated = true;
+      }
+      os << tv->m_data.pstr
+         << "c(" << tv->m_data.pstr->getCount() << ")"
+         << ":\""
+         << Util::escapeStringForCPP(tv->m_data.pstr->data(), len)
+         << "\"" << (truncated ? "..." : "");
     }
-    os << tv->m_data.pstr
-       << "c(" << tv->m_data.pstr->getCount() << ")"
-       << ":\""
-       << Util::escapeStringForCPP(tv->m_data.pstr->data(), len)
-       << "\"" << (truncated ? "..." : "");
     break;
-  }
-  case KindOfArray: {
+  case KindOfArray:
     assert(tv->m_data.parr->getCount() > 0);
     os << tv->m_data.parr
        << "c(" << tv->m_data.parr->getCount() << ")"
        << ":Array";
      break;
-  }
-  case KindOfObject: {
+  case KindOfObject:
     assert(tv->m_data.pobj->getCount() > 0);
     os << tv->m_data.pobj
        << "c(" << tv->m_data.pobj->getCount() << ")"
        << ":Object("
-       << tvAsVariant(tv).asObjRef().get()->o_getClassName().get()->data()
+       << tvAsCVarRef(tv).asCObjRef().get()->o_getClassName().get()->data()
        << ")";
     break;
-  }
-  case KindOfRef: {
+  case KindOfRef:
     not_reached();
-  }
-  case KindOfClass: {
+  case KindOfClass:
     os << tv->m_data.pcls
        << ":" << tv->m_data.pcls->name()->data();
     break;
-  }
-  default: {
+  default:
     os << "?";
     break;
   }
-  }
+
+  return os.str();
 }
 
-void Stack::toStringIter(std::ostream& os, Iter* it, bool itRef) const {
-  if (itRef) {
-    os << "I:MutableArray";
-    return;
-  }
+static std::string toStringIter(const Iter* it, bool itRef) {
+  if (itRef) return "I:MutableArray";
+
+  // TODO(#2458166): it might be a CufIter, but we're just lucky that
+  // the bit pattern for the CufIter is going to have a 0 in
+  // getIterType for now.
   switch (it->arr().getIterType()) {
-  case ArrayIter::TypeUndefined: {
-    os << "I:Undefined";
-    break;
-  }
-  case ArrayIter::TypeArray: {
-    os << "I:Array";
-    break;
-  }
-  case ArrayIter::TypeIterator: {
-    os << "I:Iterator";
-    break;
-  }
-  default: {
-    assert(false);
-    os << "I:?";
-    break;
-  }
-  }
-}
-
-void Stack::toStringFrag(std::ostream& os, const ActRec* fp,
-                         const TypedValue* top) const {
-  TypedValue* tv;
-
-  // The only way to figure out which stack elements are activation records is
-  // to follow the frame chain. However, the goal for each stack frame is to
-  // print stack fragments from deepest to shallowest -- a then b in the
-  // following example:
-  //
-  //   {func:foo,soff:51}<C:8> {func:bar} C:8 C:1 {func:biz} C:0
-  //                           aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb
-  //
-  // Use depth-first recursion to get the output order correct.
-
-  if (LIKELY(!fp->m_func->isGenerator())) {
-    tv = frameStackBase(fp);
-  } else {
-    tv = generatorStackBase(fp);
-  }
-
-  for (tv--; (uintptr_t)tv >= (uintptr_t)top; tv--) {
-    os << " ";
-    toStringElm(os, tv, fp);
-  }
-}
-
-void Stack::toStringAR(std::ostream& os, const ActRec* fp,
-                       const FPIEnt *fe, const TypedValue* top) const {
-  ActRec *ar;
-  if (LIKELY(!fp->m_func->isGenerator())) {
-    ar = arAtOffset(fp, -fe->m_fpOff);
-  } else {
-    // Deal with generators' split stacks. See unwindAR for reasoning.
-    TypedValue* genStackBase = generatorStackBase(fp);
-    ActRec* fakePrevFP =
-      (ActRec*)(genStackBase + fp->m_func->numSlotsInFrame());
-    ar = arAtOffset(fakePrevFP, -fe->m_fpOff);
-  }
-
-  if (fe->m_parentIndex != -1) {
-    toStringAR(os, fp, &fp->m_func->fpitab()[fe->m_parentIndex],
-      (TypedValue*)&ar[1]);
-  } else {
-    toStringFrag(os, fp, (TypedValue*)&ar[1]);
-  }
-
-  os << " {func:" << ar->m_func->fullName()->data() << "}";
-  TypedValue* tv = (TypedValue*)ar;
-  for (tv--; (uintptr_t)tv >= (uintptr_t)top; tv--) {
-    os << " ";
-    toStringElm(os, tv, fp);
-  }
-}
-
-void Stack::toStringFragAR(std::ostream& os, const ActRec* fp,
-                           int offset, const TypedValue* top) const {
-  const FPIEnt *fe = fp->m_func->findFPI(offset);
-  if (fe != nullptr) {
-    toStringAR(os, fp, fe, top);
-  } else {
-    toStringFrag(os, fp, top);
+  case ArrayIter::TypeUndefined:
+    return "I:Undefined";
+  case ArrayIter::TypeArray:
+    return "I:Array";
+  case ArrayIter::TypeIterator:
+    return "I:Iterator";
   }
+  assert(false);
+  return "I:?";
 }
 
 void Stack::toStringFrame(std::ostream& os, const ActRec* fp,
@@ -874,7 +804,7 @@ void Stack::toStringFrame(std::ostream& os, const ActRec* fp,
       if (i > 0) {
         os << " ";
       }
-      toStringElm(os, tv, fp);
+      os << toStringElm(tv);
     }
     os << ">";
   }
@@ -889,7 +819,7 @@ void Stack::toStringFrame(std::ostream& os, const ActRec* fp,
       }
       bool itRef;
       if (func->checkIterScope(offset, i, itRef)) {
-        toStringIter(os, it, itRef);
+        os << toStringIter(it, itRef);
       } else {
         os << "I:Undefined";
       }
@@ -897,13 +827,36 @@ void Stack::toStringFrame(std::ostream& os, const ActRec* fp,
     os << "|";
   }
 
-  toStringFragAR(os, fp, offset, ftop);
+  std::vector<std::string> stackElems;
+  visitStackElems(
+    fp, ftop, offset,
+    [&](const ActRec* ar) {
+      stackElems.push_back(
+        folly::format("{{func:{}}}", ar->m_func->fullName()->data()).str()
+      );
+    },
+    [&](const TypedValue* tv) {
+      stackElems.push_back(toStringElm(tv));
+    }
+  );
+  std::reverse(stackElems.begin(), stackElems.end());
+  os << ' ' << folly::join(' ', stackElems);
 
-  os << std::endl;
+  os << '\n';
 }
 
 string Stack::toString(const ActRec* fp, int offset,
                        const string prefix/* = "" */) const {
+  // The only way to figure out which stack elements are activation records is
+  // to follow the frame chain. However, the goal for each stack frame is to
+  // print stack fragments from deepest to shallowest -- a then b in the
+  // following example:
+  //
+  //   {func:foo,soff:51}<C:8> {func:bar} C:8 C:1 {func:biz} C:0
+  //                           aaaaaaaaaaaaaaaaaa bbbbbbbbbbbbbb
+  //
+  // Use depth-first recursion to get the output order correct.
+
   std::ostringstream os;
   os << prefix << "=== Stack at " << curUnit()->filepath()->data() << ":" <<
     curUnit()->getLineNumber(curUnit()->offsetOf(vmpc())) << " func " <<
@@ -1054,9 +1007,7 @@ void Stack::unwindAR(ActRec* fp, const FPIEnt* fe) {
     if (LIKELY(!fp->m_func->isGenerator())) {
       ar = arAtOffset(fp, -fe->m_fpOff);
     } else {
-      // fp is pointing into the continuation object. Since fpOff is given as an
-      // offset from the frame pointer as if it were in the normal place on the
-      // main stack, we have to reconstruct that "normal place".
+      // FIXME: duplicated logic from visitStackElems
       TypedValue* genStackBase = generatorStackBase(fp);
       ActRec* fakePrevFP =
         (ActRec*)(genStackBase + fp->m_func->numSlotsInFrame());
@@ -4645,7 +4596,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopRetC(PC& pc) {
 #ifdef HPHP_TRACE
     {
       std::ostringstream os;
-      m_stack.toStringElm(os, m_stack.topTV(), m_fp);
+      os << toStringElm(m_stack.topTV());
       ONTRACE(1,
               Trace::trace("Return %s from VMExecutionContext::dispatch("
                            "%p)\n", os.str().c_str(), m_fp));
@@ -6975,7 +6926,7 @@ inline void OPTBLD_INLINE VMExecutionContext::iopNativeImpl(PC& pc) {
 #ifdef HPHP_TRACE
     {
       std::ostringstream os;
-      m_stack.toStringElm(os, m_stack.topTV(), m_fp);
+      os << toStringElm(m_stack.topTV());
       ONTRACE(1,
               Trace::trace("Return %s from VMExecutionContext::dispatch("
                            "%p)\n", os.str().c_str(), m_fp));

hphp/runtime/vm/bytecode.h

@@ -17,6 +17,8 @@
 #ifndef incl_HPHP_VM_BYTECODE_H_
 #define incl_HPHP_VM_BYTECODE_H_
 
+#include <type_traits>
+
 #include <boost/optional.hpp>
 
 #include "hphp/util/util.h"
@@ -28,6 +30,7 @@
 #include "hphp/runtime/vm/class.h"
 #include "hphp/runtime/vm/instance.h"
 #include "hphp/runtime/vm/unit.h"
+#include "hphp/runtime/vm/func.h"
 #include "hphp/runtime/vm/name_value_table.h"
 #include "hphp/runtime/vm/request_arena.h"
 
@@ -55,13 +58,12 @@ namespace HPHP {
   }                                                                           \
 } while (0)
 
- class Func;
+class Func;
 class ActRec;
 
 // max number of arguments for direct call to builtin
 const int kMaxBuiltinArgs = 5;
 
-
 struct ExtraArgs : private boost::noncopyable {
   /*
    * Allocate an ExtraArgs structure, with arguments copied from the
@@ -194,7 +196,7 @@ class VarEnv {
   TypedValue* getExtraArg(unsigned argInd) const;
 };
 
-/**
+/*
  * An "ActRec" is a call activation record. The ordering of the fields assumes
  * that stacks grow toward lower addresses.
  *
@@ -494,7 +496,6 @@ enum UnwindStatus {
 
 // Interpreter evaluation stack.
 class Stack {
-private:
   TypedValue* m_elms;
   TypedValue* m_top;
   TypedValue* m_base; // Stack grows down, so m_base is beyond the end of
@@ -506,20 +507,12 @@ public:
   bool isValidAddress(uintptr_t v) {
     return v >= uintptr_t(m_elms) && v < uintptr_t(m_base);
   }
-  void toStringElm(std::ostream& os, TypedValue* vv, const ActRec* fp)
-    const;
-  void toStringIter(std::ostream& os, Iter* it, bool itRef) const;
   void protect();
   void unprotect();
   void requestInit();
   void requestExit();
+
 private:
-  void toStringFrag(std::ostream& os, const ActRec* fp,
-                    const TypedValue* top) const;
-  void toStringAR(std::ostream& os, const ActRec* fp,
-                  const FPIEnt *fe, const TypedValue* top) const;
-  void toStringFragAR(std::ostream& os, const ActRec* fp,
-                    int offset, const TypedValue* top) const;
   void toStringFrame(std::ostream& os, const ActRec* fp,
                      int offset, const TypedValue* ftop,
                      const std::string& prefix) const;
@@ -827,7 +820,71 @@ public:
   }
 };
 
-///////////////////////////////////////////////////////////////////////////////
+//////////////////////////////////////////////////////////////////////
+
+/*
+ * Visit all the slots and pre-live ActRecs on a live eval stack,
+ * handling FPI regions and generators correctly, and stopping when we
+ * reach the supplied activation record.
+ *
+ * The stack elements are visited from lower address to higher, with
+ * ActRecs visited after the stack slots below them.
+ *
+ * This will not read the VM registers (pc, fp, sp), so it will
+ * perform the requested visitation independent of modifications to
+ * the VM stack or frame pointer.
+ */
+template<class MaybeConstTVPtr, class ARFun, class TVFun>
+typename std::enable_if<
+  std::is_same<MaybeConstTVPtr,const TypedValue*>::value ||
+  std::is_same<MaybeConstTVPtr,      TypedValue*>::value
+>::type
+visitStackElems(const ActRec* const fp,
+                MaybeConstTVPtr const stackTop,
+                Offset const bcOffset,
+                ARFun arFun,
+                TVFun tvFun) {
+  const TypedValue* const base =
+    fp->m_func->isGenerator() ? Stack::generatorStackBase(fp)
+                              : Stack::frameStackBase(fp);
+  MaybeConstTVPtr cursor = stackTop;
+  assert(cursor <= base);
+
+  if (auto fe = fp->m_func->findFPI(bcOffset)) {
+    for (;;) {
+      ActRec* ar;
+      if (!fp->m_func->isGenerator()) {
+        ar = arAtOffset(fp, -fe->m_fpOff);
+      } else {
+        // fp is pointing into the continuation object. Since fpOff is
+        // given as an offset from the frame pointer as if it were in
+        // the normal place on the main stack, we have to reconstruct
+        // that "normal place".
+        auto const fakePrevFP = reinterpret_cast<const ActRec*>(
+          base + fp->m_func->numSlotsInFrame()
+        );
+        ar = arAtOffset(fakePrevFP, -fe->m_fpOff);
+      }
+
+      assert(cursor <= reinterpret_cast<TypedValue*>(ar));
+      while (cursor < reinterpret_cast<TypedValue*>(ar)) {
+        tvFun(cursor++);
+      }
+      arFun(ar);
+
+      cursor += kNumActRecCells;
+      if (fe->m_parentIndex == -1) break;
+      fe = &fp->m_func->fpitab()[fe->m_parentIndex];
+    }
+  }
+
+  while (cursor < base) {
+    tvFun(cursor++);
+  }
 }
 
-#endif // __VM_BYTECODE_H__
+///////////////////////////////////////////////////////////////////////////////
+
+}
+
+#endif

hphp/runtime/vm/func.cpp

@@ -32,10 +32,11 @@
 #include "hphp/runtime/vm/blob_helper.h"
 #include "hphp/runtime/vm/func_inline.h"
 #include "hphp/system/lib/systemlib.h"
+#include "hphp/runtime/vm/bytecode.h"
 
 namespace HPHP {
 
-static const Trace::Module TRACEMOD = Trace::bcinterp;
+static const Trace::Module TRACEMOD = Trace::hhbc;
 const StringData* Func::s___call = StringData::GetStaticString("__call");
 const StringData* Func::s___callStatic =
   StringData::GetStaticString("__callStatic");
@@ -296,6 +297,10 @@ void Func::rename(const StringData* name) {
   Unit::loadFunc(this);
 }
 
+int Func::numSlotsInFrame() const {
+  return shared()->m_numLocals + shared()->m_numIterators * kNumIterCells;
+}
+
 bool Func::checkIterScope(Offset o, Id iterId, bool& itRef) const {
   const EHEntVec& ehtab = shared()->m_ehtab;
   assert(o >= base() && o < past());

hphp/runtime/vm/func.h

@@ -17,16 +17,17 @@
 #ifndef incl_HPHP_VM_FUNC_H_
 #define incl_HPHP_VM_FUNC_H_
 
-#include "hphp/runtime/vm/bytecode.h"
 #include "hphp/runtime/vm/type_constraint.h"
 #include "hphp/runtime/vm/repo_helpers.h"
 #include "hphp/runtime/vm/indexed_string_map.h"
 #include "hphp/runtime/base/intercept.h"
+#include "hphp/runtime/base/class_info.h"
 
 namespace HPHP {
 
 const int kNumFixedPrologues = 6;
 
+struct ActRec;
 typedef TypedValue*(*BuiltinFunction)(ActRec* ar);
 
 /*
@@ -163,9 +164,7 @@ struct Func {
   void setNewFuncId();
 
   void rename(const StringData* name);
-  int numSlotsInFrame() const {
-    return shared()->m_numLocals + shared()->m_numIterators * kNumIterCells;
-  }
+  int numSlotsInFrame() const;
   Id lookupVarId(const StringData* name) const;
 
   /*