From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67415 invoked by alias); 3 Nov 2017 03:11:06 -0000 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 Received: (qmail 67406 invoked by uid 89); 3 Nov 2017 03:11:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-11.9 required=5.0 tests=BAYES_00,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=meanwhile X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 03 Nov 2017 03:11:03 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 7842E80F90; Fri, 3 Nov 2017 03:11:02 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 7842E80F90 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=sergiodj@redhat.com Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 4EC2F60317; Fri, 3 Nov 2017 03:11:02 +0000 (UTC) From: Sergio Durigan Junior To: "pcarroll\@codesourcery.com" Cc: "gdb-patches\@sourceware.org" Subject: Re: [PATCH] Assertion 'xfered>0' in target.c for remote connection References: <1155839491.1748621.1509663923992.ref@mail.yahoo.com> <1155839491.1748621.1509663923992@mail.yahoo.com> Date: Fri, 03 Nov 2017 03:11:00 -0000 In-Reply-To: <1155839491.1748621.1509663923992@mail.yahoo.com> (pcarroll@codesourcery.com's message of "Thu, 2 Nov 2017 23:05:23 +0000 (UTC)") Message-ID: <87lgjo6oqi.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00065.txt.bz2 On Thursday, November 02 2017, pcarroll@codesourcery.com wrote: > We have a customer who is using a Corelis gdb server to connect to gdb. > Occasionally, the gdb server will send a 0-byte block of memory for a rea= d. > When this happens, gdb gives an assertion from target.c: > > internal-error: target_xfer_partial: Assertion `*xfered_len > 0' failed. > > This problem is almost identical to that fixed in https://sourceware.org/= ml/gdb-patches/2014-02/msg00636.html > > In this case, remote.c needs to be modified to return TARGET_XFER_EOF ins= tead of TARGET_XFER_OK or TARGET_XFER_UNAVAILABLE when 0 bytes are transfer= red.=20 Thanks for the patch. A few comments below. BTW, your mail client seems to have messed up with the formatting of the patch, removing TABs and replacing them with whitespaces. Could you please send the patch again, but using a client that handles patches better? > The proposed fix would be: > > diff -rup fsf/gdb/ChangeLog fix/gdb/ChangeLog > --- fsf/gdb/ChangeLog=C2=A0=C2=A0 2017-11-02 16:13:19.188615000 -0500 > +++ fix/gdb/ChangeLog=C2=A0=C2=A0 2017-11-02 16:13:21.626754500 -0500 > @@ -1,3 +1,10 @@ > +2017-11-02=C2=A0 Paul Carroll=C2=A0 > + > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 PR gdb/22388 > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * remote.c (remote_write_bytes_aux,= remote_read_bytes_1, > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 remote_read_bytes, remote_write_qxf= er, remote_xfer_partial): > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Return TARGET_XFER_EOF if size of r= eturned data is 0. > + The ChangeLog entries need to be formatted with TABs instead of whitespaces. But the text looks great. > 2017-11-02=C2=A0 Yao Qi=C2=A0 > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 * frame.c (do_frame_register_r= ead): Remove aspace. > diff -rup fsf/gdb/remote.c fix/gdb/remote.c > --- fsf/gdb/remote.c=C2=A0=C2=A0=C2=A0 2017-11-02 16:06:15.979408800 -0500 > +++ fix/gdb/remote.c=C2=A0=C2=A0=C2=A0 2017-11-02 15:24:35.536391700 -0500 > @@ -8264,7 +8264,7 @@ remote_write_bytes_aux (const char *head > =C2=A0=C2=A0 /* Return UNITS_WRITTEN, not TODO_UNITS, in case escape char= s caused us to > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 send fewer units than we'd planned.=C2=A0 = */ > =C2=A0=C2=A0 *xfered_len_units =3D (ULONGEST) units_written; > -=C2=A0 return TARGET_XFER_OK; > +=C2=A0 return (*xfered_len_units !=3D 0) ? TARGET_XFER_OK : TARGET_XFER_= EOF; > } This one looks correct to me. > > /* Write memory data directly to the remote machine. > @@ -8358,7 +8358,7 @@ remote_read_bytes_1 (CORE_ADDR memaddr, > =C2=A0=C2=A0 decoded_bytes =3D hex2bin (p, myaddr, todo_units * unit_size= ); > =C2=A0=C2=A0 /* Return what we have.=C2=A0 Let higher layers handle parti= al reads.=C2=A0 */ > =C2=A0=C2=A0 *xfered_len_units =3D (ULONGEST) (decoded_bytes / unit_size); > -=C2=A0 return TARGET_XFER_OK; > +=C2=A0 return (*xfered_len_units !=3D 0) ? TARGET_XFER_OK : TARGET_XFER_= EOF; > } Likewise. > > /* Using the set of read-only target sections of remote, read live > @@ -8455,13 +8455,14 @@ remote_read_bytes (struct target_ops *op > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 res =3D remote_xfer_live_readonly_partial (ops, myaddr, memaddr, > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0 len, unit_size, xfered_len); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 if (res =3D=3D TARGET_XFER_OK) > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 return TARGET_XFER_OK; > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 return (*xfered_len !=3D 0) ? TARGET_XFER_OK : TARGET_XFER_EOF; remote_xfer_live_readonly_partial can only return TARGET_XFER_EOF or what remote_read_bytes_1 return (which already takes care of the EOF part), so I don't think you need this part here, although keeping it wouldn't hurt. > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 else > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* No use trying further, we know some memor= y starting > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 at MEMADDR isn't available= .=C2=A0 */ > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *xfered_len =3D len; > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 return TARGET_XFER_UNAVAILABLE; > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 return (*xfered_len !=3D 0) ? > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 TARGET_XFER_UNAVAILABLE : TARGET_XFER_= EOF; > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 } > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } > > @@ -10386,7 +10387,7 @@ remote_write_qxfer (struct target_ops *o > =C2=A0=C2=A0 unpack_varlen_hex (rs->buf, &n); > > =C2=A0=C2=A0 *xfered_len =3D n; > -=C2=A0 return TARGET_XFER_OK; > +=C2=A0 return (*xfered_len !=3D 0) ? TARGET_XFER_OK : TARGET_XFER_EOF; > } > > /* Read OBJECT_NAME/ANNEX from the remote target using a qXfer packet. > @@ -10687,7 +10688,7 @@ remote_xfer_partial (struct target_ops * > =C2=A0=C2=A0 strcpy ((char *) readbuf, rs->buf); > > =C2=A0=C2=A0 *xfered_len =3D strlen ((char *) readbuf); > -=C2=A0 return TARGET_XFER_OK; > +=C2=A0 return (*xfered_len !=3D 0) ? TARGET_XFER_OK : TARGET_XFER_EOF; > } > > /* Implementation of to_get_memory_xfer_limit.=C2=A0 */ The rest looks sane to me, but I'm not a global maintainer and cannot approve your patch. Meanwhile I'd recommend resending the patch to fix the formatting errors. Cheers, --=20 Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/