[PATCH v3 0/8] gdbserver improvements for AArch64 SVE support

Thiago Jung Bauermann via Gdb-patches
2023-01-30T04:46:06Z

Hello,

This is version 3 of the patch series adding support to gdbserver for
inferiors that change the SVE vector length at runtime. This is already
supported by GDB, but not by gdbserver. Version 2 was posted here:

https://inbox.sourceware.org/gdb-patches/20221126020452.1686509-1-thiago.bauermann@linaro.org/

This version incorporates the review comments from v2 (thanks!). The
biggest change is that it implements Simon's idea¹:

> If it's the case, that we need to extend the RSP to allow fetching
> per-thread tdesc, here's the idea I had in mind for a while, to avoid
> adding too much overhead. Stop replies and the qXfer:threads:read reply
> could include a per-thread tdesc identifier. That tdesc identifier
> would be something short, either an incrementing number, or some kind of
> hash of the tdesc. It would be something opaque chosen by the stub. A
> new packet would be introduced to allow GDB to request the XML given
> that ID. On the GDB side, GDB would request the XML when encountering a
> tdesc ID it doesn't know about, and then cache the result.

The only difference from the above in this series is that instead of
creating a new packet to allow GDB to request the XML given the ID, I'm
extending the qXfer:features:read request: GDB can send "target-id-%u.xml"
as the annex, where %u is the target description ID. Please let me know if
you think a separate packet would be better. The remote protocol changes
are documented in patch 5.

I had to drop the Reviewed-by tags from some patches because they have some
significant changes from the version that was reviewed.

There's also a new testcase exercising the case of an inferior's thread
changing its SVE vector length. The previous version of this series failed
the testcase, but this one passes it.

With this series applied, gdb.arch/aarch64-sve.exp passes all tests on
extended-remote aarch64-linux. Without them, it fails tests after the
testcase changes the vector length.

Tested on native and extended-remote aarch64-linux, x86_64-linux and
armv7l-linux-gnueabihf (the last one on QEMU TCG).

¹ https://inbox.sourceware.org/gdb-patches/559069a3-f3ce-2059-bf4a-44add43979f7@simark.ca/

Changes since v2:

- Patch “gdbserver: Add assert in find_register_by_number”
- Rewritten to follow Simon's suggestion of putting the assert in another
function.
- Patch “gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap”
- As suggested by Simon, directly access thread_info::id rather than use
pid_of.
- Also as suggested by Simon, updated doc comment of
process_stratum_target::read_auxv.
- Patch “gdbserver/linux-aarch64: Factor out function to get aarch64_features”
- As suggested by Simon, directly access thread_info::id rather than use
lwpid_of.
- Also as suggested by Simon, added doc comment for
aarch64_get_arch_features.
- Patch “gdbserver/linux-aarch64: When thread stops, update its target description”
- As suggested by Luis, added doc comments to thread_info::tdesc and
arch_process_info::has_sve.
- As suggested by Simon, renamed arch_update_tdesc to get_thread_tdesc,
and return a target_desc pointer (or nullptr) instead of one wrapped in
gdb::optional. The code was changed a bit as well.
- As suggested by Luis, expanded doc comment of get_thread_tdesc.
- As suggested by Luis, abstracted away trying to fetch a tdesc from a
thread and then from a process to a new function
"get_thread_target_desc".
- Export free_register_cache_thread in gdbserver/regcache.h.
- Use free_register_cache_thread in linux_process_target::filter_event to
implement Simon's suggestion of only freeing the regcache for the
thread with a different tdesc.
- “gdb/aarch64: Factor out most of the thread_architecture method”
- Dropped this patch. It's not needed anymore.
- “gdbserver: Transmit target description ID in thread list and stop reply”
- New patch.
- “gdb/remote: Parse and use tdesc field in stop reply and threads list XML”
- New patch.
- “gdb/aarch64: Detect vector length changes when debugging remotely”
- As suggested by Luis, clarified in the patch description that this patch improves
debugging programs remotely. Also, reworded description somewhat.
- As suggested by Luis, changed VL references in comments to VG. Also reworded a bit the
description of the update_architecture gdbarch method.
- Changed the update_architecture gdbarch method (and consequently also
aarch64_update_architecture) to take a target description instead of a vector of
cached_reg_t.
- Changed remote_target::update_thread_list to get the thread target description from
the remote target.
- Changed remote_target::process_stop_reply to get the thread target description from
the remote target.
- Renamed remote_thread_info::expedited_arch to arch.
- Changed code of remote_target::thread_architecture to be a little bit clearer, and
added doc comment.
- “gdb/testsuite: Add test to exercise multi-threaded AArch64 SVE inferiors”
- New patch.

Thiago Jung Bauermann (8):
gdbserver: Add assert in find_register_by_number
gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap
gdbserver/linux-aarch64: Factor out function to get aarch64_features
gdbserver/linux-aarch64: When thread stops, update its target
description
gdbserver: Transmit target description ID in thread list and stop
reply
gdb/remote: Parse tdesc field in stop reply and threads list XML
gdb/aarch64: Detect vector length changes when debugging remotely
gdb/testsuite: Add test to exercise multi-threaded AArch64 SVE
inferiors

gdb/aarch64-tdep.c | 20 +++
gdb/arch-utils.c | 8 +
gdb/arch-utils.h | 4 +
gdb/doc/gdb.texinfo | 27 ++-
gdb/features/threads.dtd | 1 +
gdb/gdbarch-components.py | 15 ++
gdb/gdbarch-gen.h | 10 ++
gdb/gdbarch.c | 22 +++
gdb/remote.c | 169 +++++++++++++++++-
gdb/testsuite/gdb.arch/aarch64-sve-threads.c | 125 +++++++++++++
.../gdb.arch/aarch64-sve-threads.exp | 70 ++++++++
gdb/xml-tdesc.c | 27 ++-
gdb/xml-tdesc.h | 6 +
gdbserver/gdbthread.h | 4 +
gdbserver/linux-aarch64-low.cc | 68 +++++--
gdbserver/linux-arm-low.cc | 2 +-
gdbserver/linux-low.cc | 35 +++-
gdbserver/linux-low.h | 15 +-
gdbserver/linux-ppc-low.cc | 6 +-
gdbserver/linux-s390-low.cc | 2 +-
gdbserver/netbsd-low.cc | 4 +-
gdbserver/netbsd-low.h | 2 +-
gdbserver/regcache.cc | 14 +-
gdbserver/regcache.h | 4 +
gdbserver/remote-utils.cc | 57 ++++++
gdbserver/remote-utils.h | 9 +
gdbserver/server.cc | 20 ++-
gdbserver/server.h | 4 +
gdbserver/target.cc | 4 +-
gdbserver/target.h | 4 +-
gdbserver/tdesc.cc | 13 +-
gdbserver/tdesc.h | 5 +
32 files changed, 717 insertions(+), 59 deletions(-)
create mode 100644 gdb/testsuite/gdb.arch/aarch64-sve-threads.c
create mode 100644 gdb/testsuite/gdb.arch/aarch64-sve-threads.exp

base-commit: 6c76a6beade05f8b2ca93e939cf73da2917d379b