From 1e9fa730163c2a445014ff8324b169cd82a50df1 Mon Sep 17 00:00:00 2001 From: Nathan Froyd Date: Wed, 3 Jun 2009 11:33:08 -0700 Subject: [PATCH] fix gdbstub support for multiple threads in usermode, v3 When debugging multi-threaded programs, QEMU's gdb stub would report the correct number of threads (the qfThreadInfo and qsThreadInfo packets). However, the stub was unable to actually switch between threads (the T packet), since it would report every thread except the first as being dead. Furthermore, the stub relied upon cpu_index as a reliable means of assigning IDs to the threads. This was a bad idea; if you have this sequence of events: initial thread created new thread #1 new thread #2 thread #1 exits new thread #3 thread #3 will have the same cpu_index as thread #1, which would confuse GDB. (This problem is partly due to the remote protocol not having a good way to send thread creation/destruction events.) We fix this by using the host thread ID for the identifier passed to GDB when debugging a multi-threaded userspace program. The thread ID might wrap, but the same sort of problems with wrapping thread IDs would come up with debugging programs natively, so this doesn't represent a problem. Signed-off-by: Nathan Froyd --- cpu-defs.h | 1 + exec.c | 2 +- gdbstub.c | 69 ++++++++++++++++++++++++++++---------------- linux-user/syscall.c | 4 ++- 4 files changed, 49 insertions(+), 27 deletions(-) diff --git a/cpu-defs.h b/cpu-defs.h index 0d0eaa1c5e..fda3044bcd 100644 --- a/cpu-defs.h +++ b/cpu-defs.h @@ -184,6 +184,7 @@ typedef struct CPUWatchpoint { \ CPUState *next_cpu; /* next CPU sharing TB cache */ \ int cpu_index; /* CPU index (informative) */ \ + uint32_t host_tid; /* host thread ID */ \ int numa_node; /* NUMA node this cpu is belonging to */ \ int running; /* Nonzero if cpu is currently running(usermode). */ \ /* user data */ \ diff --git a/exec.c b/exec.c index 723de89bf4..52f4e89251 100644 --- a/exec.c +++ b/exec.c @@ -553,7 +553,7 @@ void cpu_exec_init(CPUState *env) penv = &first_cpu; cpu_index = 0; while (*penv != NULL) { - penv = (CPUState **)&(*penv)->next_cpu; + penv = &(*penv)->next_cpu; cpu_index++; } env->cpu_index = cpu_index; diff --git a/gdbstub.c b/gdbstub.c index 7b32fab589..9bd4375852 100644 --- a/gdbstub.c +++ b/gdbstub.c @@ -1567,11 +1567,34 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc) #endif } +static inline int gdb_id(CPUState *env) +{ +#if defined(CONFIG_USER_ONLY) && defined(USE_NPTL) + return env->host_tid; +#else + return env->cpu_index + 1; +#endif +} + +static CPUState *find_cpu(uint32_t thread_id) +{ + CPUState *env; + + for (env = first_cpu; env != NULL; env = env->next_cpu) { + if (gdb_id(env) == thread_id) { + return env; + } + } + + return NULL; +} + static int gdb_handle_packet(GDBState *s, const char *line_buf) { CPUState *env; const char *p; - int ch, reg_size, type, res, thread; + uint32_t thread; + int ch, reg_size, type, res; char buf[MAX_PACKET_LENGTH]; uint8_t mem_buf[MAX_PACKET_LENGTH]; uint8_t *registers; @@ -1586,7 +1609,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) case '?': /* TODO: Make this return the correct value for user-mode. */ snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP, - s->c_cpu->cpu_index+1); + gdb_id(s->c_cpu)); put_packet(s, buf); /* Remove all the breakpoints when this query is issued, * because gdb is doing and initial connect and the state @@ -1750,9 +1773,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) put_packet(s, "OK"); break; } - for (env = first_cpu; env != NULL; env = env->next_cpu) - if (env->cpu_index + 1 == thread) - break; + env = find_cpu(thread); if (env == NULL) { put_packet(s, "E22"); break; @@ -1773,14 +1794,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) break; case 'T': thread = strtoull(p, (char **)&p, 16); -#ifndef CONFIG_USER_ONLY - if (thread > 0 && thread < smp_cpus + 1) -#else - if (thread == 1) -#endif - put_packet(s, "OK"); - else + env = find_cpu(thread); + + if (env != NULL) { + put_packet(s, "OK"); + } else { put_packet(s, "E22"); + } break; case 'q': case 'Q': @@ -1818,7 +1838,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) } else if (strcmp(p,"sThreadInfo") == 0) { report_cpuinfo: if (s->query_cpu) { - snprintf(buf, sizeof(buf), "m%x", s->query_cpu->cpu_index+1); + snprintf(buf, sizeof(buf), "m%x", gdb_id(s->query_cpu)); put_packet(s, buf); s->query_cpu = s->query_cpu->next_cpu; } else @@ -1826,16 +1846,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) break; } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) { thread = strtoull(p+16, (char **)&p, 16); - for (env = first_cpu; env != NULL; env = env->next_cpu) - if (env->cpu_index + 1 == thread) { - cpu_synchronize_state(env, 0); - len = snprintf((char *)mem_buf, sizeof(mem_buf), - "CPU#%d [%s]", env->cpu_index, - env->halted ? "halted " : "running"); - memtohex(buf, mem_buf, len); - put_packet(s, buf); - break; - } + env = find_cpu(thread); + if (env != NULL) { + cpu_synchronize_state(env, 0); + len = snprintf((char *)mem_buf, sizeof(mem_buf), + "CPU#%d [%s]", env->cpu_index, + env->halted ? "halted " : "running"); + memtohex(buf, mem_buf, len); + put_packet(s, buf); + } break; } #ifdef CONFIG_USER_ONLY @@ -1965,7 +1984,7 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) } snprintf(buf, sizeof(buf), "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";", - GDB_SIGNAL_TRAP, env->cpu_index+1, type, + GDB_SIGNAL_TRAP, gdb_id(env), type, env->watchpoint_hit->vaddr); put_packet(s, buf); env->watchpoint_hit = NULL; @@ -1976,7 +1995,7 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) } else { ret = GDB_SIGNAL_INT; } - snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, env->cpu_index+1); + snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, gdb_id(env)); put_packet(s, buf); } #endif diff --git a/linux-user/syscall.c b/linux-user/syscall.c index a0915a455b..59c91f8da9 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -3202,6 +3202,7 @@ static void *clone_func(void *arg) env = info->env; thread_env = env; info->tid = gettid(); + env->host_tid = info->tid; if (info->child_tidptr) put_user_u32(info->tid, info->child_tidptr); if (info->parent_tidptr) @@ -3792,6 +3793,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, /* FIXME: This probably breaks if a signal arrives. We should probably be disabling signals. */ if (first_cpu->next_cpu) { + TaskState *ts; CPUState **lastp; CPUState *p; @@ -3809,7 +3811,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, /* Remove the CPU from the list. */ *lastp = p->next_cpu; cpu_list_unlock(); - TaskState *ts = ((CPUState *)cpu_env)->opaque; + ts = ((CPUState *)cpu_env)->opaque; if (ts->child_tidptr) { put_user_u32(0, ts->child_tidptr); sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX,