diff mbox series

[RFC,3/3,gdb/dap] Allow WAIT_FOR_EVENTS input

Message ID 20230314130535.6369-4-tdevries@suse.de
State New
Headers
Series Handle DAP command files |

Commit Message

Tom de Vries March 14, 2023, 1:05 p.m. UTC 
  Modify the dap input format to allow a WAIT_FOR_EVENTS line:
...
Content-Length: 54
WAIT_FOR_EVENTS: 1
{"seq": 1, "type": "request", "command": "initialize"}
Content-Length: 163
...
that ensures that we wait for a specific amount of events before continuing
to process input.
---
 gdb/python/lib/gdb/dap/io.py     |  7 ++++++-
 gdb/python/lib/gdb/dap/server.py | 12 +++++++++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

Comments

Tom Tromey May 5, 2023, 1:09 p.m. UTC | #1 
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Modify the dap input format to allow a WAIT_FOR_EVENTS line:
Tom> ...
Tom> Content-Length: 54
Tom> WAIT_FOR_EVENTS: 1
Tom> {"seq": 1, "type": "request", "command": "initialize"}
Tom> Content-Length: 163
Tom> ...
Tom> that ensures that we wait for a specific amount of events before continuing
Tom> to process input.

I don't understand the use for this.

Tom
Tom de Vries May 5, 2023, 2:01 p.m. UTC | #2 
On 5/5/23 15:09, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> Tom> Modify the dap input format to allow a WAIT_FOR_EVENTS line:
> Tom> ...
> Tom> Content-Length: 54
> Tom> WAIT_FOR_EVENTS: 1
> Tom> {"seq": 1, "type": "request", "command": "initialize"}
> Tom> Content-Length: 163
> Tom> ...
> Tom> that ensures that we wait for a specific amount of events before continuing
> Tom> to process input.
> 
> I don't understand the use for this.

The idea is that I'm trying to reproduce gdb.dap test-cases outside the 
test-suite, and I need something to play the role of _dap_wait_for_event.

Otherwise, I sent requests before I have an acknowledgement that 
expected events due to previous requests have occurred.

More concretely, let's run gdb.dap/bt-nodebug.exp.

Now, we copy the gdb.in.1 to use as input:
...
$ cp leap-15-4/build/gdb/testsuite/outputs/gdb.dap/bt-nodebug/gdb.in.1 
gdb.in
...

Let's use it as described in the cover letter:
...
$ gdb -q -iex "set debug dap-log-file dap.log" -i dap < gdb.in
Content-Length: 473

{"request_seq": 1, "type": "response", "command": "initialize", "body": 
{"supportsTerminateRequest": true, "supportTerminateDebuggee": true, 
"supportsFunctionBreakpoints": true, "supportsInstructionBreakpoints": 
true, "supportsDelayedStackTraceLoading": true, 
"supportsDisassembleRequest": true, "supportsConfigurationDoneRequest": 
true, "supportsReadMemoryRequest": true, "supportsWriteMemoryRequest": 
true, "supportsSteppingGranularity": true}, "success": true, "seq": 
1}Content-Length: 51

{"type": "event", "event": "initialized", "seq": 2}Content-Length: 86

{"request_seq": 2, "type": "response", "command": "launch", "success": 
true, "seq": 3}Content-Length: 299

