From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20462 invoked by alias); 12 Aug 2009 14:11:27 -0000 Received: (qmail 20345 invoked by uid 22791); 12 Aug 2009 14:11:26 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_14,J_CHICKENPOX_63,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 12 Aug 2009 14:11:19 +0000 Received: (qmail 18876 invoked from network); 12 Aug 2009 14:11:17 -0000 Received: from unknown (HELO orlando) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 12 Aug 2009 14:11:17 -0000 From: Pedro Alves To: gdb-patches@sourceware.org, danny.backx@scarlet.be Subject: Re: Ping (Re: Patch : gdbserver get_image_name on CE) Date: Wed, 12 Aug 2009 15:10:00 -0000 User-Agent: KMail/1.9.10 References: <1250021102.5537.1123.camel@pavilion> In-Reply-To: <1250021102.5537.1123.camel@pavilion> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200908121511.20428.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: 2009-08/txt/msg00168.txt.bz2 On Tuesday 11 August 2009 21:05:02, Danny Backx wrote: > > > Please comment on its contents and on my adherence to the coding > > > standards. Apologies for that, I seem to have a hard time learning > > that. See below. > > > 2009-07-08 Danny Backx > > > > > > * win32-low.c (get_image_name) : Work around ReadProcessMemory ^ No space between ) and : please. > > > failure when reading at arbitrary addresses. > Index: win32-low.c > =================================================================== > RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v > retrieving revision 1.38 > diff -u -r1.38 win32-low.c > --- win32-low.c 4 Jul 2009 18:13:28 -0000 1.38 > +++ win32-low.c 11 Aug 2009 20:01:19 -0000 > @@ -922,7 +922,6 @@ > DWORD size = unicode ? sizeof (WCHAR) : sizeof (char); > char *address_ptr; > int len = 0; > - char b[2]; > DWORD done; > > /* Attempt to read the name of the dll that was detected. > @@ -945,21 +944,23 @@ > return NULL; > #endif > > - /* Find the length of the string */ > - while (ReadProcessMemory (h, address_ptr + len++ * size, &b, size, &done) > - && (b[0] != 0 || b[size - 1] != 0) && done == size) > - continue; > + /* ReadProcessMemory sometimes fails when reading a (w)char at a time, but > + * we can't just read MAX_PATH (w)chars either : msdn says not to cross the > + * boundary into inaccessible areas. > + * So we loop, reading more characters each time, until we find the NULL. > + */ Format comments as: /* ReadProcessMemory sometimes fails when reading a (w)char at a time, but we can't just read MAX_PATH (w)chars either: MSDN says not to cross the boundary into inaccessible areas. So we loop, reading more characters each time, until we find the NULL. */ - no '*' at the beginning of each comment line. - period and double space between sentences. - might as well uppercase msdn. In any case, the comment doesn't really explain the problem, and would make more sense when looking at the old code than at the post-patch code. From what I could tell so far, the problem is not with the size of how much we read each time, but with starting the read from any address other than address_ptr. The comment should mention that instead. BTW, did you ever try to read the dll name from GDB using 'x' 'print', etc. ? If you play with the arguments to 'x', e.g., (gdb) x /10b $address_ptr you should see the same problem triggering. (this is one of the reasons I'm relunctant to working around an issue I'm not sure is understood fully). > + WCHAR *wbuf = alloca ((MAX_PATH + 1) * size); Declarations go at the beggining of a block/function. > + while (1) > + { > + ReadProcessMemory (h, address_ptr, wbuf, ++len * size, &done); > + if (wbuf[len - 1] == 0) > + break; > + } > This is busted for the non unicode case. This doesn't check for the possibility of overflowing wbuf. The old code handled both cases. Given that this now reads the whole string, why not read directly into buf, instead of alloca'ing a big buffer, and then you wouldn't need... > if (!unicode) > ReadProcessMemory (h, address_ptr, buf, len, &done); ... this? > else > - { > - WCHAR *unicode_address = (WCHAR *) alloca (len * sizeof (WCHAR)); > - ReadProcessMemory (h, address_ptr, unicode_address, len * sizeof (WCHAR), > - &done); > - > - WideCharToMultiByte (CP_ACP, 0, unicode_address, len, buf, len, 0, 0); > - } > + WideCharToMultiByte (CP_ACP, 0, wbuf, len, buf, len, 0, 0); > > return buf; > } Sorry, I'm quite behind on reading cegcc-devel@. Do you now have a fully working non-broken libstdc++ dll (the dll that triggered this problem)? IIUC from the binutils posts, the runtime loader was ending up doing things it shouldn't do to the dll image in memory. Does this ReadProcessMemory problem still exist with a fixed dll? Did you ever see this problem with another dll? I'm very relunctant to this fix without understanding it fully, and without having seen it trigger with a sane dll, and not really understanding why some dlls cause it and other don't, even though they're apparently built the exact same way --- not because I'm hard headed, but because these workarounds tend to mask other real problems. From what I've heard so far, this only triggered on that broken dll. I wonder if anyone has tried loading an application with a dll that triggers this into MSFT's debugger, and see if it reads the dll name correctly. -- Pedro Alves