public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Tom Tromey <tromey@adacore.com>
To: gdb-patches@sourceware.org
Subject: [PATCH 4/7] Implement type checking for DAP breakpoint requests
Date: Mon, 12 Jun 2023 13:36:31 -0600	[thread overview]
Message-ID: <20230612-more-dap-v1-4-ad868f1a4cc0@adacore.com> (raw)
In-Reply-To: <20230612-more-dap-v1-0-ad868f1a4cc0@adacore.com>

I realized that with a small refactoring, it is possible to type-check
the parameters to the various DAP breakpoint requests.  This would
have caught the earlier bug involving hitCondition.
---
 gdb/python/lib/gdb/dap/breakpoint.py | 128 ++++++++++++++++++++++-------------
 1 file changed, 82 insertions(+), 46 deletions(-)

diff --git a/gdb/python/lib/gdb/dap/breakpoint.py b/gdb/python/lib/gdb/dap/breakpoint.py
index 3f40cfb7f63..227085ad73e 100644
--- a/gdb/python/lib/gdb/dap/breakpoint.py
+++ b/gdb/python/lib/gdb/dap/breakpoint.py
@@ -143,19 +143,28 @@ def _set_breakpoints(kind, specs):
     return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
 
 
-# Turn a DAP SourceBreakpoint, FunctionBreakpoint, or
-# InstructionBreakpoint into a "spec" that is used by
-# _set_breakpoints.  SPEC is a dictionary of parameters that is used
-# as the base of the result; it is modified in place.
-def _basic_spec(bp_info, spec):
-    for name in ("condition", "hitCondition"):
-        if name in bp_info:
-            spec[name] = bp_info[name]
-    return spec
+# A helper function that rewrites a SourceBreakpoint into the internal
+# form passed to the creator.  This function also allows for
+# type-checking of each SourceBreakpoint.
+@type_check
+def _rewrite_src_breakpoint(
+    *,
+    # This is a Source but we don't type-check it.
+    source,
+    line: int,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    return {
+        "source": source["path"],
+        "line": line,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
 
 
 # FIXME we do not specify a type for 'source'.
-# FIXME 'breakpoints' is really a list[SourceBreakpoint].
 @request("setBreakpoints")
 @capability("supportsHitConditionalBreakpoints")
 @capability("supportsConditionalBreakpoints")
@@ -163,17 +172,7 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
     else:
-        specs = []
-        for obj in breakpoints:
-            specs.append(
-                _basic_spec(
-                    obj,
-                    {
-                        "source": source["path"],
-                        "line": obj["line"],
-                    },
-                )
-            )
+        specs = [_rewrite_src_breakpoint(source=source, **bp) for bp in breakpoints]
         # Be sure to include the path in the key, so that we only
         # clear out breakpoints coming from this same source.
         key = "source:" + source["path"]
@@ -183,45 +182,64 @@ def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     }
 
 
+# A helper function that rewrites a FunctionBreakpoint into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each FunctionBreakpoint.
+@type_check
+def _rewrite_fn_breakpoint(
+    *,
+    name: str,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    return {
+        "function": name,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
+
+
 @request("setFunctionBreakpoints")
 @capability("supportsFunctionBreakpoints")
 def set_fn_breakpoint(*, breakpoints: Sequence, **args):
-    specs = []
-    for bp in breakpoints:
-        specs.append(
-            _basic_spec(
-                bp,
-                {
-                    "function": bp["name"],
-                },
-            )
-        )
+    specs = [_rewrite_fn_breakpoint(**bp) for bp in breakpoints]
     result = send_gdb_with_response(lambda: _set_breakpoints("function", specs))
     return {
         "breakpoints": result,
     }
 
 
+# A helper function that rewrites an InstructionBreakpoint into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each InstructionBreakpoint.
+@type_check
+def _rewrite_insn_breakpoint(
+    *,
+    instructionReference: str,
+    offset: Optional[int] = None,
+    condition: Optional[str] = None,
+    hitCondition: Optional[str] = None,
+    **args,
+):
+    # There's no way to set an explicit address breakpoint from
+    # Python, so we rely on "spec" instead.
+    val = "*" + instructionReference
+    if offset is not None:
+        val = val + " + " + str(offset)
+    return {
+        "spec": val,
+        "condition": condition,
+        "hitCondition": hitCondition,
+    }
+
+
 @request("setInstructionBreakpoints")
 @capability("supportsInstructionBreakpoints")
 def set_insn_breakpoints(
     *, breakpoints: Sequence, offset: Optional[int] = None, **args
 ):
-    specs = []
-    for bp in breakpoints:
-        # There's no way to set an explicit address breakpoint
-        # from Python, so we rely on "spec" instead.
-        val = "*" + bp["instructionReference"]
-        if offset is not None:
-            val = val + " + " + str(offset)
-        specs.append(
-            _basic_spec(
-                bp,
-                {
-                    "spec": val,
-                },
-            )
-        )
+    specs = [_rewrite_insn_breakpoint(**bp) for bp in breakpoints]
     result = send_gdb_with_response(lambda: _set_breakpoints("instruction", specs))
     return {
         "breakpoints": result,
@@ -249,6 +267,23 @@ def _set_exception_catchpoints(filter_options):
     return _set_breakpoints_callback("exception", filter_options, _catch_exception)
 
 
+# A helper function that rewrites an ExceptionFilterOptions into the
+# internal form passed to the creator.  This function also allows for
+# type-checking of each ExceptionFilterOptions.
+@type_check
+def _rewrite_exception_breakpoint(
+    *,
+    filterId: str,
+    condition: Optional[str] = None,
+    # Note that exception breakpoints do not support a hit count.
+    **args,
+):
+    return {
+        "filterId": filterId,
+        "condition": condition,
+    }
+
+
 @request("setExceptionBreakpoints")
 @capability("supportsExceptionFilterOptions")
 @capability(
@@ -272,6 +307,7 @@ def set_exception_breakpoints(
     # Convert the 'filters' to the filter-options style.
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
+    options = [_rewrite_exception_breakpoint(**bp) for bp in options]
     result = send_gdb_with_response(lambda: _set_exception_catchpoints(options))
     return {
         "breakpoints": result,

-- 
2.40.1


  parent reply	other threads:[~2023-06-12 19:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-12 19:36 [PATCH 0/7] More changes to DAP Tom Tromey
2023-06-12 19:36 ` [PATCH 1/7] Fix type of DAP hitCondition Tom Tromey
2023-06-12 19:36 ` [PATCH 2/7] Reuse breakpoints more frequently in DAP Tom Tromey
2023-06-12 19:36 ` [PATCH 3/7] Handle exceptions when creating DAP breakpoints Tom Tromey
2023-06-12 19:36 ` Tom Tromey [this message]
2023-06-12 19:36 ` [PATCH 5/7] Handle supportsVariablePaging in DAP Tom Tromey
2023-06-12 19:36 ` [PATCH 6/7] Implement DAP logging breakpoints Tom Tromey
2023-06-12 19:36 ` [PATCH 7/7] Implement DAP "hover" context Tom Tromey
2023-06-22 15:46 ` [PATCH 0/7] More changes to DAP Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230612-more-dap-v1-4-ad868f1a4cc0@adacore.com \
    --to=tromey@adacore.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).