[Dexter] Add DAP instruction and function breakpoint handling (#152718)

Add `add_function_breakpoint` and `add_instruction_breakpoint` to DebuggerBase.

Add implementations for DAP-based debuggers.

Author: OCHyams

cross-project-tests/debuginfo-tests/dexter/dex/debugger/DAP.py

@@ -218,6 +218,10 @@ def __init__(self, context, *args):
         self.file_to_bp = defaultdict(list)
         # { dex_breakpoint_id -> (file, line, condition) }
         self.bp_info = {}
+        # { dex_breakpoint_id -> function_name }
+        self.function_bp_info = {}
+        # { dex_breakpoint_id -> instruction_reference }
+        self.instruction_bp_info = {}
         # We don't rely on IDs returned directly from the debug adapter. Instead, we use dexter breakpoint IDs, and
         # maintain a two-way-mapping of dex_bp_id<->dap_bp_id. This also allows us to defer the setting of breakpoints
         # in the debug adapter itself until necessary.
@@ -226,6 +230,8 @@ def __init__(self, context, *args):
         self.dex_id_to_dap_id = {}
         self.dap_id_to_dex_ids = {}
         self.pending_breakpoints: bool = False
+        self.pending_function_breakpoints: bool = False
+        self.pending_instruction_breakpoints: bool = False
         # List of breakpoints, indexed by BP ID
         # Each entry has the source file (for use in referencing desired_bps), and the DA-assigned
         # ID for that breakpoint if it has one (if it has been removed or not yet created then it will be None).
@@ -288,6 +294,26 @@ def make_set_breakpoint_request(source: str, bps) -> dict:
             {"source": {"path": source}, "breakpoints": [bp.toDict() for bp in bps]},
         )
 
+    @staticmethod
+    def make_set_function_breakpoint_request(function_names: list) -> dict:
+        # Function breakpoints may specify conditions and hit counts, though we
+        # don't use those here (though perhaps we should use native hit count,
+        # rather than emulating it ConditionalController, now that we have a
+        # shared interface (DAP)).
+        return DAP.make_request(
+            "setFunctionBreakpoints",
+            {"breakpoints": [{"name": f} for f in function_names]},
+        )
+
+    @staticmethod
+    def make_set_instruction_breakpoint_request(addrs: list) -> dict:
+        # Instruction breakpoints have additional fields we're ignoring for the
+        # moment.
+        return DAP.make_request(
+            "setInstructionBreakpoints",
+            {"breakpoints": [{"instructionReference": a} for a in addrs]},
+        )
+
     ############################################################################
     ## DAP communication & state-handling functions
 
@@ -569,45 +595,96 @@ def clear_breakpoints(self):
     def _add_breakpoint(self, file, line):
         return self._add_conditional_breakpoint(file, line, None)
 
+    def add_function_breakpoint(self, name: str):
+        if not self._debugger_state.capabilities.supportsFunctionBreakpoints:
+            raise DebuggerException("Debugger does not support function breakpoints")
+        new_id = self.get_next_bp_id()
+        self.function_bp_info[new_id] = name
+        self.pending_function_breakpoints = True
+        return new_id
+
+    def add_instruction_breakpoint(self, addr: str):
+        if not self._debugger_state.capabilities.supportsInstructionBreakpoints:
+            raise DebuggerException("Debugger does not support instruction breakpoints")
+        new_id = self.get_next_bp_id()
+        self.instruction_bp_info[new_id] = addr
+        self.pending_instruction_breakpoints = True
+        return new_id
+
     def _add_conditional_breakpoint(self, file, line, condition):
         new_id = self.get_next_bp_id()
         self.file_to_bp[file].append(new_id)
         self.bp_info[new_id] = (file, line, condition)
         self.pending_breakpoints = True
         return new_id
 
