This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Disassembly improvements
- From: Pedro Alves <palves at redhat dot com>
- To: "Abid, Hafiz" <Hafiz_Abid at mentor dot com>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, "Mirza, Taimoor" <Taimoor_Mirza at mentor dot com>
- Date: Thu, 10 Oct 2013 14:34:37 +0100
- Subject: Re: [patch] Disassembly improvements
- Authentication-results: sourceware.org; auth=none
- References: <EB3B29AD43CA924DA27099BC85192376E0705106 at EU-MBX-03 dot mgc dot mentorg dot 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