[patch/gdbserver] Avoid a nasty little problem in the remote protocol T packet

[patch/gdbserver] Avoid a nasty little problem in the remote protocol T packet

[Daniel Jacobowitz]

The documentation for the T packet is not especially clear on when thread:
may be omitted.  Gdbserver has been assuming that if the thread was omitted,
then the same thread used for the previous T packet will be used again. 
This is, unfortunately, wrong; if no thread is supplied by number,
remote_wait returns inferior_ptid.  It's not practical to track everything
that GDB might do which changes inferior_ptid, since not all of them are
communicated to the remote target; so the only thing we can do is always
report the thread.

Without this, gdb could misinterpret which thread has stopped.  It then
may omit sending an Hg packet, request registers, and get the registers for
a different thread than it wants.  This results in info thread changing
the stack pointer.  schedlock.exp catches this - it's the "step without lock
changes thread" test.

Will commit in a bit.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

2004-02-28  Daniel Jacobowitz  <drow@mvista.com>

	* remote-utils.c (prepare_resume_reply): Always supply "thread:".

Index: gdb/gdbserver/remote-utils.c
===================================================================
RCS file: /big/fsf/rsync/src-cvs/src/gdb/gdbserver/remote-utils.c,v
retrieving revision 1.17
diff -u -p -r1.17 remote-utils.c
--- gdb/gdbserver/remote-utils.c	5 Jun 2003 14:26:58 -0000	1.17
+++ gdb/gdbserver/remote-utils.c	27 Feb 2004 20:54:11 -0000
@@ -609,7 +629,11 @@ prepare_resume_reply (char *buf, char st
 	  thread_from_wait = ((struct inferior_list_entry *)current_inferior)->id;
 	  if (debug_threads)
 	    fprintf (stderr, "Writing resume reply for %d\n\n", thread_from_wait);
-	  if (old_thread_from_wait != thread_from_wait)
+	  /* This if (1) ought to be unnecessary.  But remote_wait in GDB
+	     will claim this event belongs to inferior_ptid if we do not
+	     specify a thread, and there's no way for gdbserver to know
+	     what inferior_ptid is.  */
+	  if (1 || old_thread_from_wait != thread_from_wait)
 	    {
 	      general_thread = thread_from_wait;
 	      sprintf (buf, "thread:%x;", thread_from_wait);

Re: [patch/gdbserver] Avoid a nasty little problem in the remote protocol T packet

[Daniel Jacobowitz]

On Sat, Feb 28, 2004 at 01:22:03PM -0500, Daniel Jacobowitz wrote:
> The documentation for the T packet is not especially clear on when thread:
> may be omitted.  Gdbserver has been assuming that if the thread was omitted,
> then the same thread used for the previous T packet will be used again. 
> This is, unfortunately, wrong; if no thread is supplied by number,
> remote_wait returns inferior_ptid.  It's not practical to track everything
> that GDB might do which changes inferior_ptid, since not all of them are
> communicated to the remote target; so the only thing we can do is always
> report the thread.
> 
> Without this, gdb could misinterpret which thread has stopped.  It then
> may omit sending an Hg packet, request registers, and get the registers for
> a different thread than it wants.  This results in info thread changing
> the stack pointer.  schedlock.exp catches this - it's the "step without lock
> changes thread" test.
> 
> Will commit in a bit.

> 2004-02-28  Daniel Jacobowitz  <drow@mvista.com>
> 
> 	* remote-utils.c (prepare_resume_reply): Always supply "thread:".

I've committed this to HEAD.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer