[dexter] Add hit_count keyword arg to DexLimitSteps

[Orlando]

Summary

The DexLimitSteps command leading breakpoint will be deleted after triggering 'hit_count' number of times if the argument is provided.

All the lit tests pass on linux (with lldb), and I've tested this on windows (with vs2017) manually as the lit tests for DexLimitSteps are currently unsupported on windows.

[Orlando]

+ Update Commands.md

[Orlando]

Ping.

Alternatively we could a slightly more general keyword arg hit_count=<int> that limits the times the breakpoints can be triggered to any number, rather than just once. I'm not sure we need it right now, but it's more flexible. Does anyone have any opinion on this?

[chrisjackson]

In D101527#2738333, @Orlando wrote:

Ping.

Alternatively we could a slightly more general keyword arg hit_count=<int> that limits the times the breakpoints can be triggered to any number, rather than just once. I'm not sure we need it right now, but it's more flexible. Does anyone have any opinion on this?

That is certainly more generalised, however if a sensible use case isn't apparent I'd suggest holding fire.

[chrisjackson]

lgtm, with one question. Is it necessary to have '=True' because of Python argument syntax, as opposed to passing 'is_one_shot' as an argument on its own?

[Orlando]

Thanks for taking a look.

In D101527#2738360, @chrisjackson wrote:

lgtm, with one question. Is it necessary to have '=True' because of Python argument syntax, as opposed to passing 'is_one_shot' as an argument on its own?

That's right, yeah. It would be nice to be able to pass in only is_one_shot but I'm not sure how we can do it easily?

[krmnparedes164]

In D101527#2738357, @chrisjackson wrote:

In D101527#2738333, @Orlando wrote:

Ping.

Alternatively we could a slightly more general keyword arg hit_count=<int> that limits the times the breakpoints can be triggered to any number, rather than just once. I'm not sure we need it right now, but it's more flexible. Does anyone have any opinion on this?

That is certainly more generalised, however if a sensible use case isn't apparent I'd suggest holding fire.

[chrisjackson]

On second thought, the more generalised command shouldn't have a much more complicated implementation. Additionally, usage of the function will be nearly identical to the user while being more flexible, so I think 'hit_count=<int>' should be added in favour of 'one_shot=<bool>'.

[Orlando]

+ Changed is_one_shot to hit_count. Updated patch title and summary.

[StephenTozer]

LGTM

[Orlando]

Thanks both

debuginfo-tests/dexter/Commands.md

 
 ### Heuristic
 [TODO]
 
 
 ----
 ## DexLimitSteps
     DexLimitSteps([expr, *values][, **from_line=1][,**to_line=Max]
-                  [,**on_line])
+                  [,**on_line][,**hit_count])
 
     Args:
         expr (str): variable or value to compare.
 
     Arg list:
         values (str): At least one potential value the expr may evaluate to.
 
     Keyword args:
         from_line (int): Define the start of the limited step range.
         to_line (int): Define the end of the limited step range.
         on_line (int): Define a range with length 1 starting and ending on the
                        same line.
+        hit_count (int): If provided, limit the number of times the command
+                         triggers.
 
 ### Description
 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'.
+file defined by the range 'from_line' and 'to_line' or 'on_line'. This only
+happens 'hit_count' number of times if the argument is provided.
 
 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, 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

def __init__(self, *args, **kwargs):
             self.values = [str(arg) for arg in args[1:]]
         try:
             on_line = kwargs.pop('on_line')
             self.from_line = on_line
             self.to_line = on_line
         except KeyError:
             self.from_line = kwargs.pop('from_line', 1)
             self.to_line = kwargs.pop('to_line', 999999)
+        self.hit_count = kwargs.pop('hit_count', None)
         if kwargs:
             raise TypeError('unexpected named args: {}'.format(
                 ', '.join(kwargs)))
         super(DexLimitSteps, self).__init__()
 
     def eval(self):
         raise NotImplementedError('DexLimitSteps commands cannot be evaled.')
 
     @staticmethod
     def get_name():
         return __class__.__name__
 
     @staticmethod
     def get_subcommands() -> dict:
         return None

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

class BreakpointRange:
     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`.
+
+       hit_count: None for no limit, or int to set the number of times the
+                  leading breakpoint is triggered before it is removed.
     """
 
-    def __init__(self, expression: str, path: str, range_from: int, range_to: int, values: list):
+    def __init__(self, expression: str, path: str, range_from: int, range_to: int,
+                 values: list, hit_count: int):
         self.expression = expression
         self.path = path
         self.range_from = range_from
         self.range_to = range_to
         self.conditional_values = values
+        self.max_hit_count = hit_count
+        self.current_hit_count = 0
 
     def has_conditions(self):
         return self.expression != None
 
     def get_conditional_expression_list(self):
         conditional_list = []
         for value in self.conditional_values:
             # (<expression>) == (<value>)
             conditional_expression = '({}) == ({})'.format(self.expression, value)
             conditional_list.append(conditional_expression)
         return conditional_list
 
+    def add_hit(self):
+        self.current_hit_count += 1
+
+    def should_be_removed(self):
+        if self.max_hit_count == None:
+            return False
+        return self.current_hit_count >= self.max_hit_count
+
 
 class ConditionalController(DebuggerControllerBase):
     def __init__(self, context, step_collection):
       self.context = context
       self.step_collection = step_collection
       self._bp_ranges = None
       self._build_bp_ranges()
       self._watches = set()
def _build_bp_ranges(self):
         try:
             limit_commands = commands['DexLimitSteps']
             for lc in limit_commands:
                 bpr = BreakpointRange(
                   lc.expression,
                   lc.path,
                   lc.from_line,
                   lc.to_line,
-                  lc.values)
+                  lc.values,
+                  lc.hit_count)
                 self._bp_ranges.append(bpr)
         except KeyError:
             raise DebuggerException('Missing DexLimitSteps commands, cannot conditionally step.')
 
     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:
def _run_debugger_custom(self):
             for bp_id in self.debugger.get_triggered_breakpoint_ids():
                 try:
                     # See if this is one of our leading breakpoints.
                     bpr = self._leading_bp_handles[bp_id]
                 except KeyError:
                     # This is a trailing bp. Mark it for removal.
                     bp_to_delete.append(bp_id)
                     continue
+
+                bpr.add_hit()
+                if bpr.should_be_removed():
+                    bp_to_delete.append(bp_id)
+                    del self._leading_bp_handles[bp_id]
                 # Add a range of trailing breakpoints covering the lines
                 # requested in the DexLimitSteps command. Ignore first line as
                 # 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 trailing breakpoints we just hit.
+            # Remove any trailing or expired leading breakpoints we just hit.
             for bp_id in bp_to_delete:
                 self.debugger.delete_breakpoint(bp_id)
 
             self.debugger.go()
             time.sleep(self._pause_between_steps)

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

+// Purpose:
+//      Test that \DexLimitSteps keyword argument hit_count correctly limits
+//      the number of times the command can trigger.
+//
+// REQUIRES: system-linux
+//
+// RUN: %dexter_regression_test -- %s | FileCheck %s
+// CHECK: hit_count.cpp
+
+int a;
+int main() {
+  for (int i = 0; i < 4; i++) {
+    a = i; // DexLabel('check')
+  }
+  return 0;
+}
+
+//// Unconditionally limit dexter's view of the program to 'on_line' and check
+//// for i=0, i=1. The test will fail if dexter sees any other value for test.
+// DexLimitSteps(hit_count=2, on_line=ref('check'))
+// DexExpectWatchValue('i', '0', '1')