[dexter] Remove requirement for a condition in DexLimitSteps

Currently the DexLimitSteps command requires at least one condition. This patch
lets users elide the condition to specify that the breakpoint range should
always be activated when the leading line is stepped on. This patch also
updates the terminology used in the `ConditionalController` class from the
terms 'conditional' and 'unconditional' to 'leading' and 'trailing' when
referring to the breakpoints in the DexLimitSteps range because the leading
breakpoint can now be unconditional.

Reviewed By: chrisjackson

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

Author: OCHyams

debuginfo-tests/dexter/Commands.md

@@ -177,7 +177,7 @@ Expect the source line this is found on will never be stepped on to.
 
 ----
 ## DexLimitSteps
-    DexLimitSteps(expr, *values [, **from_line=1],[,**to_line=Max]
+    DexLimitSteps([expr, *values][, **from_line=1][,**to_line=Max]
                   [,**on_line])
 
     Args:
@@ -193,12 +193,14 @@ Expect the source line this is found on will never be stepped on to.
                        same line.
 
 ### Description
-Define a limited stepping range that is predicated on a condition. When
-'(expr) == (values[n])', set a range of temporary, unconditional break points within
-the test file defined by the range from_line and to_line or on_line.
+Define a limited stepping range that may be predicated on a condition. When the
+leading line is stepped on and any condition '(expr) == (values[n])' is true or
+there are no conditions, set a range of temporary breakpoints within the test
+file defined by the range 'from_line' and 'to_line' or 'on_line'.
 
 The condition is only evaluated on the line 'from_line' or 'on_line'. If the
-condition is not true at the start of the range, the whole range is ignored.
+condition is not true at the start of the range, or that line is never stepped
+onto, the whole range is ignored.
 
 DexLimitSteps commands are useful for reducing the amount of steps gathered in
 large test cases that would normally take much longer to complete.

debuginfo-tests/dexter/dex/command/commands/DexLimitSteps.py

@@ -12,8 +12,14 @@
 
 class DexLimitSteps(CommandBase):
     def __init__(self, *args, **kwargs):
-        self.expression = args[0]
-        self.values = [str(arg) for arg in args[1:]]
+        if len(args) == 0:
+            self.expression = None
+            self.values = []
+        elif len(args) == 1:
+            raise TypeError("expected 0 or at least 2 positional arguments")
+        else:
+            self.expression = args[0]
+            self.values = [str(arg) for arg in args[1:]]
         try:
             on_line = kwargs.pop('on_line')
             self.from_line = on_line

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

@@ -18,9 +18,20 @@
 from dex.utils.Exceptions import DebuggerException
 
 
-class ConditionalBpRange:
-    """Represents a conditional range of breakpoints within a source file descending from
-    one line to another."""
+class BreakpointRange:
+    """A range of breakpoints and a set of conditions.
+
+    The leading breakpoint (on line `range_from`) is always active.
+
+    When the leading breakpoint is hit the trailing range should be activated
+    when `expression` evaluates to any value in `values`. If there are no
+    conditions (`expression` is None) then the trailing breakpoint range should
+    always be activated upon hitting the leading breakpoint.
+
+    Args:
+       expression: None for no conditions, or a str expression to compare
+       against `values`.
+    """
 
     def __init__(self, expression: str, path: str, range_from: int, range_to: int, values: list):
         self.expression = expression
@@ -29,6 +40,9 @@ def __init__(self, expression: str, path: str, range_from: int, range_to: int, v
         self.range_to = range_to
         self.conditional_values = values
 
+    def has_conditions(self):
+        return self.expression != None
+
     def get_conditional_expression_list(self):
         conditional_list = []
         for value in self.conditional_values:
@@ -42,48 +56,54 @@ class ConditionalController(DebuggerControllerBase):
     def __init__(self, context, step_collection):
       self.context = context
       self.step_collection = step_collection
-      self._conditional_bp_ranges = None
-      self._build_conditional_bp_ranges()
+      self._bp_ranges = None
+      self._build_bp_ranges()
       self._watches = set()
       self._step_index = 0
       self._pause_between_steps = context.options.pause_between_steps
       self._max_steps = context.options.max_steps
-      # Map {id: ConditionalBpRange}
-      self._conditional_bp_handles = {}
+      # Map {id: BreakpointRange}
+      self._leading_bp_handles = {}
 
-    def _build_conditional_bp_ranges(self):
+    def _build_bp_ranges(self):
         commands = self.step_collection.commands
-        self._conditional_bp_ranges = []
+        self._bp_ranges = []
         try:
             limit_commands = commands['DexLimitSteps']
             for lc in limit_commands:
-                conditional_bp = ConditionalBpRange(
+                bpr = BreakpointRange(
                   lc.expression,
                   lc.path,
                   lc.from_line,
                   lc.to_line,
                   lc.values)
-                self._conditional_bp_ranges.append(conditional_bp)
+                self._bp_ranges.append(bpr)
         except KeyError:
             raise DebuggerException('Missing DexLimitSteps commands, cannot conditionally step.')
 
-    def _set_conditional_bps(self):
-        # Set a conditional breakpoint for each ConditionalBpRange and build a
-        # map of {id: ConditionalBpRange}.
-        for cbp in self._conditional_bp_ranges:
-            for cond_expr in cbp.get_conditional_expression_list():
-                id = self.debugger.add_conditional_breakpoint(cbp.path,
-                                                              cbp.range_from,
-                                                              cond_expr)
-                self._conditional_bp_handles[id] = cbp
+    def _set_leading_bps(self):
+        # Set a leading breakpoint for each BreakpointRange, building a
+        # map of {leading bp id: BreakpointRange}.
+        for bpr in self._bp_ranges:
+            if bpr.has_conditions():
+                # Add a conditional breakpoint for each condition.
+                for cond_expr in bpr.get_conditional_expression_list():
+                    id = self.debugger.add_conditional_breakpoint(bpr.path,
+                                                                  bpr.range_from,
+                                                                  cond_expr)
+                    self._leading_bp_handles[id] = bpr
+            else:
+                # Add an unconditional breakpoint.
+                id = self.debugger.add_breakpoint(bpr.path, bpr.range_from)
+                self._leading_bp_handles[id] = bpr
 
     def _run_debugger_custom(self):
         # TODO: Add conditional and unconditional breakpoint support to dbgeng.
         if self.debugger.get_name() == 'dbgeng':
             raise DebuggerException('DexLimitSteps commands are not supported by dbgeng')
 
         self.step_collection.clear_steps()
-        self._set_conditional_bps()
+        self._set_leading_bps()
 
         for command_obj in chain.from_iterable(self.step_collection.commands.values()):
             self._watches.update(command_obj.get_watches())
@@ -103,20 +123,20 @@ def _run_debugger_custom(self):
             bp_to_delete = []
             for bp_id in self.debugger.get_triggered_breakpoint_ids():
                 try:
-                    # See if this is one of our conditional breakpoints.
-                    cbp = self._conditional_bp_handles[bp_id]
+                    # See if this is one of our leading breakpoints.
+                    bpr = self._leading_bp_handles[bp_id]
                 except KeyError:
-                    # This is an unconditional bp. Mark it for removal.
+                    # This is a trailing bp. Mark it for removal.
                     bp_to_delete.append(bp_id)
                     continue
-                # Add a range of unconditional breakpoints covering the lines
+                # Add a range of trailing 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)
+                # that's covered by the leading bp we just hit and include the
+                # final line.
+                for line in range(bpr.range_from + 1, bpr.range_to + 1):
+                    self.debugger.add_breakpoint(bpr.path, line)
 
-            # Remove any unconditional breakpoints we just hit.
+            # Remove any trailing breakpoints we just hit.
             for bp_id in bp_to_delete:
                 self.debugger.delete_breakpoint(bp_id)
 

debuginfo-tests/dexter/feature_tests/commands/perfect/limit_steps/unconditional.cpp

@@ -0,0 +1,25 @@
+// Purpose:
+//      Test that \DexLimitSteps can be used without a condition (i.e. the
+//      breakpoint range is set any time from_line is stepped on).
+//
+// REQUIRES: system-linux
+//
+// RUN: %dexter_regression_test -- %s | FileCheck %s
+// CHECK: unconditional.cpp
+
+int glob;
+int main() {
+  int test = 0;
+  for (test = 1; test < 4; test++) {
+    glob += test; // DexLabel('from')
+    glob += test; // DexLabel('to')
+  }
+  return test; // test = 4
+}
+
+// DexLimitSteps(from_line='from', to_line='to')
+//// Unconditionally limit dexter's view of the program from line 'from' to
+//// 'to'. Check for test=0, 1, 2 so that the test will fail if dexter sees
+//// test=0 or test=4.
+// DexExpectWatchValue('test', 1, 2, 3)
+

debuginfo-tests/dexter/feature_tests/subtools/test/err_limit_steps_no_values.cpp

@@ -0,0 +1,10 @@
+// Purpose:
+//    Check that specifying an expression without any values to compare against
+//    in a \DexLimitSteps command results in a useful error message.
+//    Use --binary switch to trick dexter into skipping the build step.
+//
+// REQUIRES: system-linux
+// RUN: not %dexter_base test --binary %s --debugger 'lldb' -- %s | FileCheck %s
+// CHECK: parser error:{{.*}}err_limit_steps_no_values.cpp(10): expected 0 or at least 2 positional arguments
+
+// DexLimitSteps('test')