From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25002 invoked by alias); 22 May 2012 20:06:10 -0000 Received: (qmail 24989 invoked by uid 22791); 22 May 2012 20:06:08 -0000 X-SWARE-Spam-Status: No, hits=-4.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_CP X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 22 May 2012 20:05:43 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1SWvKw-0007EI-4q from Maciej_Rozycki@mentor.com ; Tue, 22 May 2012 13:05:42 -0700 Received: from SVR-IES-FEM-01.mgc.mentorg.com ([137.202.0.104]) by SVR-ORW-EXC-10.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.4675); Tue, 22 May 2012 13:05:22 -0700 Received: from [172.30.0.105] (137.202.0.76) by SVR-IES-FEM-01.mgc.mentorg.com (137.202.0.104) with Microsoft SMTP Server id 14.1.289.1; Tue, 22 May 2012 21:05:40 +0100 Date: Tue, 22 May 2012 20:06:00 -0000 From: "Maciej W. Rozycki" To: Jan Kratochvil CC: Pedro Alves , Subject: Re: [patch] Re: Regression for gdbserver [Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues] In-Reply-To: <20120522193521.GA26213@host2.jankratochvil.net> Message-ID: References: <4FB67E6D.2040406@redhat.com> <4FB6ACEF.2030600@redhat.com> <20120522080321.GA18378@host2.jankratochvil.net> <20120522193521.GA26213@host2.jankratochvil.net> User-Agent: Alpine 1.10 (DEB 962 2008-03-14) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" 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: 2012-05/txt/msg00848.txt.bz2 On Tue, 22 May 2012, Jan Kratochvil wrote: > > Can you check that these are not intermittent failures > > It was very 0% vs. 100% reproducible and for the single testcase. Oh well, thanks for double-checking, it's difficult to chase something you can't reproduce. > diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c > index 19f7be6..c33c976 100644 > --- a/gdb/gdbserver/linux-low.c > +++ b/gdb/gdbserver/linux-low.c > @@ -4450,10 +4450,12 @@ linux_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len) > > /* Copy appropriate bytes out of the buffer. */ > i *= sizeof (PTRACE_XFER_TYPE); > - i -= memaddr & (sizeof (PTRACE_XFER_TYPE) - 1); > - memcpy (myaddr, > - (char *) buffer + (memaddr & (sizeof (PTRACE_XFER_TYPE) - 1)), > - i < len ? i : len); > + if (i > 0) > + memcpy (myaddr, > + (char *) buffer + (memaddr & (sizeof (PTRACE_XFER_TYPE) - 1)), > + (MIN ((memaddr & -(CORE_ADDR) sizeof (PTRACE_XFER_TYPE)) + i, > + memaddr + len) > + - memaddr)); > > return ret; > } Doh, it looks like my final correction of the copy size (not to overrun the buffer supplied) missed the zero case, sorry and thanks for catching this. Presumably your alignment is different or suchlike. I think however, that this memcpy call needs a rewrite now, I find your proposal unreadable. Would you please make use of an auxiliary variable or suchlike to make this calculation? Also can we rely on MIN? It's not a standard macro AFAIK and we don't define our own copy, except from one place in gdb/target.c (I checked before making the original change already). Perhaps a piece like this would actually be enough, i.e. the original code, only guarded against zero, where we're not going to copy anything anyway, so we can skip the whole calculation altogether: /* Copy appropriate bytes out of the buffer. */ if (i > 0) { i *= sizeof (PTRACE_XFER_TYPE); i -= memaddr & (sizeof (PTRACE_XFER_TYPE) - 1); memcpy (myaddr, (char *) buffer + (memaddr & (sizeof (PTRACE_XFER_TYPE) - 1)), i < len ? i : len); } ? Maciej