Re: [RFA] Restore leading zeros in remote_thread_alive

[Pedro Alves <pedro at codesourcery dot com>]

A Wednesday 22 October 2008 14:18:22, Pedro Alves escreveu:
> On Wednesday 22 October 2008 02:02:19, Michael Snyder wrote:
> > --- remote.cÂÂÂÂ17 Oct 2008 19:43:47 -0000ÂÂÂÂÂÂ1.321
> > +++ remote.cÂÂÂÂ22 Oct 2008 01:00:16 -0000
> > @@ -1433,15 +1433,15 @@ write_ptid (char *buf, const char *endbu
> > Â Â Â{
> > Â Â Â Âpid = ptid_get_pid (ptid);
> > Â Â Â Âif (pid < 0)
> > -ÂÂÂÂÂÂÂbuf += xsnprintf (buf, endbuf - buf, "p-%x.", -pid);
> > +ÂÂÂÂÂÂÂbuf += xsnprintf (buf, endbuf - buf, "p-%08x.", -pid);
> > Â Â Â Âelse
> > -ÂÂÂÂÂÂÂbuf += xsnprintf (buf, endbuf - buf, "p%x.", pid);
> > +ÂÂÂÂÂÂÂbuf += xsnprintf (buf, endbuf - buf, "p%08x.", pid);
> > Â Â Â}
> > Â Âtid = ptid_get_tid (ptid);
> > Â Âif (tid < 0)
> > - Â Âbuf += xsnprintf (buf, endbuf - buf, "-%x", -tid);
> > + Â Âbuf += xsnprintf (buf, endbuf - buf, "-%x08", -tid);
> > Â Âelse
> > - Â Âbuf += xsnprintf (buf, endbuf - buf, "%x", tid);
> > + Â Âbuf += xsnprintf (buf, endbuf - buf, "%x08", tid);
> > Â
> > Â Âreturn buf;
> 
> ( Now that I've slept a bit, :-) ) I've gone through an old version of the code
> and documentation looking for places we use write_ptid now that used to
> output %08x vs places we used to output "-1" or %x.  Indeed, I can only see
> that in remote_thread_alive.  The change above can possibly lead to other
> stubs out there (with similar assumptions to yours) not parsing e.g., Hg-000000001
> correctly, because they were expecting Hg-1, or e.g., misparsing vCont, because
> used to be "vCont;c:%x".  Also, we don't really need to use %08 when multi-process
> is in effect.  There should be no multi-process stubs around that depend
> on leading 0's.
> 
> I guess this means we get to add a new parameter to write_ptid
> (leading_zeros ?), and pass 1 where needed...  Give me a sec to cook
> something up.
> 

Could you try this out please?  It works here against gdbserver
single|multi-process, and sends a `T000000tid' (tid alive) packet when
multi-process isn't in effect.

-- 
Pedro Alves

---
 gdb/remote.c |   62 ++++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 13 deletions(-)

Index: src/gdb/remote.c
===================================================================
--- src.orig/gdb/remote.c	2008-10-22 14:25:12.000000000 +0100
+++ src/gdb/remote.c	2008-10-22 14:58:28.000000000 +0100
@@ -209,6 +209,8 @@ static void show_remote_protocol_packet_
 					     const char *value);
 
 static char *write_ptid (char *buf, const char *endbuf, ptid_t ptid);
+static char *write_ptid_leading_zeros (char *buf, const char *endbuf,
+				       ptid_t ptid);
 static ptid_t read_ptid (char *buf, char **obuf);
 
 static void remote_query_supported (void);
@@ -1296,7 +1298,10 @@ remote_thread_alive (ptid_t ptid)
   endp = rs->buf + get_remote_packet_size ();
 
   *p++ = 'T';
-  write_ptid (p, endp, ptid);
+
+  /* Make sure we output an 8-char wide thread id (if the
+     multi-process extensions aren't in effect).  */
+  write_ptid_leading_zeros (p, endp, ptid);
 
   putpkt (rs->buf);
   getpkt (&rs->buf, &rs->buf_size, 0);
@@ -1420,32 +1425,63 @@ static int remote_newthread_step (thread
 
 
 /* Write a PTID to BUF.  ENDBUF points to one-passed-the-end of the
-   buffer we're allowed to write to.  Returns
-   BUF+CHARACTERS_WRITTEN.  */
+   buffer we're allowed to write to.  If LEADING_ZEROS, and the
+   multi-process extensions are in effect, always output an 8-char
+   long tid, padding with leading 0's if required to do so.
+   Returns BUF+CHARACTERS_WRITTEN.  */
 
 static char *
-write_ptid (char *buf, const char *endbuf, ptid_t ptid)
+write_ptid_1 (char *buf, const char *endbuf,
+	      ptid_t ptid, int leading_zeros)
 {
   int pid, tid;
   struct remote_state *rs = get_remote_state ();
 
-  if (remote_multi_process_p (rs))
+  tid = ptid_get_tid (ptid);
+
+  /* GDB has always output leading zeros in the thread id in some
+     packets.  Some stubs misparse those packets if the leading zeros
+     are removed.  GDB never sent leading zeros when multi-process
+     extensions are in effect, so it's safe to not send them in that
+     case (since it's just extra traffic).  */
+  if (leading_zeros && !remote_multi_process_p (rs))
     {
-      pid = ptid_get_pid (ptid);
-      if (pid < 0)
-	buf += xsnprintf (buf, endbuf - buf, "p-%x.", -pid);
+      if (tid < 0)
+	buf += xsnprintf (buf, endbuf - buf, "-%08x", -tid);
       else
-	buf += xsnprintf (buf, endbuf - buf, "p%x.", pid);
+	buf += xsnprintf (buf, endbuf - buf, "%08x", tid);
     }
-  tid = ptid_get_tid (ptid);
-  if (tid < 0)
-    buf += xsnprintf (buf, endbuf - buf, "-%x", -tid);
   else
-    buf += xsnprintf (buf, endbuf - buf, "%x", tid);
+    {
+      if (remote_multi_process_p (rs))
+	{
+	  pid = ptid_get_pid (ptid);
+	  if (pid < 0)
+	    buf += xsnprintf (buf, endbuf - buf, "p-%x.", -pid);
+	  else
+	    buf += xsnprintf (buf, endbuf - buf, "p%x.", pid);
+	}
+      if (tid < 0)
+	buf += xsnprintf (buf, endbuf - buf, "-%x", -tid);
+      else
+	buf += xsnprintf (buf, endbuf - buf, "%x", tid);
+    }
 
   return buf;
 }
 
+static char *
+write_ptid (char *buf, const char *endbuf, ptid_t ptid)
+{
+  return write_ptid_1 (buf, endbuf, ptid, 0);
+}
+
+static char *
+write_ptid_leading_zeros (char *buf, const char *endbuf, ptid_t ptid)
+{
+  return write_ptid_1 (buf, endbuf, ptid, 1);
+}
+
 /* Extract a PTID from BUF.  If non-null, OBUF is set to the to one
    passed the last parsed char.  Returns null_ptid on error.  */