Set breakpoints on multiple non-contiguous bytecode blocks per line

Summary:
One line in source code may be translated to multiple blocks of bytecodes. For example, the local iterator optimization in HackC creates two copies of the foreach loop body. We need to set breakpoints on each of these blocks as long as they are not overlapping or contiguous.
Unfortunately, there are some cases where this may lead to spurious breakpoints being set. For example, multiline call expressions produce two bytecode blocks for the same line that are not contiguous, so there will be two breakpoints set.

Reviewed By: paulbiss

Differential Revision: D68462841

fbshipit-source-id: 824e55946b9a18babad658b1f926f5e26721a46f

Author: arnabde03

hphp/runtime/ext/vsdebug/breakpoint_command.cpp

@@ -125,7 +125,7 @@ bool SetBreakpointsCommand::executeImpl(
   // Make a map of line -> breakpoint for all breakpoints in this file before
   // this set breakpoints operation.
   std::unordered_map<int, Breakpoint*> oldBpLines;
-  const auto oldBpIds = bpMgr->getBreakpointIdsForPath(filePath);
+  const auto oldBpIds = bpMgr->getBreakpointIdsForPath(path);
   for (auto it = oldBpIds.begin(); it != oldBpIds.end(); it++) {
     Breakpoint* bp = bpMgr->getBreakpointById(*it);
     std::pair<int, Breakpoint*> pair;

hphp/runtime/vm/debugger-hook.cpp

@@ -528,24 +528,30 @@ bool phpAddBreakPointLine(const Unit* unit, int line) {
     return false;
   }
 
-  // One source line may refer to multiple bytecodes. We want to set the
-  // breakpoint only on the first bytecode. There is one exeption:
-  // If the bytecodes refer to different functions, set breakpoints on
-  // one bytecode of each function. This can happen for lambdas.
+  // One source line may refer to multiple blocks of bytecodes, possibly in
+  // different functions (in case of lambdas).
+  // We want to set the breakpoint only on the first bytecode of each block,
+  // unless they are overlapping or contiguous.
   // Add to the breakpoint filter and the line filter.
   assertx(funcOffsets.size() > 0);
-  DEBUG_ONLY std::unordered_set<const Func*> seenFuncs;
   for (auto const& offsets : funcOffsets) {
     auto f = offsets.first;
-    assertx(seenFuncs.find(f) == seenFuncs.end());
-    if (!offsets.second.empty()) {
-      auto bpOffset = offsets.second.front().base;
+    auto currPast = -1;
+    // offsets are sorted.
+    for (auto const& offset : offsets.second) {
+      auto bpOffset = offset.base;
+      if (bpOffset <= currPast) {
+        // Overlapping or contiguous bytecode blocks for the same source line.
+        // Do not add a breakpoint, only update the currPast.
+        currPast = offset.past > currPast ? offset.past : currPast;
+        continue;
+      }
+      currPast = offset.past;
       TRACE(3, "Adding breakpoint at %s::%d, Op %s.\n",
             f->name()->data(), bpOffset, opcodeToName(f->getOp(bpOffset)));
       phpAddBreakPoint(f, bpOffset);
       auto pc = f->at(bpOffset);
       RID().m_lineBreakPointFilter.addPC(pc);
-      if (debug) seenFuncs.insert(f);
     }
   }
   return true;

hphp/test/slow/ext_vsdebug/breakpoints.php

@@ -150,6 +150,18 @@
   "body" => dict[
       "breakpoints" => vec[]
   ]]);
+
+$breakpoints = vec[
+  dict[
+    "path" => __FILE__ . ".test",
+    "breakpoints" => vec[
+      dict["line" => 34, "calibratedLine" => 34, "condition" => ""],
+    ]]
+  ];
+setBreakpoints($breakpoints, true);
+resumeTarget();
+
+verifyBpHit($breakpoints[0]['path'], $breakpoints[0]['breakpoints'][0]);
 resumeTarget();
 
 // Verify hard break was hit.

hphp/test/slow/ext_vsdebug/breakpoints.php.test

@@ -29,6 +29,12 @@ for ($i = 0; $i < 10; $i++) {
   $y++;
 }
 
+$v = vec[1, 2, 3];
+$a = 0;
+foreach($v as $u) {
+  $a = $a + $u;
+}
+
 hphp_debug_break();
 
 $dir = dirname(__FILE__);

hphp/test/slow/ext_vsdebug/common.inc

@@ -330,6 +330,7 @@ function vsDebugLaunch($scriptPath, $sendDefaultInit = true, $breakpoints = vec[
       '-vEval.JitDisabledByVSDebug='.$disable_jit,
       '-vEval.JitPGO=false',
       '-vHack.Lang.OptimizeLocalLifetimes=0',
+      '-vHack.Lang.OptimizeLocalIterators = true',
       $scriptPath,
     ];
   } else {