Re: [patch] Re: Regression for gdbserver [Re: [PATCH] Linux/gdbserver: Fix memory read ptrace fallback issues]

["Maciej W. Rozycki" <macro at codesourcery dot com>]

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