[Qemu-trivial] [PATCH 1/2] Add GDB qAttached support

[Qemu-trivial] [PATCH 1/2] Add GDB qAttached support

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1408 bytes --]

From: Jan Kiszka <jan.kiszka@siemens.com>

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 <jan.kiszka@siemens.com>
---

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(+)

diff --git a/gdbstub.c b/gdbstub.c
index e4a1a79..da3e7cb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -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;
 
-- 
2.1.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1408 bytes --]

From: Jan Kiszka <jan.kiszka@siemens.com>

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 <jan.kiszka@siemens.com>
---

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(+)

diff --git a/gdbstub.c b/gdbstub.c
index e4a1a79..da3e7cb 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -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;
 
-- 
2.1.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[Qemu-trivial] [PATCH 2/2] Revert "gdbstub: Do not kill target in system emulation mode"

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

From: Fabien Chouteau <chouteau@adacore.com>

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

This reverts commit 00e94dbc7fd0110b0555d59592b004333adfb4b8.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 gdbstub.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index da3e7cb..14e1f0c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -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();
-- 
2.1.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[Qemu-devel] [PATCH 2/2] Revert "gdbstub: Do not kill target in system emulation mode"

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 871 bytes --]

From: Fabien Chouteau <chouteau@adacore.com>

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

This reverts commit 00e94dbc7fd0110b0555d59592b004333adfb4b8.

Signed-off-by: Fabien Chouteau <chouteau@adacore.com>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 gdbstub.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index da3e7cb..14e1f0c 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -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();
-- 
2.1.4


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Pedro Alves]

Hi, I was skimming the list, and noticed:

On 01/31/2015 10:28 AM, Jan Kiszka wrote:
> @@ -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

Re: [Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Pedro Alves]

Hi, I was skimming the list, and noticed:

On 01/31/2015 10:28 AM, Jan Kiszka wrote:
> @@ -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

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On 2015-02-04 14:36, Pedro Alves wrote:
> Hi, I was skimming the list, and noticed:
> 
> On 01/31/2015 10:28 AM, Jan Kiszka wrote:
>> @@ -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 for the remark! Will update the patch using the easier readable
second variant.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 887 bytes --]

On 2015-02-04 14:36, Pedro Alves wrote:
> Hi, I was skimming the list, and noticed:
> 
> On 01/31/2015 10:28 AM, Jan Kiszka wrote:
>> @@ -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 for the remark! Will update the patch using the easier readable
second variant.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

Re: [Qemu-trivial] [Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Peter Maydell]

On 4 February 2015 at 13:36, Pedro Alves <palves@redhat.com> wrote:
>
> 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.)

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

Re: [Qemu-devel] [PATCH 1/2] Add GDB qAttached support

[Peter Maydell]

On 4 February 2015 at 13:36, Pedro Alves <palves@redhat.com> wrote:
>
> 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.)

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