gdbserver: don't pick a random thread if the current thread dies
In all-stop mode, if the current thread disappears while stopping all threads, gdbserver calls set_desired_thread(0) ['0' means "I want the continue thread"] which just picks the first thread in the list. This looks like a dangerous thing to do. GDBserver continues processing whatever it was doing, but to the wrong thread. If debugging more than one process, we may even pick the wrong process. Instead, GDBserver should detect the situation and bail out of whatever is was doing. The backends used to pay attention to the set 'cont_thread' (the Hc thread, used in the old way to resume threads, before vCont), but all such 'cont_thread' checks have been eliminated meanwhile. The remaining implicit dependencies that I found on there being a selected thread in the backends are in the Ctrl-C handling, which some backends use as thread to send a signal to. Even that seems to me to be better handled by always using the first thread in the list or by using the signal_pid PID. In order to make this a systematic approach, I'm making set_desired_thread never fallback to a random thread, and instead end up with current_thread == NULL, like already done in non-stop mode. Then I updated all callers to handle the situation. I stumbled on this while fixing other bugs exposed by gdb.threads/fork-plus-threads.exp test. The problems I saw were fixed in a different way, but in any case, I think the potential for problems is more or less obvious, and the resulting code looks a bit less magical to me. Tested on x86-64 Fedora 20, w/ native-extended-gdbserver board. gdb/gdbserver/ChangeLog: 2015-08-21 Pedro Alves <palves@redhat.com> * linux-low.c (wait_for_sigstop): Always switch to no thread selected if the previously current thread dies. * lynx-low.c (lynx_request_interrupt): Use the first thread's process instead of the current thread's. * remote-utils.c (input_interrupt): Don't check if there's no current thread. * server.c (gdb_read_memory, gdb_write_memory): If setting the current thread to the general thread fails, error out. (handle_qxfer_auxv, handle_qxfer_libraries) (handle_qxfer_libraries_svr4, handle_qxfer_siginfo) (handle_qxfer_spu, handle_qxfer_statictrace, handle_qxfer_fdpic) (handle_query): Check if there's a thread selected instead of checking whether there's any thread in the thread list. (handle_qxfer_threads, handle_qxfer_btrace) (handle_qxfer_btrace_conf): Don't error out early if there's no thread in the thread list. (handle_v_cont, myresume): Don't set the current thread to the continue thread. (process_serial_event) <Hg handling>: Also set thread_id if the previous general thread is still alive. (process_serial_event) <g/G handling>: If setting the current thread to the general thread fails, error out. * spu-low.c (spu_resume, spu_request_interrupt): Use the first thread's lwp instead of the current thread's. * target.c (set_desired_thread): If the desired thread was not found, leave the current thread pointing to NULL. Return an int (boolean) indicating success. * target.h (set_desired_thread): Change return type to int.
This commit is contained in:
Pedro Alves
parent
2d7711a367
commit
f0db101d98
|
|
@ -1,3 +1,34 @@
|
|||
2015-08-21 Pedro Alves <palves@redhat.com>
|
||||
|
||||
* linux-low.c (wait_for_sigstop): Always switch to no thread
|
||||
selected if the previously current thread dies.
|
||||
* lynx-low.c (lynx_request_interrupt): Use the first thread's
|
||||
process instead of the current thread's.
|
||||
* remote-utils.c (input_interrupt): Don't check if there's no
|
||||
current thread.
|
||||
* server.c (gdb_read_memory, gdb_write_memory): If setting the
|
||||
current thread to the general thread fails, error out.
|
||||
(handle_qxfer_auxv, handle_qxfer_libraries)
|
||||
(handle_qxfer_libraries_svr4, handle_qxfer_siginfo)
|
||||
(handle_qxfer_spu, handle_qxfer_statictrace, handle_qxfer_fdpic)
|
||||
(handle_query): Check if there's a thread selected instead of
|
||||
checking whether there's any thread in the thread list.
|
||||
(handle_qxfer_threads, handle_qxfer_btrace)
|
||||
(handle_qxfer_btrace_conf): Don't error out early if there's no
|
||||
thread in the thread list.
|
||||
(handle_v_cont, myresume): Don't set the current thread to the
|
||||
continue thread.
|
||||
(process_serial_event) <Hg handling>: Also set thread_id if the
|
||||
previous general thread is still alive.
|
||||
(process_serial_event) <g/G handling>: If setting the current
|
||||
thread to the general thread fails, error out.
|
||||
* spu-low.c (spu_resume, spu_request_interrupt): Use the first
|
||||
thread's lwp instead of the current thread's.
|
||||
* target.c (set_desired_thread): If the desired thread was not
|
||||
found, leave the current thread pointing to NULL. Return an int
|
||||
(boolean) indicating success.
|
||||
* target.h (set_desired_thread): Change return type to int.
|
||||
|
||||
2015-08-20 Max Filippov <jcmvbkbc@gmail.com>
|
||||
|
||||
* configure.srv (xtensa*-*-linux*): Add srv_linux_thread_db=yes.
|
||||
|
|
|
|||
|
|
@ -3617,18 +3617,10 @@ wait_for_sigstop (void)
|
|||
if (debug_threads)
|
||||
debug_printf ("Previously current thread died.\n");
|
||||
|
||||
if (non_stop)
|
||||
{
|
||||
/* We can't change the current inferior behind GDB's back,
|
||||
otherwise, a subsequent command may apply to the wrong
|
||||
process. */
|
||||
current_thread = NULL;
|
||||
}
|
||||
else
|
||||
{
|
||||
/* Set a valid thread as current. */
|
||||
set_desired_thread (0);
|
||||
}
|
||||
/* We can't change the current inferior behind GDB's back,
|
||||
otherwise, a subsequent command may apply to the wrong
|
||||
process. */
|
||||
current_thread = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -713,7 +713,7 @@ lynx_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
|
|||
static void
|
||||
lynx_request_interrupt (void)
|
||||
{
|
||||
ptid_t inferior_ptid = thread_to_gdb_id (current_thread);
|
||||
ptid_t inferior_ptid = thread_to_gdb_id (get_first_thread ());
|
||||
|
||||
kill (lynx_ptid_get_pid (inferior_ptid), SIGINT);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -747,7 +747,7 @@ input_interrupt (int unused)
|
|||
fprintf (stderr, "client connection closed\n");
|
||||
return;
|
||||
}
|
||||
else if (cc != 1 || c != '\003' || current_thread == NULL)
|
||||
else if (cc != 1 || c != '\003')
|
||||
{
|
||||
fprintf (stderr, "input_interrupt, count = %d c = %d ", cc, c);
|
||||
if (isprint (c))
|
||||
|
|
|
|||
|
|
@ -816,7 +816,10 @@ gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
|
|||
res = prepare_to_access_memory ();
|
||||
if (res == 0)
|
||||
{
|
||||
res = read_inferior_memory (memaddr, myaddr, len);
|
||||
if (set_desired_thread (1))
|
||||
res = read_inferior_memory (memaddr, myaddr, len);
|
||||
else
|
||||
res = 1;
|
||||
done_accessing_memory ();
|
||||
|
||||
return res == 0 ? len : -1;
|
||||
|
|
@ -840,7 +843,10 @@ gdb_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
|
|||
ret = prepare_to_access_memory ();
|
||||
if (ret == 0)
|
||||
{
|
||||
ret = write_inferior_memory (memaddr, myaddr, len);
|
||||
if (set_desired_thread (1))
|
||||
ret = write_inferior_memory (memaddr, myaddr, len);
|
||||
else
|
||||
ret = EIO;
|
||||
done_accessing_memory ();
|
||||
}
|
||||
return ret;
|
||||
|
|
@ -1171,7 +1177,7 @@ handle_qxfer_auxv (const char *annex,
|
|||
if (the_target->read_auxv == NULL || writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (annex[0] != '\0' || !target_running ())
|
||||
if (annex[0] != '\0' || current_thread == NULL)
|
||||
return -1;
|
||||
|
||||
return (*the_target->read_auxv) (offset, readbuf, len);
|
||||
|
|
@ -1316,7 +1322,7 @@ handle_qxfer_libraries (const char *annex,
|
|||
if (writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (annex[0] != '\0' || !target_running ())
|
||||
if (annex[0] != '\0' || current_thread == NULL)
|
||||
return -1;
|
||||
|
||||
total_len = 64;
|
||||
|
|
@ -1360,7 +1366,7 @@ handle_qxfer_libraries_svr4 (const char *annex,
|
|||
if (writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (!target_running () || the_target->qxfer_libraries_svr4 == NULL)
|
||||
if (current_thread == NULL || the_target->qxfer_libraries_svr4 == NULL)
|
||||
return -1;
|
||||
|
||||
return the_target->qxfer_libraries_svr4 (annex, readbuf, writebuf, offset, len);
|
||||
|
|
@ -1389,7 +1395,7 @@ handle_qxfer_siginfo (const char *annex,
|
|||
if (the_target->qxfer_siginfo == NULL)
|
||||
return -2;
|
||||
|
||||
if (annex[0] != '\0' || !target_running ())
|
||||
if (annex[0] != '\0' || current_thread == NULL)
|
||||
return -1;
|
||||
|
||||
return (*the_target->qxfer_siginfo) (annex, readbuf, writebuf, offset, len);
|
||||
|
|
@ -1405,7 +1411,7 @@ handle_qxfer_spu (const char *annex,
|
|||
if (the_target->qxfer_spu == NULL)
|
||||
return -2;
|
||||
|
||||
if (!target_running ())
|
||||
if (current_thread == NULL)
|
||||
return -1;
|
||||
|
||||
return (*the_target->qxfer_spu) (annex, readbuf, writebuf, offset, len);
|
||||
|
|
@ -1423,7 +1429,7 @@ handle_qxfer_statictrace (const char *annex,
|
|||
if (writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (annex[0] != '\0' || !target_running () || current_traceframe == -1)
|
||||
if (annex[0] != '\0' || current_thread == NULL || current_traceframe == -1)
|
||||
return -1;
|
||||
|
||||
if (traceframe_read_sdata (current_traceframe, offset,
|
||||
|
|
@ -1486,7 +1492,7 @@ handle_qxfer_threads (const char *annex,
|
|||
if (writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (!target_running () || annex[0] != '\0')
|
||||
if (annex[0] != '\0')
|
||||
return -1;
|
||||
|
||||
if (offset == 0)
|
||||
|
|
@ -1582,7 +1588,7 @@ handle_qxfer_fdpic (const char *annex, gdb_byte *readbuf,
|
|||
if (the_target->read_loadmap == NULL)
|
||||
return -2;
|
||||
|
||||
if (!target_running ())
|
||||
if (current_thread == NULL)
|
||||
return -1;
|
||||
|
||||
return (*the_target->read_loadmap) (annex, offset, readbuf, len);
|
||||
|
|
@ -1602,9 +1608,6 @@ handle_qxfer_btrace (const char *annex,
|
|||
if (the_target->read_btrace == NULL || writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (!target_running ())
|
||||
return -1;
|
||||
|
||||
if (ptid_equal (general_thread, null_ptid)
|
||||
|| ptid_equal (general_thread, minus_one_ptid))
|
||||
{
|
||||
|
|
@ -1676,7 +1679,7 @@ handle_qxfer_btrace_conf (const char *annex,
|
|||
if (the_target->read_btrace_conf == NULL || writebuf != NULL)
|
||||
return -2;
|
||||
|
||||
if (annex[0] != '\0' || !target_running ())
|
||||
if (annex[0] != '\0')
|
||||
return -1;
|
||||
|
||||
if (ptid_equal (general_thread, null_ptid)
|
||||
|
|
@ -1978,7 +1981,7 @@ handle_query (char *own_buf, int packet_len, int *new_packet_len_p)
|
|||
if (target_supports_tracepoints ())
|
||||
tracepoint_look_up_symbols ();
|
||||
|
||||
if (target_running () && the_target->look_up_symbols != NULL)
|
||||
if (current_thread != NULL && the_target->look_up_symbols != NULL)
|
||||
(*the_target->look_up_symbols) ();
|
||||
|
||||
strcpy (own_buf, "OK");
|
||||
|
|
@ -2575,8 +2578,6 @@ handle_v_cont (char *own_buf)
|
|||
if (i < n)
|
||||
resume_info[i] = default_action;
|
||||
|
||||
set_desired_thread (0);
|
||||
|
||||
resume (resume_info, n);
|
||||
free (resume_info);
|
||||
return;
|
||||
|
|
@ -2878,8 +2879,6 @@ myresume (char *own_buf, int step, int sig)
|
|||
int n = 0;
|
||||
int valid_cont_thread;
|
||||
|
||||
set_desired_thread (0);
|
||||
|
||||
valid_cont_thread = (!ptid_equal (cont_thread, null_ptid)
|
||||
&& !ptid_equal (cont_thread, minus_one_ptid));
|
||||
|
||||
|
|
@ -3908,14 +3907,13 @@ process_serial_event (void)
|
|||
(struct thread_info *) find_inferior_id (&all_threads,
|
||||
general_thread);
|
||||
if (thread == NULL)
|
||||
{
|
||||
thread = get_first_thread ();
|
||||
thread_id = thread->entry.id;
|
||||
}
|
||||
thread = get_first_thread ();
|
||||
thread_id = thread->entry.id;
|
||||
}
|
||||
|
||||
general_thread = thread_id;
|
||||
set_desired_thread (1);
|
||||
gdb_assert (current_thread != NULL);
|
||||
}
|
||||
else if (own_buf[1] == 'c')
|
||||
cont_thread = thread_id;
|
||||
|
|
@ -3947,9 +3945,13 @@ process_serial_event (void)
|
|||
{
|
||||
struct regcache *regcache;
|
||||
|
||||
set_desired_thread (1);
|
||||
regcache = get_thread_regcache (current_thread, 1);
|
||||
registers_to_string (regcache, own_buf);
|
||||
if (!set_desired_thread (1))
|
||||
write_enn (own_buf);
|
||||
else
|
||||
{
|
||||
regcache = get_thread_regcache (current_thread, 1);
|
||||
registers_to_string (regcache, own_buf);
|
||||
}
|
||||
}
|
||||
break;
|
||||
case 'G':
|
||||
|
|
@ -3960,10 +3962,14 @@ process_serial_event (void)
|
|||
{
|
||||
struct regcache *regcache;
|
||||
|
||||
set_desired_thread (1);
|
||||
regcache = get_thread_regcache (current_thread, 1);
|
||||
registers_from_string (regcache, &own_buf[1]);
|
||||
write_ok (own_buf);
|
||||
if (!set_desired_thread (1))
|
||||
write_enn (own_buf);
|
||||
else
|
||||
{
|
||||
regcache = get_thread_regcache (current_thread, 1);
|
||||
registers_from_string (regcache, &own_buf[1]);
|
||||
write_ok (own_buf);
|
||||
}
|
||||
}
|
||||
break;
|
||||
case 'm':
|
||||
|
|
|
|||
|
|
@ -383,11 +383,12 @@ spu_thread_alive (ptid_t ptid)
|
|||
static void
|
||||
spu_resume (struct thread_resume *resume_info, size_t n)
|
||||
{
|
||||
struct thread_info *thr = get_first_thread ();
|
||||
size_t i;
|
||||
|
||||
for (i = 0; i < n; i++)
|
||||
if (ptid_equal (resume_info[i].thread, minus_one_ptid)
|
||||
|| ptid_equal (resume_info[i].thread, current_ptid))
|
||||
|| ptid_equal (resume_info[i].thread, ptid_of (thr)))
|
||||
break;
|
||||
|
||||
if (i == n)
|
||||
|
|
@ -401,7 +402,7 @@ spu_resume (struct thread_resume *resume_info, size_t n)
|
|||
regcache_invalidate ();
|
||||
|
||||
errno = 0;
|
||||
ptrace (PTRACE_CONT, ptid_get_lwp (current_ptid), 0, resume_info[i].sig);
|
||||
ptrace (PTRACE_CONT, ptid_get_lwp (ptid_of (thr)), 0, resume_info[i].sig);
|
||||
if (errno)
|
||||
perror_with_name ("ptrace");
|
||||
}
|
||||
|
|
@ -633,7 +634,9 @@ spu_look_up_symbols (void)
|
|||
static void
|
||||
spu_request_interrupt (void)
|
||||
{
|
||||
syscall (SYS_tkill, ptid_get_lwp (current_ptid), SIGINT);
|
||||
struct thread_info *thr = get_first_thread ();
|
||||
|
||||
syscall (SYS_tkill, ptid_get_lwp (thr), SIGINT);
|
||||
}
|
||||
|
||||
static struct target_ops spu_target_ops = {
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@
|
|||
|
||||
struct target_ops *the_target;
|
||||
|
||||
void
|
||||
int
|
||||
set_desired_thread (int use_general)
|
||||
{
|
||||
struct thread_info *found;
|
||||
|
|
@ -33,10 +33,8 @@ set_desired_thread (int use_general)
|
|||
else
|
||||
found = find_thread_ptid (cont_thread);
|
||||
|
||||
if (found == NULL)
|
||||
current_thread = get_first_thread ();
|
||||
else
|
||||
current_thread = found;
|
||||
current_thread = found;
|
||||
return (current_thread != NULL);
|
||||
}
|
||||
|
||||
int
|
||||
|
|
|
|||
|
|
@ -645,7 +645,7 @@ int read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len);
|
|||
int write_inferior_memory (CORE_ADDR memaddr, const unsigned char *myaddr,
|
||||
int len);
|
||||
|
||||
void set_desired_thread (int id);
|
||||
int set_desired_thread (int id);
|
||||
|
||||
const char *target_pid_to_str (ptid_t);
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue