[Dexter] Enable after_hit_count for state nodes (#203846)

The after_hit_count attribute for a state node causes it to become
active only after it would have become active N times. This uses the
existing logic for incrementing hit counts, i.e. after the node becomes
"active", we will not add another hit count until it stops being active
for at least one step. Since state nodes with after_hit_count do not
become active before reaching the required hit count, this requires us
to keep track of an "early" set of state nodes, meaning nodes that would
be active if not for their after_hit_count.

cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerControllers/ScriptDebuggerController.py

@@ -19,7 +19,7 @@ from dex.debugger.DebuggerControllers.DebuggerControllerBase import (
 )
 from dex.debugger.DebuggerBase import DebuggerBase
 from dex.debugger.DAP import DAP
-from dex.evaluation.StateMatch import StateMatchContext, get_active_where_matches
+from dex.evaluation.StateMatch import StateMatchContext, get_state_match
 from dex.test_script.Nodes import Where
 from dex.test_script.Script import DexterScript, Scope
 from dex.tools import Context
@@ -133,13 +133,11 @@ class ScriptDebuggerController(DebuggerControllerBase):
                 key=lambda where: str(where),
             )
 
-            active_where_matches = get_active_where_matches(
-                script, step_info, state_match_context
-            )
+            state_match = get_state_match(script, step_info, state_match_context)
 
             watches = defaultdict(list)
             scope_watches = defaultdict(list)