+    def _update_breakpoint_ids_after_request(self, dex_bp_ids: list, response: dict):
+        dap_bp_ids = [bp["id"] for bp in response["body"]["breakpoints"]]
+        if len(dex_bp_ids) != len(dap_bp_ids):
+            self.context.logger.error(
+                f"Sent request to set {len(dex_bp_ids)} breakpoints, but received {len(dap_bp_ids)} in response."
+            )
+        visited_dap_ids = set()
+        for i, dex_bp_id in enumerate(dex_bp_ids):
+            dap_bp_id = dap_bp_ids[i]
+            self.dex_id_to_dap_id[dex_bp_id] = dap_bp_id
+            # We take the mappings in the response as the canonical mapping, meaning that if the debug server has
+            # simply *changed* the DAP ID for a breakpoint we overwrite the existing mapping rather than adding to
+            # it, but if we receive the same DAP ID for multiple Dex IDs *then* we store a one-to-many mapping.
+            if dap_bp_id in visited_dap_ids:
+                self.dap_id_to_dex_ids[dap_bp_id].append(dex_bp_id)
+            else:
+                self.dap_id_to_dex_ids[dap_bp_id] = [dex_bp_id]
+                visited_dap_ids.add(dap_bp_id)
+
     def _flush_breakpoints(self):
-        if not self.pending_breakpoints:
-            return
-        for file in self.file_to_bp.keys():
-            desired_bps = self._get_desired_bps(file)
+        # Normal and conditional breakpoints.
+        if self.pending_breakpoints:
+            self.pending_breakpoints = False
+            for file in self.file_to_bp.keys():
+                desired_bps = self._get_desired_bps(file)
+                request_id = self.send_message(
+                    self.make_set_breakpoint_request(file, desired_bps)
+                )
+                result = self._await_response(request_id, 10)
+                if not result["success"]:
+                    raise DebuggerException(f"could not set breakpoints for '{file}'")
+                # The debug adapter may have chosen to merge our breakpoints. From here we need to identify such cases and
+                # handle them so that our internal bookkeeping is correct.
+                dex_bp_ids = self.get_current_bps(file)
+                self._update_breakpoint_ids_after_request(dex_bp_ids, result)
+
+        # Function breakpoints.
+        if self.pending_function_breakpoints:
+            self.pending_function_breakpoints = False
+            desired_bps = list(self.function_bp_info.values())
             request_id = self.send_message(
-                self.make_set_breakpoint_request(file, desired_bps)
+                self.make_set_function_breakpoint_request(desired_bps)
             )
             result = self._await_response(request_id, 10)
             if not result["success"]:
