On 3/19/25 11:22, Alex Bennée wrote:
The aim of this work is to get rid of the endian aware helpers in
gdbstub/helpers.h which due to their use of tswap() mean target
gdbstubs need to be built multiple times. While this series doesn't
actually build each stub once it introduces a new helper -
gdb_get_register_value() which takes a MemOp which can describe the
current endian state of the system. This will be a lot easier to
dynamically feed from a helper function.
The most complex example is PPC which has a helper called
ppc_maybe_bswap_register() which was doing this.
This is still an RFC so I'm interested in feedback:
- is the API sane
- can we avoid lots of (uint8_t *) casting?
Even though the series has a good intent, the fact we make everything
"generic" makes that we lose all guarantees we could get by relying on
static typing, and that we had possibility of mistakes when passing size
(which happened in patch 4 if I'm correct). And explicit casting comes
as a *strong* warning about that.
By patch 7, I was really feeling it's not a win vs explicit functions
per size.
If the goal of the series is to get rid of endian aware helpers, well,
this can be fixed in the helpers themselves, without needing to
introduce a "generic" size helper. Maybe we are trying to solve two
different problems here?
- should we have a reverse helper for setting registers
If this seems like the right approach I can have a go at more of the
frontends later.
There are a few other misc clean-ups I did on the way which might be
worth cherry picking for 10.0 but I'll leave that up to maintainers.
Alex.
Alex Bennée (10):
include/gdbstub: fix include guard in commands.h
gdbstub: introduce target independent gdb register helper
target/arm: convert 32 bit gdbstub to new helper
target/arm: convert 64 bit gdbstub to new helper
target/ppc: expand comment on FP/VMX/VSX access functions
target/ppc: make ppc_maybe_bswap_register static
target/ppc: convert gdbstub to new helper (!hacky)
gdbstub: assert earlier in handle_read_all_regs
include/exec: fix assert in size_memop
target/microblaze: convert gdbstub to new helper
include/exec/memop.h | 4 +-
include/gdbstub/commands.h | 2 +-
include/gdbstub/registers.h | 30 ++++++
target/ppc/cpu.h | 8 +-
gdbstub/gdbstub.c | 24 ++++-
target/arm/gdbstub.c | 57 +++++++----
target/arm/gdbstub64.c | 53 ++++++----
target/microblaze/gdbstub.c | 44 ++++----
target/ppc/gdbstub.c | 194 ++++++++++++++++++++----------------
9 files changed, 257 insertions(+), 159 deletions(-)
create mode 100644 include/gdbstub/registers.h