-            for where, where_match in active_where_matches.items():
+            for where, where_match in state_match.where_match_results.items():
                 watches[where_match.frame_idx].extend(
                     watch
                     for expect in where_match.active_expects
@@ -158,7 +156,7 @@ class ScriptDebuggerController(DebuggerControllerBase):
 
             active_thens = [
                 then
-                for where_match in active_where_matches.values()
+                for where_match in state_match.where_match_results.values()
                 for then in where_match.active_thens
             ]
             should_step_out = any(then.command == "step_out" for then in active_thens)
@@ -175,10 +173,10 @@ class ScriptDebuggerController(DebuggerControllerBase):
                 next_action = DebuggerAction.STEP_OUT
             elif any(
                 where_match.frame_idx == 0
-                for where_match in active_where_matches.values()
+                for where_match in state_match.where_match_results.values()
             ):
                 next_action = DebuggerAction.STEP_OVER
-            elif active_where_matches:
+            elif state_match.where_match_results:
                 next_action = DebuggerAction.STEP_OUT
             elif all(
                 where in state_match_context.expired_wheres
@@ -192,7 +190,7 @@ class ScriptDebuggerController(DebuggerControllerBase):
             bp_to_delete = []
             pending_wheres = set(
                 where
-                for where_match in active_where_matches.values()
+                for where_match in state_match.where_match_results.values()
                 for where in where_match.pending_wheres
             )
 
@@ -222,7 +220,7 @@ class ScriptDebuggerController(DebuggerControllerBase):
 
             # If we have --trace enabled, report a short overview of this step.
             self.context.logger.note(
-                f"Stopped at {step_info.current_function} {step_info.current_location.short_str()}, {len(active_where_matches)} !wheres on the stack, next_action={next_action}"
+                f"Stopped at {step_info.current_function} {step_info.current_location.short_str()}, {len(state_match.where_match_results)} !wheres on the stack, next_action={next_action}"
             )
 
             if next_action == DebuggerAction.EXIT:

cross-project-tests/debuginfo-tests/dexter/dex/evaluation/ExpectRewriter.py

@@ -10,7 +10,7 @@ from collections import defaultdict
 from typing import Any, Dict, List, Optional, Set, Tuple, Union
 
 from dex.dextIR import DextIR, StepIR, ValueIR
-from dex.evaluation.StateMatch import StateMatchContext, get_active_where_matches
+from dex.evaluation.StateMatch import StateMatchContext, get_state_match
 from dex.test_script.Nodes import (
     DexRange,
     Expect,
@@ -189,10 +189,10 @@ class StepExpectRewriter:
     ):
         self.step = step
         self.script = script
-        self.state_match = get_active_where_matches(script, step, state_match_context)
+        self.state_match = get_state_match(script, step, state_match_context)
         active_expects = {
             expect: where_match.frame_idx
-            for where_match in self.state_match.values()
+            for where_match in self.state_match.where_match_results.values()
             for expect in where_match.active_expects
         }
         self.expect_value_matches: Dict[Expect, ExpectedValueRewriter] = {}

cross-project-tests/debuginfo-tests/dexter/dex/evaluation/RunMatch.py

@@ -26,7 +26,7 @@ from dex.evaluation.Metrics import (
     get_variable_metrics,
     serialize_metric_to_json,
 )
-from dex.evaluation.StateMatch import StateMatchContext, get_active_where_matches
+from dex.evaluation.StateMatch import StateMatchContext, get_state_match
 from dex.test_script import DexterScript, Scope
 from dex.test_script.Nodes import Expect, Line, Step, Value
 
@@ -45,10 +45,10 @@ class DebuggerStepMatch:
         self.step = step
         self.script = script
         self.match_context = match_context
-        self.state_match = get_active_where_matches(script, step, state_match_context)
+        self.state_match = get_state_match(script, step, state_match_context)
         expects_to_match = {
             expect: where_match.frame_idx
-            for where_match in self.state_match.values()
+            for where_match in self.state_match.where_match_results.values()
             for expect in where_match.active_expects
         }
         self.var_expect_matches: Dict[Expect, DebuggerExpectMatch] = {}
@@ -177,7 +177,10 @@ class DebuggerRunMatch(object):
             result += f"Step {step_match.step.step_index}:\n"
             result += f"  {step_match.step.current_location}\n"
             frame_active_wheres = defaultdict(list)
-            for where, where_match in step_match.state_match.items():
+            for (
+                where,
+                where_match,
+            ) in step_match.state_match.where_match_results.items():
                 frame_active_wheres[where_match.frame_idx].append(str(where))
             if not frame_active_wheres:
                 result += f"  No active !where nodes.\n"

cross-project-tests/debuginfo-tests/dexter/dex/evaluation/StateMatch.py

@@ -9,6 +9,7 @@ hitcounts) to descriptions of expected state in a DexterScript."""
 
 from collections import Counter
 from dataclasses import dataclass, field
+from enum import Enum, IntEnum
 import os
 from typing import Dict, List, Optional, Set
 
@@ -33,7 +34,10 @@ class StateMatchContext:
         if self._last_match_result is None:
             return True
         # If this !where did not appear in any frame in the previous step, this is a fresh hit.
-        if where not in self._last_match_result:
+        if (
+            where not in self._last_match_result.where_match_results
+            and where not in self._last_match_result.early_wheres
+        ):
             return True
         # If !where uses a function breakpoint and that breakpoint was hit this step, this is a fresh hit.
         if where.function and not where.lines and where in step.hit_where_bps:
@@ -44,11 +48,14 @@ class StateMatchContext:
         """Checks whether the current step can be counted as a new "hit" for `where`. Increments `where`'s hit count if
         it has a new hit, and returns True iff so."""
         assert (
-            where.for_hit_count is not None
-        ), "Tried to add hit count for !where without for_hit_count?"
+            where.for_hit_count is not None or where.after_hit_count is not None
+        ), "Tried to add hit count for !where without for/after_hit_count?"
         if self.where_hit_is_new(where, step):
             self.where_hit_counts[where] += 1
-            if self.where_hit_counts[where] >= where.for_hit_count:
+            print(f"Added hit count for {where}")
+            if where.for_hit_count is not None and self.where_hit_counts[
+                where
+            ] >= where.for_hit_count + (where.after_hit_count or 0):
                 self.expired_wheres.add(where)
             return True
         return False
@@ -65,37 +72,48 @@ def is_subpath(subpath: str, superpath: str) -> bool:
     return normalized_superpath.endswith(normalized_subpath)
 
 
+class WhereFrameMatchResult(IntEnum):
+    FALSE = 0
+    TRUE = 1
+    EARLY = 2
+
+
 def _match_where_to_frame(
     where: Where,
     frame: FrameIR,
     labels: FileLabels,
     context: StateMatchContext,
     default_path: Optional[str] = None,
-) -> bool:
+) -> WhereFrameMatchResult:
     """A very simple matcher, returns True iff `where` matches `frame`."""
     file = where.file
     if not file and where.lines and not where.function:
         file = default_path
     if file is not None and not is_subpath(file, frame.loc.path):
-        return False
+        return WhereFrameMatchResult.FALSE
     if where.function is not None:
         fn = frame.function
         if "(" in fn:
             fn = fn.split("(")[0]
         if where.function != fn:
-            return False
+            return WhereFrameMatchResult.FALSE
     if where.lines is not None:
         if frame.loc.lineno not in where.get_lines(labels):
-            return False
+            return WhereFrameMatchResult.FALSE
     if where.for_hit_count is not None:
+        after_hit_count = where.after_hit_count or 0
         where_hit_count = context.where_hit_counts[where]
-        if where_hit_count > where.for_hit_count:
-            return False
-    if where.after_hit_count is not None or where.conditions is not None:
-        raise NotImplementedError(
-            "!where hit counts and conditions currently unsupported."
-        )
-    return True
+        if where_hit_count > where.for_hit_count + (after_hit_count):
+            return WhereFrameMatchResult.FALSE
+    if where.conditions is not None:
+        raise NotImplementedError("!where conditions currently unsupported.")
+    # The check for after_hit_count must go last, as before we return EARLY, we need to know that the only condition
+    # preventing the match is after_hit_count.
+    if where.after_hit_count is not None:
+        where_hit_count = context.where_hit_counts[where]
+        if where_hit_count <= where.after_hit_count:
+            return WhereFrameMatchResult.EARLY
+    return WhereFrameMatchResult.TRUE
 
 
 def match_where_to_frame(
@@ -105,11 +123,15 @@ def match_where_to_frame(
     labels: FileLabels,
     context: StateMatchContext,
     default_path: Optional[str] = None,
-) -> bool:
-    """Returns True if `where` matches `frame`. As part of this check, we perform the check once, and if necessary we
-    may increment `where`'s hit count and check again."""
+) -> WhereFrameMatchResult:
+    """Returns True if `where` matches `frame`. As part of this check, we perform the where-to-frame check once, and if
+    we get a result that could change due to an increased hit count (i.e. if we get a match with a `for_hit_count`
+    where, or if we get an "early" result), we increment `where`'s hit count and run the check again to check whether
+    the result changes."""
     result = _match_where_to_frame(where, frame, labels, context, default_path)
