Commit 1e9fa730163c2a445014ff8324b169cd82a50df1
Committed by
Paul Brook
1 parent
4548eaea
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 <froydnj@codesourcery.com>
Showing
4 changed files
with
49 additions
and
27 deletions
cpu-defs.h
@@ -184,6 +184,7 @@ typedef struct CPUWatchpoint { | @@ -184,6 +184,7 @@ typedef struct CPUWatchpoint { | ||
184 | \ | 184 | \ |
185 | CPUState *next_cpu; /* next CPU sharing TB cache */ \ | 185 | CPUState *next_cpu; /* next CPU sharing TB cache */ \ |
186 | int cpu_index; /* CPU index (informative) */ \ | 186 | int cpu_index; /* CPU index (informative) */ \ |
187 | + uint32_t host_tid; /* host thread ID */ \ | ||
187 | int numa_node; /* NUMA node this cpu is belonging to */ \ | 188 | int numa_node; /* NUMA node this cpu is belonging to */ \ |
188 | int running; /* Nonzero if cpu is currently running(usermode). */ \ | 189 | int running; /* Nonzero if cpu is currently running(usermode). */ \ |
189 | /* user data */ \ | 190 | /* user data */ \ |
exec.c
@@ -553,7 +553,7 @@ void cpu_exec_init(CPUState *env) | @@ -553,7 +553,7 @@ void cpu_exec_init(CPUState *env) | ||
553 | penv = &first_cpu; | 553 | penv = &first_cpu; |
554 | cpu_index = 0; | 554 | cpu_index = 0; |
555 | while (*penv != NULL) { | 555 | while (*penv != NULL) { |
556 | - penv = (CPUState **)&(*penv)->next_cpu; | 556 | + penv = &(*penv)->next_cpu; |
557 | cpu_index++; | 557 | cpu_index++; |
558 | } | 558 | } |
559 | env->cpu_index = cpu_index; | 559 | env->cpu_index = cpu_index; |
gdbstub.c
@@ -1567,11 +1567,34 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc) | @@ -1567,11 +1567,34 @@ static void gdb_set_cpu_pc(GDBState *s, target_ulong pc) | ||
1567 | #endif | 1567 | #endif |
1568 | } | 1568 | } |
1569 | 1569 | ||
1570 | +static inline int gdb_id(CPUState *env) | ||
1571 | +{ | ||
1572 | +#if defined(CONFIG_USER_ONLY) && defined(USE_NPTL) | ||
1573 | + return env->host_tid; | ||
1574 | +#else | ||
1575 | + return env->cpu_index + 1; | ||
1576 | +#endif | ||
1577 | +} | ||
1578 | + | ||
1579 | +static CPUState *find_cpu(uint32_t thread_id) | ||
1580 | +{ | ||
1581 | + CPUState *env; | ||
1582 | + | ||
1583 | + for (env = first_cpu; env != NULL; env = env->next_cpu) { | ||
1584 | + if (gdb_id(env) == thread_id) { | ||
1585 | + return env; | ||
1586 | + } | ||
1587 | + } | ||
1588 | + | ||
1589 | + return NULL; | ||
1590 | +} | ||
1591 | + | ||
1570 | static int gdb_handle_packet(GDBState *s, const char *line_buf) | 1592 | static int gdb_handle_packet(GDBState *s, const char *line_buf) |
1571 | { | 1593 | { |
1572 | CPUState *env; | 1594 | CPUState *env; |
1573 | const char *p; | 1595 | const char *p; |
1574 | - int ch, reg_size, type, res, thread; | 1596 | + uint32_t thread; |
1597 | + int ch, reg_size, type, res; | ||
1575 | char buf[MAX_PACKET_LENGTH]; | 1598 | char buf[MAX_PACKET_LENGTH]; |
1576 | uint8_t mem_buf[MAX_PACKET_LENGTH]; | 1599 | uint8_t mem_buf[MAX_PACKET_LENGTH]; |
1577 | uint8_t *registers; | 1600 | uint8_t *registers; |
@@ -1586,7 +1609,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | @@ -1586,7 +1609,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | ||
1586 | case '?': | 1609 | case '?': |
1587 | /* TODO: Make this return the correct value for user-mode. */ | 1610 | /* TODO: Make this return the correct value for user-mode. */ |
1588 | snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP, | 1611 | snprintf(buf, sizeof(buf), "T%02xthread:%02x;", GDB_SIGNAL_TRAP, |
1589 | - s->c_cpu->cpu_index+1); | 1612 | + gdb_id(s->c_cpu)); |
1590 | put_packet(s, buf); | 1613 | put_packet(s, buf); |
1591 | /* Remove all the breakpoints when this query is issued, | 1614 | /* Remove all the breakpoints when this query is issued, |
1592 | * because gdb is doing and initial connect and the state | 1615 | * 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) | @@ -1750,9 +1773,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | ||
1750 | put_packet(s, "OK"); | 1773 | put_packet(s, "OK"); |
1751 | break; | 1774 | break; |
1752 | } | 1775 | } |
1753 | - for (env = first_cpu; env != NULL; env = env->next_cpu) | ||
1754 | - if (env->cpu_index + 1 == thread) | ||
1755 | - break; | 1776 | + env = find_cpu(thread); |
1756 | if (env == NULL) { | 1777 | if (env == NULL) { |
1757 | put_packet(s, "E22"); | 1778 | put_packet(s, "E22"); |
1758 | break; | 1779 | break; |
@@ -1773,14 +1794,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | @@ -1773,14 +1794,13 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | ||
1773 | break; | 1794 | break; |
1774 | case 'T': | 1795 | case 'T': |
1775 | thread = strtoull(p, (char **)&p, 16); | 1796 | thread = strtoull(p, (char **)&p, 16); |
1776 | -#ifndef CONFIG_USER_ONLY | ||
1777 | - if (thread > 0 && thread < smp_cpus + 1) | ||
1778 | -#else | ||
1779 | - if (thread == 1) | ||
1780 | -#endif | ||
1781 | - put_packet(s, "OK"); | ||
1782 | - else | 1797 | + env = find_cpu(thread); |
1798 | + | ||
1799 | + if (env != NULL) { | ||
1800 | + put_packet(s, "OK"); | ||
1801 | + } else { | ||
1783 | put_packet(s, "E22"); | 1802 | put_packet(s, "E22"); |
1803 | + } | ||
1784 | break; | 1804 | break; |
1785 | case 'q': | 1805 | case 'q': |
1786 | case 'Q': | 1806 | case 'Q': |
@@ -1818,7 +1838,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | @@ -1818,7 +1838,7 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | ||
1818 | } else if (strcmp(p,"sThreadInfo") == 0) { | 1838 | } else if (strcmp(p,"sThreadInfo") == 0) { |
1819 | report_cpuinfo: | 1839 | report_cpuinfo: |
1820 | if (s->query_cpu) { | 1840 | if (s->query_cpu) { |
1821 | - snprintf(buf, sizeof(buf), "m%x", s->query_cpu->cpu_index+1); | 1841 | + snprintf(buf, sizeof(buf), "m%x", gdb_id(s->query_cpu)); |
1822 | put_packet(s, buf); | 1842 | put_packet(s, buf); |
1823 | s->query_cpu = s->query_cpu->next_cpu; | 1843 | s->query_cpu = s->query_cpu->next_cpu; |
1824 | } else | 1844 | } else |
@@ -1826,16 +1846,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | @@ -1826,16 +1846,15 @@ static int gdb_handle_packet(GDBState *s, const char *line_buf) | ||
1826 | break; | 1846 | break; |
1827 | } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) { | 1847 | } else if (strncmp(p,"ThreadExtraInfo,", 16) == 0) { |
1828 | thread = strtoull(p+16, (char **)&p, 16); | 1848 | thread = strtoull(p+16, (char **)&p, 16); |
1829 | - for (env = first_cpu; env != NULL; env = env->next_cpu) | ||
1830 | - if (env->cpu_index + 1 == thread) { | ||
1831 | - cpu_synchronize_state(env, 0); | ||
1832 | - len = snprintf((char *)mem_buf, sizeof(mem_buf), | ||
1833 | - "CPU#%d [%s]", env->cpu_index, | ||
1834 | - env->halted ? "halted " : "running"); | ||
1835 | - memtohex(buf, mem_buf, len); | ||
1836 | - put_packet(s, buf); | ||
1837 | - break; | ||
1838 | - } | 1849 | + env = find_cpu(thread); |
1850 | + if (env != NULL) { | ||
1851 | + cpu_synchronize_state(env, 0); | ||
1852 | + len = snprintf((char *)mem_buf, sizeof(mem_buf), | ||
1853 | + "CPU#%d [%s]", env->cpu_index, | ||
1854 | + env->halted ? "halted " : "running"); | ||
1855 | + memtohex(buf, mem_buf, len); | ||
1856 | + put_packet(s, buf); | ||
1857 | + } | ||
1839 | break; | 1858 | break; |
1840 | } | 1859 | } |
1841 | #ifdef CONFIG_USER_ONLY | 1860 | #ifdef CONFIG_USER_ONLY |
@@ -1965,7 +1984,7 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) | @@ -1965,7 +1984,7 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) | ||
1965 | } | 1984 | } |
1966 | snprintf(buf, sizeof(buf), | 1985 | snprintf(buf, sizeof(buf), |
1967 | "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";", | 1986 | "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";", |
1968 | - GDB_SIGNAL_TRAP, env->cpu_index+1, type, | 1987 | + GDB_SIGNAL_TRAP, gdb_id(env), type, |
1969 | env->watchpoint_hit->vaddr); | 1988 | env->watchpoint_hit->vaddr); |
1970 | put_packet(s, buf); | 1989 | put_packet(s, buf); |
1971 | env->watchpoint_hit = NULL; | 1990 | env->watchpoint_hit = NULL; |
@@ -1976,7 +1995,7 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) | @@ -1976,7 +1995,7 @@ static void gdb_vm_state_change(void *opaque, int running, int reason) | ||
1976 | } else { | 1995 | } else { |
1977 | ret = GDB_SIGNAL_INT; | 1996 | ret = GDB_SIGNAL_INT; |
1978 | } | 1997 | } |
1979 | - snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, env->cpu_index+1); | 1998 | + snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, gdb_id(env)); |
1980 | put_packet(s, buf); | 1999 | put_packet(s, buf); |
1981 | } | 2000 | } |
1982 | #endif | 2001 | #endif |
linux-user/syscall.c
@@ -3202,6 +3202,7 @@ static void *clone_func(void *arg) | @@ -3202,6 +3202,7 @@ static void *clone_func(void *arg) | ||
3202 | env = info->env; | 3202 | env = info->env; |
3203 | thread_env = env; | 3203 | thread_env = env; |
3204 | info->tid = gettid(); | 3204 | info->tid = gettid(); |
3205 | + env->host_tid = info->tid; | ||
3205 | if (info->child_tidptr) | 3206 | if (info->child_tidptr) |
3206 | put_user_u32(info->tid, info->child_tidptr); | 3207 | put_user_u32(info->tid, info->child_tidptr); |
3207 | if (info->parent_tidptr) | 3208 | if (info->parent_tidptr) |
@@ -3792,6 +3793,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, | @@ -3792,6 +3793,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, | ||
3792 | /* FIXME: This probably breaks if a signal arrives. We should probably | 3793 | /* FIXME: This probably breaks if a signal arrives. We should probably |
3793 | be disabling signals. */ | 3794 | be disabling signals. */ |
3794 | if (first_cpu->next_cpu) { | 3795 | if (first_cpu->next_cpu) { |
3796 | + TaskState *ts; | ||
3795 | CPUState **lastp; | 3797 | CPUState **lastp; |
3796 | CPUState *p; | 3798 | CPUState *p; |
3797 | 3799 | ||
@@ -3809,7 +3811,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, | @@ -3809,7 +3811,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1, | ||
3809 | /* Remove the CPU from the list. */ | 3811 | /* Remove the CPU from the list. */ |
3810 | *lastp = p->next_cpu; | 3812 | *lastp = p->next_cpu; |
3811 | cpu_list_unlock(); | 3813 | cpu_list_unlock(); |
3812 | - TaskState *ts = ((CPUState *)cpu_env)->opaque; | 3814 | + ts = ((CPUState *)cpu_env)->opaque; |
3813 | if (ts->child_tidptr) { | 3815 | if (ts->child_tidptr) { |
3814 | put_user_u32(0, ts->child_tidptr); | 3816 | put_user_u32(0, ts->child_tidptr); |
3815 | sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, | 3817 | sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, |