Test conditional breakpoints and re-enable "called from" breakpoints

Added some test cases. Also now permit caller=>func() type of breakpoints. To make this work, I added a method, getCallingSite, to class InterruptSite and use this to walk the stack when checking if a site matches a break point that has more than one function name.

hphp/doc/debugger.refs

@@ -76,6 +76,16 @@ variables at breakpoints. For example,
 
 So every time it breaks at mypage.php line 123, it will print out $a.
 
+    -------------------------- Call chains -------------------------------
+    
+Function/method call breakpoints can be qualified with the names of
+functions or methods that must be calling the right most function/method
+name for execution to stop at the breakpoint. These calls need not be 
+direct calls. The syntax looks like this:
+    {call}=>{call}()
+where call is either a {func} or {cls}::{method} and zero or more 
+"{call}=>" clauses can be specified.
+
     --------------------- Breakpoint States and List ---------------------
 
 Every breakpoint has 3 states: ALWAYS, ONCE, DISABLED. Without keyword "once",
@@ -265,7 +275,7 @@ will not be affected by this setting. This directory will be stored in
 configuration file for future sessions as well.
 
 
-    ────────────────────────── Machine Command ──────────────────────────
+    -------------------------- Machine Command ---------------------------
 
     [m]achine            debugging remote server natively
       [c]onnect {host}

hphp/runtime/debugger/break_point.cpp

@@ -76,13 +76,13 @@ std::string InterruptSite::desc() const {
 }
 
 InterruptSite::InterruptSite(bool hardBreakPoint, CVarRef error)
-    : m_error(error), m_class(nullptr), m_function(nullptr),
-      m_file((StringData*)nullptr),
+    : m_error(error), m_activationRecord(nullptr),
+      m_callingSite(nullptr), m_class(nullptr),
+      m_function(nullptr), m_file((StringData*)nullptr),
       m_line0(0), m_char0(0), m_line1(0), m_char1(0),
       m_offset(InvalidAbsoluteOffset), m_unit(nullptr), m_valid(false),
       m_funcEntry(false) {
   TRACE(2, "InterruptSite::InterruptSite\n");
-  Transl::VMRegAnchor _;
 #define bail_on(c) if (c) { return; }
   VMExecutionContext* context = g_vmContext;
   ActRec *fp = context->getFP();
@@ -91,10 +91,6 @@ InterruptSite::InterruptSite(bool hardBreakPoint, CVarRef error)
     // for hard breakpoint, the fp is for an extension function,
     // so we need to construct the site on the caller
     fp = context->getPrevVMState(fp, &m_offset);
-    assert(fp);
-    bail_on(!fp->m_func);
-    m_unit = fp->m_func->unit();
-    bail_on(!m_unit);
   } else {
     const uchar *pc = context->getPC();
     bail_on(!fp->m_func);
@@ -105,6 +101,32 @@ InterruptSite::InterruptSite(bool hardBreakPoint, CVarRef error)
       m_funcEntry = true;
     }
   }
