From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24903 invoked by alias); 2 Sep 2013 13:38:34 -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 24893 invoked by uid 89); 2 Sep 2013 13:38:34 -0000 Received: from mailhost.u-strasbg.fr (HELO mailhost.u-strasbg.fr) (130.79.222.212) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 02 Sep 2013 13:38:34 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,KHOP_THREADED,MSGID_MULTIPLE_AT autolearn=no version=3.3.2 X-HELO: mailhost.u-strasbg.fr Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antispam (Postfix) with ESMTP id C5805140AEC; Mon, 2 Sep 2013 15:38:30 +0200 (CEST) Received: from mailhost.u-strasbg.fr (localhost [127.0.0.1]) by antivirus (Postfix) with ESMTP id B5BE7140C8B; Mon, 2 Sep 2013 15:38:30 +0200 (CEST) Received: from md16.u-strasbg.fr (md16.u-strasbg.fr [130.79.200.206]) by mr2.u-strasbg.fr (Postfix) with ESMTP id 94460140AEC; Mon, 2 Sep 2013 15:38:28 +0200 (CEST) Received: from ms13.u-strasbg.fr (ms13.u-strasbg.fr [130.79.204.113]) by md16.u-strasbg.fr (8.14.3/jtpda-5.5pre1) with ESMTP id r82DcPDf000929 ; Mon, 2 Sep 2013 15:38:25 +0200 (envelope-from pierre.muller@ics-cnrs.unistra.fr) Received: from E6510Muller (gw-ics.u-strasbg.fr [130.79.210.225]) (Authenticated sender: mullerp) by ms13.u-strasbg.fr (Postfix) with ESMTPSA id 1F8591FD84; Mon, 2 Sep 2013 15:38:24 +0200 (CEST) From: "Pierre Muller" To: "'Pedro Alves'" Cc: References: <5223bb46.c6c0420a.5a41.008dSMTPIN_ADDED_BROKEN@mx.google.com> <52248978.90500@redhat.com> <000301cea7dd$17bc4af0$4734e0d0$@muller@ics-cnrs.unistra.fr> <52249053.6050103@redhat.com> In-Reply-To: <52249053.6050103@redhat.com> Subject: [RFA] gdbserver/win32-low.c: Check Read/WriteProcessMemory return value (followup to [RFA] windows-nat.c: Handle ERROR_PARTIAL_COPY in windows_xfer_memory function) Date: Mon, 02 Sep 2013 13:38:00 -0000 Message-ID: <000301cea7e1$b0e91ab0$12bb5010$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-SW-Source: 2013-09/txt/msg00023.txt.bz2 > >>> This is not compatible with returning information that only part of the > >>> request length > >>> was read/written. > >> > >> Well, we could just change that interface to make it possible... > >> > >> The thing I don't like with doing this only on the native > >> side, is that we're trying to get to a point where we > >> can share the target backends between GDB and gdbserver: > > > > Well, when you look at the code inside child_xfer_memory, > > you can notice that the return value of ReadProcessMemory or > > WriteProcessMemory > > is discarded, which means that it does behave more or less like the > > new windows-nat.c code (at least in case of ERROR_PARTIAL_COPY) > > for other errors, it might also return garbage... > > anyhow, the calling code compares the returned value to the requested > length > > (LEN value) > > That's brittle... > > > so that the risk of generating a successful read_memory despite a failure > > of ReadProcessMemory function is small... (the uninitialized variable done > > would need to return the value LEN..) > > It could of course still happen theoretically... > > This is really no argument for not fixing gdbserver... In fact, > it's an argument _for_ fixing it. What about this patch, it still does not allow to really return the number of bytes read or written, but at least it checks correctly if the API calls succeeded. Pierre Muller 2013-09-02 Pierre Muller * win32-low.c (child_xfer_memory): Check if ReadProcessMemory or WriteProcessMemory complete successfully and handle ERROR_PARTIAL_COPY error. Index: src/gdb/gdbserver/win32-low.c =================================================================== RCS file: /cvs/src/src/gdb/gdbserver/win32-low.c,v retrieving revision 1.66 diff -u -r1.66 win32-low.c --- src/gdb/gdbserver/win32-low.c 2 Jul 2013 11:59:24 -0000 1.66 +++ src/gdb/gdbserver/win32-low.c 2 Sep 2013 13:31:31 -0000 @@ -278,21 +278,35 @@ child_xfer_memory (CORE_ADDR memaddr, char *our, int len, int write, struct target_ops *target) { - SIZE_T done; + BOOL success; + SIZE_T done = 0; + DWORD lasterror = 0; uintptr_t addr = (uintptr_t) memaddr; if (write) { - WriteProcessMemory (current_process_handle, (LPVOID) addr, - (LPCVOID) our, len, &done); + success = WriteProcessMemory (current_process_handle, (LPVOID) addr, + (LPCVOID) our, len, &done); + if (!success) + lasterror = GetLastError (); FlushInstructionCache (current_process_handle, (LPCVOID) addr, len); } else { - ReadProcessMemory (current_process_handle, (LPCVOID) addr, (LPVOID) our, - len, &done); + success = ReadProcessMemory (current_process_handle, (LPCVOID) addr, + (LPVOID) our, len, &done); + if (!success) + lasterror = GetLastError (); + } + if (success) + return done; + else + { + if (lasterror == ERROR_PARTIAL_COPY && done > 0) + return done; + else + return -1; } - return done; } /* Clear out any old thread list and reinitialize it to a pristine