Merge DebuggerProxyVM into DebuggerProxy, InterruptSiteVM into InterruptSite

The separation between these classes was a vestige of days gone by. Combined them. I ran into issue with having the proxy split up in particular when working on stepping, so doing this now as a separate diff to keep things cleaner.

hphp/runtime/eval/debugger/break_point.cpp

@@ -74,12 +74,12 @@ std::string InterruptSite::desc() const {
   return ret;
 }
 
-///////////////////////////////////////////////////////////////////////////////
-
-InterruptSiteVM::InterruptSiteVM(bool hardBreakPoint /* = false */,
-                                 CVarRef e /* = null_variant */)
-  : InterruptSite(e), m_unit(nullptr), m_valid(false), m_funcEntry(false) {
-  TRACE(2, "InterruptSite::InterruptSiteVM\n");
+InterruptSite::InterruptSite(bool hardBreakPoint, CVarRef e)
+    : m_exception(e), m_class(nullptr), m_function(nullptr),
+      m_file((StringData*)nullptr),
+      m_line0(0), m_char0(0), m_line1(0), m_char1(0),
+      m_unit(nullptr), m_valid(false), m_funcEntry(false) {
+  TRACE(2, "InterruptSite::InterruptSite\n");
   VM::Transl::VMRegAnchor _;
 #define bail_on(c) if (c) { return; }
   VMExecutionContext* context = g_vmContext;
@@ -137,27 +137,6 @@ InterruptSiteVM::InterruptSiteVM(bool hardBreakPoint /* = false */,
   m_valid = true;
 }
 
-const char *InterruptSiteVM::getFile() const {
-  TRACE(2, "InterruptSiteVM::getFile\n");
-  return m_file.data();
-}
-
-const char *InterruptSiteVM::getClass() const {
-  TRACE(2, "InterruptSiteVM::getClass\n");
-  if (m_class) {
-    return m_class;
-  }
-  return "";
-}
-
-const char *InterruptSiteVM::getFunction() const {
-  TRACE(2, "InterruptSiteVM::getFunction\n");
-  if (m_function) {
-    return m_function;
-  }
-  return "";
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 const char *BreakPointInfo::ErrorClassName = "@";
@@ -367,10 +346,9 @@ bool BreakPointInfo::match(InterruptType interrupt, InterruptSite &site) {
           checkLines(site.getLine0()) && checkStack(site) &&
           checkUrl(site.url()) && checkClause();
 
-        InterruptSiteVM &siteVM = dynamic_cast<InterruptSiteVM&>(site);
         if (!getFuncName().empty()) {
           // function entry breakpoint
-          match = match && siteVM.funcEntry();
+          match = match && site.funcEntry();
         }
         return match;
       }
@@ -866,7 +844,7 @@ bool BreakPointInfo::checkClause() {
     String output;
     {
       EvalBreakControl eval(false);
-      Variant ret = DebuggerProxyVM::ExecutePHP(m_php, output, false, 0);
+      Variant ret = DebuggerProxy::ExecutePHP(m_php, output, false, 0);
       if (m_check) {
         return ret.toBoolean();
       }

hphp/runtime/eval/debugger/break_point.h

@@ -34,33 +34,38 @@ enum InterruptType {
 };
 
 // Represents a site in the code, at the source level.
-// Note: this class currently has just one subclass, InterruptSiteVM, which will
-// be combined with this shortly.
+// Forms an InterruptSite by looking at the current thread's current PC and
+// grabbing source data out of the corresponding Unit.
 class InterruptSite {
 public:
-  virtual const char *getFile() const = 0;
-  virtual const char *getClass() const = 0;
-  virtual const char *getFunction() const = 0;
-  virtual const char *getNamespace() const { return nullptr; }
+  InterruptSite(bool hardBreakPoint, CVarRef e);
+
+  const char *getFile() const { return m_file.data(); }
+  const char *getClass() const { return m_class ? m_class : ""; }
+  const char *getFunction() const { return m_function ? m_function : ""; }
+  // Placeholder for future namespace support.
+  const char *getNamespace() const { return nullptr; }
   int getFileLen() const;
 
-  int32_t getLine0() const { return m_line0;}
-  int32_t getChar0() const { return m_char0;}
-  int32_t getLine1() const { return m_line1;}
-  int32_t getChar1() const { return m_char1;}
-  CVarRef getException() { return m_exception;}
+  int32_t getLine0() const { return m_line0; }
+  int32_t getChar0() const { return m_char0; }
+  int32_t getLine1() const { return m_line1; }
+  int32_t getChar1() const { return m_char1; }
+  CVarRef getException() { return m_exception; }
 
-  std::string &url() const { return m_url;}
+  std::string &url() const { return m_url; }
   std::string desc() const;
 
-protected:
-  explicit InterruptSite(CVarRef e = null_variant, const char *cls = nullptr,
-                         const char *function = nullptr,
-                         StringData *file = nullptr, int line0 = 0,
-                         int char0 = 0, int line1 = 0, int char1 = 0)
-    : m_exception(e), m_class(cls), m_function(function), m_file(file),
-      m_line0(line0), m_char0(char0), m_line1(line1), m_char1(char1) {}
+  const VM::SourceLoc *getSourceLoc() const { return &m_sourceLoc; }
+  const VM::OffsetRangeVec& getCurOffsetRange() const {
+    return m_offsetRangeVec;
+  }
+  const VM::Unit* getUnit() const { return m_unit; }
 
+  bool valid() const { return m_valid; }
+  bool funcEntry() const { return m_funcEntry; }
+
+private:
   Variant m_exception;
 
   // cached
@@ -73,29 +78,7 @@ protected:
   int32_t m_char0;
   int32_t m_line1;
   int32_t m_char1;
-};
 
-// Forms an InterruptSite by looking at the current thread's current PC and
-// grabbing source data out of the corresponding Unit.
-class InterruptSiteVM : public InterruptSite {
-public:
-  explicit InterruptSiteVM(bool hardBreakPoint = false,
-                           CVarRef e = null_variant);
-  virtual const char *getFile() const;
-  virtual const char *getClass() const;
-  virtual const char *getFunction() const;
-  const VM::SourceLoc *getSourceLoc() const {
-    return &m_sourceLoc;
-  }
-  const VM::OffsetRangeVec& getCurOffsetRange() const {
-    return m_offsetRangeVec;
-  }
-  const VM::Unit* getUnit() const {
-    return m_unit;
-  }
-  bool valid() const { return m_valid; }
-  bool funcEntry() const { return m_funcEntry; }
-private:
   VM::SourceLoc m_sourceLoc;
   VM::OffsetRangeVec m_offsetRangeVec;
   VM::Unit* m_unit;

hphp/runtime/eval/debugger/cmd/cmd_eval.cpp

@@ -66,7 +66,7 @@ bool CmdEval::onServer(DebuggerProxy *proxy) {
   VM::PCFilter* locSave = g_vmContext->m_lastLocFilter;
   g_vmContext->m_lastLocFilter = new VM::PCFilter();
   g_vmContext->setDebuggerBypassCheck(m_bypassAccessCheck);
-  DebuggerProxyVM::ExecutePHP(m_body, m_output, !proxy->isLocal(), m_frame);
+  DebuggerProxy::ExecutePHP(m_body, m_output, !proxy->isLocal(), m_frame);
   g_vmContext->setDebuggerBypassCheck(false);
   delete g_vmContext->m_lastLocFilter;
   g_vmContext->m_lastLocFilter = locSave;

hphp/runtime/eval/debugger/cmd/cmd_instrument.cpp

@@ -134,16 +134,15 @@ void CmdInstrument::setClientOutput(DebuggerClient *client) {
 bool CmdInstrument::onServer(DebuggerProxy *proxy) {
   m_instPoints = &m_ips;
   m_enabled = true;
-  DebuggerProxyVM* proxyVM = static_cast<DebuggerProxyVM*>(proxy);
   if (m_type == ActionRead) {
-    readFromTable(proxyVM);
+    readFromTable(proxy);
   } else if (m_type == ActionWrite) {
-    validateAndWriteToTable(proxyVM);
+    validateAndWriteToTable(proxy);
   }
   return proxy->sendToClient(this);
 }
 
-void CmdInstrument::readFromTable(DebuggerProxyVM *proxy) {
+void CmdInstrument::readFromTable(DebuggerProxy *proxy) {
   proxy->readInjTablesFromThread();
   m_ips.clear();
   if (!proxy->getInjTables()) {
@@ -186,7 +185,7 @@ void CmdInstrument::readFromTable(DebuggerProxyVM *proxy) {
   }
 }
 
-void CmdInstrument::validateAndWriteToTable(DebuggerProxyVM *proxy) {
+void CmdInstrument::validateAndWriteToTable(DebuggerProxy *proxy) {
   if (!proxy->getInjTables()) {
     proxy->setInjTables(new VM::InjectionTables());
   }

hphp/runtime/eval/debugger/cmd/cmd_instrument.h

@@ -48,8 +48,8 @@ private:
   InstPointInfoPtrVec *m_instPoints;
   InstPointInfoPtrVec m_ips;
 
-  void readFromTable(DebuggerProxyVM *proxy);
-  void validateAndWriteToTable(DebuggerProxyVM *proxy);
+  void readFromTable(DebuggerProxy *proxy);
+  void validateAndWriteToTable(DebuggerProxy *proxy);
 
   void listInst(DebuggerClient *client);
   void clearInst(DebuggerClient *client);

hphp/runtime/eval/debugger/cmd/cmd_print.cpp

@@ -371,8 +371,8 @@ bool CmdPrint::onServer(DebuggerProxy *proxy) {
   g_vmContext->setDebuggerBypassCheck(m_bypassAccessCheck);
   {
     EvalBreakControl eval(m_noBreak);
-    m_ret = DebuggerProxyVM::ExecutePHP(DebuggerProxy::MakePHPReturn(m_body),
-                                        m_output, !proxy->isLocal(), m_frame);
+    m_ret = DebuggerProxy::ExecutePHP(DebuggerProxy::MakePHPReturn(m_body),
+                                      m_output, !proxy->isLocal(), m_frame);
   }
   g_vmContext->setDebuggerBypassCheck(false);
   delete g_vmContext->m_lastLocFilter;

hphp/runtime/eval/debugger/debugger.cpp

@@ -260,7 +260,7 @@ void Debugger::InterruptVMHook(int type /* = BreakPointReached */,
   TRACE(2, "Debugger::InterruptVMHook\n");
   // Computing the interrupt site here pulls in more data from the Unit to
   // describe the current execution point.
-  InterruptSiteVM site(type == HardBreakPoint, e);
+  InterruptSite site(type == HardBreakPoint, e);
   if (!site.valid()) {
     // An invalid site is missing something like an ActRec, a func, or a
     // Unit. Currently the debugger has no action to take at such sites.
@@ -359,8 +359,7 @@ void Debugger::registerSandbox(const DSandboxInfo &sandbox) {
   DebuggerProxyPtr proxy = findProxy(sid);
   if (proxy) {
     ti->m_reqInjectionData.debugger = true;
-    DebuggerProxyVM* proxyVM = (DebuggerProxyVM*)proxy.get();
-    proxyVM->writeInjTablesToThread();
+    proxy->writeInjTablesToThread();
   }
 }
 
@@ -416,7 +415,7 @@ DebuggerProxyPtr Debugger::createProxy(SmartPtr<Socket> socket, bool local) {
   TRACE(2, "Debugger::createProxy\n");
   // Creates a proxy and threads needed to handle it. At this point, there is
   // not enough information to attach a sandbox.
-  DebuggerProxyPtr proxy(new DebuggerProxyVM(socket, local));
+  DebuggerProxyPtr proxy(new DebuggerProxy(socket, local));
   const StringData* sid =
     StringData::GetStaticString(proxy->getDummyInfo().id());
   {

hphp/runtime/eval/debugger/debugger_proxy.cpp

@@ -33,7 +33,7 @@ DebuggerProxy::DebuggerProxy(SmartPtr<Socket> socket, bool local)
     : m_stopped(false), m_local(local), m_dummySandbox(nullptr),
       m_hasBreakPoints(false), m_threadMode(Normal), m_thread(0),
       m_signalThread(this, &DebuggerProxy::pollSignal),
-      m_signum(CmdSignal::SignalNone) {
+      m_signum(CmdSignal::SignalNone), m_injTables(nullptr) {
   TRACE(2, "DebuggerProxy::DebuggerProxy\n");
   m_thrift.create(socket);
   m_dummyInfo = DSandboxInfo::CreateDummyInfo((int64_t)this);
@@ -47,6 +47,8 @@ DebuggerProxy::~DebuggerProxy() {
   if (m_dummySandbox) {
     m_dummySandbox->stop();
   }
+
+  delete m_injTables;
 }
 
 const char *DebuggerProxy::getThreadType() const {
@@ -151,29 +153,34 @@ void DebuggerProxy::notifyDummySandbox() {
 // class name into separate containers for later.
 void DebuggerProxy::setBreakPoints(BreakPointInfoPtrVec &breakpoints) {
   TRACE(2, "DebuggerProxy::setBreakPoints\n");
-  WriteLock lock(m_breakMutex);
-  m_breakpoints = breakpoints;
-  m_hasBreakPoints = !m_breakpoints.empty();
-  m_breaksEnterFunc.clear();
-  m_breaksEnterClsMethod.clear();
-  for (unsigned int i = 0; i < m_breakpoints.size(); i++) {
-    BreakPointInfoPtr bp = m_breakpoints[i];
-    std::string funcFullName = bp->getFuncName();
-    if (funcFullName.empty()) {
-      continue;
-    }
-    {
-      StringDataMap::accessor acc;
-      const StringData* sd = StringData::GetStaticString(funcFullName);
-      m_breaksEnterFunc.insert(acc, sd);
-    }
-    std::string clsName = bp->getClass();
-    if (!clsName.empty()) {
-      StringDataMap::accessor acc;
-      const StringData* sd = StringData::GetStaticString(clsName);
-      m_breaksEnterClsMethod.insert(acc, sd);
+  // Hold the break mutex while we update the proxy's state. There's no need
+  // to hold it over the longer operation to set breakpoints in each file later.
+  {
+    WriteLock lock(m_breakMutex);
+    m_breakpoints = breakpoints;
+    m_hasBreakPoints = !m_breakpoints.empty();
+    m_breaksEnterFunc.clear();
+    m_breaksEnterClsMethod.clear();
+    for (unsigned int i = 0; i < m_breakpoints.size(); i++) {
+      BreakPointInfoPtr bp = m_breakpoints[i];
+      std::string funcFullName = bp->getFuncName();
+      if (funcFullName.empty()) {
+        continue;
+      }
+      {
+        StringDataMap::accessor acc;
+        const StringData* sd = StringData::GetStaticString(funcFullName);
+        m_breaksEnterFunc.insert(acc, sd);
+      }
+      std::string clsName = bp->getClass();
+      if (!clsName.empty()) {
+        StringDataMap::accessor acc;
+        const StringData* sd = StringData::GetStaticString(clsName);
+        m_breaksEnterClsMethod.insert(acc, sd);
+      }
     }
   }
+  VM::phpSetBreakPointsInAllFiles(this); // Apply breakpoints to the code.
 }
 
 void DebuggerProxy::getBreakPoints(BreakPointInfoPtrVec &breakpoints) {
@@ -229,10 +236,48 @@ bool DebuggerProxy::needInterruptForNonBreak() {
   return m_flow || m_signum != CmdSignal::SignalNone;
 }
 
-// Handle an interrupt from the VM. Note: some work for breakpoints has already
-// occured in DebuggerProxyVM::interrupt().
+// Handle an interrupt from the VM.
 void DebuggerProxy::interrupt(CmdInterrupt &cmd) {
   TRACE(2, "DebuggerProxy::interrupt\n");
+  changeBreakPointDepth(cmd);
+
+  if (cmd.getInterruptType() == BreakPointReached) {
+    if (!needInterrupt()) return;
+
+    // NB: stepping is represented as a BreakPointReached interrupt.
+
+    // Modify m_lastLocFilter to save current location. This will short-circuit
+    // the work done up in phpDebuggerOpcodeHook() and ensure we don't break on
+    // this line until we're completely off of it.
+    InterruptSite *site = cmd.getSite();
+    if (g_vmContext->m_lastLocFilter) {
+      g_vmContext->m_lastLocFilter->clear();
+    } else {
+      g_vmContext->m_lastLocFilter = new VM::PCFilter();
+    }
+    if (debug && Trace::moduleEnabled(Trace::bcinterp, 5)) {
+      Trace::trace("prepare source loc filter\n");
+      const VM::OffsetRangeVec& offsets = site->getCurOffsetRange();
+      for (VM::OffsetRangeVec::const_iterator it = offsets.begin();
+           it != offsets.end(); ++it) {
+        Trace::trace("block source loc in %s:%d: unit %p offset [%d, %d)\n",
+                     site->getFile(), site->getLine0(),
+                     site->getUnit(), it->m_base, it->m_past);
+      }
+    }
+    g_vmContext->m_lastLocFilter->addRanges(site->getUnit(),
+                                            site->getCurOffsetRange());
+    // If the breakpoint is not to be processed, we should continue execution.
+    BreakPointInfoPtr bp = getBreakPointAtCmd(cmd);
+    if (bp) {
+      if (!bp->breakable(getRealStackDepth())) {
+        return;
+      } else {
+        bp->unsetBreakable(getRealStackDepth());
+      }
+    }
+  }
+
   // Wait until this thread is the thread this proxy wants to debug.
   // NB: breakpoints and control flow checks happen here, too, and return false
   // if we're not done with the flow, or not at a breakpoint, etc.
@@ -382,26 +427,6 @@ static void append_stderr(const char *header, const char *msg,
   }
 }
 
-Variant DebuggerProxy::ExecutePHP(const std::string &php, String &output,
-                                  bool log, int frame) {
-  TRACE(2, "DebuggerProxy::ExecutePHP\n");
-  Variant ret;
-  StringBuffer sb;
-  StringBuffer *save = g_context->swapOutputBuffer(nullptr);
-  g_context->setStdout(append_stdout, &sb);
-  if (log) {
-    Logger::SetThreadHook(append_stderr, &sb);
-  }
-  ret = uninit_null();
-  g_context->setStdout(nullptr, nullptr);
-  g_context->swapOutputBuffer(save);
-  if (log) {
-    Logger::SetThreadHook(nullptr, nullptr);
-  }
-  output = sb.detach();
-  return ret;
-}
-
 DThreadInfoPtr DebuggerProxy::createThreadInfo(const std::string &desc) {
   TRACE(2, "DebuggerProxy::createThreadInfo\n");
   DThreadInfoPtr info(new DThreadInfo());
@@ -526,7 +551,7 @@ void DebuggerProxy::checkStop() {
 
 void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) {
   TRACE(2, "DebuggerProxy::processInterrupt\n");
-  // Do the server-side work for this cmd.
+  // Do the server-side work for this cmd, which just notifies the client.
   if (!cmd.onServer(this)) {
     Debugger::RemoveProxy(shared_from_this()); // on socket error
     return;
@@ -559,7 +584,7 @@ void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) {
       }
     }
     try {
-      // Perform the server-size work for this command.
+      // Perform the server-side work for this command.
       if (!res || !res->onServer(this)) {
         Debugger::RemoveProxy(shared_from_this());
         return;
@@ -578,22 +603,11 @@ void DebuggerProxy::processInterrupt(CmdInterrupt &cmd) {
   }
 }
 
-void DebuggerProxy::processFlowControl(CmdInterrupt &cmd) {
-  TRACE(2, "DebuggerProxy::processFlowControl\n");
-  const_assert(false);
-}
-
-bool DebuggerProxy::breakByFlowControl(CmdInterrupt &cmd) {
-  TRACE(2, "DebuggerProxy::breakByFlowControl\n");
-  const_assert(false);
-}
-
 ///////////////////////////////////////////////////////////////////////////////
-// DebuggerProxyVM
 
-Variant DebuggerProxyVM::ExecutePHP(const std::string &php, String &output,
-                                    bool log, int frame) {
-  TRACE(2, "DebuggerProxyVM::ExecutePHP\n");
+Variant DebuggerProxy::ExecutePHP(const std::string &php, String &output,
+                                  bool log, int frame) {
+  TRACE(2, "DebuggerProxy::ExecutePHP\n");
   Variant ret;
   StringBuffer sb;
   StringBuffer *save = g_context->swapOutputBuffer(nullptr);
@@ -632,8 +646,8 @@ Variant DebuggerProxyVM::ExecutePHP(const std::string &php, String &output,
 
 // There could be multiple breakpoints at one place but we can manage this
 // with only one breakpoint.
-BreakPointInfoPtr DebuggerProxyVM::getBreakPointAtCmd(CmdInterrupt& cmd) {
-  TRACE(2, "DebuggerProxyVM::getBreakPointAtCmd\n");
+BreakPointInfoPtr DebuggerProxy::getBreakPointAtCmd(CmdInterrupt& cmd) {
+  TRACE(2, "DebuggerProxy::getBreakPointAtCmd\n");
   for (unsigned int i = 0; i < m_breakpoints.size(); ++i) {
     BreakPointInfoPtr bp = m_breakpoints[i];
     if (bp->m_state != BreakPointInfo::Disabled &&
@@ -644,59 +658,8 @@ BreakPointInfoPtr DebuggerProxyVM::getBreakPointAtCmd(CmdInterrupt& cmd) {
   return BreakPointInfoPtr();
 }
 
-// Handle an interrupt from the VM.
-void DebuggerProxyVM::interrupt(CmdInterrupt &cmd) {
-  TRACE(2, "DebuggerProxyVM::interrupt\n");
-  changeBreakPointDepth(cmd);
-
-  if (cmd.getInterruptType() == BreakPointReached) {
-    if (!needInterrupt()) return;
-
-    // NB: stepping is represented as a BreakPointReached interrupt.
-
-    // Modify m_lastLocFilter to save current location. This will short-circuit
-    // the work done up in phpDebuggerOpcodeHook() and ensure we don't break on
-    // this line until we're completely off of it.
-    InterruptSiteVM *site = (InterruptSiteVM*)cmd.getSite();
-    if (g_vmContext->m_lastLocFilter) {
-      g_vmContext->m_lastLocFilter->clear();
-    } else {
-      g_vmContext->m_lastLocFilter = new VM::PCFilter();
-    }
-    if (debug && Trace::moduleEnabled(Trace::bcinterp, 5)) {
-      Trace::trace("prepare source loc filter\n");
-      const VM::OffsetRangeVec& offsets = site->getCurOffsetRange();
-      for (VM::OffsetRangeVec::const_iterator it = offsets.begin();
-           it != offsets.end(); ++it) {
-        Trace::trace("block source loc in %s:%d: unit %p offset [%d, %d)\n",
-                     site->getFile(), site->getLine0(),
-                     site->getUnit(), it->m_base, it->m_past);
-      }
-    }
-    g_vmContext->m_lastLocFilter->addRanges(site->getUnit(),
-                                            site->getCurOffsetRange());
-    // If the breakpoint is not to be processed, we should continue execution.
-    BreakPointInfoPtr bp = getBreakPointAtCmd(cmd);
-    if (bp) {
-      if (!bp->breakable(getRealStackDepth())) {
-        return;
-      } else {
-        bp->unsetBreakable(getRealStackDepth());
-      }
-    }
-  }
-
-  DebuggerProxy::interrupt(cmd);
-}
-
-void DebuggerProxyVM::setBreakPoints(BreakPointInfoPtrVec& breakpoints) {
-  TRACE(2, "DebuggerProxyVM::setBreakPoints\n");
-  DebuggerProxy::setBreakPoints(breakpoints); // Hold bp's in the proxy.
-  VM::phpSetBreakPointsInAllFiles(this); // Apply breakpoints to the code.
-}
-
-void DebuggerProxyVM::readInjTablesFromThread() {
-  TRACE(2, "DebuggerProxyVM::readInjTablesFromThread\n");
+void DebuggerProxy::readInjTablesFromThread() {
+  TRACE(2, "DebuggerProxy::readInjTablesFromThread\n");
   ThreadInfo* ti = ThreadInfo::s_threadInfo.getNoCheck();
   if (ti->m_reqInjectionData.dummySandbox) {
     return;
@@ -711,8 +674,8 @@ void DebuggerProxyVM::readInjTablesFromThread() {
   m_injTables = g_vmContext->m_injTables->clone();
 }
 
-void DebuggerProxyVM::writeInjTablesToThread() {
-  TRACE(2, "DebuggerProxyVM::writeInjTablesToThread\n");
+void DebuggerProxy::writeInjTablesToThread() {
+  TRACE(2, "DebuggerProxy::writeInjTablesToThread\n");
   if (g_vmContext->m_injTables) {
     delete g_vmContext->m_injTables;
     g_vmContext->m_injTables = nullptr;
@@ -723,8 +686,8 @@ void DebuggerProxyVM::writeInjTablesToThread() {
   g_vmContext->m_injTables = m_injTables->clone();
 }
 
-int DebuggerProxyVM::getRealStackDepth() {
-  TRACE(2, "DebuggerProxyVM::getRealStackDepth\n");
+int DebuggerProxy::getRealStackDepth() {
+  TRACE(2, "DebuggerProxy::getRealStackDepth\n");
   int depth = 0;
   VMExecutionContext* context = g_vmContext;
   ActRec *fp = context->getFP();
@@ -737,8 +700,8 @@ int DebuggerProxyVM::getRealStackDepth() {
   return depth;
 }
 
-int DebuggerProxyVM::getStackDepth() {
-  TRACE(2, "DebuggerProxyVM::getStackDepth\n");
+int DebuggerProxy::getStackDepth() {
+  TRACE(2, "DebuggerProxy::getStackDepth\n");
   int depth = 0;
   VMExecutionContext* context = g_vmContext;
   ActRec *fp = context->getFP();
@@ -753,8 +716,8 @@ int DebuggerProxyVM::getStackDepth() {
 }
 
 // Handle a continue cmd, or setup stepping.
-void DebuggerProxyVM::processFlowControl(CmdInterrupt &cmd) {
-  TRACE(2, "DebuggerProxyVM::processFlowControl\n");
+void DebuggerProxy::processFlowControl(CmdInterrupt &cmd) {
+  TRACE(2, "DebuggerProxy::processFlowControl\n");
   switch (m_flow->getType()) {
     case DebuggerCommand::KindOfContinue:
       if (!m_flow->decCount()) {
@@ -789,8 +752,8 @@ void DebuggerProxyVM::processFlowControl(CmdInterrupt &cmd) {
  * after the breakpoint has passed
  */
 
-void DebuggerProxyVM::changeBreakPointDepth(CmdInterrupt& cmd) {
-  TRACE(2, "DebuggerProxyVM::changeBreakPointDepth\n");
+void DebuggerProxy::changeBreakPointDepth(CmdInterrupt& cmd) {
+  TRACE(2, "DebuggerProxy::changeBreakPointDepth\n");
   for (unsigned int i = 0; i < m_breakpoints.size(); ++i) {
     // if the site changes, then update the breakpoint depth
     BreakPointInfoPtr bp = m_breakpoints[i];
@@ -803,8 +766,8 @@ void DebuggerProxyVM::changeBreakPointDepth(CmdInterrupt& cmd) {
 
 // Determine if an outstanding flow control cmd has run it's course and we
 // should stop execution.
-bool DebuggerProxyVM::breakByFlowControl(CmdInterrupt &cmd) {
-  TRACE(2, "DebuggerProxyVM::breakByFlowControl\n");
+bool DebuggerProxy::breakByFlowControl(CmdInterrupt &cmd) {
+  TRACE(2, "DebuggerProxy::breakByFlowControl\n");
   switch (m_flow->getType()) {
     case DebuggerCommand::KindOfStep: {
       if (!m_flow->decCount()) {

hphp/runtime/eval/debugger/debugger_proxy.h

@@ -42,8 +42,6 @@ namespace HPHP { namespace Eval {
 // The client always creates its own "local proxy", which allows debugging any
 // code running on the VM within the client. The two are easily confused.
 //
-// Note: currently DebuggerProxy has a single subclass, DebuggerProxyVM.
-// There used to be other subclasses, but they're gong now. These can be merged.
 
 class CmdInterrupt;
 DECLARE_BOOST_TYPES(DebuggerProxy);
@@ -65,6 +63,9 @@ public:
                             int frame);
 
 public:
+  DebuggerProxy(SmartPtr<Socket> socket, bool local);
+  ~DebuggerProxy();
+
   bool isLocal() const { return m_local;}
 
   const char *getThreadType() const;
@@ -81,7 +82,7 @@ public:
   void startDummySandbox();
   void notifyDummySandbox();
 
-  virtual void setBreakPoints(BreakPointInfoPtrVec &breakpoints);
+  void setBreakPoints(BreakPointInfoPtrVec &breakpoints);
   void getBreakPoints(BreakPointInfoPtrVec &breakpoints);
   bool couldBreakEnterClsMethod(const StringData* className);
   bool couldBreakEnterFunc(const StringData* funcFullName);
@@ -90,7 +91,7 @@ public:
 
   bool needInterrupt();
   bool needInterruptForNonBreak();
-  virtual void interrupt(CmdInterrupt &cmd);
+  void interrupt(CmdInterrupt &cmd);
   bool sendToClient(DebuggerCommand *cmd);
 
   void startSignalThread();
@@ -99,9 +100,28 @@ public:
   void checkStop();
   void forceQuit();
 
-protected:
-  DebuggerProxy(SmartPtr<Socket> socket, bool local);
-  virtual ~DebuggerProxy();
+  // For instrumentation
+  HPHP::VM::InjectionTables* getInjTables() const { return m_injTables; }
+  void setInjTables(HPHP::VM::InjectionTables* tables) { m_injTables = tables;}
+  void readInjTablesFromThread();
+  void writeInjTablesToThread();
+
+private:
+  bool blockUntilOwn(CmdInterrupt &cmd, bool check);
+  bool checkBreakPoints(CmdInterrupt &cmd);
+  bool checkFlowBreak(CmdInterrupt &cmd);
+  bool processFlowBreak(CmdInterrupt &cmd);
+  void processInterrupt(CmdInterrupt &cmd);
+  void processFlowControl(CmdInterrupt &cmd);
+  bool breakByFlowControl(CmdInterrupt &cmd);
+
+  DThreadInfoPtr createThreadInfo(const std::string &desc);
+
+  void changeBreakPointDepth(CmdInterrupt& cmd);
+  BreakPointInfoPtr getBreakPointAtCmd(CmdInterrupt& cmd);
+
+  int getRealStackDepth();
+  int getStackDepth();
 
   bool m_stopped;
 
@@ -134,49 +154,6 @@ protected:
   Mutex m_signumMutex;
   int m_signum;
 
-  // helpers
-  bool blockUntilOwn(CmdInterrupt &cmd, bool check);
-  virtual bool checkBreakPoints(CmdInterrupt &cmd);
-  bool checkFlowBreak(CmdInterrupt &cmd);
-  virtual bool processFlowBreak(CmdInterrupt &cmd);
-  void processInterrupt(CmdInterrupt &cmd);
-  virtual void processFlowControl(CmdInterrupt &cmd);
-  virtual bool breakByFlowControl(CmdInterrupt &cmd);
-
-  DThreadInfoPtr createThreadInfo(const std::string &desc);
-};
-
-class DebuggerProxyVM : public DebuggerProxy {
-public:
-  static Variant ExecutePHP(const std::string &php, String &output, bool log,
-                            int frame);
-
-public:
-  DebuggerProxyVM(SmartPtr<Socket> socket, bool local)
-    : DebuggerProxy(socket, local), m_injTables(nullptr) {
-  }
-  virtual ~DebuggerProxyVM() {
-    delete m_injTables;
-  }
-  virtual void interrupt(CmdInterrupt &cmd);
-  virtual void setBreakPoints(BreakPointInfoPtrVec &breakpoints);
-
-  // For instrumentation
-  HPHP::VM::InjectionTables* getInjTables() const { return m_injTables; }
-  void setInjTables(HPHP::VM::InjectionTables* tables) { m_injTables = tables;}
-  void readInjTablesFromThread();
-  void writeInjTablesToThread();
-
-private:
-  void changeBreakPointDepth(CmdInterrupt& cmd);
-  BreakPointInfoPtr getBreakPointAtCmd(CmdInterrupt& cmd);
-
-  int getStackDepth();
-  int getRealStackDepth();
-
-  virtual void processFlowControl(CmdInterrupt &cmd);
-  virtual bool breakByFlowControl(CmdInterrupt &cmd);
-
   // For instrumentation
   HPHP::VM::InjectionTables* m_injTables;
 };

hphp/runtime/vm/debugger_hook.cpp

@@ -211,7 +211,7 @@ void phpDebuggerDefFuncHook(const Func* func) {
 
 // Helper which will look at every loaded file and attempt to see if any
 // existing file:line breakpoints should be set.
-void phpSetBreakPointsInAllFiles(Eval::DebuggerProxyVM* proxy) {
+void phpSetBreakPointsInAllFiles(Eval::DebuggerProxy* proxy) {
   for (EvaledFilesMap::const_iterator it =
        g_vmContext->m_evaledFiles.begin();
        it != g_vmContext->m_evaledFiles.end(); ++it) {

hphp/runtime/vm/debugger_hook.h

@@ -21,7 +21,7 @@
 
 namespace HPHP {
 namespace Eval{
-class DebuggerProxyVM;
+class DebuggerProxy;
 class PhpFile;
 }}
 
@@ -47,7 +47,7 @@ void phpDebuggerDefClassHook(const Class* cls);
 void phpDebuggerDefFuncHook(const Func* func);
 
 // Helper to apply pending breakpoints to all files.
-void phpSetBreakPointsInAllFiles(Eval::DebuggerProxyVM* proxy);
+void phpSetBreakPointsInAllFiles(Eval::DebuggerProxy* proxy);
 
 // Is this thread being debugged?
 static inline bool isDebuggerAttached() {