diff mbox series

[v9,20/27] gdbstub: Implement target halted (? pkt) with new infra

Message ID 20190502081554.5521-21-arilou@gmail.com
State New
Headers show
Series gdbstub: Refactor command packets handler | expand

Commit Message

Jon Doron May 2, 2019, 8:15 a.m. UTC 
Signed-off-by: Jon Doron <arilou@gmail.com>
---
 gdbstub.c | 36 ++++++++++++++++++++++++++----------
 1 file changed, 26 insertions(+), 10 deletions(-)

Comments

Alex Bennée May 15, 2019, 5:20 p.m. UTC | #1 
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?

> +    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
Jon Doron May 20, 2019, 5:32 a.m. UTC | #2 
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?

> > +    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 May 20, 2019, 12:54 p.m. UTC | #3 
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
diff mbox series

Patch

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.  */
+    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
+     * 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':
         {