RE: [patch] Disassembly improvements

["Abid, Hafiz" <Hafiz_Abid@mentor.com>]

[-- Attachment #1: Type: text/plain, Size: 802 bytes --]

Thanks for review. Here is updated patch.

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

> > +     useless to keep retrying reading that buffer line.  Simply
> > +     fallback to reading directly from target memory.  */
> 
> Should be "retrying to read", I think.
Fixed.

> > +	      /* 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.
Changed to following line which should take care of this.
unsigned int length = min (len, info->buffer_length);

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

> 
> 
> --
> Pedro Alves

[-- Attachment #2: disasm.patch --]
[-- Type: application/octet-stream, Size: 2960 bytes --]

diff --git a/gdb/disasm.c b/gdb/disasm.c
index e643c2d..7e3d908 100644
--- a/gdb/disasm.c
+++ b/gdb/disasm.c
@@ -42,11 +42,62 @@ struct dis_line_entry
   CORE_ADDR end_pc;
 };
 
+/* Size of the disassembly memory buffer.  */
+#define DIS_BUF_SIZE 1024
+
 /* Like target_read_memory, but slightly different parameters.  */
 static int
 dis_asm_read_memory (bfd_vma memaddr, gdb_byte *myaddr, unsigned int len,
 		     struct disassemble_info *info)
 {
+  /* Assume the disassembler always reads memory forwards.  If we
+     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 to read that buffer line.  Simply
+     fallback to reading directly from target memory.  */
+  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 length = min (len, info->buffer_length);
+
+	      /* Try fetching a new buffer line from the target.  */
+
+	      /* 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, length);
+	      if (rval == 0)
+		{
+		  info->buffer_vma = memaddr;
+		  info->buffer_length = length;
+		}
+	      else
+		{
+		  /* Read from target memory directly from now on.  */
+		  break;
+		}
+	    }
+	}
+    }
   return target_read_memory (memaddr, myaddr, len);
 }
 
@@ -415,6 +466,10 @@ gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
   struct symtab *symtab = NULL;
   struct linetable_entry *le = NULL;
   int nlines = -1;
+  int buff_size = DIS_BUF_SIZE;
+  int req_size = high - low;
+  int err = 0;
+  gdb_byte *pbuffer = NULL;
 
   /* Assume symtab is valid for whole PC range.  */
   symtab = find_pc_symtab (low);
@@ -425,6 +480,23 @@ gdb_disassembly (struct gdbarch *gdbarch, struct ui_out *uiout,
       le = symtab->linetable->item;
       nlines = symtab->linetable->nitems;
     }
+  if (req_size < buff_size)
+    buff_size = req_size;
+
+  if (req_size > 0)
+    {
+      /* Allocate buffer and read memory region in buffer.  */
+      pbuffer = xmalloc (buff_size);
+      make_cleanup (xfree, pbuffer);
+      err = target_read_memory (low, pbuffer, buff_size);
+      if (err == 0)
+	{
+	  di.buffer = pbuffer;
+	  di.buffer_length = buff_size;
+	  di.buffer_vma = low;
+	}
+    }
+
 
   if (!(flags & DISASSEMBLY_SOURCE) || nlines <= 0
       || symtab == NULL || symtab->linetable == NULL)