[Dexter] Allow Dexter watch commands to specify a range of acceptable FP values

[StephenTozer]

Summary

This patch adds an optional argument to DexExpectWatchBase, float_range, which defines a +- acceptance range for expected floating point values. If passed, this assumes every expected value to be a floating point value, and an exception will be thrown if this is not the case.

[Orlando]

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

183

It's been a little while since I've looked in this code, am I reading this right - expected_value is the value returned by the debugger?

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/float_range_watch/float_range_zero.cpp

2–3 ↗

(On Diff #425449)

I'm not sure about this feature. What's the benefit of overriding 0.0 rather than just letting it mean zero?

[StephenTozer]

Remove unused "zero range" test.

[StephenTozer]

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

183

Correct, I think the etymology is that it refers to the field of the watch value that we're expecting, i.e. the "value that we have an expectation for". (I do think it's confusing.)

[Orlando]

Just to confirm, does this comment:

Remove unused "zero range" test.

mean that the feature I was worried about here:

I'm not sure about this feature. What's the benefit of overriding 0.0 rather than just letting it mean zero?

doesn't exist, and the test was not correct for this iteration of the patch?

Please can you add a test for when the float range is set to 0.0 (easy / common edge case). LGTM with that plus inline comments addressed.

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

156

Should this be <=? Otherwise an exact match with float_range of zero fails, right?

183

Thanks for clarifying. It would be great if we're able to improve that name at some point in the future.

cross-project-tests/debuginfo-tests/dexter/dex/utils/Exceptions.py

59

nit: IF -> if and it's -> its

[Orlando]

cross-project-tests/debuginfo-tests/dexter/dex/utils/Exceptions.py

59

If* and while you're there, removing "instruction" from that sentence works better IMO.

[StephenTozer]

Make some changes to correctly handle the float_range=0.0 case, add tests to cover it, and change comment as requested.

[Orlando]

LGTM!

[jmorse]

LGTM, with a request for a doc comment

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

145

IMHO: this wants a docustring explaining what it's doing, which as I understand it, is replacing expected_value with a nearby float value, if it's within the float_range, to ensure that it passes an equality comparision. (Writing this down will ease the future readers task).

[StephenTozer]

Update name and add docstring to requested function.

[StephenTozer]

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

36

New name + docstring here, since the new revision has messed up line numbers apparently.

cross-project-tests/debuginfo-tests/dexter/Commands.md

 
 ### Heuristic
 [TODO]
 
 
 ---
 ## DexExpectWatchValue
     DexExpectWatchValue(expr, *values [,**from_line=1][,**to_line=Max]
-                        [,**on_line][,**require_in_order=True])
+                        [,**on_line][,**require_in_order=True][,**float_range])
 
     Args:
         expr (str): expression to evaluate.
 
     Arg list:
         values (str): At least one expected value. NOTE: string type.
 
     Keyword args:
         from_line (int): Evaluate the expression from this line. Defaults to 1.
         to_line (int): Evaluate the expression to this line. Defaults to end of
             source.
         on_line (int): Only evaluate the expression on this line. If provided,
             this overrides from_line and to_line.
         require_in_order (bool): If False the values can appear in any order.
+        float_range (float): If provided, `values` must be floats, and will
+            match an actual value if they are within `float_range` of each other.
+
 
 ### Description
 Expect the expression `expr` to evaluate to the list of `values`
 sequentially.
 
 ### Heuristic
 [TODO]
 

