[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)
Pedro Alves
palves@redhat.com
Mon Sep 2 13:50:00 GMT 2013
On 09/02/2013 02:38 PM, Pierre Muller wrote:
>>>>> 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.
No, as long as the read_memory/write_memory interfaces do not
support partial transfers, we should only return true if the
all of LEN was transferred. Otherwise, things like:
static int
gdb_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
{
...
{
res = read_inferior_memory (memaddr, myaddr, len);
done_accessing_memory ();
return res == 0 ? len : -1;
}
}
will behave incorrectly in the ERROR_PARTIAL_COPY scenario...
--
Pedro Alves
More information about the Gdb-patches
mailing list