Re: [Qemu-devel] [PULL 06/37] gdbstub: add multiprocess support to 'H' and 'T' packets

[Peter Maydell]

On Mon, 7 Jan 2019 at 16:31, Peter Maydell <address@hidden> wrote:
>
> From: Luc Michel <address@hidden>
>
> Add a couple of helper functions to cope with GDB threads and processes.
>
> The gdb_get_process() function looks for a process given a pid.
>
> The gdb_get_cpu() function returns the CPU corresponding to the (pid,
> tid) pair given as parameters.
>
> The read_thread_id() function parses the thread-id sent by the peer.
> This function supports the multiprocess extension thread-id syntax.  The
> return value specifies if the parsing failed, or if a special case was
> encountered (all processes or all threads).
>
> Use them in 'H' and 'T' packets handling to support the multiprocess
> extension.

> +static CPUState *gdb_get_cpu(const GDBState *s, uint32_t pid, uint32_t tid)
> +{
> +    GDBProcess *process;
> +    CPUState *cpu;
> +
> +    if (!tid) {
> +        /* 0 means any thread, we take the first one */
> +        tid = 1;
> +    }
> +
> +    cpu = find_cpu(tid);
> +
> +    if (cpu == NULL) {
> +        return NULL;
> +    }
> +
> +    process = gdb_get_cpu_process(s, cpu);
> +
> +    if (process->pid != pid) {
> +        return NULL;
> +    }
> +
> +    if (!process->attached) {
> +        return NULL;
> +    }
> +
> +    return cpu;
> +}

I'm finding that (with my out-of-tree board model) gdb
asks for "process 0 thread 0", which should mean "anything you
like", so we end up calling gdb_get_cpu(s, 0, 0). Then we
find a CPU via its TID, and of course it has some specific
PID in process->pid which is not zero, so we fail the
"process->pid != pid" test and return NULL here incorrectly.

Should that check be
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -736,7 +736,8 @@ static CPUState *gdb_get_cpu(const GDBState *s,
uint32_t pid, uint32_t tid)

     process = gdb_get_cpu_process(s, cpu);

-    if (process->pid != pid) {
+    /* pid == 0 means any process, so this one is fine */
+    if (pid != 0 && process->pid != pid) {
         return NULL;
     }

?

thanks
-- PMM