cross-project-tests/debuginfo-tests/dexter/dex/command/ParseCommand.py

 """
 
 import os
 import unittest
 from copy import copy
 from pathlib import PurePath
 from collections import defaultdict, OrderedDict
 
-from dex.utils.Exceptions import CommandParseError
+from dex.utils.Exceptions import CommandParseError, NonFloatValueInCommand
 
 from dex.command.CommandBase import CommandBase
 from dex.command.commands.DexCommandLine import DexCommandLine
 from dex.command.commands.DexDeclareFile import DexDeclareFile
 from dex.command.commands.DexDeclareAddress import DexDeclareAddress
 from dex.command.commands.DexExpectProgramState import DexExpectProgramState
 from dex.command.commands.DexExpectStepKind import DexExpectStepKind
 from dex.command.commands.DexExpectStepOrder import DexExpectStepOrder
for region_start.line in range(len(file_lines)):
                 err_point.line += e.lineno - 1 # e.lineno is a position, not index.
                 err_point.char += e.offset - 1 # e.offset is a position, not index.
                 raise format_parse_err(e.msg, path, file_lines, err_point)
             except TypeError as e:
                 # This err should always point to the end of the command name.
                 err_point = copy(cmd_point)
                 err_point.char += len(command_name)
                 raise format_parse_err(str(e), path, file_lines, err_point)
+            except NonFloatValueInCommand as e:
+                err_point = copy(cmd_point)
+                err_point.char += len(command_name)
+                raise format_parse_err(str(e), path, file_lines, err_point)
             else:
                 if type(command) is DexLabel:
                     add_line_label(labels, command, path, cmd_point.get_lineno())
                 elif type(command) is DexDeclareAddress:
                     add_address(addresses, command, path, cmd_point.get_lineno())
                 elif type(command) is DexDeclareFile:
                     cmd_path = command.declared_file
                     if not os.path.isabs(cmd_path):

cross-project-tests/debuginfo-tests/dexter/dex/command/commands/DexExpectWatchBase.py

 import abc
 import difflib
 import os
 import math
 from collections import namedtuple
 
 from dex.command.CommandBase import CommandBase, StepExpectInfo
 from dex.command.StepValueInfo import StepValueInfo
+from dex.utils.Exceptions import NonFloatValueInCommand
 
 class AddressExpression(object):
     def __init__(self, name, offset=0):
         self.name = name
         self.offset = offset
 
     def is_resolved(self, resolutions):
         return self.name in resolutions
 
     # Given the resolved value of the address, resolve the final value of
     # this expression.
     def resolved_value(self, resolutions):
         if not self.name in resolutions or resolutions[self.name] is None:
             return None
         # Technically we should fill(8) if we're debugging on a 32bit architecture?
         return format_address(resolutions[self.name] + self.offset)
 
 def format_address(value, address_width=64):
     return "0x" + hex(value)[2:].zfill(math.ceil(address_width/4))
 
 def resolved_value(value, resolutions):
     return value.resolved_value(resolutions) if isinstance(value, AddressExpression) else value
 
 class DexExpectWatchBase(CommandBase):
     def __init__(self, *args, **kwargs):
         if len(args) < 2:
             raise TypeError('expected at least two args')
 
         self.expression = args[0]
         self.values = [arg if isinstance(arg, AddressExpression) else 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._require_in_order = kwargs.pop('require_in_order', True)
+        self.float_range = kwargs.pop('float_range', None)
+        if self.float_range is not None:
+            for value in self.values:
+                try:
+                    float(value)
+                except ValueError:
+                    raise NonFloatValueInCommand(f'Non-float value \'{value}\' when float_range arg provided')
         if kwargs:
             raise TypeError('unexpected named args: {}'.format(
                 ', '.join(kwargs)))
 
         # Number of times that this watch has been encountered.
         self.times_encountered = 0
 
         # We'll pop from this set as we encounter values so anything left at
class DexExpectWatchBase(CommandBase):
     def encountered_values(self):
         return sorted(list(set(self.describe_value(v) for v in set(self.values) - self._missing_values)))
 
     @abc.abstractmethod
     def _get_expected_field(self, watch):
         """Return a field from watch that this ExpectWatch command is checking.
         """
 
+    def _evaluate_as_floating_point(self, expected_value):
+        try:
+            value_as_float = float(expected_value)
+        except ValueError:
+            return expected_value
+
+        possible_values = self.values
+        for expected in possible_values:
+          try:
+              expected_as_float = float(expected)
+              difference = abs(value_as_float - expected_as_float)
+              if difference <= self.float_range:
+                  return expected
+          except ValueError:
+              pass
+        return expected_value
+
+    def _maybe_fix_float(self, value):
+        if self.float_range is not None:
+            return self._evaluate_as_floating_point(value)
+        else:
+            return value
+
     def _handle_watch(self, step_info):
         self.times_encountered += 1
 
         if not step_info.watch_info.could_evaluate:
             self.invalid_watches.append(step_info)
             return
 
         if step_info.watch_info.is_optimized_away:
             self.optimized_out_watches.append(step_info)
             return
 
         if step_info.watch_info.is_irretrievable:
             self.irretrievable_watches.append(step_info)
             return
 
+        expected_value = self._maybe_fix_float(step_info.expected_value)
+
         # Check to see if this value matches with a resolved address.
         matching_address = None
         for v in self.values:
             if (isinstance(v, AddressExpression) and
                     v.name in self.address_resolutions and
-                    self.resolve_value(v) == step_info.expected_value):
+                    self.resolve_value(v) == expected_value):
                 matching_address = v
                 break
 
         # If this is not an expected value, either a direct value or an address,
         # then this is an unexpected watch.
-        if step_info.expected_value not in self.values and matching_address is None:
+        if expected_value not in self.values and matching_address is None:
             self.unexpected_watches.append(step_info)
             return
 
         self.expected_watches.append(step_info)
-        value_to_remove = matching_address if matching_address is not None else step_info.expected_value
+        value_to_remove = matching_address if matching_address is not None else expected_value
         try:
             self._missing_values.remove(value_to_remove)
         except KeyError:
             pass
 
     def _check_watch_order(self, actual_watches, expected_values):
         """Use difflib to figure out whether the values are in the expected order
         or not.
         """
         differences = []
-        actual_values = [w.expected_value for w in actual_watches]
+        actual_values = [self._maybe_fix_float(w.expected_value) for w in actual_watches]
         value_differences = list(difflib.Differ().compare(actual_values,
                                                           expected_values))
 
         missing_value = False
         index = 0
         for vd in value_differences:
             kind = vd[0]
             if kind == '+':
def eval(self, step_collection):
                     step_info = StepValueInfo(step.step_index, watch, 
                                               expected_field)
                     self._handle_watch(step_info)
 
         if self._require_in_order:
             # A list of all watches where the value has changed.
             value_change_watches = []
             prev_value = None
+            all_expected_values = []
             for watch in self.expected_watches:
-                if watch.expected_value != prev_value:
+                expected_value = self._maybe_fix_float(watch.expected_value)
+                all_expected_values.append(expected_value)
+                if expected_value != prev_value:
                     value_change_watches.append(watch)
-                    prev_value = watch.expected_value
+                    prev_value = expected_value
 
             resolved_values = [self.resolve_value(v) for v in self.values]
             self.misordered_watches = self._check_watch_order(
                 value_change_watches, [
-                    v for v in resolved_values if v in
-                    [w.expected_value for w in self.expected_watches]
+                    v for v in resolved_values if v in all_expected_values
                 ])

cross-project-tests/debuginfo-tests/dexter/dex/utils/Exceptions.py

def __init__(self, *args, **kwargs):
         super(CommandParseError, self).__init__(*args, **kwargs)
         self.filename = None
         self.lineno = None
         self.info = None
         self.src = None
         self.caret = None
 
 
+class NonFloatValueInCommand(CommandParseError):
+    """If a command has the float_range arg but at least one of its expected
+    values cannot be converted to a float."""
+
+    def __init__(self, *args, **kwargs):
+        super(NonFloatValueInCommand, self).__init__(*args, **kwargs)
+        self.value = None
+
 class ToolArgumentError(Dexception):
     """If a tool argument is invalid."""
     pass
 
 
 class BuildScriptException(Dexception):
     """If there is an error in a build script file."""
 
     def __init__(self, *args, **kwargs):
         self.script_error = kwargs.pop('script_error', None)
         super(BuildScriptException, self).__init__(*args, **kwargs)
 
 
 class HeuristicException(Dexception):
     """If there was a problem with the heuristic."""
     pass

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/penalty/float_range_out_range.cpp

+// Purpose:
+//      Check that a \DexExpectWatchValue float_range that is not large enough
+//      detects unexpected watch values.
+//
+// UNSUPPORTED: system-darwin
+//
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
+// CHECK: float_range_out_range.cpp:
+
+int main() {
+  float a = 1.0f;
+  a = a - 0.5f;
+  return a;  //DexLabel('check')
+}
+
+// DexExpectWatchValue('a', '1.00000', from_line=ref('check1'), to_line=ref('check2'), float_range=0.4)

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/penalty/float_range_zero_nonmatch.cpp

+// Purpose:
+//      Check that \DexExpectWatchValue float_range=0.0 matches only exact
+//      values.
+//
+// UNSUPPORTED: system-darwin
+//
+// RUN: not %dexter_regression_test -- %s | FileCheck %s
+// CHECK: float_range_zero_nonmatch.cpp:
+
+int main() {
+  float a = 1.0f;
+  return a; //DexLabel('check')
+}
+
+// DexExpectWatchValue('a', '1.0000001', on_line=ref('check'), float_range=0.0)

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/float_range_watch/float_range_multiple.cpp

+// Purpose:
+//      Check that \DexExpectWatchValue float_range=0.5 considers a range
+//      difference of 0.49999 to be an expected watch value for multple values.
+//
+// UNSUPPORTED: system-darwin
+//
+// RUN: %dexter_regression_test -- %s | FileCheck %s
+// CHECK: float_range_multiple.cpp:
+
+int main() {
+  float a = 1.0f;
+  float b = 100.f;
+  a = a + 0.4999f;
+  a = a + b; // DexLabel('check1')
+  return a;  //DexLabel('check2')
+}
+
+// DexExpectWatchValue('a', '1.0', '100.0', from_line=ref('check1'), to_line=ref('check2'), float_range=0.5)

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/float_range_watch/float_range_no_arg.cpp

+// Purpose:
+//      Check that omitted float_range from \DexExpectWatchValue turns off
+//      the floating point range evalution and defaults back to
+//      pre-float evalution.
+//
+// UNSUPPORTED: system-darwin
+//
+// RUN: %dexter_regression_test -- %s | FileCheck %s
+// CHECK: float_range_no_arg.cpp:
+
+int main() {
+  float a = 1.0f;
+  return a;  //DexLabel('check')
+}
+
+// DexExpectWatchValue('a', '1.00000', on_line=ref('check'))

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/float_range_watch/float_range_small.cpp

+// Purpose:
+//      Check that \DexExpectWatchValue float_range=0.5 considers a range
+//      difference of 0.49999 to be an expected watch value.
+//
+// UNSUPPORTED: system-darwin
+//
+// RUN: %dexter_regression_test -- %s | FileCheck %s
+// CHECK: float_range_small.cpp:
+
+int main() {
+  float a = 1.0f;
+  a = a - 0.49999f;
+  return a; //DexLabel('check')
+}
+
+// DexExpectWatchValue('a', '1.0', on_line=ref('check'), float_range=0.5)

cross-project-tests/debuginfo-tests/dexter/feature_tests/commands/perfect/float_range_watch/float_range_zero_match.cpp

+// Purpose:
+//      Check that \DexExpectWatchValue float_range=0.0 matches exact values.
+//
+// UNSUPPORTED: system-darwin
+//
+// RUN: %dexter_regression_test -- %s | FileCheck %s
+// CHECK: float_range_zero_match.cpp:
+
+int main() {
+  float a = 1.0f;
+  return a; //DexLabel('check')
+}
+
+// DexExpectWatchValue('a', '1.0000000', on_line=ref('check'), float_range=0.0)