From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27657 invoked by alias); 22 Oct 2008 14:17:58 -0000 Received: (qmail 27639 invoked by uid 22791); 22 Oct 2008 14:17:56 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 22 Oct 2008 14:16:06 +0000 Received: (qmail 11198 invoked from network); 22 Oct 2008 14:16:03 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 22 Oct 2008 14:16:03 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFA] Restore leading zeros in remote_thread_alive Date: Wed, 22 Oct 2008 14:17:00 -0000 User-Agent: KMail/1.9.9 Cc: Michael Snyder References: <48FE7B9B.3040905@vmware.com> <200810221418.22409.pedro@codesourcery.com> In-Reply-To: <200810221418.22409.pedro@codesourcery.com> MIME-Version: 1.0 Content-Type: Multipart/Mixed; boundary="Boundary-00=_pWz/IfThJLxoN00" Message-Id: <200810221516.09164.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-10/txt/msg00543.txt.bz2 --Boundary-00=_pWz/IfThJLxoN00 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Content-length: 2421 A Wednesday 22 October 2008 14:18:22, Pedro Alves escreveu: > On Wednesday 22 October 2008 02:02:19, Michael Snyder wrote: > > --- remote.c=C2=A0=C2=A0=C2=A0=C2=A017 Oct 2008 19:43:47 -0000=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A01.321 > > +++ remote.c=C2=A0=C2=A0=C2=A0=C2=A022 Oct 2008 01:00:16 -0000 > > @@ -1433,15 +1433,15 @@ write_ptid (char *buf, const char *endbu > > =C2=A0 =C2=A0 =C2=A0{ > > =C2=A0 =C2=A0 =C2=A0 =C2=A0pid =3D ptid_get_pid (ptid); > > =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pid < 0) > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0buf +=3D xsnprintf (buf, end= buf - buf, "p-%x.", -pid); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0buf +=3D xsnprintf (buf, end= buf - buf, "p-%08x.", -pid); > > =C2=A0 =C2=A0 =C2=A0 =C2=A0else > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0buf +=3D xsnprintf (buf, end= buf - buf, "p%x.", pid); > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0buf +=3D xsnprintf (buf, end= buf - buf, "p%08x.", pid); > > =C2=A0 =C2=A0 =C2=A0} > > =C2=A0 =C2=A0tid =3D ptid_get_tid (ptid); > > =C2=A0 =C2=A0if (tid < 0) > > - =C2=A0 =C2=A0buf +=3D xsnprintf (buf, endbuf - buf, "-%x", -tid); > > + =C2=A0 =C2=A0buf +=3D xsnprintf (buf, endbuf - buf, "-%x08", -tid); > > =C2=A0 =C2=A0else > > - =C2=A0 =C2=A0buf +=3D xsnprintf (buf, endbuf - buf, "%x", tid); > > + =C2=A0 =C2=A0buf +=3D xsnprintf (buf, endbuf - buf, "%x08", tid); > > =C2=A0 > > =C2=A0 =C2=A0return buf; >=20 > ( Now that I've slept a bit, :-) ) I've gone through an old version of th= e 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 s= ee > 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, b= ecause > used to be "vCont;c:%x". Also, we don't really need to use %08 when mult= i-process > is in effect. There should be no multi-process stubs around that depend > on leading 0's. >=20 > 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. >=20 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. --=20 Pedro Alves --Boundary-00=_pWz/IfThJLxoN00 Content-Type: text/x-diff; charset="utf-8"; name="write_ptid_arg.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="write_ptid_arg.diff" Content-length: 3489 --- 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. */ --Boundary-00=_pWz/IfThJLxoN00--