On Mon, 7 Jan 2019 at 16:31, Peter Maydell <[email protected]> wrote:
>
> From: Luc Michel <[email protected]>
>
> 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
