Re: [patch] Disassembly improvements

[Pedro Alves <palves@redhat.com>]

On 10/10/2013 02:14 PM, Abid, Hafiz wrote:
> Hi Pedro,
> I am attaching the patch that was mentioned in the following thread. I resurrected it from our internal repo, did a bit of manual testing and run the regression suite without any problem. It basically reads memory from the target in a buffer in gdb_disassembly and tries to use this buffer in dis_asm_read_memory instead of reading from the target. This saves us on repeated memory read calls. The problem was noted when eclipse was trying to fill its disassembly view.
> https://sourceware.org/ml/gdb-patches/2013-10/msg00221.html

Thanks.

> +  /* Assume the disassembler always read memory forwards.  If we

"always reads"

> +     failed to read a buffer line in a previous call, assume we're
> +     reading close to the end of a mapped page or section, and so it's
> +     useless to keep retrying reading that buffer line.  Simply
> +     fallback to reading directly from target memory.  */

Should be "retrying to read", I think.


> +  if (info->buffer_length > 0)
> +    {
> +      while (len)
> +	{
> +	  if (memaddr >= info->buffer_vma
> +	      && memaddr < info->buffer_vma + info->buffer_length)
> +	    {
> +	      unsigned int offset = (memaddr - info->buffer_vma);
> +	      unsigned int l = min (len, info->buffer_length - offset);
> +
> +	      memcpy (myaddr, info->buffer + offset, l);
> +
> +	      memaddr += l;
> +	      myaddr += l;
> +	      len -= l;
> +
> +	      if (len == 0)
> +		return 0;
> +	    }
> +	  else
> +	    {
> +	      int rval;
> +	      unsigned int len = info->buffer_length;
> +
> +	      /* Try fetching a new buffer line from the target.  */

Hmm, this seems to miss making sure LEN doesn't read beyond the
original requested memory range.  It'd be good to add that.

That "unsigned int len" variable shadows the function's "len"
parameter.  It'd be good to rename it.

> +
> +	      /* If we fail reading memory halfway, we'll have clobbered
> +		 the buffer, so don't trust it anymore, even on fail.  */
> +	      info->buffer_length = 0;
> +	      rval = target_read_memory (memaddr, info->buffer, len);
> +	      if (rval == 0)
> +		{
> +		  info->buffer_vma = memaddr;
> +		  info->buffer_length = len;
> +		}
> +	      else
> +		{
> +		  /* Read from target memory directly from now on.  */
> +		  break;
> +		}
> +	    }
> +	}
> +    }
>    return target_read_memory (memaddr, myaddr, len);
>  }


-- 
Pedro Alves