From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32672 invoked by alias); 18 May 2012 16:53:25 -0000 Received: (qmail 32657 invoked by uid 22791); 18 May 2012 16:53:22 -0000 X-SWARE-Spam-Status: No, hits=-7.4 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,SPF_HELO_PASS,TW_CP,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 18 May 2012 16:53:06 +0000 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q4IGr3Uk006057 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 18 May 2012 12:53:03 -0400 Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx12.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id q4IGr1rQ010186; Fri, 18 May 2012 12:53:02 -0400 Message-ID: <4FB67E6D.2040406@redhat.com> Date: Fri, 18 May 2012 16:53:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120430 Thunderbird/12.0.1 MIME-Version: 1.0 To: "Maciej W. Rozycki" CC: gdb-patches@sourceware.org Subject: Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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/msg00693.txt.bz2 Hi Maciej, On 05/16/2012 11:56 PM, Maciej W. Rozycki wrote: > While trying to investigate suspicious MIPS16/Linux test suite results > noted with the recent MIPS ISA bit solution proposal I have come across > this problem in gdbserver. It contributed to about 2550 failures per > multilib. > > In linux_read_memory we make a two-way choice as to how we read > debuggee's memory -- we prefer pread64 or lseek/read of /proc//mem > and failing that we resort to a long boring series of PEEKTEXT ptrace > requests. We use this in particular in linux_qxfer_libraries_svr4 to > access names of shared libraries loaded. There we rely on > linux_read_memory to return data in the buffer provided even if the > function wasn't able to read the whole number of bytes requested. > > This has two problems as I revealed. If any call in the pread64 fails > then the supplied buffer is obviously not filled. It looks like the pread64/lseek/read path actually reads directly into MYADDR (the supplied buffer). > Then if the ptrace > fallback path is not able to retrieve the whole number of bytes requested, > then it does not supply partially retrieved data to the buffer. This is > the scenario that I observed. Right. > The change below addresses these two problems. Any data successfully > retrieved by the ptrace loop is copied over to the user buffer even if > linux_read_memory returns unsuccessfully. If pread64 successfully > retrieves any data, then ptrace is not used to read that data again, the > attempt to read data is resumed at the point where pread64 stopped. This > will normally not retrieve any further data as pread64 would have provided > that (internally, in the Linux kernel, the read handlers for > /proc//maps special files are implemented as a wrapper around the > very same worker function that the PEEKTEXT ptrace request uses. So how about just returning immediately if reading from /proc manages to read something? From what you say, the PEEKTEXT fallback then won't just normally fail; it'll _always_ fail. I suspect that'd make the code a bit simpler. > /* 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)), > - len); > + i < len ? i : len); > > - return 0; > + return errno; You shouldn't assume that "errno" is preserved across library calls (memcpy in this case). > } > -- Pedro Alves