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

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

[Pedro Alves]

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(-)

diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 792c178..04ec50b 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -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;
     }
 }
 
diff --git a/gdb/gdbserver/lynx-low.c b/gdb/gdbserver/lynx-low.c
index 5cf03be..ee4d0e8 100644
--- a/gdb/gdbserver/lynx-low.c
+++ b/gdb/gdbserver/lynx-low.c
@@ -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);
 }
diff --git a/gdb/gdbserver/remote-utils.c b/gdb/gdbserver/remote-utils.c
index bb31456..05563be 100644
--- a/gdb/gdbserver/remote-utils.c
+++ b/gdb/gdbserver/remote-utils.c
@@ -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))
diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c
index f15b7be..cfe97fc 100644
--- a/gdb/gdbserver/server.c
+++ b/gdb/gdbserver/server.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));
 
@@ -3902,14 +3901,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;
@@ -3941,9 +3939,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':
@@ -3954,10 +3956,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':
diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c
index cbee960..2ca9159 100644
--- a/gdb/gdbserver/spu-low.c
+++ b/gdb/gdbserver/spu-low.c
@@ -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 = {
diff --git a/gdb/gdbserver/target.c b/gdb/gdbserver/target.c
index 14999e6..8fcfe9b 100644
--- a/gdb/gdbserver/target.c
+++ b/gdb/gdbserver/target.c
@@ -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
diff --git a/gdb/gdbserver/target.h b/gdb/gdbserver/target.h
index fefd8d1..3e3b80f 100644
--- a/gdb/gdbserver/target.h
+++ b/gdb/gdbserver/target.h
@@ -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);
 
-- 
1.9.3

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Pedro Alves]

On 08/06/2015 03:44 PM, Pedro Alves wrote:
> 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.

I pushed this in.

Thanks,
Pedro Alves

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Ulrich Weigand]

Pedro Alves wrote:

> 	* spu-low.c (spu_resume, spu_request_interrupt): Use the first
> 	thread's lwp instead of the current thread's.

>  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);
>  }

This doesn't compile due to:

gdbserver/spu-low.c: In function 'spu_request_interrupt':
gdbserver/spu-low.c:639: error: incompatible type for argument 1 of 'ptid_get_lwp'

When adding the obvious fix ("ptid_of (thr)"), it does compile, but now
gdbserver crashes as soon as GDB attaches to it:

Program received signal SIGSEGV, Segmentation fault.                                                                                                                               
0x10008274 in read_ptid (buf=0x1005045a "0", obuf=0x0) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/remote-utils.c:578
578       pid = ptid_get_pid (current_ptid);
(gdb) bt
#0  0x10008274 in read_ptid (buf=0x1005045a "0", obuf=0x0) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/remote-utils.c:578
#1  0x1001045c in process_serial_event (err=<value optimized out>, client_data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/server.c:3864
#2  handle_serial_event (err=<value optimized out>, client_data=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/server.c:4206
#3  0x100176c0 in handle_file_event (event_file_desc=<value optimized out>) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/event-loop.c:429
#4  0x10017e90 in process_event () at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/event-loop.c:184
#5  start_event_loop () at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/event-loop.c:547
#6  0x1000ce3c in captured_main (argc=3, argv=0x1) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/server.c:3559
#7  0x1000d370 in main (argc=3, argv=0xffbbf994) at /home/uweigand/fsf/binutils-gdb/gdb/gdbserver/server.c:3634
(gdb) list
573       /* No multi-process.  Just a tid.  */
574       tid = hex_or_minus_one (p, &pp);
575
576       /* Since the stub is not sending a process id, then default to
577          what's in the current inferior.  */
578       pid = ptid_get_pid (current_ptid);
579
580       if (obuf)
581         *obuf = pp;
582       return ptid_build (pid, tid, 0);


Looks like current_thread is NULL at this point.  Since this is generic
code, I'm not quite sure if this is a SPU-specific problem or not ...

Do you think this is related to your change?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Pedro Alves]

On 08/24/2015 03:59 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
> 
>> 	* spu-low.c (spu_resume, spu_request_interrupt): Use the first
>> 	thread's lwp instead of the current thread's.
> 
>>  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);
>>  }
> 
> This doesn't compile due to:
> 
> gdbserver/spu-low.c: In function 'spu_request_interrupt':
> gdbserver/spu-low.c:639: error: incompatible type for argument 1 of 'ptid_get_lwp'
> 
> When adding the obvious fix ("ptid_of (thr)"), it does compile, but now
> gdbserver crashes as soon as GDB attaches to it:

I guess lwpid_of would be a little better even.

> Looks like current_thread is NULL at this point.  Since this is generic
> code, I'm not quite sure if this is a SPU-specific problem or not ...
> 
> Do you think this is related to your change?

Sounds likely.  I can trigger the same on GNU/Linux if I force-disable
the multi-process extensions:

