This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Re: Regression for gdbserver [Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues]
On Tue, 22 May 2012, Jan Kratochvil wrote:
> > Can you check that these are not intermittent failures
>
> It was very 0% vs. 100% reproducible and for the single testcase.
Oh well, thanks for double-checking, it's difficult to chase something
you can't reproduce.
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index 19f7be6..c33c976 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -4450,10 +4450,12 @@ linux_read_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
>
> /* 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)),
> - i < len ? i : len);
> + if (i > 0)
> + memcpy (myaddr,
> + (char *) buffer + (memaddr & (sizeof (PTRACE_XFER_TYPE) - 1)),
> + (MIN ((memaddr & -(CORE_ADDR) sizeof (PTRACE_XFER_TYPE)) + i,
> + memaddr + len)
> + - memaddr));
>
> return ret;
> }
Doh, it looks like my final correction of the copy size (not to overrun
the buffer supplied) missed the zero case, sorry and thanks for catching
this. Presumably your alignment is different or suchlike.
I think however, that this memcpy call needs a rewrite now, I find your
proposal unreadable. Would you please make use of an auxiliary variable
or suchlike to make this calculation? Also can we rely on MIN? It's not
a standard macro AFAIK and we don't define our own copy, except from one
place in gdb/target.c (I checked before making the original change
already).
Perhaps a piece like this would actually be enough, i.e. the original
code, only guarded against zero, where we're not going to copy anything
anyway, so we can skip the whole calculation altogether:
/* Copy appropriate bytes out of the buffer. */
if (i > 0)
{
i *= sizeof (PTRACE_XFER_TYPE);
i -= memaddr & (sizeof (PTRACE_XFER_TYPE) - 1);
memcpy (myaddr,
(char *) buffer + (memaddr & (sizeof (PTRACE_XFER_TYPE) - 1)),
i < len ? i : len);
}
?
Maciej