[v2,5/7] target: consider addressable unit size when reading/writing memory

Message ID 557B49FC.5000701@ericsson.com
State New, archived
Headers

Commit Message

Simon Marchi June 12, 2015, 9:07 p.m. UTC
  On 15-05-21 01:46 PM, Pedro Alves wrote:
> On 04/15/2015 08:47 PM, Simon Marchi wrote:
>> If we are reading/writing from a memory object, the length represents
>> the number of "addresses" to read/write, so the addressable unit size
>> needs to be taken into account when allocating memory on gdb's side.
>>
>> gdb/ChangeLog:
>>
>> 	* target.c (target_read): Consider addressable unit size when
>> 	reading from a memory object.
>> 	(read_memory_robust): Same.
>> 	(read_whatever_is_readable): Same.
>> 	(target_write_with_progress): Consider addressable unit size
>> 	when writing to a memory object.
>> 	* target.h (target_read): Update documentation.
>> 	(target_write): Add documentation.
> 
>> --- a/gdb/target.c
>> +++ b/gdb/target.c
>> @@ -1589,6 +1589,15 @@ target_read (struct target_ops *ops,
>>  	     ULONGEST offset, LONGEST len)
>>  {
>>    LONGEST xfered_total = 0;
>> +  int unit_size = 1;
>> +
>> +  /* If we are reading from a memory object, find the length of an addressable
>> +     unit for that architecture.  */
>> +  if (object == TARGET_OBJECT_MEMORY
>> +      || object == TARGET_OBJECT_STACK_MEMORY
>> +      || object == TARGET_OBJECT_CODE_MEMORY
>> +      || object == TARGET_OBJECT_RAW_MEMORY)
>> +    unit_size = gdbarch_addressable_memory_unit_size (target_gdbarch());
> 
> Missing space before parens.  Otherwise looks OK.
> 
> Thanks,
> Pedro Alves

Thanks, pushed with the missing space added. Since we need to post what we
push, here's the same patch with the extra space :).


From d309493c38fcef624f6f85aee4aa37f4f9e3e62a Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Fri, 12 Jun 2015 17:02:44 -0400
Subject: [PATCH] target: consider addressable unit size when reading/writing
 memory

If we are reading/writing from a memory object, the length represents
the number of "addresses" to read/write, so the addressable unit size
needs to be taken into account when allocating memory on gdb's side.

gdb/ChangeLog:

	* target.c (target_read): Consider addressable unit size when
	reading from a memory object.
	(read_memory_robust): Same.
	(read_whatever_is_readable): Same.
	(target_write_with_progress): Consider addressable unit size
	when writing to a memory object.
	* target.h (target_read): Update documentation.
	(target_write): Add documentation.
---
 gdb/ChangeLog | 11 +++++++++++
 gdb/target.c  | 35 ++++++++++++++++++++++++++++-------
 gdb/target.h  | 33 ++++++++++++++++++++++++++-------
 3 files changed, 65 insertions(+), 14 deletions(-)
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1b28992..bc30a8c 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@ 
 2015-06-12  Simon Marchi  <simon.marchi@ericsson.com>

+	* target.c (target_read): Consider addressable unit size when
+	reading from a memory object.
+	(read_memory_robust): Same.
+	(read_whatever_is_readable): Same.
+	(target_write_with_progress): Consider addressable unit size
+	when writing to a memory object.
+	* target.h (target_read): Update documentation.
+	(target_write): Add documentation.
+
+2015-06-12  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* arch-utils.h (default_addressable_memory_unit_size): New.
 	* arch-utils.c (default_addressable_memory_unit_size): New.
 	* gdbarch.sh (addressable_memory_unit_size): New.
diff --git a/gdb/target.c b/gdb/target.c
index dd2393a..4e2d005 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -1593,6 +1593,15 @@  target_read (struct target_ops *ops,
 	     ULONGEST offset, LONGEST len)
 {
   LONGEST xfered_total = 0;
+  int unit_size = 1;
+
+  /* If we are reading from a memory object, find the length of an addressable
+     unit for that architecture.  */
+  if (object == TARGET_OBJECT_MEMORY
+      || object == TARGET_OBJECT_STACK_MEMORY
+      || object == TARGET_OBJECT_CODE_MEMORY
+      || object == TARGET_OBJECT_RAW_MEMORY)
+    unit_size = gdbarch_addressable_memory_unit_size (target_gdbarch ());

   while (xfered_total < len)
     {
@@ -1600,7 +1609,7 @@  target_read (struct target_ops *ops,
       enum target_xfer_status status;

       status = target_read_partial (ops, object, annex,
-				    buf + xfered_total,
+				    buf + xfered_total * unit_size,
 				    offset + xfered_total, len - xfered_total,
 				    &xfered_partial);

@@ -1643,6 +1652,7 @@  target_read (struct target_ops *ops,
 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)
 {
   gdb_byte *buf = xmalloc (end - begin);
@@ -1709,7 +1719,7 @@  read_whatever_is_readable (struct target_ops *ops,
 	}

       xfer = target_read (ops, TARGET_OBJECT_MEMORY, NULL,
-			  buf + (first_half_begin - begin),
+			  buf + (first_half_begin - begin) * unit_size,
 			  first_half_begin,
 			  first_half_end - first_half_begin);

@@ -1745,8 +1755,9 @@  read_whatever_is_readable (struct target_ops *ops,
       /* The [current_end, end) range has been read.  */
       LONGEST region_len = end - current_end;

-      r.data = xmalloc (region_len);
-      memcpy (r.data, buf + current_end - begin, region_len);
+      r.data = xmalloc (region_len * unit_size);
+      memcpy (r.data, buf + (current_end - begin) * unit_size,
+	      region_len * unit_size);
       r.begin = current_end;
       r.end = end;
       xfree (buf);
@@ -1773,6 +1784,7 @@  read_memory_robust (struct target_ops *ops,
 		    const ULONGEST offset, const LONGEST len)
 {
   VEC(memory_read_result_s) *result = 0;
+  int unit_size = gdbarch_addressable_memory_unit_size (target_gdbarch ());

   LONGEST xfered_total = 0;
   while (xfered_total < len)
@@ -1798,7 +1810,7 @@  read_memory_robust (struct target_ops *ops,
       else
 	{
 	  LONGEST to_read = min (len - xfered_total, region_len);
-	  gdb_byte *buffer = (gdb_byte *)xmalloc (to_read);
+	  gdb_byte *buffer = (gdb_byte *) xmalloc (to_read * unit_size);

 	  LONGEST xfered_partial =
 	      target_read (ops, TARGET_OBJECT_MEMORY, NULL,
@@ -1810,7 +1822,7 @@  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,
+	      read_whatever_is_readable (ops, offset + xfered_total, unit_size,
 					 offset + xfered_total + to_read, &result);
 	      xfered_total += to_read;
 	    }
@@ -1840,6 +1852,15 @@  target_write_with_progress (struct target_ops *ops,
 			    void (*progress) (ULONGEST, void *), void *baton)
 {
   LONGEST xfered_total = 0;
+  int unit_size = 1;
+
+  /* If we are writing to a memory object, find the length of an addressable
+     unit for that architecture.  */
+  if (object == TARGET_OBJECT_MEMORY
+      || object == TARGET_OBJECT_STACK_MEMORY
+      || object == TARGET_OBJECT_CODE_MEMORY
+      || object == TARGET_OBJECT_RAW_MEMORY)
+    unit_size = gdbarch_addressable_memory_unit_size (target_gdbarch ());

   /* Give the progress callback a chance to set up.  */
   if (progress)
@@ -1851,7 +1872,7 @@  target_write_with_progress (struct target_ops *ops,
       enum target_xfer_status status;

       status = target_write_partial (ops, object, annex,
-				     (gdb_byte *) buf + xfered_total,
+				     buf + xfered_total * unit_size,
 				     offset + xfered_total, len - xfered_total,
 				     &xfered_partial);

diff --git a/gdb/target.h b/gdb/target.h
index c7046be..32234f7 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -265,13 +265,17 @@  typedef enum target_xfer_status
 			     ULONGEST len,
 			     ULONGEST *xfered_len);

-/* Request that OPS transfer up to LEN 8-bit bytes of the target's
-   OBJECT.  The OFFSET, for a seekable object, specifies the
-   starting point.  The ANNEX can be used to provide additional
-   data-specific information to the target.
-
-   Return the number of bytes actually transfered, or a negative error
-   code (an 'enum target_xfer_error' value) if the transfer is not
+/* Request that OPS transfer up to LEN addressable units of the target's
+   OBJECT.  When reading from a memory object, the size of an addressable unit
+   is architecture dependent and can be found using
+   gdbarch_addressable_memory_unit_size.  Otherwise, an addressable unit is 1
+   byte long.  BUF should point to a buffer large enough to hold the read data,
+   taking into account the addressable unit size.  The OFFSET, for a seekable
+   object, specifies the starting point.  The ANNEX can be used to provide
+   additional data-specific information to the target.
+
+   Return the number of addressable units actually transferred, or a negative
+   error code (an 'enum target_xfer_error' value) if the transfer is not
    supported or otherwise fails.  Return of a positive value less than
    LEN indicates that no further transfer is possible.  Unlike the raw
    to_xfer_partial interface, callers of these functions do not need
@@ -300,6 +304,21 @@  extern VEC(memory_read_result_s)* read_memory_robust (struct target_ops *ops,
 						      const ULONGEST offset,
 						      const LONGEST len);

+/* Request that OPS transfer up to LEN addressable units from BUF to the
+   target's OBJECT.  When writing to a memory object, the addressable unit
+   size is architecture dependent and can be found using
+   gdbarch_addressable_memory_unit_size.  Otherwise, an addressable unit is 1
+   byte long.  The OFFSET, for a seekable object, specifies the starting point.
+   The ANNEX can be used to provide additional data-specific information to
+   the target.
+
+   Return the number of addressable units actually transferred, or a negative
+   error code (an 'enum target_xfer_status' value) if the transfer is not
+   supported or otherwise fails.  Return of a positive value less than
+   LEN indicates that no further transfer is possible.  Unlike the raw
+   to_xfer_partial interface, callers of these functions do not need to
+   retry partial transfers.  */
+
 extern LONGEST target_write (struct target_ops *ops,
 			     enum target_object object,
 			     const char *annex, const gdb_byte *buf,