diff mbox series

[v3,19/20] gdbstub: Implement qGDBServerVersion packet

Message ID 20250521164250.135776-20-alex.bennee@linaro.org
State New
Headers show
Series Maintainer updates for May (testing, plugins, virtio-gpu) - pre-PR | expand

Commit Message

Alex Bennée May 21, 2025, 4:42 p.m. UTC 
From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>

This commit adds support for the `qGDBServerVersion` packet to the qemu
gdbstub  which could be used by clients to detect the QEMU version
(and, e.g., use a workaround for known bugs).

This packet is not documented/standarized by GDB but it was implemented
by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1].

This has been implemented by Patryk, who I included in Co-authored-by
and who asked me to send the patch.

[0] https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion
[1] https://github.com/pwndbg/pwndbg/issues/2648

Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com>
Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com>
[AJB: fix include, checkpatch linewrap]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 gdbstub/gdbstub.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

Comments

Akihiko Odaki May 22, 2025, 6:29 a.m. UTC | #1 
On 2025/05/22 1:42, Alex Bennée wrote:
> From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
> 
> This commit adds support for the `qGDBServerVersion` packet to the qemu
> gdbstub  which could be used by clients to detect the QEMU version
> (and, e.g., use a workaround for known bugs).
> 
> This packet is not documented/standarized by GDB but it was implemented
> by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1].
> 
> This has been implemented by Patryk, who I included in Co-authored-by
> and who asked me to send the patch.
> 
> [0] https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion
> [1] https://github.com/pwndbg/pwndbg/issues/2648
> 
> Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com>
> Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
> Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com>
> [AJB: fix include, checkpatch linewrap]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   gdbstub/gdbstub.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 6023c80d25..def0b7e877 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -28,6 +28,7 @@
>   #include "qemu/cutils.h"
>   #include "qemu/module.h"
>   #include "qemu/error-report.h"
> +#include "qemu/target-info.h"
>   #include "trace.h"
>   #include "exec/gdbstub.h"
>   #include "gdbstub/commands.h"
> @@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx)
>       gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
>   }
>   
> +static void handle_query_gdb_server_version(GArray *params, void *user_ctx)
> +{
> +#if defined(CONFIG_USER_ONLY)
> +    g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;",
> +                    target_name(), QEMU_VERSION);
> +#else
> +    g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;",
> +                    target_name(), QEMU_VERSION);
> +#endif

I sugguest passing "qemu" as the name property.

I guess LLDB developers chose to explicitly have the key-value pair 
syntax so that users can have one unified logic for parsing and avoid 
the mess of HTTP's User-Agent; there is a proposal for Web that 
introduces key-value pairs in a similar manner:
https://developer.chrome.com/docs/privacy-security/user-agent-client-hints

If we embed more information like to the name property, users will need 
to have an additional logic to know if it's QEMU or to know other 
information. Instead, we can emit:
name:qemu;version:10.0.50;

and we can use something like follows if we want to tell more:
name:qemu;version:10.0.50;qemu-mode:system;qemu-target:hexagon;
Alex Bennée May 22, 2025, 10:05 a.m. UTC | #2 
Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> On 2025/05/22 1:42, Alex Bennée wrote:
>> From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
>> This commit adds support for the `qGDBServerVersion` packet to the
>> qemu
>> gdbstub  which could be used by clients to detect the QEMU version
>> (and, e.g., use a workaround for known bugs).
>> This packet is not documented/standarized by GDB but it was
>> implemented
>> by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1].
>> This has been implemented by Patryk, who I included in
>> Co-authored-by
>> and who asked me to send the patch.
>> [0]
>> https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion
>> [1] https://github.com/pwndbg/pwndbg/issues/2648
>> Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com>
>> Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
>> Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com>
>> [AJB: fix include, checkpatch linewrap]
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   gdbstub/gdbstub.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
>> index 6023c80d25..def0b7e877 100644
>> --- a/gdbstub/gdbstub.c
>> +++ b/gdbstub/gdbstub.c
>> @@ -28,6 +28,7 @@
>>   #include "qemu/cutils.h"
>>   #include "qemu/module.h"
>>   #include "qemu/error-report.h"
>> +#include "qemu/target-info.h"
>>   #include "trace.h"
>>   #include "exec/gdbstub.h"
>>   #include "gdbstub/commands.h"
>> @@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx)
>>       gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
>>   }
>>   +static void handle_query_gdb_server_version(GArray *params, void
>> *user_ctx)
>> +{
>> +#if defined(CONFIG_USER_ONLY)
>> +    g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;",
>> +                    target_name(), QEMU_VERSION);
>> +#else
>> +    g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;",
>> +                    target_name(), QEMU_VERSION);
>> +#endif
>
> I sugguest passing "qemu" as the name property.
>
> I guess LLDB developers chose to explicitly have the key-value pair
> syntax so that users can have one unified logic for parsing and avoid
> the mess of HTTP's User-Agent; there is a proposal for Web that
> introduces key-value pairs in a similar manner:
> https://developer.chrome.com/docs/privacy-security/user-agent-client-hints
>
> If we embed more information like to the name property, users will
> need to have an additional logic to know if it's QEMU or to know other
> information. Instead, we can emit:
> name:qemu;version:10.0.50;
>
> and we can use something like follows if we want to tell more:
> name:qemu;version:10.0.50;qemu-mode:system;qemu-target:hexagon;

