[PATCH] PR remote/14786 - thread list returned by qfThreadInfo clobbered by g packet

[Pedro Alves <palves at redhat dot com>]

Richard writes (edited):

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The function remote_threads_info in remote.c gets a list of threads
from the target and is supposed call remote_notice_new_inferior for
each of them.  However, while processing one the list of threads (in
rs->buf) can be clobbered by other calls to putpkt and getpk that use
rs->buf.  Some calls (e.g. qAttached) only change the first few bytes
of rs->buf but under some cases send_g_packet can be called and this
overwrites many bytes.

Example:

(gdb) info threads
Sending packet: $qfThreadInfo#bb...Ack
Packet received: m19b1e00,19b1ac0l

Breakpoint 33, remote_threads_info (ops=0xdb41a0 <extended_remote_ops>) at
../../gdb/remote.c:2775
(top-gdb) p bufp
$29 = 0xeabb00 "m19b1e00,19b1ac0l"
(top-gdb) x/s bufp
0xeabb00:    "m19b1e00,19b1ac0l"
(top-gdb) n
(top-gdb) n
(top-gdb) p bufp
$30 = 0xeabb08 ",19b1ac0l"

gdb is about to process the first thread, 19b1e00, and then when done
should work on 19b1ac0.  However, it asks for the registers for
19b1e00 and when done bufp is no longer valid because it has been
overwritten with the reply to the 'g' packet.

Sending packet: $g#67...Ack
Packet received: 0000000001a5adf000000000000000000000000 <truncated>

now back in remote_threads_info:

(top-gdb) p bufp
$31 = 0xeabb08 "01a5adf", '0' <repeats 193 times>...
(top-gdb) x/s bufp
0xeabb08:    "01a5adf", '0' <repeats 193 times>...

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

GDB's own gdbserver, and I think all stubs I've seen before, only ever
sends one thread at a time per qfThreadInfo/qsThreadInfo, which
explains how this was missed.

This is in essense Richard's patch, converted to use a cleanup,
with a comment added, and formatting fixed.

Regression tested on x86_64 Fedora 17, with gdbserver, with the xml
thread list packet disabled so qfThreadInfo is used.

Applied.

2013-01-14  Richard Sharman  <richard_sharman@mitel.com>
	    Pedro Alves  <palves@redhat.com>

	PR remote/14786

	* remote.c (remote_threads_info): Make a copy of the reply from
	qfThreadInfo and use that instead of rs->buf.
---
 gdb/remote.c |   13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 59b2eb6..7ea9597 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -2759,6 +2759,15 @@ remote_threads_info (struct target_ops *ops)
       bufp = rs->buf;
       if (bufp[0] != '\0')		/* q packet recognized */
 	{
+	  struct cleanup *old_chain;
+	  char *saved_reply;
+
+	  /* remote_notice_new_inferior (in the loop below) may make
+	     new RSP calls, which clobber rs->buf.  Work with a
+	     copy.  */
+	  bufp = saved_reply = xstrdup (rs->buf);
+	  old_chain = make_cleanup (free_current_contents, &saved_reply);
+
 	  while (*bufp++ == 'm')	/* reply contains one or more TID */
 	    {
 	      do
@@ -2776,10 +2785,12 @@ remote_threads_info (struct target_ops *ops)
 		    }
 		}
 	      while (*bufp++ == ',');	/* comma-separated list */
+	      free_current_contents (&saved_reply);
 	      putpkt ("qsThreadInfo");
 	      getpkt (&rs->buf, &rs->buf_size, 0);
-	      bufp = rs->buf;
+	      bufp = saved_reply = xstrdup (rs->buf);
 	    }
+	  do_cleanups (old_chain);
 	  return;	/* done */
 	}
     }