On 05/07/2019 14:17, Alex Bennée wrote: > Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> 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 <ari...@gmail.com> >> Date: Wed May 29 09:41:36 2019 +0300 >> >> gdbstub: Implement set register (P pkt) with new infra >> >> Signed-off-by: Jon Doron <ari...@gmail.com> >> Message-Id: <20190529064148.19856-9-ari...@gmail.com> >> Signed-off-by: Alex Bennée <alex.ben...@linaro.org> >> >> 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.