I think we are getting into bike shedding territory here. GDB does need
a decent way to communicate about supported targets and when it comes up
with one we shall implement it. But I don't think we need to give too
much thought to this custom response for now.
Akihiko Odaki May 22, 2025, 10:15 a.m. UTC | #3 
On 2025/05/22 19:05, Alex Bennée wrote:
> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
> 
>> On 2025/05/22 1:42, Alex Bennée wrote:
>>> From: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
>>> This commit adds support for the `qGDBServerVersion` packet to the
>>> qemu
>>> gdbstub  which could be used by clients to detect the QEMU version
>>> (and, e.g., use a workaround for known bugs).
>>> This packet is not documented/standarized by GDB but it was
>>> implemented
>>> by LLDB gdbstub [0] and is helpful for projects like Pwndbg [1].
>>> This has been implemented by Patryk, who I included in
>>> Co-authored-by
>>> and who asked me to send the patch.
>>> [0]
>>> https://lldb.llvm.org/resources/lldbgdbremote.html#qgdbserverversion
>>> [1] https://github.com/pwndbg/pwndbg/issues/2648
>>> Co-authored-by: Patryk 'patryk4815' Sondej <patryk.sondej@gmail.com>
>>> Signed-off-by: Dominik 'Disconnect3d' Czarnota <dominik.b.czarnota@gmail.com>
>>> Message-Id: <20250403191340.53343-1-dominik.b.czarnota@gmail.com>
>>> [AJB: fix include, checkpatch linewrap]
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    gdbstub/gdbstub.c | 17 +++++++++++++++++
>>>    1 file changed, 17 insertions(+)
>>> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
>>> index 6023c80d25..def0b7e877 100644
>>> --- a/gdbstub/gdbstub.c
>>> +++ b/gdbstub/gdbstub.c
>>> @@ -28,6 +28,7 @@
>>>    #include "qemu/cutils.h"
>>>    #include "qemu/module.h"
>>>    #include "qemu/error-report.h"
>>> +#include "qemu/target-info.h"
>>>    #include "trace.h"
>>>    #include "exec/gdbstub.h"
>>>    #include "gdbstub/commands.h"
>>> @@ -1597,6 +1598,18 @@ static void handle_query_threads(GArray *params, void *user_ctx)
>>>        gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
>>>    }
>>>    +static void handle_query_gdb_server_version(GArray *params, void
>>> *user_ctx)
>>> +{
>>> +#if defined(CONFIG_USER_ONLY)
>>> +    g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;",
>>> +                    target_name(), QEMU_VERSION);
>>> +#else
>>> +    g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;",
>>> +                    target_name(), QEMU_VERSION);
>>> +#endif
>>
>> I sugguest passing "qemu" as the name property.
>>
>> I guess LLDB developers chose to explicitly have the key-value pair
>> syntax so that users can have one unified logic for parsing and avoid
>> the mess of HTTP's User-Agent; there is a proposal for Web that
>> introduces key-value pairs in a similar manner:
>> https://developer.chrome.com/docs/privacy-security/user-agent-client-hints
>>
>> If we embed more information like to the name property, users will
>> need to have an additional logic to know if it's QEMU or to know other
>> information. Instead, we can emit:
>> name:qemu;version:10.0.50;
>>
>> and we can use something like follows if we want to tell more:
>> name:qemu;version:10.0.50;qemu-mode:system;qemu-target:hexagon;
> 
> I think we are getting into bike shedding territory here. GDB does need
> a decent way to communicate about supported targets and when it comes up
> with one we shall implement it. But I don't think we need to give too
> much thought to this custom response for now.
> 

My suggestion is to pass just "qemu" as the name. The example with 
"qemu-mode" and "qemu-target" is a demonstration of how additional 
information should be provided, and I don't think we need to provide the 
mode and target information now. We can think of details when adding 
such information.

Regards,
Akihiko Odaki
diff mbox series

Patch

diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
index 6023c80d25..def0b7e877 100644
--- a/gdbstub/gdbstub.c
+++ b/gdbstub/gdbstub.c
@@ -28,6 +28,7 @@ 
 #include "qemu/cutils.h"
 #include "qemu/module.h"
 #include "qemu/error-report.h"
+#include "qemu/target-info.h"
 #include "trace.h"
 #include "exec/gdbstub.h"
 #include "gdbstub/commands.h"
@@ -1597,6 +1598,18 @@  static void handle_query_threads(GArray *params, void *user_ctx)
     gdbserver_state.query_cpu = gdb_next_attached_cpu(gdbserver_state.query_cpu);
 }
 
+static void handle_query_gdb_server_version(GArray *params, void *user_ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    g_string_printf(gdbserver_state.str_buf, "name:qemu-%s;version:%s;",
+                    target_name(), QEMU_VERSION);
+#else
+    g_string_printf(gdbserver_state.str_buf, "name:qemu-system-%s;version:%s;",
+                    target_name(), QEMU_VERSION);
+#endif
+    gdb_put_strbuf();
+}
+
 static void handle_query_first_threads(GArray *params, void *user_ctx)
 {
     gdbserver_state.query_cpu = gdb_first_attached_cpu();
@@ -1842,6 +1855,10 @@  static const GdbCmdParseEntry gdb_gen_query_table[] = {
         .handler = handle_query_threads,
         .cmd = "sThreadInfo",
     },
+    {
+        .handler = handle_query_gdb_server_version,
+        .cmd = "GDBServerVersion",
+    },
     {
         .handler = handle_query_first_threads,
         .cmd = "fThreadInfo",