+#undef bail_on
+  this->Initialize(fp);
+}
+
+// Only used to look for callers by function name. No need to
+// to retrieve source line information for this kind of site.
+InterruptSite::InterruptSite(ActRec *fp, Offset offset, CVarRef error)
+  : m_error(error), m_activationRecord(nullptr),
+    m_callingSite(nullptr), m_class(nullptr),
+    m_function(nullptr), m_file((StringData*)nullptr),
+    m_line0(0), m_char0(0), m_line1(0), m_char1(0),
+    m_offset(offset), m_unit(nullptr), m_valid(false),
+    m_funcEntry(false) {
+  TRACE(2, "InterruptSite::InterruptSite(fp)\n");
+  this->Initialize(fp);
+}
+
+void InterruptSite::Initialize(ActRec *fp) {
+  TRACE(2, "InterruptSite::Initialize\n");
+
+#define bail_on(c) if (c) { return; }
+  assert(fp);
+  m_activationRecord = fp;
+  bail_on(!fp->m_func);
+  m_unit = fp->m_func->unit();
+  bail_on(!m_unit);
   m_file = m_unit->filepath()->data();
   if (m_unit->getSourceLoc(m_offset, m_sourceLoc)) {
     m_line0 = m_sourceLoc.line0;
@@ -122,6 +144,21 @@ InterruptSite::InterruptSite(bool hardBreakPoint, CVarRef error)
   m_valid = true;
 }
 
+// Returns an Interrupt site for the function that called the
+// function that contains this site. This site retains ownership
+// of the returned site and it will be deleted when this site
+// is destructed, so do not hold on to the returned object for
+// longer than there is a guarantee that this site will be alive.
+const InterruptSite *InterruptSite::getCallingSite() const {
+  if (m_callingSite != nullptr) return m_callingSite.get();
+  VMExecutionContext* context = g_vmContext;
+  Offset parentOffset;
+  auto parentFp = context->getPrevVMState(m_activationRecord, &parentOffset);
+  if (parentFp == nullptr) return nullptr;
+  m_callingSite.reset(new InterruptSite(parentFp, parentOffset, m_error));
+  return m_callingSite.get();
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 const char *BreakPointInfo::ErrorClassName = "@";
@@ -294,7 +331,7 @@ bool BreakPointInfo::valid() {
             return false;
           }
         }
-        if (m_regex || m_funcs.size() > 1) {
+        if (m_regex) {
           return false;
         }
         return (m_line1 && m_line2) || !m_file.empty() || !m_funcs.empty();
@@ -674,6 +711,7 @@ void BreakPointInfo::parseBreakPointReached(const std::string &exp,
       // check for {something other than a name}
       if (offset2 == offset1) goto returnInvalid;
       name = exp.substr(offset1, offset2-offset1);
+      offset1 = offset2;
     }
     while (offset1 < len && exp[offset1] == '\\') {
       if (!m_namespace.empty()) m_namespace += "\\";
@@ -939,17 +977,26 @@ bool BreakPointInfo::checkLines(int line) {
   return true;
 }
 
+// Checks if m_funcs[0] matches the top of the execution stack and
+// if m_funcs[1] (if not null) matches an earlier stack frame, and so on.
+// I.e. m_funcs[1] need only be caller, not a direct caller, of m_funcs[0].
 bool BreakPointInfo::checkStack(InterruptSite &site) {
   TRACE(2, "BreakPointInfo::checkStack\n");
-  if (m_funcs.empty()) return true;
-
-  if (!Match(site.getNamespace(), 0, m_funcs[0]->m_namespace, m_regex, true) ||
-      !Match(site.getFunction(),  0, m_funcs[0]->m_function,  m_regex, true) ||
-      !MatchClass(site.getClass(), m_funcs[0]->m_class, m_regex,
-                  site.getFunction())) {
-    return false;
+  const InterruptSite* s = &site;
+  for (int i = 0; i < m_funcs.size(); ) {
+    if (s == nullptr) return false;
+    if (!Match(s->getNamespace(), 0, m_funcs[i]->m_namespace, m_regex, true) ||
+        !Match(s->getFunction(),  0, m_funcs[i]->m_function,  m_regex, true) ||
+        !MatchClass(s->getClass(), m_funcs[i]->m_class, m_regex,
+                    s->getFunction())) {
+      if (i == 0) return false; //m_funcs[0] must match the very first frame.
+      // there is a mismatch for this frame, but the calling frame may match
+      // so carry on.
+    } else {
+      i++; // matched m_funcs[i], proceed to match m_funcs[i+1]
+    }
+    s = s->getCallingSite();
   }
-
   return true;
 }
 

hphp/runtime/debugger/break_point.h

@@ -62,6 +62,7 @@ class InterruptSite {
 public:
   InterruptSite(bool hardBreakPoint, CVarRef e);
 
+  const InterruptSite *getCallingSite() const;
   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 : ""; }
@@ -89,9 +90,14 @@ public:
   bool funcEntry() const { return m_funcEntry; }
 
 private:
+  InterruptSite(ActRec* fp, Offset offset, CVarRef error);
+  void Initialize(ActRec *fp);
+
   Variant m_error;
+  ActRec *m_activationRecord;
 
   // cached
+  mutable std::unique_ptr<const InterruptSite> m_callingSite;
   mutable const char *m_class;
   mutable const char *m_function;
   mutable String m_file;

hphp/runtime/debugger/cmd/cmd_break.cpp

@@ -160,6 +160,17 @@ void CmdBreak::help(DebuggerClient &client) {
     "So every time it breaks at mypage.php line 123, it will print out $a."
   );
 
+  client.helpTitle("Call chains");
+  client.helpSection(
+    "Function/method call breakpoints can be qualified with the names of "
+    "functions or methods that must be calling the right most function/method "
+    "name for execution to stop at the breakpoint. These calls need not be "
+    "direct calls. The syntax looks like this:\n"
+    "  {call}=>{call}()\n"
+    "where call is either a {func} or {cls}::{method} and zero or more "
+    "\"{call}=>\" clauses can be specified."
+   );
+
   client.helpTitle("Breakpoint States and List");
   client.helpSection(
     "Every breakpoint has 3 states: ALWAYS, ONCE, DISABLED. Without keyword "

hphp/test/quick/debugger/break1.php

@@ -22,6 +22,12 @@ class cls {
 }
 
 class derived extends cls {
+  public function callPubObj($x) {
+    $this->pubObj($x);
+  }
+  public function callCallPubObj($x) {
+    $this->callPubObj($x);
+  }
 }
 
 error_log('break1.php loaded');

hphp/test/quick/debugger/break1.php.expectf

@@ -99,6 +99,8 @@ break clear all
 All breakpoints are cleared.
 break derived::pubObj()
 Breakpoint 1 set upon entering derived::pubObj()
+break list
+  1	ALWAYS    upon entering derived::pubObj()
 @ $break6 = new derived()
 
 @ $break6->pubObj('test_break6')
@@ -110,4 +112,94 @@ Breakpoint 1 reached at cls::pubObj() on line 12 of %s/break1.php
 continue
 pubObj:test_break6
 
+break clear all
+All breakpoints are cleared.
+b cls::pubObj() if $x == 'yes'
+Breakpoint 1 set upon entering cls::pubObj() if $x == 'yes'
+@ $break7 = new cls()
+
+@ $break7->pubObj('yes');
+Breakpoint 1 reached at cls::pubObj() on line 12 of %s/break1.php
+  11   public function pubObj($x) {
+  12     error_log("pubObj:".$x);
+  13   }
+
+break list
+  1	ALWAYS    upon entering cls::pubObj() if $x == 'yes'
+b 12 if $x == 'yes sir'
+Breakpoint 2 set on line 12 of %s/break1.php if $x == 'yes sir'
+@ $this->pubObj('yes sir');
+Breakpoint 2 reached at cls::pubObj() on line 12 of %s/break1.php
+  11   public function pubObj($x) {
+  12     error_log("pubObj:".$x);
+  13   }
+
+break list
+  1	ALWAYS    upon entering cls::pubObj() if $x == 'yes'
+  2	ALWAYS    on line 12 of %s/break1.php if $x == 'yes sir'
+break clear all
+All breakpoints are cleared.
+continue
+pubObj:yes sir
+
+break derived::callPubObj=>cls::pubObj()
+Breakpoint 2 set upon entering cls::pubObj() called by derived::callPubObj()
+break list
+  2	ALWAYS    upon entering cls::pubObj() called by derived::callPubObj()
+@ $break8 = new derived();
+
+@ $break8->pubObj('no')
+pubObj:no
+
+@ $break8->callPubObj('yes')
+Breakpoint 2 reached at cls::pubObj() on line 12 of %s/break1.php
+  11   public function pubObj($x) {
+  12     error_log("pubObj:".$x);
+  13   }
+
+break clear all
+All breakpoints are cleared.
+continue
+pubObj:yes
+
+break derived::callCallPubObj=>derived::callPubObj=>cls::pubObj()
+Breakpoint 2 set upon entering cls::pubObj() called by derived::callPubObj() called by derived::callCallPubObj()
+break list
+  2	ALWAYS    upon entering cls::pubObj() called by derived::callPubObj() called by derived::callCallPubObj()
+@ $break9 = new derived();
+
+@ $break9->callPubObj('no')
+pubObj:no
+
+@ $break9->callCallPubObj('yes')
+Breakpoint 2 reached at cls::pubObj() on line 12 of %s/break1.php
+  11   public function pubObj($x) {
+  12     error_log("pubObj:".$x);
+  13   }
+
+break clear all
+All breakpoints are cleared.
+continue
+pubObj:yes
+
+break derived::callCallPubObj=>cls::pubObj()
+Breakpoint 2 set upon entering cls::pubObj() called by derived::callCallPubObj()
+break list
+  2	ALWAYS    upon entering cls::pubObj() called by derived::callCallPubObj()
+@ $break10 = new derived();
+
+@ $break10->callPubObj('no')
+pubObj:no
+
+@ $break10->callCallPubObj('yes')
+Breakpoint 2 reached at cls::pubObj() on line 12 of %s/break1.php
+  11   public function pubObj($x) {
+  12     error_log("pubObj:".$x);
+  13   }
+
+break clear all
+All breakpoints are cleared.
+continue
+pubObj:yes
+
 quit

hphp/test/quick/debugger/break1.php.in

@@ -33,7 +33,39 @@ break noSuchFunction()
 break list
 break clear all
 break derived::pubObj()
+break list
 @ $break6 = new derived()
 @ $break6->pubObj('test_break6')
 continue
+break clear all
+b cls::pubObj() if $x == 'yes'
+@ $break7 = new cls()
+@ $break7->pubObj('yes');
+break list
+b 12 if $x == 'yes sir'
+@ $this->pubObj('yes sir');
+break list
+break clear all
+continue
+break derived::callPubObj=>cls::pubObj()
+break list
+@ $break8 = new derived();
+@ $break8->pubObj('no')
+@ $break8->callPubObj('yes')
+break clear all
+continue
+break derived::callCallPubObj=>derived::callPubObj=>cls::pubObj()
+break list
+@ $break9 = new derived();
+@ $break9->callPubObj('no')
+@ $break9->callCallPubObj('yes')
+break clear all
+continue
+break derived::callCallPubObj=>cls::pubObj()
+break list
+@ $break10 = new derived();
+@ $break10->callPubObj('no')
+@ $break10->callCallPubObj('yes')
+break clear all
+continue
 quit