diff --git c/gdb/gdbserver/linux-low.c w/gdb/gdbserver/linux-low.c
index 2bc91c2..44c0feb 100644
--- c/gdb/gdbserver/linux-low.c
+++ w/gdb/gdbserver/linux-low.c
@@ -5785,7 +5785,7 @@ linux_start_non_stop (int nonstop)
 static int
 linux_supports_multi_process (void)
 {
-  return 1;
+  return 0;
 }

 /* Check if fork events are supported.  */


Let me take a closer look.

Thanks,
Pedro Alves

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Pedro Alves]

On 08/24/2015 04:30 PM, Pedro Alves wrote:

> Let me take a closer look.

I'm testing a patch.

Thanks,
Pedro Alves

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Pedro Alves]

On 08/24/2015 04:30 PM, Pedro Alves wrote:

> I guess lwpid_of would be a little better even.

I went ahead a pushed this bit in, like below.  Build-tested on
the gcc110 ppc64 machine on the GCC compile farm.

The fix for the crash will follow.

From a8c6d4fcd6b2a30c2b5b87d656ce035dcf8b0035 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Mon, 24 Aug 2015 17:58:22 +0100
Subject: [PATCH] Fix gdbserver SPU build

Ref: https://sourceware.org/ml/gdb-patches/2015-08/msg00675.html

 gdbserver/spu-low.c: In function 'spu_request_interrupt':
 gdbserver/spu-low.c:639: error: incompatible type for argument 1 of 'ptid_get_lwp'

gdb/gdbserver/ChangeLog:
2015-08-24  Pedro Alves  <palves@redhat.com>

	* spu-low.c (spu_request_interrupt): Use lwpid_of instead of
	ptid_get_lwp.
---
 gdb/gdbserver/ChangeLog | 5 +++++
 gdb/gdbserver/spu-low.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index cd5e046..12d8bb3 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2015-08-24  Pedro Alves  <palves@redhat.com>
+
+	* spu-low.c (spu_request_interrupt): Use lwpid_of instead of
+	ptid_get_lwp.
+
 2015-08-21  Pedro Alves  <palves@redhat.com>

 	* ax.c (gdb_eval_agent_expr): Return expr_eval_unhandled_opcode
diff --git a/gdb/gdbserver/spu-low.c b/gdb/gdbserver/spu-low.c
index 2ca9159..a110a0e 100644
--- a/gdb/gdbserver/spu-low.c
+++ b/gdb/gdbserver/spu-low.c
@@ -636,7 +636,7 @@ spu_request_interrupt (void)
 {
   struct thread_info *thr = get_first_thread ();

-  syscall (SYS_tkill, ptid_get_lwp (thr), SIGINT);
+  syscall (SYS_tkill, lwpid_of (thr), SIGINT);
 }

 static struct target_ops spu_target_ops = {
-- 
1.9.3

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Ulrich Weigand]

Pedro Alves wrote:
> On 08/24/2015 04:30 PM, Pedro Alves wrote:
> 
> > I guess lwpid_of would be a little better even.
> 
> I went ahead a pushed this bit in, like below.  Build-tested on
> the gcc110 ppc64 machine on the GCC compile farm.
> 
> The fix for the crash will follow.

Thanks!

I'll go ahead and run a test on Cell once the fix is in ...

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Pedro Alves]

On 08/24/2015 06:08 PM, Ulrich Weigand wrote:

> I'll go ahead and run a test on Cell once the fix is in ...

I've posted the fix now:

 https://sourceware.org/ml/gdb-patches/2015-08/msg00690.html

I've pushed that to the users/palves/gdbserver-crash-without-multiprocess
branch, but if you prefer, I'll push it to master instead.

Thanks,
Pedro Alves

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Ulrich Weigand]

Pedro Alves wrote:

> I've posted the fix now:
> 
>  https://sourceware.org/ml/gdb-patches/2015-08/msg00690.html
> 
> I've pushed that to the users/palves/gdbserver-crash-without-multiprocess
> branch, but if you prefer, I'll push it to master instead.

I've tested the branch on Cell/B.E. SPU, both native and via gdbserver.
Everything looks good again.  Please push to master ...

Thanks,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

Re: [PATCH] gdbserver: don't pick a random thread if the current thread dies

[Pedro Alves]

On 08/24/2015 07:39 PM, Ulrich Weigand wrote:
> Pedro Alves wrote:
> 
>> I've posted the fix now:
>>
>>  https://sourceware.org/ml/gdb-patches/2015-08/msg00690.html
>>
>> I've pushed that to the users/palves/gdbserver-crash-without-multiprocess
>> branch, but if you prefer, I'll push it to master instead.
> 
> I've tested the branch on Cell/B.E. SPU, both native and via gdbserver.
> Everything looks good again.  Please push to master ...

Thanks much for testing.  (And sorry for the breakage, of course!)

Now pushed.

-- 
Pedro Alves