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

[Alex Bennée]

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?

> 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):

    /*
     * 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.

--
Alex Bennée