-                raise DebuggerException(f"could not set breakpoints for '{file}'")
-            # The debug adapter may have chosen to merge our breakpoints. From here we need to identify such cases and
-            # handle them so that our internal bookkeeping is correct.
-            dex_bp_ids = self.get_current_bps(file)
-            dap_bp_ids = [bp["id"] for bp in result["body"]["breakpoints"]]
-            if len(dex_bp_ids) != len(dap_bp_ids):
-                self.context.logger.error(
-                    f"Sent request to set {len(dex_bp_ids)} breakpoints, but received {len(dap_bp_ids)} in response."
+                raise DebuggerException(
+                    f"could not set function breakpoints: '{desired_bps}'"
                 )
-            visited_dap_ids = set()
-            for i, dex_bp_id in enumerate(dex_bp_ids):
-                dap_bp_id = dap_bp_ids[i]
-                self.dex_id_to_dap_id[dex_bp_id] = dap_bp_id
-                # We take the mappings in the response as the canonical mapping, meaning that if the debug server has
-                # simply *changed* the DAP ID for a breakpoint we overwrite the existing mapping rather than adding to
-                # it, but if we receive the same DAP ID for multiple Dex IDs *then* we store a one-to-many mapping.
-                if dap_bp_id in visited_dap_ids:
-                    self.dap_id_to_dex_ids[dap_bp_id].append(dex_bp_id)
-                else:
-                    self.dap_id_to_dex_ids[dap_bp_id] = [dex_bp_id]
-                    visited_dap_ids.add(dap_bp_id)
-        self.pending_breakpoints = False
+            # We expect the breakpoint order to match in request and response.
+            dex_bp_ids = list(self.function_bp_info.keys())
+            self._update_breakpoint_ids_after_request(dex_bp_ids, result)
+
+        # Address / instruction breakpoints.
+        if self.pending_instruction_breakpoints:
+            self.pending_instruction_breakpoints = False
+            desired_bps = list(self.instruction_bp_info.values())
+            request_id = self.send_message(
+                self.make_set_instruction_breakpoint_request(desired_bps)
+            )
+            result = self._await_response(request_id, 10)
+            if not result["success"]:
+                raise DebuggerException(
+                    f"could not set instruction breakpoints: '{desired_bps}'"
+                )
+            # We expect the breakpoint order to match in request and response.
+            dex_bp_ids = list(self.instruction_bp_info.keys())
+            self._update_breakpoint_ids_after_request(dex_bp_ids, result)
 
     def _confirm_triggered_breakpoint_ids(self, dex_bp_ids):
         """Can be overridden for any specific implementations that need further processing from the debug server's
@@ -632,8 +709,16 @@ def get_triggered_breakpoint_ids(self):
     def delete_breakpoints(self, ids):
         per_file_deletions = defaultdict(list)
         for dex_bp_id in ids:
-            source, _, _ = self.bp_info[dex_bp_id]
-            per_file_deletions[source].append(dex_bp_id)
+            if dex_bp_id in self.bp_info:
+                source, _, _ = self.bp_info[dex_bp_id]
+                per_file_deletions[source].append(dex_bp_id)
+            elif dex_bp_id in self.function_bp_info:
+                del self.function_bp_info[dex_bp_id]
+                self.pending_function_breakpoints = True
+            elif dex_bp_id in self.instruction_bp_info:
+                del self.instruction_bp_info[dex_bp_id]
+                self.pending_instruction_breakpoints = True
+
         for file, deleted_ids in per_file_deletions.items():
             old_len = len(self.file_to_bp[file])
             self.file_to_bp[file] = [
@@ -651,7 +736,13 @@ def _get_launch_params(self, cmdline):
         """ "Set the debugger-specific params used in a launch request."""
 
     def launch(self, cmdline):
-        assert len(self.file_to_bp.keys()) > 0
+        # FIXME: Should this be a warning or exception, rather than assert?
+        assert (
+            len(self.file_to_bp)
+            + len(self.function_bp_info)
+            + len(self.instruction_bp_info)
+            > 0
+        ), "Expected at least one breakpoint before launching"
 
         if self.context.options.target_run_args:
             cmdline += shlex.split(self.context.options.target_run_args)

cross-project-tests/debuginfo-tests/dexter/dex/debugger/DebuggerBase.py

@@ -166,6 +166,22 @@ def _add_conditional_breakpoint(self, file_, line, condition):
         """Returns a unique opaque breakpoint id."""
         pass
 
+    def add_function_breakpoint(self, name):
+        """Returns a unique opaque breakpoint id.
+
+        The ID type depends on the debugger being used, but will probably be
+        an int.
+        """
+        raise NotImplementedError()
+
+    def add_instruction_breakpoint(self, addr):
+        """Returns a unique opaque breakpoint id.
+
+        The ID type depends on the debugger being used, but will probably be
+        an int.
+        """
+        raise NotImplementedError()
+
     @abc.abstractmethod
     def delete_breakpoints(self, ids):
         """Delete a set of breakpoints by ids.

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

@@ -528,6 +528,16 @@ def _confirm_triggered_breakpoint_ids(self, dex_bp_ids):
         manually check conditions here."""
         confirmed_breakpoint_ids = set()
         for dex_bp_id in dex_bp_ids:
+            # Function and instruction breakpoints don't use conditions.
+            # FIXME: That's not a DAP restriction, so they could in future.
+            if dex_bp_id not in self.bp_info:
+                assert (
+                    dex_bp_id in self.function_bp_info
+                    or dex_bp_id in self.instruction_bp_info
+                )
+                confirmed_breakpoint_ids.add(dex_bp_id)
+                continue
+
             _, _, cond = self.bp_info[dex_bp_id]
             if cond is None:
                 confirmed_breakpoint_ids.add(dex_bp_id)