Re: [PATCH v3 19/20] gdbstub: Implement qGDBServerVersion packet

["Alex Bennée" <alex.bennee@linaro.org>]

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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro