Re: [Qemu-devel] Unable to set register on qemu-system-sparc64 via gdbstub

[Mark Cave-Ayland]

On 05/07/2019 14:17, Alex Bennée wrote:

> Mark Cave-Ayland <address@hidden> writes:
> 
>> Hi all,
>>
>> It looks as if the recent gdbstub code rework has broken the ability to set 
>> registers
>> under qemu-system-sparc64:
>>
>> $ sparc64-linux-gdb obj-sparc64/openbios-builtin.elf.nostrip
>> GNU gdb (GDB) 8.1
>> Copyright (C) 2018 Free Software Foundation, Inc.
>> License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
>> This is free software: you are free to change and redistribute it.
>> There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
>> and "show warranty" for details.
>> This GDB was configured as "--host=x86_64-pc-linux-gnu 
>> --target=sparc64-linux".
>> Type "show configuration" for configuration details.
>> For bug reporting instructions, please see:
>> <http://www.gnu.org/software/gdb/bugs/>.
>> Find the GDB manual and other documentation resources online at:
>> <http://www.gnu.org/software/gdb/documentation/>.
>> For help, type "help".
>> Type "apropos word" to search for commands related to "word"...
>> Reading symbols from obj-sparc64/openbios-builtin.elf.nostrip...done.
>> (gdb) target remote :1234
>> Remote debugging using :1234
>> 0x000001fff0000020 in ?? ()
>> (gdb) info regi $g1
>> g1             0x0      0
>> (gdb) set $g1 = 0x55
>> Could not write register "g1"; remote failure reply 'E00'
>> (gdb)
>>
>> I managed to narrow this down to the recent gdbstub rework, and in 
>> particular to this
>> patch:
>>
>> commit 62b3320bddd79c050553ea7f81f20c6d3b401ce3
>> Author: Jon Doron <address@hidden>
>> Date:   Wed May 29 09:41:36 2019 +0300
>>
>>     gdbstub: Implement set register (P pkt) with new infra
>>
>>     Signed-off-by: Jon Doron <address@hidden>
>>     Message-Id: <address@hidden>
>>     Signed-off-by: Alex Bennée <address@hidden>
>>
>> Tracing through I see that the problem occurs because of this code in 
>> gdbstub's
>> handle_set_reg:
>>
>> static void handle_set_reg(GdbCmdContext *gdb_ctx, void *user_ctx)
>> {
>>     int reg_size;
>>
>>     if (!gdb_has_xml) {
>>         put_packet(gdb_ctx->s, "E00");
>>         return;
>>     }
>>
>>     ...
>>     ...
>> }
>>
>> Because SPARC doesn't have any GDB XML files then this check always
>> fails which is
> 
> I think the gdb_has_xml test in this case is if the gdb protocol
> understand XML (as well as us having an XML spec in QEMU). I assume your
> gdb is fairly modern?

Ah right, I missed that it was set for both the spec being present and support 
in
gdb. Currently I'm running with gdb 8.1.

>> why the E00 error code is being returned.
> 
> Previously we'd fallback to unknown_command which would send an empty
> packet. That results in gdb sending a series of additional packets
> instead of just failing:
> 
>   10423@1562330441.283482:gdbstub_io_command Received: P8=0000000000103500
>   10423@1562330441.283505:gdbstub_io_reply Sent:
>   10423@1562330441.283670:gdbstub_io_command Received: 
> G000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000103500000000000000000100000040008013380000000000000000000000000000000000000000000000000000004000800a810000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010045000000000001004540000000082009207000000000000000000000000000000000000000000000000
>   10423@1562330441.283710:gdbstub_io_reply Sent: OK
>   10423@1562330441.283902:gdbstub_io_command Received: g
>   10423@1562330441.283934:gdbstub_io_reply Sent: 
> 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000103500000000000000000100000040008013380000000000000000000000000000000000000000000000000000004000800a810000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010045000000000001004540000000082009207000000000000000000000000000000000000000000000000
>   10423@1562330441.284267:gdbstub_io_command Received: m100450,4
>   10423@1562330441.284289:gdbstub_io_reply Sent: 1700040f
>   10423@1562330441.284453:gdbstub_io_command Received: m10044c,4
>   10423@1562330441.284471:gdbstub_io_reply Sent: 1100040d
>   10423@1562330441.284562:gdbstub_io_command Received: m10043c,4
>   10423@1562330441.284573:gdbstub_io_reply Sent: bc100000
>   10423@1562330441.284661:gdbstub_io_command Received: m10043c,4
>   10423@1562330441.284671:gdbstub_io_reply Sent: bc100000
> 
> Which is the G packet (Write general registers) instead of the "ignored"
> P packet (Write register n). As you have seen skipping the gdb_xml test
> results in:
> 
> 15029@1562331181.902711:gdbstub_io_command Received: P8=0000000000103500
> 15029@1562331181.902733:gdbstub_io_reply Sent: OK
> 15029@1562331181.902925:gdbstub_io_command Received: g
> 15029@1562331181.902951:gdbstub_io_reply Sent: 
> 000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000103500000000000000000100000040008013380000000000000000000000000000000000000000000000000000004000800a810000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010045000000000001004540000000082009207000000000000000000000000000000000000000000000000
> 15029@1562331181.903286:gdbstub_io_command Received: m100450,4
> 15029@1562331181.903311:gdbstub_io_reply Sent: 1700040f
> 15029@1562331181.903454:gdbstub_io_command Received: m10044c,4
> 15029@1562331181.903466:gdbstub_io_reply Sent: 1100040d
> 15029@1562331181.903566:gdbstub_io_command Received: m10043c,4
> 15029@1562331181.903582:gdbstub_io_reply Sent: bc100000
> 15029@1562331181.903695:gdbstub_io_command Received: m10043c,4
> 15029@1562331181.903710:gdbstub_io_reply Sent: bc100000
> 
> 
>> In fact if I simply comment out the above check then everything appears to 
>> work
>> again, however I'm not sure that this is the correct fix because there are 
>> several
>> other references to gdb_has_xml remaining in the file?
> 
> I suspect the best fix would be to define an XML for sparc so we can use
> the more modern features. However given the age of the architecture and
> without knowing if it is actively developed in gdb I suspect the easiest
> fix would be to do the same as handle_get_reg (and what it did pre re-factor):

There are certainly some XML files present, although I'm not familiar enough 
with gdb
to know off-hand whether they can be used in their current form:
http://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=tree;f=gdb/features/sparc;h=7a7b6c48ce0e7176120d5f27028ab01716af4bd2;hb=98602811d838077269e361e9d807fe530c780011.

>     /*
>      * Older gdb are really dumb, and don't use 'g' if 'p' is avaialable.
>      * This works, but can be very slow.  Anything new enough to
>      * understand XML also knows how to use this properly.
>      */
>     if (!gdb_has_xml) {
>         put_packet(gdb_ctx->s, "");
>         return;
>     }
> 
> I'll spin up a patch doing that.

That certainly seems the best option for 4.1. I've seen the patch in my inbox 
so I'll
go give it a quick test...


ATB,

Mark.