Re: [Qemu-devel] [PATCH v9 20/27] gdbstub: Implement target halted (? pkt) with new infra

["Alex Bennée" <alex.bennee@linaro.org>]

Jon Doron <arilou@gmail.com> writes:

> On Wed, May 15, 2019 at 8:20 PM Alex Bennée <alex.bennee@linaro.org> wrote:
>>
>>
>> Jon Doron <arilou@gmail.com> writes:
>>
>> > Signed-off-by: Jon Doron <arilou@gmail.com>
>> > ---
>> >  gdbstub.c | 36 ++++++++++++++++++++++++++----------
>> >  1 file changed, 26 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/gdbstub.c b/gdbstub.c
>> > index 2fd0d66f4d..d678191705 100644
>> > --- a/gdbstub.c
>> > +++ b/gdbstub.c
>> > @@ -2239,13 +2239,30 @@ static void handle_gen_set(GdbCmdContext *gdb_ctx, void *user_ctx)
>> >      put_packet(gdb_ctx->s, "");
>> >  }
>> >
>> > +static void handle_target_halt(GdbCmdContext *gdb_ctx, void *user_ctx)
>> > +{
>> > +    char thread_id[16];
>> > +
>> > +    /* TODO: Make this return the correct value for user-mode.  */
>>
>> Can this be cleaned up as we convert?
>>
>
> To be honest i have no idea what the "correct value" is or how to get
> it, can you tell me what it should be and ill add it to the patch?

Actually I think you can delete the comment and mention the thread-id
has been correctly reported in usermode since bd88c780e6

>
>> > +    gdb_fmt_thread_id(gdb_ctx->s, gdb_ctx->s->c_cpu, thread_id,
>> > +                      sizeof(thread_id));
>> > +    snprintf(gdb_ctx->str_buf, sizeof(gdb_ctx->str_buf), "T%02xthread:%s;",
>> > +             GDB_SIGNAL_TRAP, thread_id);
>> > +    put_packet(gdb_ctx->s, gdb_ctx->str_buf);
>> > +    /*
>> > +     * Remove all the breakpoints when this query is issued,
>> > +     * because gdb is doing and initial connect and the state
>>
>> s/and/an/
>>
>> > +     * should be cleaned up.
>> > +     */
>> > +    gdb_breakpoint_remove_all();
>> > +}
>> > +
>> >  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>> >  {
>> >      const char *p;
>> >      int ch;
>> >      uint8_t mem_buf[MAX_PACKET_LENGTH];
>> >      char buf[sizeof(mem_buf) + 1 /* trailing NUL */];
>> > -    char thread_id[16];
>> >      const GdbCmdParseEntry *cmd_parser = NULL;
>> >
>> >      trace_gdbstub_io_command(line_buf);
>> > @@ -2257,15 +2274,14 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf)
>> >          put_packet(s, "OK");
>> >          break;
>> >      case '?':
>> > -        /* TODO: Make this return the correct value for user-mode.  */
>> > -        snprintf(buf, sizeof(buf), "T%02xthread:%s;", GDB_SIGNAL_TRAP,
>> > -                 gdb_fmt_thread_id(s, s->c_cpu, thread_id, sizeof(thread_id)));
>> > -        put_packet(s, buf);
>> > -        /* Remove all the breakpoints when this query is issued,
>> > -         * because gdb is doing and initial connect and the state
>> > -         * should be cleaned up.
>> > -         */
>> > -        gdb_breakpoint_remove_all();
>> > +        {
>> > +            static const GdbCmdParseEntry target_halted_cmd_desc = {
>> > +                .handler = handle_target_halt,
>> > +                .cmd = "?",
>> > +                .cmd_startswith = 1
>> > +            };
>> > +            cmd_parser = &target_halted_cmd_desc;
>> > +        }
>> >          break;
>> >      case 'c':
>> >          {
>>
>>
>> --
>> Alex Bennée


--
Alex Bennée