HHVM Debugger: Fix conditional breakpoints not working correctly

ebluestein · fredemmott · commit bbba234980bc · 2018-08-10T13:41:12.000-07:00
Summary: The breakpoints were being erroneously removed from the VM after the first time the condition evaluated to false.

Reviewed By: Goom11

Differential Revision: D9039623

fbshipit-source-id: ac806bbaa00f0baadbf190908c8db4e94c06258e
diff --git a/hphp/runtime/ext/vsdebug/debugger.cpp b/hphp/runtime/ext/vsdebug/debugger.cpp
@@ -1681,7 +1681,6 @@ void Debugger::onBreakpointHit(
   int line
 ) {
   std::string stopReason;
-  int matchingBpId = -1;
   const std::string filePath = getFilePathForUnit(compilationUnit);
 
   if (prepareToPauseTarget(ri) != PrepareToPauseResult::ReadyToPause) {
@@ -1713,12 +1712,10 @@ void Debugger::onBreakpointHit(
     bool lineInRange = line >= bp->m_resolvedLocation.m_startLine &&
         line <= bp->m_resolvedLocation.m_endLine;
 
-    bool conditionSatisfied = bpMgr->isBreakConditionSatisified(ri, bp);
     if (lineInRange) {
-      if (conditionSatisfied) {
-        matchingBpId = bpId;
+      if (bpMgr->isBreakConditionSatisified(ri, bp)) {
         stopReason = getStopReasonForBp(
-          matchingBpId,
+          bpId,
           !bp->m_resolvedLocation.m_path.empty()
             ? bp->m_resolvedLocation.m_path
             : bp->m_path,
@@ -1728,7 +1725,16 @@ void Debugger::onBreakpointHit(
         // Breakpoint hit!
         pauseTarget(ri, stopReason.c_str());
         bpMgr->onBreakpointHit(bpId);
-        break;
+
+        processCommandQueue(
+          getCurrentThreadId(),
+          ri,
+          "breakpoint",
+          stopReason.c_str(),
+          true
+        );
+
+        return;
       } else {
         VSDebugLogger::Log(
           VSDebugLogger::LogLevelInfo,
@@ -1740,17 +1746,8 @@ void Debugger::onBreakpointHit(
     }
   }
 
-  if (matchingBpId >= 0) {
-    // If an active breakpoint was found at this location, enter the debugger.
-    processCommandQueue(
-      getCurrentThreadId(),
-      ri,
-      "breakpoint",
-      stopReason.c_str(),
-      true
-    );
-  } else if (ri->m_runToLocationInfo.path == filePath &&
-             line == ri->m_runToLocationInfo.line) {
+  if (ri->m_runToLocationInfo.path == filePath &&
+      line == ri->m_runToLocationInfo.line) {
 
     // Hit our run to location destination!
     stopReason = "Run to location";
@@ -1782,19 +1779,6 @@ void Debugger::onBreakpointHit(
       stopReason.c_str(),
       true
     );
-  } else {
-    // This breakpoint no longer exists. Remove it from the VM.
-    VSDebugLogger::Log(
-      VSDebugLogger::LogLevelInfo,
-      "Request hit bp that no longer exists, removing from VM. %s:%d",
-      filePath.c_str(),
-      line
-    );
-    removeBreakpoint(
-      func != nullptr
-        ? BreakpointType::Function
-        : BreakpointType::Source
-    );
   }
 }
 
