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

[Alex Bennée]

Jon Doron <address@hidden> writes:

> 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?

Sure

>
> 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


--
Alex Bennée