| Message ID | 9bb3ec5d407a5278524d54d7ad53db830b35a74c.1661260622.git.quic_mathbern@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | gdbstub: only send stop-reply upon request | expand |
On Thu, Aug 32, 2022 at 1:39 PM Matheus Tavares Bernardino wrote: > > --- > gdbstub.c | 22 +++++++++++++++++++++- > 1 file changed, 21 insertions(+), 1 deletion(-) Please ignore this patch. This version is broken, I'm working on a new one. Sorry for the noise.
On Tue, 23 Aug 2022 at 16:07, Matheus Tavares Bernardino <quic_mathbern@quicinc.com> wrote: > > 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. That document is 14 years old and is not a reliable reference for the protocol today... In particular, https://sourceware.org/gdb/download/onlinedocs/gdb/Notification-Packets.html says: "The GDB remote serial protocol includes notifications, packets that require no acknowledgment. Both the GDB and the stub may send notifications (although the only notifications defined at present are sent by the stub)." That said, the thing we're sending here isn't a notification, so it's not like we're in compliance with the protocol. So I guess I'm just saying the commit message could be improved. (The notification mechanism does allow notification of "we just stopped" if the stub implements non-stop mode, but we don't and it would be a real pain to try, so that's a bit of a red herring.) thanks -- PMM
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? */
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(-)