Re: [PULL 19/25] gdbstub: Replace gdb_regs with an array

[Fabiano Rosas]

Alex Bennée <alex.bennee@linaro.org> writes:

> From: Akihiko Odaki <akihiko.odaki@daynix.com>
>
> An array is a more appropriate data structure than a list for gdb_regs
> since it is initialized only with append operation and read-only after
> initialization.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Message-Id: <20230912224107.29669-13-akihiko.odaki@daynix.com>
> [AJB: fixed a checkpatch violation]
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20231009164104.369749-20-alex.bennee@linaro.org>
>
> diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h
> index 7b8347ed5a..3968369554 100644
> --- a/include/hw/core/cpu.h
> +++ b/include/hw/core/cpu.h
> @@ -502,7 +502,7 @@ struct CPUState {
>  
>      CPUJumpCache *tb_jmp_cache;
>  
> -    struct GDBRegisterState *gdb_regs;
> +    GArray *gdb_regs;
>      int gdb_num_regs;
>      int gdb_num_g_regs;
>      QTAILQ_ENTRY(CPUState) node;
> diff --git a/gdbstub/gdbstub.c b/gdbstub/gdbstub.c
> index 62608a5389..b1532118d1 100644
> --- a/gdbstub/gdbstub.c
> +++ b/gdbstub/gdbstub.c
> @@ -51,7 +51,6 @@ typedef struct GDBRegisterState {
>      gdb_get_reg_cb get_reg;
>      gdb_set_reg_cb set_reg;
>      const char *xml;
> -    struct GDBRegisterState *next;
>  } GDBRegisterState;
>  
>  GDBState gdbserver_state;
> @@ -386,7 +385,8 @@ static const char *get_feature_xml(const char *p, const 
> char **newp,
>                  xml,
>                  g_markup_printf_escaped("<xi:include href=\"%s\"/>",
>                                          cc->gdb_core_xml_file));
> -            for (r = cpu->gdb_regs; r; r = r->next) {
> +            for (guint i = 0; i < cpu->gdb_regs->len; i++) {

It seems we can reach here before having initialized gdb_regs at
gdb_register_coprocessor():

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x0000555555e5310b in get_feature_xml (p=0x555556a99118
<gdbserver_state+56> "target.xml:0,ffb", newp=0x7fffffffc6b0,
process=0x555557a21dd0) at ../gdbstub/gdbstub.c:388

(gdb) p/x cpu->gdb_regs
$1 = 0x0


Using:
qemu-system-x86 ... -s -s

just connect GDB and it crashes.