[PATCH] gdbstub: only send stop-reply upon request

* [PATCH] gdbstub: only send stop-reply upon request
@ 2022-08-23 13:39 Matheus Tavares Bernardino
  2022-08-23 15:37 ` Matheus Tavares Bernardino
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Matheus Tavares Bernardino @ 2022-08-23 13:39 UTC (permalink / raw)
  To: qemu-devel; +Cc: alex.bennee, f4bug

The GDB remote serial protocol[1] specifies that the communication
between GDB and the stub is driven by GDB:

    The host (GDB) sends commands, and the target (the debugging
    stub incorporated in your program) sends a response.

This is further emphasized by Embecosm's "Howto: GDB Remote Serial
Protocol" document[2], which says:

    This is the only circumstance under which the server sends a
    packet: in reply to a packet from the client requiring a
    response.

However, QEMU may send stop-reply packets even when not solicited by
GDB, as these are currently issued whenever the vm stops. Although this
behavior doesn't seem to cause problems with GDB itself, it does with
other debuggers that implement the GDB remote serial protocol, like
hexagon-lldb. In this case, the debugger fails when it receives an
unexpected stop-reply from QEMU as attaching to it.

Instead, let's make QEMU send stop-reply messages only as a response to
a previous GDB command, and only to commands that accept a stop-reply
packet. According to [3], these are: 'C', 'c', 'S', 's', '?', 'vCont',
'vAttach', 'vRun', and 'vStopped'. The ones starting with 'v' are not
implemented by gdbstup.c, so we only have to handle the single-letter
ones.

[1]: https://sourceware.org/gdb/download/onlinedocs/gdb/Overview.html#Overview
[2]: https://www.embecosm.com/appnotes/ean4/embecosm-howto-rsp-server-ean4-issue-2.html
[3]: https://sourceware.org/gdb/download/onlinedocs/gdb/Stop-Reply-Packets.html#Stop-Reply-Packets

Signed-off-by: Matheus Tavares Bernardino <quic_mathbern@quicinc.com>
---
 gdbstub.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/gdbstub.c b/gdbstub.c
index cf869b10e3..b8fc0ce07b 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -339,6 +339,7 @@ enum RSState {
     RS_GETLINE_RLE,
     RS_CHKSUM1,
     RS_CHKSUM2,
+    RS_HANDLING_CMD,
 };
 typedef struct GDBState {
     bool init;       /* have we been initialised? */
@@ -2562,6 +2563,7 @@ static int gdb_handle_packet(const char *line_buf)
 {
     const GdbCmdParseEntry *cmd_parser = NULL;
 
+    gdbserver_state.state = RS_HANDLING_CMD;
     trace_gdbstub_io_command(line_buf);
 
     switch (line_buf[0]) {
@@ -2821,6 +2823,23 @@ void gdb_set_stop_cpu(CPUState *cpu)
 }
 
 #ifndef CONFIG_USER_ONLY
+static bool cmd_allows_stop_reply(const char *cmd)
+{
+    if (strlen(cmd) != 1) {
+        return false;
+    }
+    switch (cmd[0]) {
+    case 'C':
+    case 'c':
+    case 'S':
+    case 's':
+    case '?':
+        return true;
+    default:
+        return false;
+    }
+}
+
 static void gdb_vm_state_change(void *opaque, bool running, RunState state)
 {
     CPUState *cpu = gdbserver_state.c_cpu;
@@ -2829,7 +2848,8 @@ static void gdb_vm_state_change(void *opaque, bool running, RunState state)
     const char *type;
     int ret;
 
-    if (running || gdbserver_state.state == RS_INACTIVE) {
+    if (running || gdbserver_state.state != RS_HANDLING_CMD ||
+        !cmd_allows_stop_reply(gdbserver_state.line_buf)) {
         return;
     }
     /* Is there a GDB syscall waiting to be sent?  */
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread