gdb/DAP: Add customRequest

Message ID 20230507094144.894866-1-simon.farre.cx@gmail.com
State New
Headers
Series gdb/DAP: Add customRequest |

Commit Message

Simon Farre May 7, 2023, 9:41 a.m. UTC
  First, some preface; as a maintainer on one of the few implementations of a pure GDB-only
debug adapter extension for VSCode (technically, it's designed for GDB+RR)
called "Midas", there is one thing that is crucial for making
the DAP interpreter useful, or rather, to make it be able to fully
utilize all the different super powers that GDB has and that is custom requests.
Midas implements the DA by making GDB feign the DAP in python, but using
CLI and MI, by parsing MI and turning it into JSON. Not great,
but it was the only solution that existed at the time.

The Python interpreter, when in CLI-mode, currently provides something similar to custom requests,
with custom commands from the command line via the base class `gdb.Command`.
But with the current implementation of the DAP-interpreter, no class hierarchies are needed.

This patch introduces the customRequest, which allows for DA's (debug adapters)
that want to use the full extent of GDB's functionality by working a bit
like the extension of `gdb.Command`, But instead of deriving from a base class
and instantiating a command object the DA would source python code
that will be added to the custom requests handler of the DAP-interpreter, for example like:
`-iex "source foo.py"` or `-iex "source debug-adapter-std-lib.py"` or whatever she/he wants
to call it.

Here's an example of what `foo.py` might look like:

> import gdb
> import gdb.dap
> from gdb.dap import server # import logic, possibly should look different
>
> @server.custom_request("foo")
> def fooRequestHandler(argumentBaz):
>  # full Python interpreter access, full access to all of GDB, like we're @ the CLI
>  # say goodbye to MI! Yay! :P
>  return { "bar": gdb.execute("print foo", to_string=True), "someParam": argumentBaz }

Now, the DA would be able to send a request that looks like
> header omitted
> {"seq":6,"type":"request","command":"customRequest","arguments":{"command":"foo","args":["baz"]}}

And in this particular case the response to that request would be:

> {
>  "request_seq":6,
>  "type":"response",
>  "command":"customRequest",
>  "body": {
>   "bar": "resultFromPrintCommand",
>   "someParam": "baz"
>  },
>  "success":true,"seq":19
> }

If the command is unrecognized it will respond with a "success" : false, and an error message.

As a debug adapter maintainer for VSCode, a customRequest like this would be not only useful,
but crucial. VSCode itself in it's debug adapter interface specification,
i.e. not the protocol spec, but the Typescript interface, defines customRequest-functionality,
which can be found here;
https://github.com/microsoft/vscode-debugadapter-node/blob/main/adapter/src/debugSession.ts#L638

Unfortunately, there's nothing about such a request in the DAP-spec, but I'd argue that's really irrelevant;
the largest code editor for debug adapter protocol implementers, has a notion of it,
though it's up to DA-implementers to make sure they understand their own requests.

There are other, (in my opinion) bonuses of the customRequest functionality. The major one
being it can be a driving factor for further development of Python in GDB, by providing
an access point to GDB that returns "real python objects" (instead of the above example
of returning a string from the CLI-command) from commands and make it more
possible for future debug adapters to migrate away from MI (if they so choose),
which is difficult to work with, hard to read and even harder to test. JSON is superior
since there's so much open source code that supports JSON and in
a lot of languages it's even got built in support. "Driving GDB" via the DAP-interpreter
from any modern language is trivial because of this.

Beyond this patch, documentation would be needed also, to document how
to create customRequests, like the one above. I'm hoping Eli can provide me with
some direction here.
---
 gdb/python/lib/gdb/dap/server.py | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
  

Comments

Matt Rice May 7, 2023, 10:45 a.m. UTC | #1
On Sun, May 7, 2023 at 9:42 AM Simon Farre via Gdb-patches
<gdb-patches@sourceware.org> wrote:
>
> First, some preface; as a maintainer on one of the few implementations of a pure GDB-only
> debug adapter extension for VSCode (technically, it's designed for GDB+RR)
> called "Midas", there is one thing that is crucial for making
> the DAP interpreter useful, or rather, to make it be able to fully
> utilize all the different super powers that GDB has and that is custom requests.
> Midas implements the DA by making GDB feign the DAP in python, but using
> CLI and MI, by parsing MI and turning it into JSON. Not great,
> but it was the only solution that existed at the time.

My somewhat initial thought was that there was some overlap here with
the 'MI commands in Python' feature,
https://sourceware.org/gdb/current/onlinedocs/gdb.html/GDB_002fMI-Commands-In-Python.html#GDB_002fMI-Commands-In-Python
But that still leaves one needing to convert MI to JSON.

I'd want to note that Tromey's recent patch should allow MI commands
to be trivially converted through json,
https://sourceware.org/pipermail/gdb-patches/2023-April/198606.html
`json.dumps(gdb.execute_mi("break-list"))`, or something of similar effect.

It would seem like combining your customRequest patch with that, would
lead to a fairly trivial command
enabling access to all mi commands as json. This at least seems like
it would be much less involved than
manual translation of MI to JSON.

> The Python interpreter, when in CLI-mode, currently provides something similar to custom requests,
> with custom commands from the command line via the base class `gdb.Command`.
> But with the current implementation of the DAP-interpreter, no class hierarchies are needed.
>
> This patch introduces the customRequest, which allows for DA's (debug adapters)
> that want to use the full extent of GDB's functionality by working a bit
> like the extension of `gdb.Command`, But instead of deriving from a base class
> and instantiating a command object the DA would source python code
> that will be added to the custom requests handler of the DAP-interpreter, for example like:
> `-iex "source foo.py"` or `-iex "source debug-adapter-std-lib.py"` or whatever she/he wants
> to call it.
>
> Here's an example of what `foo.py` might look like:
>
> > import gdb
> > import gdb.dap
> > from gdb.dap import server # import logic, possibly should look different
> >
> > @server.custom_request("foo")
> > def fooRequestHandler(argumentBaz):
> >  # full Python interpreter access, full access to all of GDB, like we're @ the CLI
> >  # say goodbye to MI! Yay! :P
> >  return { "bar": gdb.execute("print foo", to_string=True), "someParam": argumentBaz }
>
> Now, the DA would be able to send a request that looks like
> > header omitted
> > {"seq":6,"type":"request","command":"customRequest","arguments":{"command":"foo","args":["baz"]}}
>
> And in this particular case the response to that request would be:
>
> > {
> >  "request_seq":6,
> >  "type":"response",
> >  "command":"customRequest",
> >  "body": {
> >   "bar": "resultFromPrintCommand",
> >   "someParam": "baz"
> >  },
> >  "success":true,"seq":19
> > }
>
> If the command is unrecognized it will respond with a "success" : false, and an error message.
>
> As a debug adapter maintainer for VSCode, a customRequest like this would be not only useful,
> but crucial. VSCode itself in it's debug adapter interface specification,
> i.e. not the protocol spec, but the Typescript interface, defines customRequest-functionality,
> which can be found here;
> https://github.com/microsoft/vscode-debugadapter-node/blob/main/adapter/src/debugSession.ts#L638
>
> Unfortunately, there's nothing about such a request in the DAP-spec, but I'd argue that's really irrelevant;
> the largest code editor for debug adapter protocol implementers, has a notion of it,
> though it's up to DA-implementers to make sure they understand their own requests.
>
> There are other, (in my opinion) bonuses of the customRequest functionality. The major one
> being it can be a driving factor for further development of Python in GDB, by providing
> an access point to GDB that returns "real python objects" (instead of the above example
> of returning a string from the CLI-command) from commands and make it more
> possible for future debug adapters to migrate away from MI (if they so choose),
> which is difficult to work with, hard to read and even harder to test. JSON is superior
> since there's so much open source code that supports JSON and in
> a lot of languages it's even got built in support. "Driving GDB" via the DAP-interpreter
> from any modern language is trivial because of this.
>
> Beyond this patch, documentation would be needed also, to document how
> to create customRequests, like the one above. I'm hoping Eli can provide me with
> some direction here.
> ---
>  gdb/python/lib/gdb/dap/server.py | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
> index ff88282049f..7cd60873319 100644
> --- a/gdb/python/lib/gdb/dap/server.py
> +++ b/gdb/python/lib/gdb/dap/server.py
> @@ -33,6 +33,8 @@ _capabilities = {}
>  # Map command names to callables.
>  _commands = {}
>
> +_custom_requests = {}
> +
>  # The global server.
>  _server = None
>
> @@ -171,6 +173,27 @@ def request(name):
>      return wrap
>
>
> +def custom_request(name):
> +    """A decorator that indicates that the wrapper function should be
> +    handled by a custom request"""
> +
> +    def wrap(func):
> +        global _custom_requests
> +        _custom_requests[name] = func
> +        return in_dap_thread(func)
> +
> +    return wrap
> +
> +@request("customRequest")
> +def custom_request_handler(**args):
> +    global _custom_requests
> +    cmd = args["command"]
> +    if _custom_requests.get(cmd) is not None:
> +        return _custom_requests[cmd](args["args"])
> +    else:
> +        raise Exception(f"Unrecognized customRequest {cmd}")
> +
> +
>  def capability(name):
>      """A decorator that indicates that the wrapper function implements
>      the DAP capability NAME."""
> --
> 2.40.1
>
  
Tom Tromey May 7, 2023, 9:19 p.m. UTC | #2
>>>>> "Simon" == Simon Farre via Gdb-patches <gdb-patches@sourceware.org> writes:

Simon> This patch introduces the customRequest, which allows for DA's (debug adapters)
Simon> that want to use the full extent of GDB's functionality by working a bit
Simon> like the extension of `gdb.Command`

Seems reasonable to me.

Simon> Unfortunately, there's nothing about such a request in the
Simon> DAP-spec, but I'd argue that's really irrelevant; the largest
Simon> code editor for debug adapter protocol implementers, has a notion
Simon> of it, though it's up to DA-implementers to make sure they
Simon> understand their own requests.

Makes sense.

Simon> Beyond this patch, documentation would be needed also, to document how
Simon> to create customRequests, like the one above. I'm hoping Eli can provide me with
Simon> some direction here.

There's a DAP node in the docs where I think it would make sense to
document the existence of the custom request.  For implementing requests
themselves, I think a new node in python.texi would be appropriate.

>> @server.custom_request("foo")
>> def fooRequestHandler(argumentBaz):
>> # full Python interpreter access, full access to all of GDB, like we're @ the CLI
>> # say goodbye to MI! Yay! :P
>> return { "bar": gdb.execute("print foo", to_string=True), "someParam": argumentBaz }

Simon> +@request("customRequest")
Simon> +def custom_request_handler(**args):
Simon> +    global _custom_requests
Simon> +    cmd = args["command"]
Simon> +    if _custom_requests.get(cmd) is not None:
Simon> +        return _custom_requests[cmd](args["args"])

An issue here is that the DAP thread can't directly invoke code in gdb
-- basically the 'gdb' module is off-limits.  This is why there are
decorators to assert that a given function is on the DAP thread or gdb
thread.  This rule is in place because the core of gdb isn't
thread-safe.

Probably the fix is as simple as wrapping the call with
send_gdb_with_response, and having the custom_request decorator
use in_gdb_thread.

Tom
  
Lancelot SIX May 10, 2023, 9:40 a.m. UTC | #3
Hi Simon,

I have a couple of remarks below.

> +@request("customRequest")
> +def custom_request_handler(**args):

It looks like the args dict must contain the "command" and "args".  I
would find it clearer if this was stated in the function definition like
so:

def custom_request_handler(*, command, args):

If args can contain more keys (which will be ignored by the
implementation), it could be:

def custom_request_handler(*, command, args, **kwargs):

> +    global _custom_requests
> +    cmd = args["command"]
> +    if _custom_requests.get(cmd) is not None:
> +        return _custom_requests[cmd](args["args"])

Just a nit, but the cmd lookup could be done just once:

cmd = _custom_requests.get(command)
if cmd is not None:
    return cmd(args)

Also see Tom's comment regarding which thread you want this to execute
in.  This part could become

    return send_gdb_with_response(lambda: cmd(args))

> +    else:
> +        raise Exception(f"Unrecognized customRequest {cmd}")

I am not sure we guarantee python >= 3.6, so f-strings might not be
available.  That being said, as python-3.6 istelf is not supported
anymore we could maybe just bump the required version.

Best,
Lancelot.
  

Patch

diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index ff88282049f..7cd60873319 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -33,6 +33,8 @@  _capabilities = {}
 # Map command names to callables.
 _commands = {}
 
+_custom_requests = {}
+
 # The global server.
 _server = None
 
@@ -171,6 +173,27 @@  def request(name):
     return wrap
 
 
+def custom_request(name):
+    """A decorator that indicates that the wrapper function should be
+    handled by a custom request"""
+
+    def wrap(func):
+        global _custom_requests
+        _custom_requests[name] = func
+        return in_dap_thread(func)
+
+    return wrap
+
+@request("customRequest")
+def custom_request_handler(**args):
+    global _custom_requests
+    cmd = args["command"]
+    if _custom_requests.get(cmd) is not None:
+        return _custom_requests[cmd](args["args"])
+    else:
+        raise Exception(f"Unrecognized customRequest {cmd}")
+
+
 def capability(name):
     """A decorator that indicates that the wrapper function implements
     the DAP capability NAME."""