Re: [Qemu-devel] [PATCH v9 14/27] gdbstub: Implement read all registers (g pkt) with new infra

[Jon Doron]

I agree but I guess to really fix it we need to change
gdb_read_register implementation to support returning the size of the
register for mem_buffer = NULL
Let's leave it for another patchset?

On Wed, May 15, 2019 at 7:10 PM Alex Bennée <address@hidden> wrote:
>
>
> Jon Doron <address@hidden> writes:
>
> > Signed-off-by: Jon Doron <address@hidden>
> > ---
> >  gdbstub.c | 31 +++++++++++++++++++++++--------
> >  1 file changed, 23 insertions(+), 8 deletions(-)
> >
> > diff --git a/gdbstub.c b/gdbstub.c
> > index adfe39b3a3..3478ac778d 100644
> > --- a/gdbstub.c
> > +++ b/gdbstub.c
> > @@ -1757,6 +1757,21 @@ static void handle_write_all_regs(GdbCmdContext 
> > *gdb_ctx, void *user_ctx)
> >      put_packet(gdb_ctx->s, "OK");
> >  }
> >
> > +static void handle_read_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx)
> > +{
> > +    target_ulong addr, len;
> > +
> > +    cpu_synchronize_state(gdb_ctx->s->g_cpu);
> > +    len = 0;
> > +    for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs; addr++) {
> > +        len += gdb_read_register(gdb_ctx->s->g_cpu, gdb_ctx->mem_buf + len,
> > +                                 addr);
> > +    }
>
> Again no bounds checking - we get away with it because for hppa:
>
>   (* 8 128 2) = 2048
>
> Anyway:
>
> Reviewed-by: Alex Bennée <address@hidden>
>
> > +
> > +    memtohex(gdb_ctx->str_buf, gdb_ctx->mem_buf, len);
> > +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
> > +}
> > +
> >  static int gdb_handle_packet(GDBState *s, const char *line_buf)
> >  {
> >      CPUState *cpu;
> > @@ -1764,7 +1779,7 @@ static int gdb_handle_packet(GDBState *s, const char 
> > *line_buf)
> >      CPUClass *cc;
> >      const char *p;
> >      uint32_t pid, tid;
> > -    int ch, reg_size, type, res;
> > +    int ch, type, res;
> >      uint8_t mem_buf[MAX_PACKET_LENGTH];
> >      char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
> >      char thread_id[16];
> > @@ -1923,14 +1938,14 @@ static int gdb_handle_packet(GDBState *s, const 
> > char *line_buf)
> >          }
> >          break;
> >      case 'g':
> > -        cpu_synchronize_state(s->g_cpu);
> > -        len = 0;
> > -        for (addr = 0; addr < s->g_cpu->gdb_num_g_regs; addr++) {
> > -            reg_size = gdb_read_register(s->g_cpu, mem_buf + len, addr);
> > -            len += reg_size;
> > +        {
> > +            static const GdbCmdParseEntry read_all_regs_cmd_desc = {
> > +                .handler = handle_read_all_regs,
> > +                .cmd = "g",
> > +                .cmd_startswith = 1
> > +            };
> > +            cmd_parser = &read_all_regs_cmd_desc;
> >          }
> > -        memtohex(buf, mem_buf, len);
> > -        put_packet(s, buf);
> >          break;
> >      case 'G':
> >          {
>
>
> --
> Alex Bennée