Add type-checking to DAP requests
authorTom Tromey <tromey@adacore.com>
Thu, 11 May 2023 18:20:05 +0000 (12:20 -0600)
committerTom Tromey <tromey@adacore.com>
Mon, 12 Jun 2023 18:09:28 +0000 (12:09 -0600)
It occurred to me recently that gdb's DAP implementation should
probably check the types of objects coming from the client.  This
patch implements this idea by reusing Python's existing type
annotations, and supplying a decorator that verifies these at runtime.

Python doesn't make it very easy to do runtime type-checking, so the
core of the checker is written by hand.  I haven't tried to make a
fully generic runtime type checker.  Instead, this only checks the
subset that is needed by DAP.  For example, only keyword-only
functions are handled.

Furthermore, in a few spots, it wasn't convenient to spell out the
type that is accepted.  I've added a couple of comments to this effect
in breakpoint.py.

I've tried to make this code compatible with older versions of Python,
but I've only been able to try it with 3.9 and 3.10.

13 files changed:
gdb/data-directory/Makefile.in
gdb/python/lib/gdb/dap/breakpoint.py
gdb/python/lib/gdb/dap/bt.py
gdb/python/lib/gdb/dap/disassemble.py
gdb/python/lib/gdb/dap/evaluate.py
gdb/python/lib/gdb/dap/launch.py
gdb/python/lib/gdb/dap/memory.py
gdb/python/lib/gdb/dap/next.py
gdb/python/lib/gdb/dap/scopes.py
gdb/python/lib/gdb/dap/server.py
gdb/python/lib/gdb/dap/typecheck.py [new file with mode: 0644]
gdb/testsuite/gdb.dap/type_check.exp [new file with mode: 0644]
gdb/testsuite/gdb.dap/type_check.py [new file with mode: 0644]

index a95c2d7ab3784ea6dfebc8c686ed70f9396e9c43..9e34d4cffd3c96804e7e1a18d35ae1580f143f0b 100644 (file)
@@ -105,6 +105,7 @@ PYTHON_FILE_LIST = \
        gdb/dap/startup.py \
        gdb/dap/state.py \
        gdb/dap/threads.py \
+       gdb/dap/typecheck.py \
        gdb/dap/varref.py \
        gdb/function/__init__.py \
        gdb/function/as_string.py \
index 8ffa18290311b6b66dffcbcb31ffc22459ec161e..ad019333fea329a3a2406d124d1b357dacdb8029 100644 (file)
@@ -16,6 +16,9 @@
 import gdb
 import os
 
+# These are deprecated in 3.9, but required in older versions.
+from typing import Optional, Sequence
+
 from .server import request, capability
 from .startup import send_gdb_with_response, in_gdb_thread
 
@@ -95,8 +98,10 @@ def _set_breakpoints(kind, specs):
     return _set_breakpoints_callback(kind, specs, gdb.Breakpoint)
 
 
+# FIXME we do not specify a type for 'source'.
+# FIXME 'breakpoints' is really a list[SourceBreakpoint].
 @request("setBreakpoints")
-def set_breakpoint(*, source, breakpoints=(), **args):
+def set_breakpoint(*, source, breakpoints: Sequence = (), **args):
     if "path" not in source:
         result = []
     else:
@@ -119,7 +124,7 @@ def set_breakpoint(*, source, breakpoints=(), **args):
 
 @request("setFunctionBreakpoints")
 @capability("supportsFunctionBreakpoints")
