Disable the debugger's Instrument command, and remove the code that checks for instrumentation from the interpreter

The debugger's Instrument command does not work with the JIT; any instrumentation requests will be ignored for jitted code. We believe that no one is using this at this time. I'm starting with a small diff to disable the command and remove the impact it has on the interpreter for now, just in case we're wrong. Once the command has been disabled for a few weeks I'll come back and remove all of the code (task 2376711).

InstHelpers was just pure dead code, so I nuked it.

hphp/runtime/eval/debugger/debugger_client.cpp

@@ -1570,7 +1570,6 @@ do {                                         \
   // give gdb users some love
   if (m_command == "bt") NEW_CMD_NAME("where", CmdWhere);
   if (m_command == "set") NEW_CMD_NAME("config", CmdConfig);
-  if (m_command == "inst") NEW_CMD_NAME("instrument", CmdInstrument);
   if (m_command == "complete") NEW_CMD_NAME("complete", CmdComplete);
 
   switch (tolower(m_command[0])) {

hphp/runtime/eval/debugger/debugger_command.cpp

@@ -121,8 +121,6 @@ bool DebuggerCommand::Receive(DebuggerThriftBuffer &thrift,
     case KindOfSignal   :  cmd = DebuggerCommandPtr(new CmdSignal   ()); break;
     case KindOfShell    :  cmd = DebuggerCommandPtr(new CmdShell    ()); break;
 
-    case KindOfInstrument: cmd = DebuggerCommandPtr(new CmdInstrument()); break;
-
     case KindOfExtended: {
       assert(!clsname.empty());
       cmd = CmdExtended::CreateExtendedCommand(clsname);

hphp/runtime/vm/bytecode.cpp

@@ -43,7 +43,6 @@
 #include <util/debug.h>
 #include <runtime/base/stat_cache.h>
 
-#include <runtime/vm/instrumentation_hook.h>
 #include <runtime/vm/php_debug.h>
 #include <runtime/vm/debugger_hook.h>
 #include <runtime/vm/runtime.h>
@@ -1850,7 +1849,6 @@ void VMExecutionContext::enterVMWork(ActRec* enterFnAr) {
   TCA start = nullptr;
   if (enterFnAr) {
     if (!EventHook::FunctionEnter(enterFnAr, EventHook::NormalFunc)) return;
-    INST_HOOK_FENTRY(enterFnAr->m_func->fullName());
     start = enterFnAr->m_func->getFuncBody();
   }
   Stats::inc(Stats::VMEnter);
@@ -5933,9 +5931,7 @@ void VMExecutionContext::doFCall(ActRec* ar, PC& pc) {
   assert(pcOff() > m_fp->m_func->base());
   prepareFuncEntry<false>(ar, pc, 0);
   SYNC();
-  if (EventHook::FunctionEnter(ar, EventHook::NormalFunc)) {
-    INST_HOOK_FENTRY(ar->m_func->fullName());
-  } else {
+  if (!EventHook::FunctionEnter(ar, EventHook::NormalFunc)) {
     pc = m_pc;
   }
 }
@@ -6211,7 +6207,6 @@ bool VMExecutionContext::doFCallArray(PC& pc) {
     pc = m_pc;
     return false;
   }
-  INST_HOOK_FENTRY(func->fullName());
   return true;
 }
 
@@ -6903,9 +6898,7 @@ void VMExecutionContext::iopContEnter(PC& pc) {
   pc = contAR->m_func->getEntry();
   SYNC();
 
-  if (LIKELY(EventHook::FunctionEnter(contAR, EventHook::NormalFunc))) {
-    INST_HOOK_FENTRY(contAR->m_func->fullName());
-  } else {
+  if (UNLIKELY(!EventHook::FunctionEnter(contAR, EventHook::NormalFunc))) {
     pc = m_pc;
   }
 }
@@ -7203,12 +7196,6 @@ inline void VMExecutionContext::dispatchImpl(int numInstrs) {
 #define O(name, imm, push, pop, flags) \
     &&LabelDbg##name,
     OPCODES
-#undef O
-  };
-  static const void *optabInst[] __attribute__((unused)) = {
-#define O(name, imm, push, pop, flags) \
-    &&LabelInst##name,
-    OPCODES
 #undef O
   };
   static const void *optabCover[] = {
@@ -7220,13 +7207,9 @@ inline void VMExecutionContext::dispatchImpl(int numInstrs) {
   assert(sizeof(optabDirect) / sizeof(const void *) == Op_count);
   assert(sizeof(optabDbg) / sizeof(const void *) == Op_count);
   const void **optab = optabDirect;
-  InjectionTableInt64* injTable = g_vmContext->m_injTables ?
-    g_vmContext->m_injTables->getInt64Table(InstHookTypeBCPC) : nullptr;
   bool collectCoverage = ThreadInfo::s_threadInfo->
     m_reqInjectionData.getCoverage();
-  if (injTable) {
-    optab = optabInst;
-  } else if (collectCoverage) {
+  if (collectCoverage) {
     optab = optabCover;
   }
   DEBUGGER_ATTACHED_ONLY(optab = optabDbg);
@@ -7272,8 +7255,6 @@ inline void VMExecutionContext::dispatchImpl(int numInstrs) {
 #define O(name, imm, pusph, pop, flags)                       \
   LabelDbg##name:                                             \
     phpDebuggerOpcodeHook(pc);                                \
-  LabelInst##name:                                            \
-    INST_HOOK_PC(injTable, pc);                               \
   LabelCover##name:                                           \
     if (collectCoverage) {                                    \
       recordCodeCoverage(pc);                                 \

hphp/runtime/vm/instrumentation.cpp

@@ -268,67 +268,6 @@ int InjectionTables::countInjections() {
 }
 
 
-///////////////////////////////////////////////////////////////////////////////
-
-static InjectionTables* s_globalInjTables = nullptr;
-static ReadWriteMutex s_globalInjTableLock;
-
-void InstHelpers::InstCustomStringCallback(const StringData* hook,
-                                           Injection::Callback callback,
-                                           void *arg, const StringData* desc) {
-  const Injection* inj = InjectionCache::GetInjection(callback, arg, desc);
-  assert(inj);
-  const StringData* hookCached = InjectionCache::GetStringData(hook);
-  if (!g_vmContext->m_injTables) {
-    g_vmContext->m_injTables = new InjectionTables();
-  }
-  if (!g_vmContext->m_injTables->getSDTable(InstHookTypeCustomEvt)) {
-    g_vmContext->m_injTables->setSDTable(InstHookTypeCustomEvt,
-                                       new InjectionTableSD());
-  }
-  InjectionTableSD* table =
-    g_vmContext->m_injTables->getSDTable(InstHookTypeCustomEvt);
-  (*table)[hookCached] = inj;
-}
-
-void InstHelpers::PushInstToGlobal() {
-  WriteLock lock(s_globalInjTableLock);
-  if (s_globalInjTables) {
-    delete s_globalInjTables;
-    s_globalInjTables = nullptr;
-  }
-  if (g_vmContext->m_injTables) {
-    s_globalInjTables = g_vmContext->m_injTables->clone();
-  }
-}
-
-void InstHelpers::PullInstFromGlobal() {
-  if (g_vmContext->m_injTables) {
-    delete g_vmContext->m_injTables;
-    g_vmContext->m_injTables = nullptr;
-  }
-  ReadLock lock(s_globalInjTableLock);
-  if (s_globalInjTables) {
-    g_vmContext->m_injTables = s_globalInjTables->clone();
-  }
-}
-
-int InstHelpers::CountGlobalInst() {
-  ReadLock lock(s_globalInjTableLock);
-  if (s_globalInjTables) {
-    return s_globalInjTables->countInjections();
-  }
-  return 0;
-}
-
-void InstHelpers::ClearGlobalInst() {
-  WriteLock lock(s_globalInjTableLock);
-  if (s_globalInjTables) {
-    delete s_globalInjTables;
-    s_globalInjTables = nullptr;
-  }
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 } }    // HPHP::VM

hphp/runtime/vm/instrumentation.h

@@ -148,19 +148,6 @@ private:
   std::vector<InjectionTableSD*> m_sdTables;
 };
 
-class InstHelpers {
-public:
-  static void InstCustomStringCallback(const StringData* hook,
-                                       Injection::Callback callback,
-                                       void *arg, const StringData* desc);
-
-  static void PushInstToGlobal();
-  static void PullInstFromGlobal();
-
-  static int CountGlobalInst();
-  static void ClearGlobalInst();
-};
-
 ///////////////////////////////////////////////////////////////////////////////
 
 } }    // HPHP::VM