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-06  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.
---
 gdb/gdbserver/linux-low.c    | 16 +++--------
 gdb/gdbserver/lynx-low.c     |  2 +-
 gdb/gdbserver/remote-utils.c |  2 +-
 gdb/gdbserver/server.c       | 66 ++++++++++++++++++++++++--------------------
 gdb/gdbserver/spu-low.c      |  9 ++++--
 gdb/gdbserver/target.c       |  8 ++----
 gdb/gdbserver/target.h       |  2 +-
 7 files changed, 52 insertions(+), 53 deletions(-)

gdbserver: don't pick a random thread if the current thread dies

Commit Message

Comments

Patch