GDB's remote serial protocol allows stop-reply messages to be sent by
the stub either as a notification packet or as a reply to a GDB command
(provided that the cmd accepts such a response). QEMU currently does not
implement notification packets, so it should only send stop-replies
synchronously and when requested. Nevertheless, it may still issue
unsolicited stop messages through gdb_vm_state_change().
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 upon an
unexpected stop-reply message from QEMU when lldb attaches to it.
Instead, let's change gdb_set_stop_cpu() to send stop messages only as a
response to a previous GDB command, also making sure to check that the
command accepts such a reply.
Signed-off-by: Matheus Tavares Bernardino <quic_mathb...@quicinc.com>
---
Thanks Peter for pointing out about the notification packets at v1.
Changes in this version include: improvements in the commit message;
proper handling of the idle state (so that stop-replies can be sent
later, e.g. when the program is stopped due to a watchpoint); and
inclusion of other implemented GDB cmds that accept stop-reply as a
response.
There are three additional places that I think may send stop-reply
packages asynchronously, but I haven't touched those as I'm not sure if
that is really needed:
- gdb_exit() sends a "W" reply.
- gdb_signalled() sends "X".
- gdb_handlesig() sends "T".
Should we also restrict the message sending at these functions with the
same rules added to gdb_vm_state_change()?
gdbstub.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/gdbstub.c b/gdbstub.c
index cf869b10e3..23507f21ca 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -350,6 +350,7 @@ typedef struct GDBState {
int line_buf_index;
int line_sum; /* running checksum */
int line_csum; /* checksum at the end of the packet */
+ char last_cmd[MAX_PACKET_LENGTH];
GByteArray *last_packet;
int signal;
#ifdef CONFIG_USER_ONLY
@@ -412,6 +413,7 @@ static void reset_gdbserver_state(void)
g_free(gdbserver_state.processes);
gdbserver_state.processes = NULL;
gdbserver_state.process_num = 0;
+ gdbserver_state.last_cmd[0] = '\0';
}
#endif
@@ -2558,7 +2560,7 @@ static void handle_target_halt(GArray *params, void
*user_ctx)
gdb_breakpoint_remove_all();
}
-static int gdb_handle_packet(const char *line_buf)
+static void gdb_handle_packet(const char *line_buf)
{
const GdbCmdParseEntry *cmd_parser = NULL;
@@ -2800,8 +2802,6 @@ static int gdb_handle_packet(const char *line_buf)
if (cmd_parser) {
run_cmd_parser(line_buf, cmd_parser);
}
-
- return RS_IDLE;
}
void gdb_set_stop_cpu(CPUState *cpu)
@@ -2821,8 +2821,14 @@ void gdb_set_stop_cpu(CPUState *cpu)
}
#ifndef CONFIG_USER_ONLY
+static inline bool char_in(char c, const char *str)
+{
+ return strchr(str, c) != NULL;
+}
+
static void gdb_vm_state_change(void *opaque, bool running, RunState state)
{
+ const char *cmd = gdbserver_state.last_cmd;
CPUState *cpu = gdbserver_state.c_cpu;
g_autoptr(GString) buf = g_string_new(NULL);
g_autoptr(GString) tid = g_string_new(NULL);
@@ -2843,6 +2849,18 @@ static void gdb_vm_state_change(void *opaque, bool
running, RunState state)
return;
}
+ /*
+ * We don't implement notification packets, so we should only send a
+ * stop-reply in response to a previous GDB command. Commands that accept
+ * stop-reply packages are: C, c, S, s, ?, vCont, vAttach, vRun, and
+ * vStopped. We don't implement vRun, and vStopped. For vAttach and ?, the
+ * stop-reply is already sent from their respective cmd handler functions.
+ */
+ if (gdbserver_state.state != RS_IDLE || /* still parsing the cmd */
+ !(startswith(cmd, "vCont;") || (strlen(cmd) == 1 && char_in(cmd[0],
"cCsS")))) {
+ return;
+ }
+
gdb_append_thread_id(cpu, tid);
switch (state) {
@@ -3130,11 +3148,14 @@ static void gdb_read_byte(uint8_t ch)
reply = '-';
put_buffer(&reply, 1);
gdbserver_state.state = RS_IDLE;
+ gdbserver_state.last_cmd[0] = '\0';
} else {
/* send ACK reply */
reply = '+';
put_buffer(&reply, 1);
- gdbserver_state.state =
gdb_handle_packet(gdbserver_state.line_buf);
+ strcpy(gdbserver_state.last_cmd, gdbserver_state.line_buf);
+ gdbserver_state.state = RS_IDLE;
+ gdb_handle_packet(gdbserver_state.line_buf);
}
break;
default:
--
2.25.1
