This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Robustify -data-read-memory


The -data-read-memory, as presently documented, is allowed to return "N/A" for
memory if could not read. In practice, it tends to fail completely if there's
any error reading memory, and this is not very nice when trying to look at
memory of some embedded system.

-data-read-memory uses target.c:target_read internally and target_read
uses target_read_partial, which uses target_xfer_partial/memory_xfer_partial,
which ends up using to_xfer_partial target method. Both that method,
and the 'm' remote protocol packet are not very clearly specified -- 
they are allowed to return partial data and they can return error, and when
exactly they should return partial data is not clear. As result, I think
we should expect the remote stub, generally, to completely fail a memory
transfer on any error.

This patch adds a fallback on gdb side -- if we try to read memory and fail,
we try again, with half the size. This allows us to read all the data until
the address that cannot be read. To reduce the number of attempts in case
we try to read completely bogus memory, we first try to read a single
byte at the start of the block, and bail out if that fails. 

The net result is that if the stub fails entire read operation on error, this
fallback code will read smaller blocks, until the error address. If the stub
does return partial data on error, we'll do an extra 1-byte read at the error
address, which does not seem like a performance issue.

Note that with this patch, GDB will return error if we have read exactly zero
bytes; I'm not yet sure that's good, and we should not return a bunch of "N/A"
in that case too. However, if we decide we want "N/A", it's a one-line change.

OK?

- Volodya

        * target.c (target_read_until_error): New.
        * target.h (target_read_until_error): Declare.
        * mi/mi-main.c (mi_cmd_data_read_memory): Use
        target_read_until_error.
Index: src/gdb-stable/gdb/target.c
===================================================================
--- src/gdb-stable/gdb/target.c	(revision 207775)
+++ src/gdb-stable/gdb/target.c	(working copy)
@@ -1412,6 +1412,72 @@
   return len;
 }
 
+LONGEST
+target_read_until_error (struct target_ops *ops,
+			 enum target_object object,
+			 const char *annex, gdb_byte *buf,
+			 ULONGEST offset, LONGEST len)
+{
+  LONGEST xfered = 0;
+  while (xfered < len)
+    {
+      LONGEST xfer = target_read_partial (ops, object, annex,
+					  (gdb_byte *) buf + xfered,
+					  offset + xfered, len - xfered);
+      /* Call an observer, notifying them of the xfer progress?  */
+      if (xfer == 0)
+	return xfered;
+      if (xfer < 0)
+	{
+	  /* We've got an error.  Try to read in smaller blocks.  */
+	  ULONGEST start = offset + xfered;
+	  ULONGEST remaining = len - xfered;
+	  ULONGEST half;
+
+	  /* If an attempt was made to read a random memory address,
+	     it's likely that the very first byte is not accessible.
+	     Try reading the first byte, to avoid doing log N tries
+	     below.  */
+	  xfer = target_read_partial (ops, object, annex, 
+				      (gdb_byte *) buf + xfered, start, 1);
+	  if (xfer <= 0)
+	    return xfered;
+	  start += 1;
+	  remaining -= 1;
+	  half = remaining/2;
+	  
+	  while (half > 0)
+	    {
+	      xfer = target_read_partial (ops, object, annex,
+					  (gdb_byte *) buf + xfered,
+					  start, half);
+	      if (xfer == 0)
+		return xfered;
+	      if (xfer < 0)
+		{
+		  remaining = half;		  
+		}
+	      else
+		{
+		  /* We have successfully read the first half.  So, the
+		     error must be in the second half.  Adjust start and
+		     remaining to point at the second half.  */
+		  xfered += xfer;
+		  start += xfer;
+		  remaining -= xfer;
+		}
+	      half = remaining/2;
+	    }
+
+	  return xfered;
+	}
+      xfered += xfer;
+      QUIT;
+    }
+  return len;
+}
+
+
 /* An alternative to target_write with progress callbacks.  */
 
 LONGEST
Index: src/gdb-stable/gdb/target.h
===================================================================
--- src/gdb-stable/gdb/target.h	(revision 207775)
+++ src/gdb-stable/gdb/target.h	(working copy)
@@ -237,6 +237,11 @@
 			    const char *annex, gdb_byte *buf,
 			    ULONGEST offset, LONGEST len);
 
+extern LONGEST target_read_until_error (struct target_ops *ops,
+					enum target_object object,
+					const char *annex, gdb_byte *buf,
+					ULONGEST offset, LONGEST len);
+  
 extern LONGEST target_write (struct target_ops *ops,
 			     enum target_object object,
 			     const char *annex, const gdb_byte *buf,
Index: src/gdb-stable/gdb/mi/mi-main.c
===================================================================
--- src/gdb-stable/gdb/mi/mi-main.c	(revision 207775)
+++ src/gdb-stable/gdb/mi/mi-main.c	(working copy)
@@ -851,8 +851,8 @@
   mbuf = xcalloc (total_bytes, 1);
   make_cleanup (xfree, mbuf);
 
-  nr_bytes = target_read (&current_target, TARGET_OBJECT_MEMORY, NULL,
-			  mbuf, addr, total_bytes);
+  nr_bytes = target_read_until_error (&current_target, TARGET_OBJECT_MEMORY, 
+				      NULL, mbuf, addr, total_bytes);
   if (nr_bytes <= 0)
     {
       do_cleanups (cleanups);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]