{"type": "event", "event": "breakpoint", "body": {"reason": "new", 
"breakpoint": {"id": 1, "verified": true, "source": {"name": 
"bt-main.c", "path": 
"/data/vries/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}, "line": 23, "instructionReference": "0x4004ab"}}, 
"seq": 4}Content-Length: 337

{"request_seq": 3, "type": "response", "command": 
"setFunctionBreakpoints", "body": {"breakpoints": [{"id": 1, "verified": 
true, "source": {"name": "bt-main.c", "path": 
"/data/vries/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}, "line": 23, "instructionReference": 
"0x4004ab"}]}, "success": true, "seq": 5}Content-Length: 97

{"request_seq": 4, "type": "response", "command": "configurationDone", 
"success": true, "seq": 6}Content-Length: 186

{"type": "event", "event": "output", "body": {"category": "stdout", 
"output": "Breakpoint 1 at 0x4004ab: file 
/data/vries/gdb/src/gdb/testsuite/gdb.dap/bt-main.c, line 23.\n"}, 
"seq": 7}Content-Length: 92

{"type": "event", "event": "thread", "body": {"reason": "started", 
"threadId": 1}, "seq": 8}Content-Length: 137

{"request_seq": 5, "type": "response", "command": "stackTrace", "body": 
{"stackFrames": [], "totalFrames": 0}, "success": true, "seq": 
9}Content-Length: 91

{"request_seq": 6, "type": "response", "command": "disconnect", 
"success": true, "seq": 10}$
...

As we can see the backtrace is empty.

After playing with this a few times, we get instead:
...
$ gdb -q -iex "set debug dap-log-file dap.log" -i dap < gdb.in
Content-Length: 473

{"request_seq": 1, "type": "response", "command": "initialize", "body": 
{"supportsTerminateRequest": true, "supportTerminateDebuggee": true, 
"supportsFunctionBreakpoints": true, "supportsInstructionBreakpoints": 
true, "supportsDelayedStackTraceLoading": true, 
"supportsDisassembleRequest": true, "supportsConfigurationDoneRequest": 
true, "supportsReadMemoryRequest": true, "supportsWriteMemoryRequest": 
true, "supportsSteppingGranularity": true}, "success": true, "seq": 
1}Content-Length: 51

{"type": "event", "event": "initialized", "seq": 2}Content-Length: 86

{"request_seq": 2, "type": "response", "command": "launch", "success": 
true, "seq": 3}Content-Length: 299

{"type": "event", "event": "breakpoint", "body": {"reason": "new", 
"breakpoint": {"id": 1, "verified": true, "source": {"name": 
"bt-main.c", "path": 
"/data/vries/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}, "line": 23, "instructionReference": "0x4004ab"}}, 
"seq": 4}Content-Length: 186

{"type": "event", "event": "output", "body": {"category": "stdout", 
"output": "Breakpoint 1 at 0x4004ab: file 
/data/vries/gdb/src/gdb/testsuite/gdb.dap/bt-main.c, line 23.\n"}, 
"seq": 5}Content-Length: 337

{"request_seq": 3, "type": "response", "command": 
"setFunctionBreakpoints", "body": {"breakpoints": [{"id": 1, "verified": 
true, "source": {"name": "bt-main.c", "path": 
"/data/vries/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}, "line": 23, "instructionReference": 
"0x4004ab"}]}, "success": true, "seq": 6}Content-Length: 97

{"request_seq": 4, "type": "response", "command": "configurationDone", 
"success": true, "seq": 7}Content-Length: 92

{"type": "event", "event": "thread", "body": {"reason": "started", 
"threadId": 1}, "seq": 8}Content-Length: 303

{"type": "event", "event": "breakpoint", "body": {"reason": "changed", 
"breakpoint": {"id": 1, "verified": true, "source": {"name": 
"bt-main.c", "path": 
"/data/vries/gdb/binutils-gdb.git/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}, "line": 23, "instructionReference": "0x4004ab"}}, 
"seq": 9}Content-Length: 149

{"type": "event", "event": "stopped", "body": {"threadId": 1, 
"allThreadsStopped": true, "hitBreakpointIds": [1], "reason": 
"breakpoint"}, "seq": 10}Content-Length: 685

{"request_seq": 5, "type": "response", "command": "stackTrace", "body": 
{"stackFrames": [{"id": 0, "name": "function_breakpoint_here", "line": 
23, "column": 0, "instructionPointerReference": "0x4004ab", "source": 
{"name": "bt-main.c", "path": 
"/data/vries/gdb/src/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}}, {"id": 1, "name": "no_debug_info", "line": 0, 
"column": 0, "instructionPointerReference": "0x4004c7"}, {"id": 2, 
"name": "main", "line": 27, "column": 0, "instructionPointerReference": 
"0x4004b7", "source": {"name": "bt-main.c", "path": 
"/data/vries/gdb/src/gdb/testsuite/gdb.dap/bt-main.c", 
"sourceReference": 0}}], "totalFrames": 3}, "success": true, "seq": 
11}Content-Length: 91

{"request_seq": 6, "type": "response", "command": "disconnect", 
"success": true, "seq": 12}$
...

As we can see the backtrace is now not empty.

The WAIT_FOR_EVENTS bit ensures that we get a non-empty backtrace each time.

Thanks,
- Tom
Tom Tromey May 10, 2023, 4:23 p.m. UTC | #3 
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

>> I don't understand the use for this.

Tom> The idea is that I'm trying to reproduce gdb.dap test-cases outside
Tom> the test-suite, and I need something to play the role of
Tom> _dap_wait_for_event.

Tom> Otherwise, I sent requests before I have an acknowledgement that
Tom> expected events due to previous requests have occurred.

Ok, I understand now.

Tom> {"request_seq": 5, "type": "response", "command": "stackTrace",
Tom> "body": {"stackFrames": [], "totalFrames": 0}, "success": true, "seq":
Tom> 9}Content-Length: 91

...
Tom> As we can see the backtrace is empty.

I wonder if the implementation should reject certain requests when the
inferior (really the specified thread) is running.

Tom
Tom Tromey May 10, 2023, 4:29 p.m. UTC | #4 
>>>>> "Tom" == Tom de Vries via Gdb-patches <gdb-patches@sourceware.org> writes:

Tom> Modify the dap input format to allow a WAIT_FOR_EVENTS line:
Tom> ...
Tom> Content-Length: 54
Tom> WAIT_FOR_EVENTS: 1
Tom> {"seq": 1, "type": "request", "command": "initialize"}
Tom> Content-Length: 163

I think the commit message would benefit from an explanation of the
rationale.

Tom> +        if line.startswith(b"WAIT_FOR_EVENTS:") and content_length != None:
Tom> +            line = line[16:].strip()
Tom> +            wait_for_events = int(line)
Tom> +            break

A comment here explaining why gdb accepts this header and what it's
useful for would also be good.

Tom> +            while True:
Tom> +                if wait_for_events <= self.send_event_cnt:
Tom> +                    self.send_event_cnt -= wait_for_events
Tom> +                    break
Tom> +                time.sleep(1)

Can't this be

   while wait_for_events > self.send_event_cnt:
       time.sleep(1)

Instead of sleep it is probably more efficient to use a queue of some
kind and have the event-generation code write to it when the required
number of events have been sent.  Not sure if we really care in debug
mode though.

Tom
diff mbox series

Patch

diff --git a/gdb/python/lib/gdb/dap/io.py b/gdb/python/lib/gdb/dap/io.py
index fd9953a7aaa..27d89e7a175 100644
--- a/gdb/python/lib/gdb/dap/io.py
+++ b/gdb/python/lib/gdb/dap/io.py
@@ -22,6 +22,7 @@  from .startup import start_thread, send_gdb, log
 def read_json(stream):
     """Read a JSON-RPC message from STREAM.
     The decoded object is returned."""
+    wait_for_events = 0
     # First read and parse the header.
     content_length = None
     while True:
@@ -36,13 +37,17 @@  def read_json(stream):
             line = line[15:].strip()
             content_length = int(line)
             continue
+        if line.startswith(b"WAIT_FOR_EVENTS:") and content_length != None:
+            line = line[16:].strip()
+            wait_for_events = int(line)
+            break
         log("IGNORED: <<<%s>>>" % line)
     data = bytes()
     while len(data) < content_length:
         new_data = stream.read(content_length - len(data))
         data += new_data
     result = json.loads(data)
-    return result
+    return (result, wait_for_events)
 
 
 def start_json_writer(stream, queue):
diff --git a/gdb/python/lib/gdb/dap/server.py b/gdb/python/lib/gdb/dap/server.py
index 92b4eee1c5e..88b41684602 100644
--- a/gdb/python/lib/gdb/dap/server.py
+++ b/gdb/python/lib/gdb/dap/server.py
@@ -16,6 +16,7 @@ 
 import json
 import queue
 import sys
+import time
 
 from .io import start_json_writer, read_json
 from .startup import (
@@ -45,6 +46,7 @@  class Server:
         self.out_stream = out_stream
         self.child_stream = child_stream
         self.delayed_events = []
+        self.send_event_cnt = 0
         # This queue accepts JSON objects that are then sent to the
         # DAP client.  Writing is done in a separate thread to avoid
         # blocking the read loop.
@@ -110,7 +112,9 @@  class Server:
         start_thread("output reader", self._read_inferior_output)
         start_json_writer(self.out_stream, self.write_queue)
         while not self.done:
-            cmd = read_json(self.in_stream)
+            res = read_json(self.in_stream)
+            cmd = res[0]
+            wait_for_events = res[1]
             log("READ: <<<" + json.dumps(cmd) + ">>>")
             result = self._handle_command(cmd)
             self._send_json(result)
@@ -118,6 +122,11 @@  class Server:
             self.delayed_events = []
             for event, body in events:
                 self.send_event(event, body)
+            while True:
+                if wait_for_events <= self.send_event_cnt:
+                    self.send_event_cnt -= wait_for_events
+                    break
+                time.sleep(1)
         # Got the terminate request.  This is handled by the
         # JSON-writing thread, so that we can ensure that all
         # responses are flushed to the client before exiting.
@@ -143,6 +152,7 @@  class Server:
         if body is not None:
             obj["body"] = body
         self._send_json(obj)
+        self.send_event_cnt += 1
 
     def shutdown(self):
         """Request that the server shut down."""