-    if result == True and where.for_hit_count is not None:
+    if result == WhereFrameMatchResult.EARLY or (
+        result == WhereFrameMatchResult.TRUE and where.for_hit_count is not None
+    ):
         if context.add_hit_if_where_hit_is_new(where, step):
             result = _match_where_to_frame(where, frame, labels, context, default_path)
     return result
@@ -126,19 +148,28 @@ class WhereMatchResult:
     active_thens: List[Then] = field(default_factory=list)
     pending_wheres: List[Where] = field(default_factory=list)
     expired_wheres: List[Where] = field(default_factory=list)
+    early_wheres: List[Where] = field(default_factory=list)
 
 
-StateMatchResult = Dict[Where, WhereMatchResult]
+class StateMatchResult:
+    def __init__(
+        self,
+        where_match_results: Dict[Where, WhereMatchResult],
+        early_wheres: Set[Where],
+    ):
+        self.where_match_results = where_match_results
+        self.early_wheres = early_wheres
 
 
-def get_active_where_matches(
+def get_state_match(
     script: DexterScript, step_info: StepIR, match_context: StateMatchContext
-) -> Dict[Where, WhereMatchResult]:
+) -> StateMatchResult:
     """Match the script against the step_info, producing a dict that maps each !where that matches a stack frame to the
     index of the (rootmost) stack frame that it matches, and if the frame that it matches is the current stack frame
     (i.e. the frame index is 0), also includes a list of every direct child !expect node for that !where.
     """
     active_where_expects: Dict[Where, WhereMatchResult] = {}
