Re: [Qemu-devel] [PATCH v11 02/20] gdbstub: Implement deatch (D pkt) with new infra

[Alex Bennée]

Alex Bennée <address@hidden> writes:

> Jon Doron <address@hidden> writes:
>
>> Signed-off-by: Jon Doron <address@hidden>
>
> Reviewed-by: Alex Bennée <address@hidden>

Hmm although I bisected to this patch which fails on:

09:49 address@hidden/x86_64  [linux.git/address@hidden >gdb 
./builds/arm64/vmlinux -x ~/lsrc/qemu.git/tests/guest-debug/test-gdbstub.py
GNU gdb (GDB) 8.3.50.20190424-git
Copyright (C) 2019 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-pc-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Executed .gdbinit
Reading symbols from ./builds/arm64/vmlinux...
Traceback (most recent call last):
  File "/home/alex/lsrc/linux.git/builds/arm64/vmlinux-gdb.py", line 30, in 
<module>
    import linux.config
ImportError: No module named config
Connecting to remote
0x0000000040000000 in ?? ()
Checking we can step the first few instructions
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: single step in boot code
Checking HW breakpoint works
Hardware assisted breakpoint 1 at 0xffffff8010778f0c: file 
/home/alex/lsrc/linux.git/init/main.c, line 1068.
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
0x40000000 == {int (void *)} 0xffffff8010778f0c <kernel_init>
FAIL: hbreak @ kernel_init
Setup catch-all for run_init_process
Breakpoint 2 at 0xffffff8010083dc4: file /home/alex/lsrc/linux.git/init/main.c, 
line 1009.
Breakpoint 3 at 0xffffff8010083e10: file /home/alex/lsrc/linux.git/init/main.c, 
line 1020.
Checking Normal breakpoint works
Breakpoint 4 at 0xffffff801077b300: file 
/home/alex/lsrc/linux.git/kernel/sched/completion.c, line 136.
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
0x40000000 == {void (struct completion *)} 0xffffff801077b300 
<wait_for_completion> 0
FAIL: break @ wait_for_completion
Checking watchpoint works
Hardware access (read/write) watchpoint 5: *(enum system_states 
*)(&system_state)
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: awatch for system_state (SYSTEM_BOOTING)
Hardware read watchpoint 6: *(enum system_states *)(&system_state)
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: rwatch for system_state (SYSTEM_BOOTING)
Hardware watchpoint 7: *(enum system_states *)(&system_state)
warning: Invalid remote reply:

Thread 1 received signal SIGINT, Interrupt.
0x0000000040000000 in ?? ()
FAIL: watch for system_state (SYSTEM_BOOTING)
[Inferior 1 (process 1) killed]


>
>> ---
>>  gdbstub.c | 93 +++++++++++++++++++++++++++++++------------------------
>>  1 file changed, 53 insertions(+), 40 deletions(-)
>>
>> diff --git a/gdbstub.c b/gdbstub.c
>> index e6d895177b..307366b250 100644
>> --- a/gdbstub.c
>> +++ b/gdbstub.c
>> @@ -1413,11 +1413,6 @@ static inline int startswith(const char *string, 
>> const char *pattern)
>>    return !strncmp(string, pattern, strlen(pattern));
>>  }
>>
>> -static int process_string_cmd(
>> -        GDBState *s, void *user_ctx, const char *data,
>> -        const GdbCmdParseEntry *cmds, int num_cmds)
>> -        __attribute__((unused));
>> -
>>  static int process_string_cmd(GDBState *s, void *user_ctx, const char *data,
>>                                const GdbCmdParseEntry *cmds, int num_cmds)
>>  {
>> @@ -1463,6 +1458,41 @@ static int process_string_cmd(GDBState *s, void 
>> *user_ctx, const char *data,
>>      return -1;
>>  }
>>
>> +static void handle_detach(GdbCmdContext *gdb_ctx, void *user_ctx)
>> +{
>> +    GDBProcess *process;
>> +    GDBState *s = gdb_ctx->s;
>> +    uint32_t pid = 1;
>> +
>> +    if (s->multiprocess) {
>> +        if (!gdb_ctx->num_params) {
>> +            put_packet(s, "E22");
>> +            return;
>> +        }
>> +
>> +        pid = gdb_ctx->params[0].val_ul;
>> +    }
>> +
>> +    process = gdb_get_process(s, pid);
>> +    gdb_process_breakpoint_remove_all(s, process);
>> +    process->attached = false;
>> +
>> +    if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
>> +        s->c_cpu = gdb_first_attached_cpu(s);
>> +    }
>> +
>> +    if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
>> +        s->g_cpu = gdb_first_attached_cpu(s);
>> +    }
>> +
>> +    if (!s->c_cpu) {
>> +        /* No more process attached */
>> +        gdb_syscall_mode = GDB_SYS_DISABLED;
>> +        gdb_continue(s);
>> +    }
>> +    put_packet(s, "OK");
>> +}
>> +
>>  static int gdb_handle_packet(GDBState *s, const char *line_buf)
>>  {
>>      CPUState *cpu;
>> @@ -1477,6 +1507,7 @@ static int gdb_handle_packet(GDBState *s, const char 
>> *line_buf)
>>      uint8_t *registers;
>>      target_ulong addr, len;
>>      GDBThreadIdKind thread_kind;
>> +    const GdbCmdParseEntry *cmd_parser = NULL;
>>
>>      trace_gdbstub_io_command(line_buf);
>>
>> @@ -1577,42 +1608,15 @@ static int gdb_handle_packet(GDBState *s, const char 
>> *line_buf)
>>          error_report("QEMU: Terminated via GDBstub");
>>          exit(0);
>>      case 'D':
>> -        /* Detach packet */
>> -        pid = 1;
>> -
>> -        if (s->multiprocess) {
>> -            unsigned long lpid;
>> -            if (*p != ';') {
>> -                put_packet(s, "E22");
>> -                break;
>> -            }
>> -
>> -            if (qemu_strtoul(p + 1, &p, 16, &lpid)) {
>> -                put_packet(s, "E22");
>> -                break;
>> -            }
>> -
>> -            pid = lpid;
>> -        }
>> -
>> -        process = gdb_get_process(s, pid);
>> -        gdb_process_breakpoint_remove_all(s, process);
>> -        process->attached = false;
>> -
>> -        if (pid == gdb_get_cpu_pid(s, s->c_cpu)) {
>> -            s->c_cpu = gdb_first_attached_cpu(s);
>> -        }
>> -
>> -        if (pid == gdb_get_cpu_pid(s, s->g_cpu)) {
>> -            s->g_cpu = gdb_first_attached_cpu(s);
>> -        }
>> -
>> -        if (s->c_cpu == NULL) {
>> -            /* No more process attached */
>> -            gdb_syscall_mode = GDB_SYS_DISABLED;
>> -            gdb_continue(s);
>> +        {
>> +            static const GdbCmdParseEntry detach_cmd_desc = {
>> +                .handler = handle_detach,
>> +                .cmd = "D",
>> +                .cmd_startswith = 1,
>> +                .schema = "?.l0"
>> +            };
>> +            cmd_parser = &detach_cmd_desc;
>>          }
>> -        put_packet(s, "OK");
>>          break;
>>      case 's':
>>          if (*p != '\0') {
>> @@ -1985,6 +1989,15 @@ static int gdb_handle_packet(GDBState *s, const char 
>> *line_buf)
>>          put_packet(s, buf);
>>          break;
>>      }
>> +
>> +    if (cmd_parser) {
>> +        /* helper will respond */
>> +        process_string_cmd(s, NULL, line_buf, cmd_parser, 1);
>> +    } else {
>> +        /* unknown command, empty respone */
>> +        put_packet(s, "");
>> +    }
>> +
>>      return RS_IDLE;
>>  }


--
Alex Bennée