[dexter] Remove unnecessary double check on conditional breakpoints

Remove the `ConditionalController._conditional_met` method. This was missed in
the recent ConditionalController refactor (D98699). We don't need to check that
the conditions for a conditional breakpoint have been met because
`DebuggerBase.get_triggered_breakpoint_ids` returns the set of ids for
breakpoints which have been triggered.

To get the "triggered breakpoints" from lldb we use `GetStopReasonDataCount`
and `GetStopReasonDataAtIndex`. It seems that these functions count all
breakpoints associated with the location which lldb has stopped at, regardless
of their condition. i.e. Even if we have two breakpoints at the same source
location that have mutually exclusive conditions, both will be found this way
when either condition is true. To get around this, we store a map of breakpoint
{id: condition} `_breakpoint_conditions` and evaluate the conditions of the
triggered breakpoints to filter the set down to those which are unconditional
or have a condition which evaluates to true.

Essentially we are just moving the condition double check from a general
debugger controller into the lldb specific wrapper. This tidy up will help make
upcoming patches simpler.

Reviewed By: chrisjackson

Differential Revision: https://reviews.llvm.org/D101431

Author: OCHyams

debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ConditionalController.py

@@ -77,13 +77,6 @@ def _set_conditional_bps(self):
                                                               cond_expr)
                 self._conditional_bp_handles[id] = cbp
 
-    def _conditional_met(self, cbp):
-        for cond_expr in cbp.get_conditional_expression_list():
-            valueIR = self.debugger.evaluate_expression(cond_expr)
-            if valueIR.type_name == 'bool' and valueIR.value == 'true':
-                return True
-        return False
-
     def _run_debugger_custom(self):
         # TODO: Add conditional and unconditional breakpoint support to dbgeng.
         if self.debugger.get_name() == 'dbgeng':
@@ -116,15 +109,12 @@ def _run_debugger_custom(self):
                     # This is an unconditional bp. Mark it for removal.
                     bp_to_delete.append(bp_id)
                     continue
-                # We have triggered a breakpoint with a condition. Check that
-                # the condition has been met.
-                if self._conditional_met(cbp):
-                    # Add a range of unconditional breakpoints covering the
-                    # lines requested in the DexLimitSteps command. Ignore
-                    # first line as that's the conditional bp we just hit and
-                    # include the final line.
-                    for line in range(cbp.range_from + 1, cbp.range_to + 1):
-                        self.debugger.add_breakpoint(cbp.path, line)
+                # Add a range of unconditional breakpoints covering the lines
+                # requested in the DexLimitSteps command. Ignore first line as
+                # that's the conditional bp we just hit and include the final
+                # line.
+                for line in range(cbp.range_from + 1, cbp.range_to + 1):
+                    self.debugger.add_breakpoint(cbp.path, line)
 
             # Remove any unconditional breakpoints we just hit.
             for bp_id in bp_to_delete:

debuginfo-tests/dexter/dex/debugger/lldb/LLDB.py

@@ -26,6 +26,9 @@ def __init__(self, context, *args):
         self._target = None
         self._process = None
         self._thread = None
+        # Map {id (int): condition (str)} for breakpoints which have a
+        # condition. See get_triggered_breakpoint_ids usage for more info.
+        self._breakpoint_conditions = {}
         super(LLDB, self).__init__(context, *args)
 
     def _custom_init(self):
@@ -104,37 +107,67 @@ def clear_breakpoints(self):
         self._target.DeleteAllBreakpoints()
 
     def _add_breakpoint(self, file_, line):
-        bp = self._target.BreakpointCreateByLocation(file_, line)
-        if not bp:
-            raise DebuggerException(
-                'could not add breakpoint [{}:{}]'.format(file_, line))
-        return bp.GetID()
+        return self._add_conditional_breakpoint(file_, line, None)
 
     def _add_conditional_breakpoint(self, file_, line, condition):
         bp = self._target.BreakpointCreateByLocation(file_, line)
-        if bp:
-            bp.SetCondition(condition)
-        else:
+        if not bp:
             raise DebuggerException(
                   'could not add breakpoint [{}:{}]'.format(file_, line))
-        return bp.GetID()
+        id = bp.GetID()
+        if condition:
+            bp.SetCondition(condition)
+            assert id not in self._breakpoint_conditions
+            self._breakpoint_conditions[id] = condition
+        return id
+
+    def _evaulate_breakpoint_condition(self, id):
+        """Evaluate the breakpoint condition and return the result.
+
+        Returns True if a conditional breakpoint with the specified id cannot
+        be found (i.e. assume it is an unconditional breakpoint).
+        """
+        try:
+            condition = self._breakpoint_conditions[id]
+        except KeyError:
+            # This must be an unconditional breakpoint.
+            return True
+        valueIR = self.evaluate_expression(condition)
+        return valueIR.type_name == 'bool' and valueIR.value == 'true'
 
     def get_triggered_breakpoint_ids(self):
         # Breakpoints can only have been triggered if we've hit one.
         stop_reason = self._translate_stop_reason(self._thread.GetStopReason())
         if stop_reason != StopReason.BREAKPOINT:
             return []
+        breakpoint_ids = set()
+        # When the stop reason is eStopReasonBreakpoint, GetStopReasonDataCount
+        # counts all breakpoints associated with the location that lldb has
+        # stopped at, regardless of their condition. I.e. Even if we have two
+        # breakpoints at the same source location that have mutually exclusive
+        # conditions, both will be counted by GetStopReasonDataCount when
+        # either condition is true. Check each breakpoint condition manually to
+        # filter the list down to breakpoints that have caused this stop.
+        #
         # Breakpoints have two data parts: Breakpoint ID, Location ID. We're
         # only interested in the Breakpoint ID so we skip every other item.
-        return set([self._thread.GetStopReasonDataAtIndex(i)
-                    for i in range(0, self._thread.GetStopReasonDataCount(), 2)])
+        for i in range(0, self._thread.GetStopReasonDataCount(), 2):
+            id = self._thread.GetStopReasonDataAtIndex(i)
+            if self._evaulate_breakpoint_condition(id):
+                breakpoint_ids.add(id)
+        return breakpoint_ids
 
     def delete_breakpoint(self, id):
         bp = self._target.FindBreakpointByID(id)
         if not bp:
             # The ID is not valid.
             raise KeyError
-        self._target.BreakpointDelete(bp.GetID())
+        try:
+            del self._breakpoint_conditions[id]
+        except KeyError:
+            # This must be an unconditional breakpoint.
+            pass
+        self._target.BreakpointDelete(id)
 
     def launch(self):
         self._process = self._target.LaunchSimple(None, None, os.getcwd())