+    early_wheres: Set[Where] = set()
 
     def get_active_wheres(where: Where, scope: Scope):
         # For nested !wheres, we must match a specific frame relative to the parent !where.
@@ -167,27 +198,31 @@ def get_active_where_matches(
             labels = script.get_labels(
                 expected_file or step_info.frames[target_frame_idx].loc.path
             )
-            if match_where_to_frame(
+            match_result = match_where_to_frame(
                 where,
                 step_info.frames[target_frame_idx],
                 step_info,
                 labels,
                 match_context,
-            ):
+            )
+            if match_result == WhereFrameMatchResult.TRUE:
                 active_where_expects[where] = WhereMatchResult(target_frame_idx)
+            elif match_result == WhereFrameMatchResult.EARLY:
+                early_wheres.add(where)
             return
         # For this !where, search for the rootmost stack frame that matches it.
-        matching_frame_idx = None
         for frame_idx, frame in reversed(list(enumerate(step_info.frames))):
             labels = script.get_labels(expected_file or frame.loc.path)
-            if match_where_to_frame(
+            match_result = match_where_to_frame(
                 where, frame, step_info, labels, match_context, script.root_scope.file
-            ):
-                matching_frame_idx = frame_idx
-                break
+            )
+            if match_result == WhereFrameMatchResult.TRUE:
+                active_where_expects[where] = WhereMatchResult(frame_idx)
+                return
+            if match_result == WhereFrameMatchResult.EARLY:
+                early_wheres.add(where)
+                return
 
-        if matching_frame_idx is not None:
-            active_where_expects[where] = WhereMatchResult(matching_frame_idx)
 
     # As we visit the script nodes in pre-order traversal, we can always assume that an expect's parent !where
     # has already been visited, and thus should have an entry in active_where_expects if it is active.
@@ -211,5 +246,6 @@ def get_active_where_matches(
         visit_then=get_active_thens,
     )
 
-    match_context.update(active_where_expects)
-    return active_where_expects
+    result = StateMatchResult(active_where_expects, early_wheres)
+    match_context.update(result)
+    return result

cross-project-tests/debuginfo-tests/dexter/dex/test_script/Nodes.py

@@ -52,6 +52,19 @@ class FileLabels:
 class Where:
     """One or more instances of this class define a range of steps in a debugging session. Any expects in the script
     within scope of a "Where" will only be evaluated for the steps where the Where applies.
+
+    Supports a set of attributes to specify the state of a stack frame that this node matches against:
+    - file: The path (absolute or relative) of the frame source location.
+    - line: The line number of the frame source location.
+    - function: The function name (exactly matching the name shown by the debugger) of the frame.
+    - at_frame_idx: Only specifiable for !and nodes; changes the frame that this node matches against from its parent
+                    frame to the frame at the specified index.
+    - after_hit_count: Requires that this node must become "active" a specified number of times before it will be
+                       considered as active. If for_hit_count is also specified, the "for" hit count will only begin
+                       accumulating after the "after" hit count.
+    - for_hit_count: Limits this node to becoming active a specified number of times, after which it will not become
+                     active again. If after_hit_count is also specified, the "for" hit count will only begin
+                     accumulating after the "after" hit count.
     """
 
     def __init__(self, attributes: dict, is_and: bool):
@@ -70,14 +83,6 @@ class Where:
             raise DexterNodeError(
                 self, f"unexpected attributes {', '.join(attributes)}"
             )
-        if (
-            not self.function
-            and not self.lines
-            and (self.for_hit_count or self.after_hit_count)
-        ):
-            raise DexterNodeError(
-                self, "can't check hit counts without an explicit lines or function arg"
-            )
         if self.at_frame_idx is not None and not self.is_and:
             raise DexterNodeError(self, "at_frame_idx can only be used with !and nodes")
 

cross-project-tests/debuginfo-tests/dexter/feature_tests/scripts/debugging/then_after_hit_count.cpp

@@ -0,0 +1,32 @@
+// RUN: %dexter_regression_test_cxx_build %s -o %t
+// RUN: %dexter_regression_test_run --use-script --skip-evaluate --binary %t \
+// RUN:   -- %s | FileCheck %s
+
+/// Test !then finish with !and{after_hit_count}.
+/// The long loop will be exited when we hit the `!then finish` command,
+/// which we will see after 101 hits of main: 50 hits on the loop line before
+/// after_hit_count is reached, 50 hits from stepping off of the loop line, and
+/// 1 hit from the step where we trigger the !then node.
+
+// CHECK-LABEL:      Step 0
+// CHECK-COUNT-101:   main
+// CHECK-NOT: Step
+
+int Until = 1000;
+bool checkCows() { return --Until < 0; }
+
+int main() {
+  bool AreCowsHomeYet = false;
+  while (!AreCowsHomeYet) {
+    AreCowsHomeYet = checkCows(); // !dex_label loop
+  }
+  return 0;
+}
+
+/*
+---
+!where {lines: !label loop}:
+    !value AreCowsHomeYet: false
+    !and {after_hit_count: 50}: !then finish
+...
+*/

cross-project-tests/debuginfo-tests/dexter/feature_tests/scripts/debugging/where_hit_count_early_exit.cpp

@@ -1,28 +1,28 @@
 // RUN: %dexter_regression_test_cxx_build %s -o %t
 // RUN: %dexter_regression_test_run --use-script --skip-evaluate --binary %t \
-// RUN:   --timeout-total 10 -- %s 2>&1 | FileCheck %s
+// RUN:   -- %s 2>&1 | FileCheck %s
 
 /// Test that when all root !where nodes have expired, we exit without waiting
 /// for the debuggee to finish.
 
-// CHECK-NOT: timeout reached
-
 // CHECK-LABEL: Step 0
 // CHECK-COUNT-3: getRandomNumber
 // CHECK-NOT: getRandomNumber
 
 /// All on one line for simplicity so that we only get one step per call.
 int getRandomNumber(int Max) { return 4 % Max; }
+int Attempts = 1000;
 
 int main() {
   // Bogo search
   int List[] = {0, 0, 0, 0, 5, 0, 0, 0, 0, 0};
   int SearchTarget = 0;
-  while (true) {
+  while (--Attempts > 0) {
     int NextSearch = getRandomNumber(10);
     if (List[NextSearch] == SearchTarget)
       return NextSearch;
   }
+  return -1;
 }
 
 /*

cross-project-tests/debuginfo-tests/dexter/feature_tests/scripts/where_hit_count.cpp

@@ -0,0 +1,38 @@
+// RUN: %dexter_regression_test_cxx_build %s -o %t
+// RUN: %dexter_regression_test_run --use-script --binary %t -- %s \
+// RUN:   | FileCheck %s
+
+/// Test that Dexter respects for and after hit_counts.
+
+void receive(int N) {}
+
+int main() {
+  int Current = 0;
+  int Increment = 1;
+  for (int I = 0; I < 10; ++I) {
+    receive(Current); // !dex_label loop
+    Current += Increment++;
+  }
+  return 0;
+}
+
+// CHECK: total_watched_steps: 10
+// CHECK: correct_steps: 10
+// CHECK: incorrect_steps: 0
+// CHECK: partial_step_correctness: 10.0
+// CHECK: missing_var_steps: 0
+// CHECK: unexpected_value_steps: 0
+// CHECK: correct_step_coverage: 100.0% (10/10)
+// CHECK: seen_values: 10
+// CHECK: missing_values: 0
+
+/*
+---
+!where {function: receive, after_hit_count: 3, for_hit_count: 4}:
+  !value N: [6, 10, 15, 21]
+!where {lines: !label loop, for_hit_count: 3}:
+  !value Current: [0, 1, 3]
+!where {lines: !label loop, after_hit_count: 7}:
+  !value Current: [28, 36, 45]
+...
+*/