From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3905 invoked by alias); 22 May 2012 23:34:33 -0000 Received: (qmail 3742 invoked by uid 22791); 22 May 2012 23:34:32 -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 23:34:18 +0000 Received: from svr-orw-exc-10.mgc.mentorg.com ([147.34.98.58]) by relay1.mentorg.com with esmtp id 1SWyan-0000zN-IB from Maciej_Rozycki@mentor.com ; Tue, 22 May 2012 16:34:17 -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 16:33:57 -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; Wed, 23 May 2012 00:34:15 +0100 Date: Tue, 22 May 2012 23:34: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: <20120522204235.GA31756@host2.jankratochvil.net> Message-ID: References: <4FB67E6D.2040406@redhat.com> <4FB6ACEF.2030600@redhat.com> <20120522080321.GA18378@host2.jankratochvil.net> <20120522193521.GA26213@host2.jankratochvil.net> <20120522204235.GA31756@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/msg00857.txt.bz2 On Tue, 22 May 2012, Jan Kratochvil wrote: > > it's difficult to chase something you can't reproduce. > > I see that patch > [RFC patch] non-release srctrees: --enable-targets=all & 64bit & -lmcheck > http://sourceware.org/ml/gdb-patches/2012-05/msg00714.html > > should include also better CFLAGS. But the patch does not seem to go in so > far so we may continue with mail threads like this one. Hmm, it wouldn't have triggered at the compilation time probably anyway. I have switched to --enable-targets=all already, based on some past experience. > > I think however, that this memcpy call needs a rewrite now, I find your > > proposal unreadable. > > I find the whole function unreadable but this was true also before your patch. > But I did not try to change that in this fix up. It also has 64-bit unsafe > bug using 'int' for memory sizes. As noted in the original thread, this whole stuff asks for a rewrite and that will be a good opportunity to make it more readable too. The 'int' bug hardly ever triggers probably, as you'd have to request more than 2GB in one go that is I believe very rare, but I agree that should be size_t instead. You'd have to check the callers if they don't cope with that limitation already somehow however -- though I think it's unlikely and I am too lazy to go chase it right now. It could be that this is how the RSP has been specified too. > I find always more clear to calculate everything as START ADDRESS and ONE BYTE > AFTER THE LAST ADDRESS till the very last moment. That indeed, or START & SIZE in bytes. > > /* 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); > > } > > > > ? > > This code has equal functionality in my local testing. And has passed my regression testing on mips-linux-gnu and i686-linux-gnu as well. It did fix a number of failures on the latter, sorry for not testing my change on another target before, I should have. I have now checked it in, the actual diff follows. 2012-05-22 Maciej W. Rozycki * linux-low.c (linux_store_registers): Avoid the copying sequence when no data has been retrieved by ptrace. Maciej gdb-gdbserver-linux-read-memory-ptrace-fix.diff Index: gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c =================================================================== --- gdb-fsf-trunk-quilt.orig/gdb/gdbserver/linux-low.c 2012-05-22 19:20:59.000000000 +0100 +++ gdb-fsf-trunk-quilt/gdb/gdbserver/linux-low.c 2012-05-22 21:15:36.545454255 +0100 @@ -4447,11 +4447,14 @@ linux_read_memory (CORE_ADDR memaddr, un ret = errno; /* 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) + { + 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); + } return ret; }