[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)

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

> >>> 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  <muller@sourceware.org>

	* 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