| Message ID | 20190502081554.5521-14-arilou@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | gdbstub: Refactor command packets handler | expand |
Jon Doron <arilou@gmail.com> writes: > Signed-off-by: Jon Doron <arilou@gmail.com> > --- > gdbstub.c | 41 +++++++++++++++++++++++++++++++---------- > 1 file changed, 31 insertions(+), 10 deletions(-) > > diff --git a/gdbstub.c b/gdbstub.c > index daa602edc3..adfe39b3a3 100644 > --- a/gdbstub.c > +++ b/gdbstub.c > @@ -1734,6 +1734,29 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx) > put_packet(gdb_ctx->s, gdb_ctx->str_buf); > } > > +static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) > +{ > + target_ulong addr, len; > + uint8_t *registers; > + int reg_size; > + > + if (!gdb_ctx->num_params) { > + return; > + } > + > + cpu_synchronize_state(gdb_ctx->s->g_cpu); > + registers = gdb_ctx->mem_buf; > + len = strlen(gdb_ctx->params[0].data) / 2; > + hextomem(registers, gdb_ctx->params[0].data, len); Admittedly the legacy code didn't check either but we are assuming that registers (i.e. gdb_ctx->mem_buf) won't overflow. It's probably still safe as the incoming packets are limited in size. I was trying to find where MAX_PACKET_LENGTH came from and AFAICT it's an arbitrary number we made up. I wonder if it would make sense in another series to convert the various buffers to GString and GBytes so we can dynamically handle longer packets without all this inconsistent application of bounds checking. Anyway: Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > + for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs && len > 0; > + addr++) { > + reg_size = gdb_write_register(gdb_ctx->s->g_cpu, registers, addr); > + len -= reg_size; > + registers += reg_size; > + } > + put_packet(gdb_ctx->s, "OK"); > +} > + > static int gdb_handle_packet(GDBState *s, const char *line_buf) > { > CPUState *cpu; > @@ -1745,7 +1768,6 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) > uint8_t mem_buf[MAX_PACKET_LENGTH]; > char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; > char thread_id[16]; > - uint8_t *registers; > target_ulong addr, len; > const GdbCmdParseEntry *cmd_parser = NULL; > > @@ -1911,16 +1933,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) > put_packet(s, buf); > break; > case 'G': > - cpu_synchronize_state(s->g_cpu); > - registers = mem_buf; > - len = strlen(p) / 2; > - hextomem((uint8_t *)registers, p, len); > - for (addr = 0; addr < s->g_cpu->gdb_num_g_regs && len > 0; addr++) { > - reg_size = gdb_write_register(s->g_cpu, registers, addr); > - len -= reg_size; > - registers += reg_size; > + { > + static const GdbCmdParseEntry write_all_regs_cmd_desc = { > + .handler = handle_write_all_regs, > + .cmd = "G", > + .cmd_startswith = 1, > + .schema = "s0" > + }; > + cmd_parser = &write_all_regs_cmd_desc; > } > - put_packet(s, "OK"); > break; > case 'm': > { -- Alex Bennée
diff --git a/gdbstub.c b/gdbstub.c index daa602edc3..adfe39b3a3 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1734,6 +1734,29 @@ static void handle_read_mem(GdbCmdContext *gdb_ctx, void *user_ctx) put_packet(gdb_ctx->s, gdb_ctx->str_buf); } +static void handle_write_all_regs(GdbCmdContext *gdb_ctx, void *user_ctx) +{ + target_ulong addr, len; + uint8_t *registers; + int reg_size; + + if (!gdb_ctx->num_params) { + return; + } + + cpu_synchronize_state(gdb_ctx->s->g_cpu); + registers = gdb_ctx->mem_buf; + len = strlen(gdb_ctx->params[0].data) / 2; + hextomem(registers, gdb_ctx->params[0].data, len); + for (addr = 0; addr < gdb_ctx->s->g_cpu->gdb_num_g_regs && len > 0; + addr++) { + reg_size = gdb_write_register(gdb_ctx->s->g_cpu, registers, addr); + len -= reg_size; + registers += reg_size; + } + put_packet(gdb_ctx->s, "OK"); +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *cpu; @@ -1745,7 +1768,6 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) uint8_t mem_buf[MAX_PACKET_LENGTH]; char buf[sizeof(mem_buf) + 1 /* trailing NUL */]; char thread_id[16]; - uint8_t *registers; target_ulong addr, len; const GdbCmdParseEntry *cmd_parser = NULL; @@ -1911,16 +1933,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, buf); break; case 'G': - cpu_synchronize_state(s->g_cpu); - registers = mem_buf; - len = strlen(p) / 2; - hextomem((uint8_t *)registers, p, len); - for (addr = 0; addr < s->g_cpu->gdb_num_g_regs && len > 0; addr++) { - reg_size = gdb_write_register(s->g_cpu, registers, addr); - len -= reg_size; - registers += reg_size; + { + static const GdbCmdParseEntry write_all_regs_cmd_desc = { + .handler = handle_write_all_regs, + .cmd = "G", + .cmd_startswith = 1, + .schema = "s0" + }; + cmd_parser = &write_all_regs_cmd_desc; } - put_packet(s, "OK"); break; case 'm': {
Signed-off-by: Jon Doron <arilou@gmail.com> --- gdbstub.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-)