-def set_fn_breakpoint(*, breakpoints, **args):
+def set_fn_breakpoint(*, breakpoints: Sequence, **args):
     specs = []
     for bp in breakpoints:
         specs.append(
@@ -135,7 +140,9 @@ def set_fn_breakpoint(*, breakpoints, **args):
 
 @request("setInstructionBreakpoints")
 @capability("supportsInstructionBreakpoints")
-def set_insn_breakpoints(*, breakpoints, offset=None, **args):
+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
@@ -179,16 +186,24 @@ def _set_exception_catchpoints(filter_options):
 
 @request("setExceptionBreakpoints")
 @capability("supportsExceptionFilterOptions")
-@capability("exceptionBreakpointFilters", ({
-    "filter": "assert",
-    "label": "Ada assertions",
-    "supportsCondition": True,
-}, {
-    "filter": "exception",
-    "label": "Ada exceptions",
-    "supportsCondition": True,
-}))
-def set_exception_breakpoints(*, filters, filterOptions=(), **args):
+@capability(
+    "exceptionBreakpointFilters",
+    (
+        {
+            "filter": "assert",
+            "label": "Ada assertions",
+            "supportsCondition": True,
+        },
+        {
+            "filter": "exception",
+            "label": "Ada exceptions",
+            "supportsCondition": True,
+        },
+    ),
+)
+def set_exception_breakpoints(
+    *, filters: Sequence[str], filterOptions: Sequence = (), **args
+):
     # Convert the 'filters' to the filter-options style.
     options = [{"filterId": filter} for filter in filters]
     options.extend(filterOptions)
index 21cedb73d26e6e19ece4c3736f64ab02e86dec15..a38573fbba899de6e83749006ff59e8d6445c962 100644 (file)
@@ -88,5 +88,5 @@ def _backtrace(thread_id, levels, startFrame):
 
 @request("stackTrace")
 @capability("supportsDelayedStackTraceLoading")
-def stacktrace(*, levels=0, startFrame=0, threadId, **extra):
+def stacktrace(*, levels: int = 0, startFrame: int = 0, threadId: int, **extra):
     return send_gdb_with_response(lambda: _backtrace(threadId, levels, startFrame))
index 900a3c6adbb28afca8b91a2119e555c39b33d370..bc091eb2c89decc3f7071786aa4df101968dab00 100644 (file)
@@ -43,7 +43,12 @@ def _disassemble(pc, skip_insns, count):
 @request("disassemble")
 @capability("supportsDisassembleRequest")
 def disassemble(
-    *, memoryReference, offset=0, instructionOffset=0, instructionCount, **extra
+    *,
+    memoryReference: str,
+    offset: int = 0,
+    instructionOffset: int = 0,
+    instructionCount: int,
+    **extra
 ):
     pc = int(memoryReference, 0) + offset
     return send_gdb_with_response(
index fffd255417bf15b350218a2e79b833d50bd622f2..4fc0f31486cbd822546e6c84a820a3c34aff7160 100644 (file)
@@ -16,6 +16,9 @@
 import gdb
 import gdb.printing
 
+# This is deprecated in 3.9, but required in older versions.
+from typing import Optional
+
 from .frames import frame_for_id
 from .server import request
 from .startup import send_gdb_with_response, in_gdb_thread
@@ -55,7 +58,13 @@ def _repl(command, frame_id):
 
 # FIXME supportsVariableType handling
 @request("evaluate")
-def eval_request(*, expression, frameId=None, context="variables", **args):
+def eval_request(
+    *,
+    expression: str,
+    frameId: Optional[int] = None,
+    context: str = "variables",
+    **args,
+):
     if context in ("watch", "variables"):
         # These seem to be expression-like.
         return send_gdb_with_response(lambda: _evaluate(expression, frameId))
@@ -75,7 +84,7 @@ def _variables(ref, start, count):
 @request("variables")
 # Note that we ignore the 'filter' field.  That seems to be
 # specific to javascript.
-def variables(*, variablesReference, start=0, count=0, **args):
+def variables(*, variablesReference: int, start: int = 0, count: int = 0, **args):
     result = send_gdb_with_response(
         lambda: _variables(variablesReference, start, count)
     )
index dc4bf3c6d150c8659fa0b43466ca6d3ac3c41351..e187c3144d7c9e0e319bafc08333043f8941134f 100644 (file)
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import gdb
+
+# These are deprecated in 3.9, but required in older versions.
+from typing import Mapping, Optional, Sequence
+
 from .events import ExecutionInvoker
 from .server import request, capability
 from .startup import send_gdb, send_gdb_with_response, in_gdb_thread
@@ -36,7 +40,13 @@ def _set_args_env(args, env):
 # from implementations.  Any additions or changes here should be
 # documented in the gdb manual.
 @request("launch")
-def launch(*, program=None, args=(), env=None, **extra):
+def launch(
+    *,
+    program: Optional[str] = None,
+    args: Sequence[str] = (),
+    env: Optional[Mapping[str, str]] = None,
+    **extra,
+):
     if program is not None:
         global _program
         _program = program
@@ -46,7 +56,7 @@ def launch(*, program=None, args=(), env=None, **extra):
 
 
 @request("attach")
-def attach(*, pid, **args):
+def attach(*, pid: int, **args):
     # Ensure configurationDone does not try to run.
     global _program
     _program = None
index 7eb8a27ce4746ef2fac4d517b675b12a3fc4cc4d..85948bda9f4759b7fb85b832fd77f93fa872d77c 100644 (file)
@@ -28,7 +28,7 @@ def _read_memory(addr, count):
 
 @request("readMemory")
 @capability("supportsReadMemoryRequest")
-def read_memory(*, memoryReference, offset=0, count, **extra):
+def read_memory(*, memoryReference: str, offset: int = 0, count: int, **extra):
     addr = int(memoryReference, 0) + offset
     buf = send_gdb_with_response(lambda: _read_memory(addr, count))
     return {
@@ -45,7 +45,7 @@ def _write_memory(addr, contents):
 
 @request("writeMemory")
 @capability("supportsWriteMemoryRequest")
-def write_memory(*, memoryReference, offset=0, data, **extra):
+def write_memory(*, memoryReference: str, offset: int = 0, data: str, **extra):
     addr = int(memoryReference, 0) + offset
     send_gdb_with_response(lambda: _write_memory(addr, data))
     return {}
index 75f2fa6f31ac5fad00d97b33d115a8c4735a71f3..8b112770a0c0971fc1811bba9f695119391b05bc 100644 (file)
@@ -45,7 +45,9 @@ def _handle_thread_step(thread_id, single_thread):
 
 
 @request("next")
-def next(*, threadId, singleThread=False, granularity="statement", **args):
+def next(
+    *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
+):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "next"
     if granularity == "instruction":
@@ -56,7 +58,9 @@ def next(*, threadId, singleThread=False, granularity="statement", **args):
 @capability("supportsSteppingGranularity")
 @capability("supportsSingleThreadExecutionRequests")
 @request("stepIn")
-def step_in(*, threadId, singleThread=False, granularity="statement", **args):
+def step_in(
+    *, threadId: int, singleThread: bool = False, granularity: str = "statement", **args
+):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     cmd = "step"
     if granularity == "instruction":
@@ -65,13 +69,13 @@ def step_in(*, threadId, singleThread=False, granularity="statement", **args):
 
 
 @request("stepOut")
-def step_out(*, threadId, singleThread=False):
+def step_out(*, threadId: int, singleThread: bool = False):
     send_gdb(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("finish", StopKinds.STEP))
 
 
 @request("continue")
-def continue_request(*, threadId, singleThread=False, **args):
+def continue_request(*, threadId: int, singleThread: bool = False, **args):
     locked = send_gdb_with_response(lambda: _handle_thread_step(threadId, singleThread))
     send_gdb(ExecutionInvoker("continue", None))
     return {"allThreadsContinued": not locked}
index 8e9af5064c296855aee1b6d81bf14071168b02ca..9b80dd9ce806baf0dc74cabcd1f03e157b68110b 100644 (file)
@@ -109,6 +109,6 @@ def _get_scope(id):
 
 
 @request("scopes")
-def scopes(*, frameId, **extra):
+def scopes(*, frameId: int, **extra):
     scopes = send_gdb_with_response(lambda: _get_scope(frameId))
     return {"scopes": scopes}
index 8abe475b031c21b14f80dab54b3b4cff3ed65bad..be66676f73019b7e7f5dd7f6d502473208bc30d4 100644 (file)
@@ -25,6 +25,7 @@ from .startup import (
     log_stack,
     send_gdb_with_response,
 )
+from .typecheck import type_check
 
 
 # Map capability names to values.
@@ -165,7 +166,8 @@ def request(name):
     def wrap(func):
         global _commands
         # All requests must run in the DAP thread.
-        func = in_dap_thread(func)
+        # Also type-check the calls.
+        func = in_dap_thread(type_check(func))
         _commands[name] = func
         return func
 
@@ -202,7 +204,7 @@ def terminate(**args):
 
 @request("disconnect")
 @capability("supportTerminateDebuggee")
-def disconnect(*, terminateDebuggee=False, **args):
+def disconnect(*, terminateDebuggee: bool = False, **args):
     if terminateDebuggee:
         terminate()
     _server.shutdown()
diff --git a/gdb/python/lib/gdb/dap/typecheck.py b/gdb/python/lib/gdb/dap/typecheck.py
new file mode 100644 (file)
index 0000000..791dc75
--- /dev/null
@@ -0,0 +1,88 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# A simple runtime type checker.
+
+import collections.abc
+import functools
+import typing
+
+
+# 'isinstance' won't work in general for type variables, so we
+# implement the subset that is needed by DAP.
+def _check_instance(value, typevar):
+    base = typing.get_origin(typevar)
+    if base is None:
+        return isinstance(value, typevar)
+    arg_types = typing.get_args(typevar)
+    if base == collections.abc.Mapping or base == typing.Mapping:
+        if not isinstance(value, collections.abc.Mapping):
+            return False
+        assert len(arg_types) == 2
+        (keytype, valuetype) = arg_types
+        return all(
+            _check_instance(k, keytype) and _check_instance(v, valuetype)
+            for k, v in value.items()
+        )
+    elif base == collections.abc.Sequence or base == typing.Sequence:
+        # In some places we simply use 'Sequence' without arguments.
+        if not isinstance(value, base):
+            return False
+        if len(arg_types) == 0:
+            return True
+        assert len(arg_types) == 1
+        arg_type = arg_types[0]
+        return all(_check_instance(item, arg_type) for item in value)
+    elif base == typing.Union:
+        return any(_check_instance(value, arg_type) for arg_type in arg_types)
+    raise TypeError("unsupported type variable '" + str(typevar) + "'")
+
+
+def type_check(func):
+    """A decorator that checks FUNC's argument types at runtime."""
+
+    # The type checker relies on 'typing.get_origin', which was added
+    # in Python 3.8.  (It also relies on 'typing.get_args', but that
+    # was added at the same time.)
+    if not hasattr(typing, "get_origin"):
+        return func
+
+    hints = typing.get_type_hints(func)
+    # We don't check the return type, but we allow it in case someone
+    # wants to use it on a function definition.
+    if "return" in hints:
+        del hints["return"]
+
+    # Note that keyword-only is fine for our purposes, because this is
+    # only used for DAP requests, and those are always called this
+    # way.
+    @functools.wraps(func)
+    def check_arguments(**kwargs):
+        for key in hints:
+            # The argument might not be passed in; we could type-check
+            # any default value here, but it seems fine to just rely
+            # on the code being correct -- the main goal of this
+            # checking is to verify JSON coming from the client.
+            if key in kwargs and not _check_instance(kwargs[key], hints[key]):
+                raise TypeError(
+                    "value for '"
+                    + key
+                    + "' does not have expected type '"
+                    + str(hints[key])
+                    + "'"
+                )
+        return func(**kwargs)
+
+    return check_arguments
diff --git a/gdb/testsuite/gdb.dap/type_check.exp b/gdb/testsuite/gdb.dap/type_check.exp
new file mode 100644 (file)
index 0000000..346b4ba
--- /dev/null
@@ -0,0 +1,29 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test the DAP type-checker.
+
+require allow_dap_tests allow_python_tests
+
+load_lib dap-support.exp
+load_lib gdb-python.exp
+
+clean_restart
+
+set remote_python_file \
+    [gdb_remote_download host ${srcdir}/${subdir}/${gdb_test_file_name}.py]
+gdb_test_no_output "source ${remote_python_file}" "load python file"
+
+gdb_test "python check_everything()" OK "type checker"
diff --git a/gdb/testsuite/gdb.dap/type_check.py b/gdb/testsuite/gdb.dap/type_check.py
new file mode 100644 (file)
index 0000000..1fdb595
--- /dev/null
@@ -0,0 +1,96 @@
+# Copyright 2023 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test the type checker.
+
+import typing
+from gdb.dap.typecheck import type_check
+
+
+# A wrapper to call a function that should cause a type-checking
+# failure.  Returns if the exception was seen.  Throws an exception if
+# a TypeError was not seen.
+def should_fail(func, **args):
+    try:
+        func(**args)
+    except TypeError:
+        return
+    raise RuntimeError("function failed to throw TypeError")
+
+
+# Also specify a return type to make sure return types do not confuse
+# the checker.
+@type_check
+def simple_types(*, b: bool, s: str, i: int = 23) -> int:
+    return i
+
+
+def check_simple():
+    simple_types(b=True, s="DEI", i=97)
+    # Check the absence of a defaulted argument.
+    simple_types(b=True, s="DEI")
+    simple_types(b=False, s="DEI", i=97)
+    should_fail(simple_types, b=97, s="DEI", i=97)
+    should_fail(simple_types, b=True, s=None, i=97)
+    should_fail(simple_types, b=True, s="DEI", i={})
+
+
+@type_check
+def sequence_type(*, s: typing.Sequence[str]):
+    pass
+
+
+def check_sequence():
+    sequence_type(s=("hi", "out", "there"))
+    sequence_type(s=["hi", "out", "there"])
+    sequence_type(s=())
+    sequence_type(s=[])
+    should_fail(sequence_type, s=23)
+    should_fail(sequence_type, s=["hi", 97])
+
+
+@type_check
+def map_type(*, m: typing.Mapping[str, int]):
+    pass
+
+
+def check_map():
+    map_type(m={})
+    map_type(m={"dei": 23})
+    should_fail(map_type, m=[1, 2, 3])
+    should_fail(map_type, m=None)
+    should_fail(map_type, m={"dei": "string"})
+
+
+@type_check
+def opt_type(*, u: typing.Optional[int]):
+    pass
+
+
+def check_opt():
+    opt_type(u=23)
+    opt_type(u=None)
+    should_fail(opt_type, u="string")
+
+
+def check_everything():
+    # Older versions of Python can't really implement this.
+    if hasattr(typing, "get_origin"):
+        # Just let any exception propagate and end up in the log.
+        check_simple()
+        check_sequence()
+        check_map()
+        check_opt()
+        print("OK")