Re: [PATCH] Fix '-data-read-memory-bytes' typo/assertion

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 15-11-18 07:53 PM, Don Breazeal wrote:
> This patch fixes a typo in target.c:read_memory_robust, where
> it calls read_whatever_is_readable with the function arguments
> in the wrong order.  Depending on the address being read, it
> can cause an xmalloc with a huge size, resulting in an assertion
> failure, or just read something other than what was requested.
> 
> The problem only arises when GDB is handling an MI
> "-data-read-memory-bytes" request and the initial target_read returns
> an error status.  Note that read_memory_robust is only called from
> the MI code.
> 
> Function definition:
>   static void
>   read_whatever_is_readable (struct target_ops *ops,
>                              const ULONGEST begin, const ULONGEST end,
>                              int unit_size,
>                              VEC(memory_read_result_s) **result)
> 
> Function call:
>     read_whatever_is_readable (ops, offset + xfered_total, unit_size,
>                                offset + xfered_total + to_read, &result);
> 
> If we debug gdb, generate the error, and break just before the call to
> read_whatever_is_readable, we see:
> 
> # Generate an error by trying to read a bogus address in the GDB
> # that is under debug.
> (gdb) interpreter-exec mi "-data-read-memory-bytes 1073741752 216"
> 
> # GDB-under-debug stops at the breakpoint on the call to
> # read_whatever_is_readable.
> Breakpoint 1, read_memory_robust (ops=0xe70150, offset=1073741752, len=216)
>     at /scratch/dbreazea/sandbox/gdb-5611/binutils-gdb/gdb/target.c:1825
> 1825	              read_whatever_is_readable (ops, offset + xfered_total, unit_size,
> (top) p unit_size
> $1 = 1
> 
> # Step into the function.
> (top) step
> read_whatever_is_readable (ops=0xe70150, begin=1073741752, end=1, 
>     unit_size=1073741968, result=0x7fffffffdd40)
>     at /scratch/dbreazea/sandbox/gdb-5611/binutils-gdb/gdb/target.c:1658
> 1658	  gdb_byte *buf = (gdb_byte *) xmalloc (end - begin);
> 
> # unit_size was passed as 'end', and we are going to xmalloc a large
> # number and assert.
> (top) p end-begin
> $2 = 18446744072635809865
> (top) c
> Continuing.
> "/scratch/dbreazea/sandbox/gdb-5611/binutils-gdb/gdb/utils.c:1072: internal-error: virtual memory exhausted.\nA problem internal to GDB has been detected,\nfurther debugging may prove unreliable.\nQuit this debugging session? (y or n) "
> 
> # With the fixed version, (end - begin) gives the 'len' passed to
> # read_memory_robust and specified by -data-read-memory-bytes
> (top) p end-begin
> $2 = 216
> 
> Tested on native x86_64 Linux with the gdb.mi tests.
> 
> OK?
> thanks
> --Don
> 
> gdb/
> 2015-11-18  Don Breazeal  <donb@codesourcery.com>
> 
> 	* gdb/target.c (read_memory_robust): Call
> 	read_whatever_is_readable with arguments in the correct order.
> 
> ---
>  gdb/target.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/target.c b/gdb/target.c
> index 93786c3..950a1b7 100644
> --- a/gdb/target.c
> +++ b/gdb/target.c
> @@ -1822,8 +1822,9 @@ read_memory_robust (struct target_ops *ops,
>  	      /* Got an error reading full chunk.  See if maybe we can read
>  		 some subrange.  */
>  	      xfree (buffer);
> -	      read_whatever_is_readable (ops, offset + xfered_total, unit_size,
> -					 offset + xfered_total + to_read, &result);
> +	      read_whatever_is_readable (ops, offset + xfered_total,
> +					 offset + xfered_total + to_read,
> +					 unit_size, &result);
>  	      xfered_total += to_read;
>  	    }
>  	  else
> 

Whoops.  LGTM.