Skip to contentSkip to conversation

Add GDB qAttached support

PATCH In review Jan Kiszka 3 participants 5 messages Jan 31, 2015 – Feb 4, 2015

lore
Share & tools
lore

Jan Kiszka

author
#0 lore raw

PATCH 1/2 Add GDB qAttached support

From: Jan Kiszka <[email protected]>

With this patch QEMU handles qAttached request from gdb. When QEMU replies 1, GDB sends a "detach" command at the end of a debugging session otherwise GDB sends "kill".

The default value for qAttached is 1 on system emulation and 0 on user emulation.

Based on original version by Fabien Chouteau.

Signed-off-by Jan Kiszka <[email protected]>

---

Long pending in my queue. Hope we can finally get these two in via trivial (that's what they are).

 gdbstub.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
gdbstub.c +10 -0
index e4a1a79..da3e7cb 100644
@@ -41,6 +41,12 @@
 #include "qemu/sockets.h"
 #include "sysemu/kvm.h"
 
+#ifdef CONFIG_USER_ONLY
+#define GDB_ATTACHED "0"
+#else
+#define GDB_ATTACHED "1"
+#endif
+
 static inline int target_memory_rw_debug(CPUState *cpu, target_ulong addr,
                                          uint8_t *buf, int len, bool is_write)
 {

@@ -1187,6 +1193,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             put_packet_binary(s, buf, len + 1);
             break;
         }
+        if (strncmp(p, "Attached", 8) == 0) {
+            put_packet(s, GDB_ATTACHED);
+            break;
+        }
         /* Unrecognised 'q' command.  */
         goto unknown_command;
Signature · 1 line
2.1.4
author
#1 lore raw

PATCH 2/2 Revert "gdbstub: Do not kill target in system emulation mode"

From: Fabien Chouteau <[email protected]>

The requirements described in this patch are implemented by "Add GDB qAttached support".

This reverts commit 00e94dbc7fd0110b0555d59592b004333adfb4b8.

Signed-off-by Fabien Chouteau <[email protected]>
Signed-off-by Jan Kiszka <[email protected]>
---
 gdbstub.c | 2 --
 1 file changed, 2 deletions(-)
gdbstub.c +0 -2
index da3e7cb..14e1f0c 100644
@@ -880,11 +880,9 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             goto unknown_command;
         }
     case 'k':
-#ifdef CONFIG_USER_ONLY
         /* Kill the target */
         fprintf(stderr, "\nQEMU: Terminated via GDBstub\n");
         exit(0);
-#endif
     case 'D':
         /* Detach packet */
         gdb_breakpoint_remove_all();
Signature · 1 line
2.1.4
#2 lore raw
In reply to Jan Kiszka

Hi, I was skimming the list, and noticed:

gdbstub.c static int gdb_handle_packet(GDBState *s, const char *line_buf) · line 1196

Quoted 5 lines · Jan Kiszka
gdbstub.c +1 -0
@@ -1187,6 +1193,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             put_packet_binary(s, buf, len + 1);
             break;
         }
+        if (strncmp(p, "Attached", 8) == 0) {

This looks like it'd mishandle a future qAttached2 packet.

It should be doing something like:

       if (strncmp(p, "Attached", 8) == 0 &&
          (p[8] == '\0' || p[8] == ':')) {

or:

       if (strcmp(p, "Attached") == 0 || strncmp(p, "Attached:", 9) == 0) {

Likewise other packets, if they have the same issue. (I'm not familiar with qemu's stub's internals.)

Thanks,
Pedro Alves

author
#3 lore raw
Quoted 16 lines · Pedro Alves

Hi, I was skimming the list, and noticed:

gdbstub.c static int gdb_handle_packet(GDBState *s, const char *line_buf) · line 1196

Quoted 5 lines · Jan Kiszka
gdbstub.c +1 -0
@@ -1187,6 +1193,10 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
             put_packet_binary(s, buf, len + 1);
             break;
         }
+        if (strncmp(p, "Attached", 8) == 0) {

This looks like it'd mishandle a future qAttached2 packet.

It should be doing something like:

       if (strncmp(p, "Attached", 8) == 0 &&
          (p[8] == '\0' || p[8] == ':')) {
Quoted 7 lines

or:

       if (strcmp(p, "Attached") == 0 || strncmp(p, "Attached:", 9) == 0) {

Likewise other packets, if they have the same issue. (I'm not familiar with qemu's stub's internals.)

Thanks for the remark! Will update the patch using the easier readable second variant.

Jan

#4 lore raw
In reply to Pedro Alves
Quoted 8 lines · Pedro Alves

This looks like it'd mishandle a future qAttached2 packet.

It should be doing something like:

       if (strncmp(p, "Attached", 8) == 0 &&
          (p[8] == '\0' || p[8] == ':')) {
Quoted 7 lines

or:

       if (strcmp(p, "Attached") == 0 || strncmp(p, "Attached:", 9) == 0) {

Likewise other packets, if they have the same issue. (I'm not familiar with qemu's stub's internals.)

Looks like we get this wrong for a lot of our existing query packet handling too... Maybe worth having a utility function for "is this a foo query packet" rather than raw strcmp/strncmp?

-- PMM