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

[Peter Maydell]

On Thu, 17 Jan 2019 at 18:13, Peter Maydell <address@hidden> wrote:
>
> 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.

Also, the "use TID 1 if asked for any-task" logic assumes
that TID 1 is actually part of the requested process and
that that task is attached. Shouldn't we instead find the
requested process (or any random attached process, if tid == 0)
and then pick one of the TIDs it has?

thanks
-- PMM