[PATCH 1/5] Update comment for struct type's length field, introduce type_length_units

[PATCH 1/5] Update comment for struct type's length field, introduce type_length_units

[Simon Marchi]

This patch tries to clean up a bit the blur around the length field in
struct type, regarding its use with architectures with non-8-bits
addressable memory.  It clarifies that the field is expressed in bytes,
which is what is the closest to the current reality.

It also introduces a new function to get the length of the type in
addressable memory units.

gdb/ChangeLog:

	* gdbtypes.c (type_length_units): New function.
	* gdbtypes.h (type_length_units): New declaration.
	(struct type): Update LENGTH's comment.
---
 gdb/gdbtypes.c | 11 +++++++++++
 gdb/gdbtypes.h | 45 +++++++++++++++++++++------------------------
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index e44fd4f..b94bc7b 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -252,6 +252,17 @@ get_target_type (struct type *type)
   return type;
 }
 
+/* See gdbtypes.h.  */
+
+unsigned int
+type_length_units (struct type *type)
+{
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
+  return TYPE_LENGTH (type) / unit_size;
+}
+
 /* Alloc a new type instance structure, fill it with some defaults,
    and point it at OLDTYPE.  Allocate the new type instance from the
    same place as OLDTYPE.  */
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c166e48..83f85a6 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -780,31 +780,23 @@ struct type
      check_typedef.  */
   int instance_flags;
 
-  /* * Length of storage for a value of this type.  This is what
-     sizeof(type) would return; use it for address arithmetic, memory
-     reads and writes, etc.  This size includes padding.  For example,
-     an i386 extended-precision floating point value really only
-     occupies ten bytes, but most ABI's declare its size to be 12
-     bytes, to preserve alignment.  A `struct type' representing such
-     a floating-point type would have a `length' value of 12, even
-     though the last two bytes are unused.
-
-     There's a bit of a host/target mess here, if you're concerned
-     about machines whose bytes aren't eight bits long, or who don't
-     have byte-addressed memory.  Various places pass this to memcpy
-     and such, meaning it must be in units of host bytes.  Various
-     other places expect they can calculate addresses by adding it
-     and such, meaning it must be in units of target bytes.  For
-     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
-     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
-
-     One fix would be to make this field in bits (requiring that it
-     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
-     the other choice would be to make it consistently in units of
-     HOST_CHAR_BIT.  However, this would still fail to address
-     machines based on a ternary or decimal representation.  */
+  /* * Length of storage for a value of this type.  The value is the
+     expression in bytes of of what sizeof(type) would return.  This
+     size includes padding.  For example, an i386 extended-precision
+     floating point value really only occupies ten bytes, but most
+     ABI's declare its size to be 12 bytes, to preserve alignment.
+     A `struct type' representing such a floating-point type would
+     have a `length' value of 12, even though the last two bytes are
+     unused.
+
+     Since this field is expressed in bytes, its value is appropriate to
+     pass to memcpy and such (it is assumed that GDB itself always runs
+     on an 8-bits addressable architecture).  However, when using it for
+     target address arithmetic (e.g. adding it to a target address), the
+     type_length_units function should be used in order to get the length
+     expressed in addressable memory units.  */
   
-  unsigned length;
+  unsigned int length;
 
   /* * Core type, shared by a group of qualified types.  */
 
@@ -1659,6 +1651,11 @@ extern struct gdbarch *get_type_arch (const struct type *);
 
 extern struct type *get_target_type (struct type *type);
 
+/* Return the equivalent of TYPE_LENGTH, but in number of addressable memory
+   units of the associated gdbarch instead of bytes.  */
+
+extern unsigned int type_length_units (struct type *type);
+
 /* * Helper function to construct objfile-owned types.  */
 
 extern struct type *init_type (enum type_code, int, int, const char *,
-- 
2.1.4

[PATCH 3/5] Introduce get_value_arch

[Simon Marchi]

Similar to get_type_arch, used to get the gdbarch associated to a
struct value.

gdb/ChangeLog:

	* value.c (get_value_arch): New function.
	* value.h (get_value_arch): New declaration.
---
 gdb/value.c | 6 ++++++
 gdb/value.h | 4 ++++
 2 files changed, 10 insertions(+)

diff --git a/gdb/value.c b/gdb/value.c
index 6314036..af354de 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -340,6 +340,12 @@ struct value
   VEC(range_s) *optimized_out;
 };
 
+struct gdbarch *
+get_value_arch (const struct value *value)
+{
+  return get_type_arch (value_type (value));
+}
+
 int
 value_bits_available (const struct value *value, int offset, int length)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 7ff6aa8..968882d 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -99,6 +99,10 @@ struct value *value_next (struct value *);
 
 extern struct type *value_type (const struct value *);
 
+/* Return the gdbarch associated to the value. */
+
+extern struct gdbarch *get_value_arch (const struct value *value);
+
 /* This is being used to change the type of an existing value, that
    code should instead be creating a new value with the changed type
    (but possibly shared content).  */
-- 
2.1.4

Re: [PATCH 3/5] Introduce get_value_arch

[Pedro Alves]

On 07/16/2015 07:51 PM, Simon Marchi wrote:
> Similar to get_type_arch, used to get the gdbarch associated to a
> struct value.
> 
> gdb/ChangeLog:
> 
> 	* value.c (get_value_arch): New function.
> 	* value.h (get_value_arch): New declaration.

LGTM, with:

> ---
>  gdb/value.c | 6 ++++++
>  gdb/value.h | 4 ++++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/gdb/value.c b/gdb/value.c
> index 6314036..af354de 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -340,6 +340,12 @@ struct value
>    VEC(range_s) *optimized_out;
>  };
>  

Missing "/* See foo.h.  */ breadcrumb.

> +struct gdbarch *
> +get_value_arch (const struct value *value)
> +{
> +  return get_type_arch (value_type (value));
> +}
> +

Thanks,
-- 
Pedro Alves

Re: [PATCH 3/5] Introduce get_value_arch

[Simon Marchi]

On 15-07-24 07:27 AM, Pedro Alves wrote:
>> diff --git a/gdb/value.c b/gdb/value.c
>> index 6314036..af354de 100644
>> --- a/gdb/value.c
>> +++ b/gdb/value.c
>> @@ -340,6 +340,12 @@ struct value
>>    VEC(range_s) *optimized_out;
>>  };
>>  
> 
> Missing "/* See foo.h.  */ breadcrumb.

Done, updated version:

From 89ef69ae7265ae62b7c0faeb502021262c9d0103 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Fri, 3 Jul 2015 14:36:44 -0400
Subject: [PATCH] Introduce get_value_arch

Similar to get_type_arch, used to get the gdbarch associated to a
struct value.

gdb/ChangeLog:

	* value.c (get_value_arch): New function.
	* value.h (get_value_arch): New declaration.
---
 gdb/value.c | 8 ++++++++
 gdb/value.h | 4 ++++
 2 files changed, 12 insertions(+)

diff --git a/gdb/value.c b/gdb/value.c
index 7fb7e2b..0d540d5 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -340,6 +340,14 @@ struct value
   VEC(range_s) *optimized_out;
 };

+/* See value.h.  */
+
+struct gdbarch *
+get_value_arch (const struct value *value)
+{
+  return get_type_arch (value_type (value));
+}
+
 int
 value_bits_available (const struct value *value, int offset, int length)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 7ff6aa8..968882d 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -99,6 +99,10 @@ struct value *value_next (struct value *);

 extern struct type *value_type (const struct value *);

+/* Return the gdbarch associated to the value. */
+
+extern struct gdbarch *get_value_arch (const struct value *value);
+
 /* This is being used to change the type of an existing value, that
    code should instead be creating a new value with the changed type
    (but possibly shared content).  */
-- 
2.1.4

Re: [PATCH 3/5] Introduce get_value_arch

[Pedro Alves]

On 07/27/2015 10:47 PM, Simon Marchi wrote:
> On 15-07-24 07:27 AM, Pedro Alves wrote:
>>> diff --git a/gdb/value.c b/gdb/value.c
>>> index 6314036..af354de 100644
>>> --- a/gdb/value.c
>>> +++ b/gdb/value.c
>>> @@ -340,6 +340,12 @@ struct value
>>>    VEC(range_s) *optimized_out;
>>>  };
>>>  
>>
>> Missing "/* See foo.h.  */ breadcrumb.
> 
> Done, updated version:
> 
> From 89ef69ae7265ae62b7c0faeb502021262c9d0103 Mon Sep 17 00:00:00 2001
> From: Simon Marchi <simon.marchi@ericsson.com>
> Date: Fri, 3 Jul 2015 14:36:44 -0400
> Subject: [PATCH] Introduce get_value_arch
> 
> Similar to get_type_arch, used to get the gdbarch associated to a
> struct value.
> 

LGTM.

> > +/* Return the gdbarch associated to the value. */

(
FWIW, to my non-native ears, "associated with" sounds more
natural.  I'd write:

/* Return the gdbarch associated with the value's type.  */
)

Thanks,
Pedro Alves

Re: [PATCH 3/5] Introduce get_value_arch

[Simon Marchi]

On 15-07-28 06:25 AM, Pedro Alves wrote:
>>> +/* Return the gdbarch associated to the value. */
> 
> (
> FWIW, to my non-native ears, "associated with" sounds more
> natural.  I'd write:
> 
> /* Return the gdbarch associated with the value's type.  */
> )
> 
> Thanks,
> Pedro Alves

Looks like you are right:
http://english.stackexchange.com/questions/78403/acceptable-uses-for-associated-with-or-associated-to

Re: [PATCH 3/5] Introduce get_value_arch

[Simon Marchi]

On 15-07-28 06:25 AM, Pedro Alves wrote:
> FWIW, to my non-native ears, "associated with" sounds more
> natural.  I'd write:
> 
> /* Return the gdbarch associated with the value's type.  */
> )
> 
> Thanks,
> Pedro Alves

Pushed version:


From e512cdbdffafefa63baeb835ba6636fcef56e17d Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 28 Jul 2015 11:01:50 -0400
Subject: [PATCH] Introduce get_value_arch

Similar to get_type_arch, used to get the gdbarch associated to a
struct value.

gdb/ChangeLog:

	* value.c (get_value_arch): New function.
	* value.h (get_value_arch): New declaration.
---
 gdb/ChangeLog | 5 +++++
 gdb/value.c   | 8 ++++++++
 gdb/value.h   | 4 ++++
 3 files changed, 17 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1e83384..93e054b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2015-07-28  Simon Marchi  <simon.marchi@ericsson.com>

+	* value.c (get_value_arch): New function.
+	* value.h (get_value_arch): New declaration.
+
+2015-07-28  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* value.c (struct value): Update comments.

 2015-07-28  Simon Marchi  <simon.marchi@ericsson.com>
diff --git a/gdb/value.c b/gdb/value.c
index 7fb7e2b..0d540d5 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -340,6 +340,14 @@ struct value
   VEC(range_s) *optimized_out;
 };

+/* See value.h.  */
+
+struct gdbarch *
+get_value_arch (const struct value *value)
+{
+  return get_type_arch (value_type (value));
+}
+
 int
 value_bits_available (const struct value *value, int offset, int length)
 {
diff --git a/gdb/value.h b/gdb/value.h
index 7ff6aa8..e25f52b 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -99,6 +99,10 @@ struct value *value_next (struct value *);

 extern struct type *value_type (const struct value *);

+/* Return the gdbarch associated with the value. */
+
+extern struct gdbarch *get_value_arch (const struct value *value);
+
 /* This is being used to change the type of an existing value, that
    code should instead be creating a new value with the changed type
    (but possibly shared content).  */
-- 
2.1.4

[PATCH 2/5] Update comments in struct value for non-8-bits architectures

[Simon Marchi]

gdb/ChangeLog:

	* value.c (struct value): Update comments.
---
 gdb/value.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/gdb/value.c b/gdb/value.c
index 4399493..6314036 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -234,11 +234,11 @@ struct value
     } computed;
   } location;
 
-  /* Describes offset of a value within lval of a structure in bytes.
-     If lval == lval_memory, this is an offset to the address.  If
-     lval == lval_register, this is a further offset from
-     location.address within the registers structure.  Note also the
-     member embedded_offset below.  */
+  /* Describes offset of a value within lval of a structure in addressable
+     memory units.  If lval == lval_memory, this is an offset to the address.
+     If lval == lval_register, this is a further offset from location.address
+     within the registers structure.  Note also the member embedded_offset
+     below.  */
   int offset;
 
   /* Only used for bitfields; number of bits contained in them.  */
@@ -291,19 +291,19 @@ struct value
 
      When we store the entire object, `enclosing_type' is the run-time
      type -- the complete object -- and `embedded_offset' is the
-     offset of `type' within that larger type, in bytes.  The
-     value_contents() macro takes `embedded_offset' into account, so
+     offset of `type' within that larger type, in addressable memory units.
+     The value_contents() macro takes `embedded_offset' into account, so
      most GDB code continues to see the `type' portion of the value,
      just as the inferior would.
 
      If `type' is a pointer to an object, then `enclosing_type' is a
      pointer to the object's run-time type, and `pointed_to_offset' is
-     the offset in bytes from the full object to the pointed-to object
-     -- that is, the value `embedded_offset' would have if we followed
-     the pointer and fetched the complete object.  (I don't really see
-     the point.  Why not just determine the run-time type when you
-     indirect, and avoid the special case?  The contents don't matter
-     until you indirect anyway.)
+     the offset in addressable memory units from the full object to the
+     pointed-to object -- that is, the value `embedded_offset' would
+     have if we followed the pointer and fetched the complete object.
+     (I don't really see the point.  Why not just determine the
+     run-time type when you indirect, and avoid the special case?  The
+     contents don't matter until you indirect anyway.)
 
      If we're not doing anything fancy, `enclosing_type' is equal to
      `type', and `embedded_offset' is zero, so everything works
-- 
2.1.4

Re: [PATCH 2/5] Update comments in struct value for non-8-bits architectures

[Pedro Alves]

On 07/16/2015 07:51 PM, Simon Marchi wrote:
> gdb/ChangeLog:
> 
> 	* value.c (struct value): Update comments.

Looks good to me, though as mentioned in the other patch, I think
these comments should be explicit in saying "host" and "target".  These are
central structures that people study first, and being crystal clear
should help grasp the byte vs memory units concepts sooner.

Thanks,
Pedro Alves

Re: [PATCH 2/5] Update comments in struct value for non-8-bits architectures

[Simon Marchi]

On 15-07-24 07:27 AM, Pedro Alves wrote:
> On 07/16/2015 07:51 PM, Simon Marchi wrote:
>> gdb/ChangeLog:
>>
>> 	* value.c (struct value): Update comments.
> 
> Looks good to me, though as mentioned in the other patch, I think
> these comments should be explicit in saying "host" and "target".  These are
> central structures that people study first, and being crystal clear
> should help grasp the byte vs memory units concepts sooner.
> 
> Thanks,
> Pedro Alves

Updated version:


From 4441bdcc4e55d2397d2ca7918669a1ccf1676a25 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Thu, 2 Jul 2015 17:52:40 -0400
Subject: [PATCH] Update comments in struct value for non-8-bits architectures

gdb/ChangeLog:

	* value.c (struct value): Update comments.
---
 gdb/value.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/gdb/value.c b/gdb/value.c
index 4399493..7fb7e2b 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -234,11 +234,11 @@ struct value
     } computed;
   } location;

-  /* Describes offset of a value within lval of a structure in bytes.
-     If lval == lval_memory, this is an offset to the address.  If
-     lval == lval_register, this is a further offset from
-     location.address within the registers structure.  Note also the
-     member embedded_offset below.  */
+  /* Describes offset of a value within lval of a structure in target
+     addressable memory units.  If lval == lval_memory, this is an offset to
+     the address.  If lval == lval_register, this is a further offset from
+     location.address within the registers structure.  Note also the member
+     embedded_offset below.  */
   int offset;

   /* Only used for bitfields; number of bits contained in them.  */
@@ -291,19 +291,19 @@ struct value

      When we store the entire object, `enclosing_type' is the run-time
      type -- the complete object -- and `embedded_offset' is the
-     offset of `type' within that larger type, in bytes.  The
-     value_contents() macro takes `embedded_offset' into account, so
-     most GDB code continues to see the `type' portion of the value,
-     just as the inferior would.
+     offset of `type' within that larger type, in target addressable memory
+     units.  The value_contents() macro takes `embedded_offset' into account,
+     so most GDB code continues to see the `type' portion of the value, just
+     as the inferior would.

      If `type' is a pointer to an object, then `enclosing_type' is a
      pointer to the object's run-time type, and `pointed_to_offset' is
-     the offset in bytes from the full object to the pointed-to object
-     -- that is, the value `embedded_offset' would have if we followed
-     the pointer and fetched the complete object.  (I don't really see
-     the point.  Why not just determine the run-time type when you
-     indirect, and avoid the special case?  The contents don't matter
-     until you indirect anyway.)
+     the offset in target addressable memory units from the full object
+     to the pointed-to object -- that is, the value `embedded_offset' would
+     have if we followed the pointer and fetched the complete object.
+     (I don't really see the point.  Why not just determine the
+     run-time type when you indirect, and avoid the special case?  The
+     contents don't matter until you indirect anyway.)

      If we're not doing anything fancy, `enclosing_type' is equal to
      `type', and `embedded_offset' is zero, so everything works
-- 
2.1.4

Re: [PATCH 2/5] Update comments in struct value for non-8-bits architectures

[Pedro Alves]

On 07/27/2015 10:46 PM, Simon Marchi wrote:

> Updated version:

LGTM.

Thanks,
Pedro Alves

[PATCH 4/5] Consider addressable memory unit size in various value functions

[Simon Marchi]

This patch updates various value handling functions to make them
consider the addressable memory unit size of the current architecture.
This allows to correctly extract and print values on architectures whose
addressable memory unit is not 8 bits.

The patch doesn't cover all the code that would ideally need to be
adjusted, only the code paths that we happen to use, plus a few obvious
ones.  Specifically, those areas are not covered by this patch:

 - Management of unavailable bits
 - Bitfields
 - C++ stuff

Regression-tested on x86-64 Ubuntu 14.04.  I saw no related test result
change.

gdb/ChangeLog:

	* c-valprint.c (c_val_print_array): Consider addressable memory
	unit size.
	(c_val_print_ptr): Likewise.
	(c_val_print_int): Likewise.
	* findvar.c (read_frame_register_value): Likewise.
	* valarith.c (find_size_for_pointer_math): Likewise.
	(value_ptrdiff): Likewise.
	(value_subscripted_rvalue): Likewise.
	* valops.c (read_value_memory): Likewise (and rename variables).
	(value_assign): Likewise.
	(value_repeat): Likewise.
	(value_array): Likewise.
	(value_slice): Likewise.
	* valprint.c (generic_val_print): Likewise.
	(val_print_scalar_formatted): Likewise.
	(val_print_array_elements): Likewise.
	* value.c (set_value_parent): Likewise.
	(value_contents_copy_raw): Likewise.
	(set_internalvar_component): Likewise.
	(value_primitive_field): Likewise.
	(value_fetch_lazy): Likewise.
	* value.h (read_value_memory): Update comment.
---
 gdb/c-valprint.c | 24 ++++++++++++++++++------
 gdb/findvar.c    |  4 ++--
 gdb/valarith.c   |  8 ++++----
 gdb/valops.c     | 36 ++++++++++++++++++++----------------
 gdb/valprint.c   | 31 ++++++++++++++++++-------------
 gdb/value.c      | 37 +++++++++++++++++++++++--------------
 gdb/value.h      |  4 ++--
 7 files changed, 87 insertions(+), 57 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 2009e95..0cf2d7d 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -236,6 +236,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 {
   struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
   struct type *elttype = check_typedef (unresolved_elttype);
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
 
   if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (unresolved_elttype) > 0)
     {
@@ -276,7 +278,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	      for (temp_len = 0;
 		   (temp_len < len
 		    && temp_len < options->print_max
-		    && extract_unsigned_integer (valaddr + embedded_offset
+		    && extract_unsigned_integer (valaddr
+						 + embedded_offset * unit_size
 						 + temp_len * eltlen,
 						 eltlen, byte_order) != 0);
 		   ++temp_len)
@@ -288,7 +291,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	      if (temp_len == options->print_max && temp_len < len)
 		{
 		  ULONGEST val
-		    = extract_unsigned_integer (valaddr + embedded_offset
+		    = extract_unsigned_integer (valaddr
+						+ embedded_offset * unit_size
 						+ temp_len * eltlen,
 						eltlen, byte_order);
 		  if (val != 0)
@@ -299,7 +303,7 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	    }
 
 	  LA_PRINT_STRING (stream, unresolved_elttype,
-			   valaddr + embedded_offset, len,
+			   valaddr + embedded_offset * unit_size, len,
 			   NULL, force_ellipses, options);
 	  i = len;
 	}
@@ -342,6 +346,9 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
 		 const struct value *original_value,
 		 const struct value_print_options *options)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   if (options->format && options->format != 's')
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -363,7 +370,8 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
     {
       struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
       struct type *elttype = check_typedef (unresolved_elttype);
-      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+      CORE_ADDR addr = unpack_pointer (type,
+				       valaddr + embedded_offset * unit_size);
 
       print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
 			      embedded_offset, addr, stream, recurse, options);
@@ -430,6 +438,9 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
 		 struct ui_file *stream, const struct value *original_value,
 		 const struct value_print_options *options)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   if (options->format || options->output_format)
     {
       struct value_print_options opts = *options;
@@ -441,7 +452,7 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
     }
   else
     {
-      val_print_type_code_int (type, valaddr + embedded_offset,
+      val_print_type_code_int (type, valaddr + embedded_offset * unit_size,
 			       stream);
       /* C and C++ has no single byte int type, char is used
 	 instead.  Since we don't know whether the value is really
@@ -450,7 +461,8 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
       if (c_textual_element_type (unresolved_type, options->format))
 	{
 	  fputs_filtered (" ", stream);
-	  LA_PRINT_CHAR (unpack_long (type, valaddr + embedded_offset),
+	  LA_PRINT_CHAR (unpack_long (type,
+				      valaddr + embedded_offset * unit_size),
 			 unresolved_type, stream);
 	}
     }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 2079b4b..83b4fca 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -662,7 +662,7 @@ read_frame_register_value (struct value *value, struct frame_info *frame)
   int offset = 0;
   int reg_offset = value_offset (value);
   int regnum = VALUE_REGNUM (value);
-  int len = TYPE_LENGTH (check_typedef (value_type (value)));
+  int len = type_length_units (check_typedef (value_type (value)));
 
   gdb_assert (VALUE_LVAL (value) == lval_register);
 
@@ -677,7 +677,7 @@ read_frame_register_value (struct value *value, struct frame_info *frame)
   while (len > 0)
     {
       struct value *regval = get_frame_register_value (frame, regnum);
-      int reg_len = TYPE_LENGTH (value_type (regval)) - reg_offset;
+      int reg_len = type_length_units (value_type (regval)) - reg_offset;
 
       /* If the register length is larger than the number of bytes
          remaining to copy, then only copy the appropriate bytes.  */
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 0162c10..3e349f2 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -54,7 +54,7 @@ find_size_for_pointer_math (struct type *ptr_type)
   gdb_assert (TYPE_CODE (ptr_type) == TYPE_CODE_PTR);
   ptr_target = check_typedef (TYPE_TARGET_TYPE (ptr_type));
 
-  sz = TYPE_LENGTH (ptr_target);
+  sz = type_length_units (ptr_target);
   if (sz == 0)
     {
       if (TYPE_CODE (ptr_type) == TYPE_CODE_VOID)
@@ -121,7 +121,7 @@ value_ptrdiff (struct value *arg1, struct value *arg2)
 	     "second argument is neither\n"
 	     "an integer nor a pointer of the same type."));
 
-  sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
+  sz = type_length_units (check_typedef (TYPE_TARGET_TYPE (type1)));
   if (sz == 0) 
     {
       warning (_("Type size unknown, assuming 1. "
@@ -192,12 +192,12 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
 {
   struct type *array_type = check_typedef (value_type (array));
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
-  unsigned int elt_size = TYPE_LENGTH (elt_type);
+  unsigned int elt_size = type_length_units (elt_type);
   unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
   struct value *v;
 
   if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
-			     && elt_offs >= TYPE_LENGTH (array_type)))
+			     && elt_offs >= type_length_units (array_type)))
     error (_("no such vector element"));
 
   if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
diff --git a/gdb/valops.c b/gdb/valops.c
index d68e9f3..dac0274 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -958,30 +958,33 @@ read_value_memory (struct value *val, int embedded_offset,
 		   int stack, CORE_ADDR memaddr,
 		   gdb_byte *buffer, size_t length)
 {
-  ULONGEST xfered = 0;
+  ULONGEST xfered_total = 0;
+  struct gdbarch *arch = get_value_arch (val);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
 
-  while (xfered < length)
+  while (xfered_total < length)
     {
       enum target_xfer_status status;
-      ULONGEST xfered_len;
+      ULONGEST xfered_partial;
 
       status = target_xfer_partial (current_target.beneath,
 				    TARGET_OBJECT_MEMORY, NULL,
-				    buffer + xfered, NULL,
-				    memaddr + xfered, length - xfered,
-				    &xfered_len);
+				    buffer + xfered_total * unit_size, NULL,
+				    memaddr + xfered_total,
+				    length - xfered_total,
+				    &xfered_partial);
 
       if (status == TARGET_XFER_OK)
 	/* nothing */;
       else if (status == TARGET_XFER_UNAVAILABLE)
-	mark_value_bytes_unavailable (val, embedded_offset + xfered,
-				      xfered_len);
+	mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
+				      xfered_partial);
       else if (status == TARGET_XFER_EOF)
-	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
+	memory_error (TARGET_XFER_E_IO, memaddr + xfered_total);
       else
-	memory_error (status, memaddr + xfered);
+	memory_error (status, memaddr + xfered_total);
 
-      xfered += xfered_len;
+      xfered_total += xfered_partial;
       QUIT;
     }
 }
@@ -1089,7 +1092,7 @@ value_assign (struct value *toval, struct value *fromval)
 	else
 	  {
 	    changed_addr = value_address (toval);
-	    changed_len = TYPE_LENGTH (type);
+	    changed_len = type_length_units (type);
 	    dest_buffer = value_contents (fromval);
 	  }
 
@@ -1280,7 +1283,7 @@ value_repeat (struct value *arg1, int count)
 
   read_value_memory (val, 0, value_stack (val), value_address (val),
 		     value_contents_all_raw (val),
-		     TYPE_LENGTH (value_enclosing_type (val)));
+		     type_length_units (value_enclosing_type (val)));
 
   return val;
 }
@@ -1606,10 +1609,11 @@ value_array (int lowbound, int highbound, struct value **elemvec)
     {
       error (_("bad array bounds (%d, %d)"), lowbound, highbound);
     }
-  typelength = TYPE_LENGTH (value_enclosing_type (elemvec[0]));
+  typelength = type_length_units (value_enclosing_type (elemvec[0]));
   for (idx = 1; idx < nelem; idx++)
     {
-      if (TYPE_LENGTH (value_enclosing_type (elemvec[idx])) != typelength)
+      if (type_length_units (value_enclosing_type (elemvec[idx]))
+	  != typelength)
 	{
 	  error (_("array elements must all be the same size"));
 	}
@@ -3812,7 +3816,7 @@ value_slice (struct value *array, int lowbound, int length)
       {
 	slice = allocate_value (slice_type);
 	value_contents_copy (slice, 0, array, offset,
-			     TYPE_LENGTH (slice_type));
+			     type_length_units (slice_type));
       }
 
     set_value_component_location (slice, array);
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 52a386a..874b50d 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -379,6 +379,7 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 		   const struct generic_val_print_decorations *decorations)
 {
   struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
   unsigned int i = 0;	/* Number of characters printed.  */
   unsigned len;
   struct type *elttype, *unresolved_elttype;
@@ -431,7 +432,7 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
       unresolved_elttype = TYPE_TARGET_TYPE (type);
       elttype = check_typedef (unresolved_elttype);
 	{
-	  addr = unpack_pointer (type, valaddr + embedded_offset);
+	  addr = unpack_pointer (type, valaddr + embedded_offset * unit_size);
 	print_unpacked_pointer:
 
 	  if (TYPE_CODE (elttype) == TYPE_CODE_FUNC)
@@ -495,7 +496,7 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 	  break;
 	}
       len = TYPE_NFIELDS (type);
-      val = unpack_long (type, valaddr + embedded_offset);
+      val = unpack_long (type, valaddr + embedded_offset * unit_size);
       for (i = 0; i < len; i++)
 	{
 	  QUIT;
@@ -583,7 +584,7 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 	}
       else
 	{
-	  val = unpack_long (type, valaddr + embedded_offset);
+	  val = unpack_long (type, valaddr + embedded_offset * unit_size);
 	  if (val == 0)
 	    fputs_filtered (decorations->false_name, stream);
 	  else if (val == 1)
@@ -615,7 +616,8 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 				      original_value, &opts, 0, stream);
 	}
       else
-	val_print_type_code_int (type, valaddr + embedded_offset, stream);
+	val_print_type_code_int (type, valaddr + embedded_offset * unit_size,
+				 stream);
       break;
 
     case TYPE_CODE_CHAR:
@@ -630,7 +632,7 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 	}
       else
 	{
-	  val = unpack_long (type, valaddr + embedded_offset);
+	  val = unpack_long (type, valaddr + embedded_offset * unit_size);
 	  if (TYPE_UNSIGNED (type))
 	    fprintf_filtered (stream, "%u", (unsigned int) val);
 	  else
@@ -648,7 +650,7 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 	}
       else
 	{
-	  print_floating (valaddr + embedded_offset, type, stream);
+	  print_floating (valaddr + embedded_offset * unit_size, type, stream);
 	}
       break;
 
@@ -657,8 +659,8 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 	val_print_scalar_formatted (type, valaddr, embedded_offset,
 				    original_value, options, 0, stream);
       else
-	print_decimal_floating (valaddr + embedded_offset,
-				type, stream);
+	print_decimal_floating (valaddr + embedded_offset * unit_size, type,
+				stream);
       break;
 
     case TYPE_CODE_VOID:
@@ -684,19 +686,19 @@ generic_val_print (struct type *type, const gdb_byte *valaddr,
 				    valaddr, embedded_offset,
 				    original_value, options, 0, stream);
       else
-	print_floating (valaddr + embedded_offset,
+	print_floating (valaddr + embedded_offset * unit_size,
 			TYPE_TARGET_TYPE (type),
 			stream);
       fprintf_filtered (stream, "%s", decorations->complex_infix);
       if (options->format)
 	val_print_scalar_formatted (TYPE_TARGET_TYPE (type),
 				    valaddr,
-				    embedded_offset
+				    embedded_offset * unit_size
 				    + TYPE_LENGTH (TYPE_TARGET_TYPE (type)),
 				    original_value,
 				    options, 0, stream);
       else
-	print_floating (valaddr + embedded_offset
+	print_floating (valaddr + embedded_offset * unit_size
 			+ TYPE_LENGTH (TYPE_TARGET_TYPE (type)),
 			TYPE_TARGET_TYPE (type),
 			stream);
@@ -964,6 +966,9 @@ val_print_scalar_formatted (struct type *type,
 			    int size,
 			    struct ui_file *stream)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   gdb_assert (val != NULL);
   gdb_assert (valaddr == value_contents_for_printing_const (val));
 
@@ -989,7 +994,7 @@ val_print_scalar_formatted (struct type *type,
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
-    print_scalar_formatted (valaddr + embedded_offset, type,
+    print_scalar_formatted (valaddr + embedded_offset * unit_size, type,
 			    options, size, stream);
 }
 
@@ -1637,7 +1642,7 @@ val_print_array_elements (struct type *type,
   LONGEST low_pos, high_pos;
 
   elttype = TYPE_TARGET_TYPE (type);
-  eltlen = TYPE_LENGTH (check_typedef (elttype));
+  eltlen = type_length_units (check_typedef (elttype));
   index_type = TYPE_INDEX_TYPE (type);
 
   if (get_array_bounds (type, &low_bound, &high_bound))
diff --git a/gdb/value.c b/gdb/value.c
index af354de..7cc67d9 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1089,8 +1089,10 @@ set_value_parent (struct value *value, struct value *parent)
 gdb_byte *
 value_contents_raw (struct value *value)
 {
+  struct gdbarch *arch = get_value_arch (value);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
   allocate_value_contents (value);
-  return value->contents + value->embedded_offset;
+  return value->contents + value->embedded_offset * unit_size;
 }
 
 gdb_byte *
@@ -1240,7 +1242,7 @@ value_ranges_copy_adjusted (struct value *dst, int dst_bit_offset,
 			bit_length);
 }
 
-/* Copy LENGTH bytes of SRC value's (all) contents
+/* Copy LENGTH addressable memory units of SRC value's (all) contents
    (value_contents_all) starting at SRC_OFFSET, into DST value's (all)
    contents, starting at DST_OFFSET.  If unavailable contents are
    being copied from SRC, the corresponding DST contents are marked
@@ -1255,8 +1257,9 @@ value_contents_copy_raw (struct value *dst, int dst_offset,
 			 struct value *src, int src_offset, int length)
 {
   range_s *r;
-  int i;
   int src_bit_offset, dst_bit_offset, bit_length;
+  struct gdbarch *arch = get_value_arch (src);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
 
   /* A lazy DST would make that this copy operation useless, since as
      soon as DST's contents were un-lazied (by a later value_contents
@@ -1273,14 +1276,14 @@ value_contents_copy_raw (struct value *dst, int dst_offset,
 					     TARGET_CHAR_BIT * length));
 
   /* Copy the data.  */
-  memcpy (value_contents_all_raw (dst) + dst_offset,
-	  value_contents_all_raw (src) + src_offset,
-	  length);
+  memcpy (value_contents_all_raw (dst) + dst_offset * unit_size,
+	  value_contents_all_raw (src) + src_offset * unit_size,
+	  length * unit_size);
 
   /* Copy the meta-data, adjusted.  */
-  src_bit_offset = src_offset * TARGET_CHAR_BIT;
-  dst_bit_offset = dst_offset * TARGET_CHAR_BIT;
-  bit_length = length * TARGET_CHAR_BIT;
+  src_bit_offset = src_offset * unit_size * CHAR_BIT;
+  dst_bit_offset = dst_offset * unit_size * CHAR_BIT;
+  bit_length = length * unit_size * CHAR_BIT;
 
   value_ranges_copy_adjusted (dst, dst_bit_offset,
 			      src, src_bit_offset,
@@ -2277,17 +2280,21 @@ set_internalvar_component (struct internalvar *var, int offset, int bitpos,
 			   int bitsize, struct value *newval)
 {
   gdb_byte *addr;
+  struct gdbarch *arch;
+  int unit_size;
 
   switch (var->kind)
     {
     case INTERNALVAR_VALUE:
       addr = value_contents_writeable (var->u.value);
+      arch = get_value_arch (var->u.value);
+      unit_size = gdbarch_addressable_memory_unit_size (arch);
 
       if (bitsize)
 	modify_field (value_type (var->u.value), addr + offset,
 		      value_as_long (newval), bitpos, bitsize);
       else
-	memcpy (addr + offset, value_contents (newval),
+	memcpy (addr + offset * unit_size, value_contents (newval),
 		TYPE_LENGTH (value_type (newval)));
       break;
 
@@ -2998,6 +3005,8 @@ value_primitive_field (struct value *arg1, int offset,
 {
   struct value *v;
   struct type *type;
+  struct gdbarch *arch = get_value_arch (arg1);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
 
   arg_type = check_typedef (arg_type);
   type = TYPE_FIELD_TYPE (arg_type, fieldno);
@@ -3076,7 +3085,7 @@ value_primitive_field (struct value *arg1, int offset,
   else
     {
       /* Plain old data member */
-      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
+      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) / (CHAR_BIT * unit_size);
 
       /* Lazy register values with offsets are not supported.  */
       if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
@@ -3089,7 +3098,7 @@ value_primitive_field (struct value *arg1, int offset,
 	  v = allocate_value (type);
 	  value_contents_copy_raw (v, value_embedded_offset (v),
 				   arg1, value_embedded_offset (arg1) + offset,
-				   TYPE_LENGTH (type));
+				   type_length_units (type));
 	}
       v->offset = (value_offset (arg1) + offset
 		   + value_embedded_offset (arg1));
@@ -3831,7 +3840,7 @@ value_fetch_lazy (struct value *val)
       if (TYPE_LENGTH (type))
 	read_value_memory (val, 0, value_stack (val),
 			   addr, value_contents_all_raw (val),
-			   TYPE_LENGTH (type));
+			   type_length_units (type));
     }
   else if (VALUE_LVAL (val) == lval_register)
     {
@@ -3890,7 +3899,7 @@ value_fetch_lazy (struct value *val)
       set_value_lazy (val, 0);
       value_contents_copy (val, value_embedded_offset (val),
 			   new_val, value_embedded_offset (new_val),
-			   TYPE_LENGTH (type));
+			   type_length_units (type));
 
       if (frame_debug)
 	{
diff --git a/gdb/value.h b/gdb/value.h
index 968882d..36aa860 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -571,8 +571,8 @@ extern int value_contents_eq (const struct value *val1, int offset1,
 			      const struct value *val2, int offset2,
 			      int length);
 
-/* Read LENGTH bytes of memory starting at MEMADDR into BUFFER, which
-   is (or will be copied to) VAL's contents buffer offset by
+/* Read LENGTH addressable memory units starting at MEMADDR into BUFFER,
+   which is (or will be copied to) VAL's contents buffer offset by
    EMBEDDED_OFFSET (that is, to &VAL->contents[EMBEDDED_OFFSET]).
    Marks value contents ranges as unavailable if the corresponding
    memory is likewise unavailable.  STACK indicates whether the memory
-- 
2.1.4

Re: [PATCH 4/5] Consider addressable memory unit size in various value functions

[Pedro Alves]

On 07/16/2015 07:51 PM, Simon Marchi wrote:
> This patch updates various value handling functions to make them
> consider the addressable memory unit size of the current architecture.
> This allows to correctly extract and print values on architectures whose
> addressable memory unit is not 8 bits.
> 
> The patch doesn't cover all the code that would ideally need to be
> adjusted, only the code paths that we happen to use, plus a few obvious
> ones.  Specifically, those areas are not covered by this patch:
> 
>  - Management of unavailable bits
>  - Bitfields
>  - C++ stuff
> 
> Regression-tested on x86-64 Ubuntu 14.04.  I saw no related test result
> change.

LGTM, with:

> diff --git a/gdb/value.c b/gdb/value.c
> index af354de..7cc67d9 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -1089,8 +1089,10 @@ set_value_parent (struct value *value, struct value *parent)
>  gdb_byte *
>  value_contents_raw (struct value *value)
>  {
> +  struct gdbarch *arch = get_value_arch (value);
> +  int unit_size = gdbarch_addressable_memory_unit_size (arch);

Missing line break.

>    allocate_value_contents (value);
> -  return value->contents + value->embedded_offset;
> +  return value->contents + value->embedded_offset * unit_size;
>  }
>  

>  
>    /* Copy the meta-data, adjusted.  */
> -  src_bit_offset = src_offset * TARGET_CHAR_BIT;
> -  dst_bit_offset = dst_offset * TARGET_CHAR_BIT;
> -  bit_length = length * TARGET_CHAR_BIT;
> +  src_bit_offset = src_offset * unit_size * CHAR_BIT;
> +  dst_bit_offset = dst_offset * unit_size * CHAR_BIT;
> +  bit_length = length * unit_size * CHAR_BIT;

AFAICS, we don't use CHAR_BIT anywhere.  Instead, use HOST_CHAR_BIT,
which has a fallback definition in common/host-defs.h.

Thanks,
Pedro Alves

Re: [PATCH 4/5] Consider addressable memory unit size in various value functions

[Simon Marchi]

On 15-07-24 07:27 AM, Pedro Alves wrote:
> LGTM, with:
> 
>> diff --git a/gdb/value.c b/gdb/value.c
>> index af354de..7cc67d9 100644
>> --- a/gdb/value.c
>> +++ b/gdb/value.c
>> @@ -1089,8 +1089,10 @@ set_value_parent (struct value *value, struct value *parent)
>>  gdb_byte *
>>  value_contents_raw (struct value *value)
>>  {
>> +  struct gdbarch *arch = get_value_arch (value);
>> +  int unit_size = gdbarch_addressable_memory_unit_size (arch);
> 
> Missing line break.

Done.

>>    allocate_value_contents (value);
>> -  return value->contents + value->embedded_offset;
>> +  return value->contents + value->embedded_offset * unit_size;
>>  }
>>  
> 
>>  
>>    /* Copy the meta-data, adjusted.  */
>> -  src_bit_offset = src_offset * TARGET_CHAR_BIT;
>> -  dst_bit_offset = dst_offset * TARGET_CHAR_BIT;
>> -  bit_length = length * TARGET_CHAR_BIT;
>> +  src_bit_offset = src_offset * unit_size * CHAR_BIT;
>> +  dst_bit_offset = dst_offset * unit_size * CHAR_BIT;
>> +  bit_length = length * unit_size * CHAR_BIT;
> 
> AFAICS, we don't use CHAR_BIT anywhere.  Instead, use HOST_CHAR_BIT,
> which has a fallback definition in common/host-defs.h.

Done.

Updated patch. I needed to update it after the series that split generic_val_print.


From ccd5044ed7b1eb70b0d79de922d3ee9bcd1c6e42 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Fri, 10 Jul 2015 13:45:59 -0400
Subject: [PATCH] Consider addressable memory unit size in various value
 functions

This patch updates various value handling functions to make them
consider the addressable memory unit size of the current architecture.
This allows to correctly extract and print values on architectures whose
addressable memory unit is not 8 bits.

The patch doesn't cover all the code that would ideally need to be
adjusted, only the code paths that we happen to use, plus a few obvious
ones.  Specifically, those areas are not covered by this patch:

 - Management of unavailable bits
 - Bitfields
 - C++ stuff

Regression-tested on x86-64 Ubuntu 14.04.  I saw no related test result
change.

gdb/ChangeLog:

	* c-valprint.c (c_val_print_array): Consider addressable memory
	unit size.
	(c_val_print_ptr): Likewise.
	(c_val_print_int): Likewise.
	* findvar.c (read_frame_register_value): Likewise.
	* valarith.c (find_size_for_pointer_math): Likewise.
	(value_ptrdiff): Likewise.
	(value_subscripted_rvalue): Likewise.
	* valops.c (read_value_memory): Likewise (and rename variables).
	(value_assign): Likewise.
	(value_repeat): Likewise.
	(value_array): Likewise.
	(value_slice): Likewise.
	* valprint.c (generic_val_print_ptr): Likewise.
	(generic_val_print_enum): Likewise.
	(generic_val_print_bool): Likewise.
	(generic_val_print_int): Likewise.
	(generic_val_print_char): Likewise.
	(generic_val_print_float): Likewise.
	(generic_val_print_decfloat): Likewise.
	(generic_val_print_complex): Likewise.
	(val_print_scalar_formatted): Likewise.
	(val_print_array_elements): Likewise.
	* value.c (set_value_parent): Likewise.
	(value_contents_copy_raw): Likewise.
	(set_internalvar_component): Likewise.
	(value_primitive_field): Likewise.
	(value_fetch_lazy): Likewise.
	* value.h (read_value_memory): Update comment.
---
 gdb/c-valprint.c | 24 ++++++++++++++++++------
 gdb/findvar.c    |  4 ++--
 gdb/valarith.c   |  8 ++++----
 gdb/valops.c     | 36 ++++++++++++++++++++----------------
 gdb/valprint.c   | 53 ++++++++++++++++++++++++++++++++++++++++-------------
 gdb/value.c      | 39 +++++++++++++++++++++++++--------------
 gdb/value.h      |  4 ++--
 7 files changed, 111 insertions(+), 57 deletions(-)

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 2009e95..0cf2d7d 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -236,6 +236,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 {
   struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
   struct type *elttype = check_typedef (unresolved_elttype);
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

   if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (unresolved_elttype) > 0)
     {
@@ -276,7 +278,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	      for (temp_len = 0;
 		   (temp_len < len
 		    && temp_len < options->print_max
-		    && extract_unsigned_integer (valaddr + embedded_offset
+		    && extract_unsigned_integer (valaddr
+						 + embedded_offset * unit_size
 						 + temp_len * eltlen,
 						 eltlen, byte_order) != 0);
 		   ++temp_len)
@@ -288,7 +291,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	      if (temp_len == options->print_max && temp_len < len)
 		{
 		  ULONGEST val
-		    = extract_unsigned_integer (valaddr + embedded_offset
+		    = extract_unsigned_integer (valaddr
+						+ embedded_offset * unit_size
 						+ temp_len * eltlen,
 						eltlen, byte_order);
 		  if (val != 0)
@@ -299,7 +303,7 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	    }

 	  LA_PRINT_STRING (stream, unresolved_elttype,
-			   valaddr + embedded_offset, len,
+			   valaddr + embedded_offset * unit_size, len,
 			   NULL, force_ellipses, options);
 	  i = len;
 	}
@@ -342,6 +346,9 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
 		 const struct value *original_value,
 		 const struct value_print_options *options)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   if (options->format && options->format != 's')
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -363,7 +370,8 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
     {
       struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
       struct type *elttype = check_typedef (unresolved_elttype);
-      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+      CORE_ADDR addr = unpack_pointer (type,
+				       valaddr + embedded_offset * unit_size);

       print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
 			      embedded_offset, addr, stream, recurse, options);
@@ -430,6 +438,9 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
 		 struct ui_file *stream, const struct value *original_value,
 		 const struct value_print_options *options)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   if (options->format || options->output_format)
     {
       struct value_print_options opts = *options;
@@ -441,7 +452,7 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
     }
   else
     {
-      val_print_type_code_int (type, valaddr + embedded_offset,
+      val_print_type_code_int (type, valaddr + embedded_offset * unit_size,
 			       stream);
       /* C and C++ has no single byte int type, char is used
 	 instead.  Since we don't know whether the value is really
@@ -450,7 +461,8 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
       if (c_textual_element_type (unresolved_type, options->format))
 	{
 	  fputs_filtered (" ", stream);
-	  LA_PRINT_CHAR (unpack_long (type, valaddr + embedded_offset),
+	  LA_PRINT_CHAR (unpack_long (type,
+				      valaddr + embedded_offset * unit_size),
 			 unresolved_type, stream);
 	}
     }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 2079b4b..83b4fca 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -662,7 +662,7 @@ read_frame_register_value (struct value *value, struct frame_info *frame)
   int offset = 0;
   int reg_offset = value_offset (value);
   int regnum = VALUE_REGNUM (value);
-  int len = TYPE_LENGTH (check_typedef (value_type (value)));
+  int len = type_length_units (check_typedef (value_type (value)));

   gdb_assert (VALUE_LVAL (value) == lval_register);

@@ -677,7 +677,7 @@ read_frame_register_value (struct value *value, struct frame_info *frame)
   while (len > 0)
     {
       struct value *regval = get_frame_register_value (frame, regnum);
-      int reg_len = TYPE_LENGTH (value_type (regval)) - reg_offset;
+      int reg_len = type_length_units (value_type (regval)) - reg_offset;

       /* If the register length is larger than the number of bytes
          remaining to copy, then only copy the appropriate bytes.  */
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 0162c10..3e349f2 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -54,7 +54,7 @@ find_size_for_pointer_math (struct type *ptr_type)
   gdb_assert (TYPE_CODE (ptr_type) == TYPE_CODE_PTR);
   ptr_target = check_typedef (TYPE_TARGET_TYPE (ptr_type));

-  sz = TYPE_LENGTH (ptr_target);
+  sz = type_length_units (ptr_target);
   if (sz == 0)
     {
       if (TYPE_CODE (ptr_type) == TYPE_CODE_VOID)
@@ -121,7 +121,7 @@ value_ptrdiff (struct value *arg1, struct value *arg2)
 	     "second argument is neither\n"
 	     "an integer nor a pointer of the same type."));

-  sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
+  sz = type_length_units (check_typedef (TYPE_TARGET_TYPE (type1)));
   if (sz == 0)
     {
       warning (_("Type size unknown, assuming 1. "
@@ -192,12 +192,12 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
 {
   struct type *array_type = check_typedef (value_type (array));
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
-  unsigned int elt_size = TYPE_LENGTH (elt_type);
+  unsigned int elt_size = type_length_units (elt_type);
   unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
   struct value *v;

   if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
-			     && elt_offs >= TYPE_LENGTH (array_type)))
+			     && elt_offs >= type_length_units (array_type)))
     error (_("no such vector element"));

   if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
diff --git a/gdb/valops.c b/gdb/valops.c
index d68e9f3..dac0274 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -958,30 +958,33 @@ read_value_memory (struct value *val, int embedded_offset,
 		   int stack, CORE_ADDR memaddr,
 		   gdb_byte *buffer, size_t length)
 {
-  ULONGEST xfered = 0;
+  ULONGEST xfered_total = 0;
+  struct gdbarch *arch = get_value_arch (val);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

-  while (xfered < length)
+  while (xfered_total < length)
     {
       enum target_xfer_status status;
-      ULONGEST xfered_len;
+      ULONGEST xfered_partial;

       status = target_xfer_partial (current_target.beneath,
 				    TARGET_OBJECT_MEMORY, NULL,
-				    buffer + xfered, NULL,
-				    memaddr + xfered, length - xfered,
-				    &xfered_len);
+				    buffer + xfered_total * unit_size, NULL,
+				    memaddr + xfered_total,
+				    length - xfered_total,
+				    &xfered_partial);

       if (status == TARGET_XFER_OK)
 	/* nothing */;
       else if (status == TARGET_XFER_UNAVAILABLE)
-	mark_value_bytes_unavailable (val, embedded_offset + xfered,
-				      xfered_len);
+	mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
+				      xfered_partial);
       else if (status == TARGET_XFER_EOF)
-	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
+	memory_error (TARGET_XFER_E_IO, memaddr + xfered_total);
       else
-	memory_error (status, memaddr + xfered);
+	memory_error (status, memaddr + xfered_total);

-      xfered += xfered_len;
+      xfered_total += xfered_partial;
       QUIT;
     }
 }
@@ -1089,7 +1092,7 @@ value_assign (struct value *toval, struct value *fromval)
 	else
 	  {
 	    changed_addr = value_address (toval);
-	    changed_len = TYPE_LENGTH (type);
+	    changed_len = type_length_units (type);
 	    dest_buffer = value_contents (fromval);
 	  }

@@ -1280,7 +1283,7 @@ value_repeat (struct value *arg1, int count)

   read_value_memory (val, 0, value_stack (val), value_address (val),
 		     value_contents_all_raw (val),
-		     TYPE_LENGTH (value_enclosing_type (val)));
+		     type_length_units (value_enclosing_type (val)));

   return val;
 }
@@ -1606,10 +1609,11 @@ value_array (int lowbound, int highbound, struct value **elemvec)
     {
       error (_("bad array bounds (%d, %d)"), lowbound, highbound);
     }
-  typelength = TYPE_LENGTH (value_enclosing_type (elemvec[0]));
+  typelength = type_length_units (value_enclosing_type (elemvec[0]));
   for (idx = 1; idx < nelem; idx++)
     {
-      if (TYPE_LENGTH (value_enclosing_type (elemvec[idx])) != typelength)
+      if (type_length_units (value_enclosing_type (elemvec[idx]))
+	  != typelength)
 	{
 	  error (_("array elements must all be the same size"));
 	}
@@ -3812,7 +3816,7 @@ value_slice (struct value *array, int lowbound, int length)
       {
 	slice = allocate_value (slice_type);
 	value_contents_copy (slice, 0, array, offset,
-			     TYPE_LENGTH (slice_type));
+			     type_length_units (slice_type));
       }

     set_value_component_location (slice, array);
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 8893830..713998c 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -433,6 +433,9 @@ generic_val_print_ptr (struct type *type, const gdb_byte *valaddr,
 		       const struct value *original_value,
 		       const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format && options->format != 's')
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -442,7 +445,8 @@ generic_val_print_ptr (struct type *type, const gdb_byte *valaddr,
     {
       struct type *unresolved_elttype = TYPE_TARGET_TYPE(type);
       struct type *elttype = check_typedef (unresolved_elttype);
-      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+      CORE_ADDR addr = unpack_pointer (type,
+				       valaddr + embedded_offset * unit_size);

       print_unpacked_pointer (type, elttype, addr, stream, options);
     }
@@ -520,6 +524,8 @@ generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
   unsigned int i;
   unsigned int len;
   LONGEST val;
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);

   if (options->format)
     {
@@ -528,7 +534,7 @@ generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
       return;
     }
   len = TYPE_NFIELDS (type);
-  val = unpack_long (type, valaddr + embedded_offset);
+  val = unpack_long (type, valaddr + embedded_offset * unit_size);
   for (i = 0; i < len; i++)
     {
       QUIT;
@@ -633,6 +639,8 @@ generic_val_print_bool (struct type *type, const gdb_byte *valaddr,
 			const struct generic_val_print_decorations *decorations)
 {
   LONGEST val;
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);

   if (options->format || options->output_format)
     {
@@ -644,7 +652,7 @@ generic_val_print_bool (struct type *type, const gdb_byte *valaddr,
     }
   else
     {
-      val = unpack_long (type, valaddr + embedded_offset);
+      val = unpack_long (type, valaddr + embedded_offset * unit_size);
       if (val == 0)
 	fputs_filtered (decorations->false_name, stream);
       else if (val == 1)
@@ -662,6 +670,9 @@ generic_val_print_int (struct type *type, const gdb_byte *valaddr,
 		       const struct value *original_value,
 		       const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format || options->output_format)
     {
       struct value_print_options opts = *options;
@@ -672,7 +683,8 @@ generic_val_print_int (struct type *type, const gdb_byte *valaddr,
 				  original_value, &opts, 0, stream);
     }
   else
-    val_print_type_code_int (type, valaddr + embedded_offset, stream);
+    val_print_type_code_int (type, valaddr + embedded_offset * unit_size,
+			     stream);
 }

 /* generic_val_print helper for TYPE_CODE_CHAR.  */
@@ -685,6 +697,8 @@ generic_val_print_char (struct type *type, struct type *unresolved_type,
 			const struct value_print_options *options)
 {
   LONGEST val;
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);

   if (options->format || options->output_format)
     {
@@ -697,7 +711,7 @@ generic_val_print_char (struct type *type, struct type *unresolved_type,
     }
   else
     {
-      val = unpack_long (type, valaddr + embedded_offset);
+      val = unpack_long (type, valaddr + embedded_offset * unit_size);
       if (TYPE_UNSIGNED (type))
 	fprintf_filtered (stream, "%u", (unsigned int) val);
       else
@@ -715,6 +729,9 @@ generic_val_print_float (struct type *type, const gdb_byte *valaddr,
 			 const struct value *original_value,
 			 const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format)
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -722,7 +739,7 @@ generic_val_print_float (struct type *type, const gdb_byte *valaddr,
     }
   else
     {
-      print_floating (valaddr + embedded_offset, type, stream);
+      print_floating (valaddr + embedded_offset * unit_size, type, stream);
     }
 }

@@ -734,11 +751,15 @@ generic_val_print_decfloat (struct type *type, const gdb_byte *valaddr,
 			    const struct value *original_value,
 			    const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format)
     val_print_scalar_formatted (type, valaddr, embedded_offset, original_value,
 				options, 0, stream);
   else
-    print_decimal_floating (valaddr + embedded_offset, type, stream);
+    print_decimal_floating (valaddr + embedded_offset * unit_size, type,
+			    stream);
 }

 /* generic_val_print helper for TYPE_CODE_COMPLEX.  */
@@ -751,22 +772,25 @@ generic_val_print_complex (struct type *type, const gdb_byte *valaddr,
 			   const struct generic_val_print_decorations
 			     *decorations)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   fprintf_filtered (stream, "%s", decorations->complex_prefix);
   if (options->format)
     val_print_scalar_formatted (TYPE_TARGET_TYPE (type), valaddr,
 				embedded_offset, original_value, options, 0,
 				stream);
   else
-    print_floating (valaddr + embedded_offset, TYPE_TARGET_TYPE (type),
-		    stream);
+    print_floating (valaddr + embedded_offset * unit_size,
+		    TYPE_TARGET_TYPE (type), stream);
   fprintf_filtered (stream, "%s", decorations->complex_infix);
   if (options->format)
     val_print_scalar_formatted (TYPE_TARGET_TYPE (type), valaddr,
 				embedded_offset
-				+ TYPE_LENGTH (TYPE_TARGET_TYPE (type)),
+				+ type_length_units (TYPE_TARGET_TYPE (type)),
 				original_value, options, 0, stream);
   else
-    print_floating (valaddr + embedded_offset
+    print_floating (valaddr + embedded_offset * unit_size
 		    + TYPE_LENGTH (TYPE_TARGET_TYPE (type)),
 		    TYPE_TARGET_TYPE (type), stream);
   fprintf_filtered (stream, "%s", decorations->complex_suffix);
@@ -1150,6 +1174,9 @@ val_print_scalar_formatted (struct type *type,
 			    int size,
 			    struct ui_file *stream)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   gdb_assert (val != NULL);
   gdb_assert (valaddr == value_contents_for_printing_const (val));

@@ -1175,7 +1202,7 @@ val_print_scalar_formatted (struct type *type,
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
-    print_scalar_formatted (valaddr + embedded_offset, type,
+    print_scalar_formatted (valaddr + embedded_offset * unit_size, type,
 			    options, size, stream);
 }

@@ -1823,7 +1850,7 @@ val_print_array_elements (struct type *type,
   LONGEST low_pos, high_pos;

   elttype = TYPE_TARGET_TYPE (type);
-  eltlen = TYPE_LENGTH (check_typedef (elttype));
+  eltlen = type_length_units (check_typedef (elttype));
   index_type = TYPE_INDEX_TYPE (type);

   if (get_array_bounds (type, &low_bound, &high_bound))
diff --git a/gdb/value.c b/gdb/value.c
index 0d540d5..d9b3947 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1091,8 +1091,11 @@ set_value_parent (struct value *value, struct value *parent)
 gdb_byte *
 value_contents_raw (struct value *value)
 {
+  struct gdbarch *arch = get_value_arch (value);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   allocate_value_contents (value);
-  return value->contents + value->embedded_offset;
+  return value->contents + value->embedded_offset * unit_size;
 }

 gdb_byte *
@@ -1242,7 +1245,7 @@ value_ranges_copy_adjusted (struct value *dst, int dst_bit_offset,
 			bit_length);
 }

-/* Copy LENGTH bytes of SRC value's (all) contents
+/* Copy LENGTH target addressable memory units of SRC value's (all) contents
    (value_contents_all) starting at SRC_OFFSET, into DST value's (all)
    contents, starting at DST_OFFSET.  If unavailable contents are
    being copied from SRC, the corresponding DST contents are marked
@@ -1257,8 +1260,9 @@ value_contents_copy_raw (struct value *dst, int dst_offset,
 			 struct value *src, int src_offset, int length)
 {
   range_s *r;
-  int i;
   int src_bit_offset, dst_bit_offset, bit_length;
+  struct gdbarch *arch = get_value_arch (src);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

   /* A lazy DST would make that this copy operation useless, since as
      soon as DST's contents were un-lazied (by a later value_contents
@@ -1275,14 +1279,14 @@ value_contents_copy_raw (struct value *dst, int dst_offset,
 					     TARGET_CHAR_BIT * length));

   /* Copy the data.  */
-  memcpy (value_contents_all_raw (dst) + dst_offset,
-	  value_contents_all_raw (src) + src_offset,
-	  length);
+  memcpy (value_contents_all_raw (dst) + dst_offset * unit_size,
+	  value_contents_all_raw (src) + src_offset * unit_size,
+	  length * unit_size);

   /* Copy the meta-data, adjusted.  */
-  src_bit_offset = src_offset * TARGET_CHAR_BIT;
-  dst_bit_offset = dst_offset * TARGET_CHAR_BIT;
-  bit_length = length * TARGET_CHAR_BIT;
+  src_bit_offset = src_offset * unit_size * HOST_CHAR_BIT;
+  dst_bit_offset = dst_offset * unit_size * HOST_CHAR_BIT;
+  bit_length = length * unit_size * HOST_CHAR_BIT;

   value_ranges_copy_adjusted (dst, dst_bit_offset,
 			      src, src_bit_offset,
@@ -2279,17 +2283,21 @@ set_internalvar_component (struct internalvar *var, int offset, int bitpos,
 			   int bitsize, struct value *newval)
 {
   gdb_byte *addr;
+  struct gdbarch *arch;
+  int unit_size;

   switch (var->kind)
     {
     case INTERNALVAR_VALUE:
       addr = value_contents_writeable (var->u.value);
+      arch = get_value_arch (var->u.value);
+      unit_size = gdbarch_addressable_memory_unit_size (arch);

       if (bitsize)
 	modify_field (value_type (var->u.value), addr + offset,
 		      value_as_long (newval), bitpos, bitsize);
       else
-	memcpy (addr + offset, value_contents (newval),
+	memcpy (addr + offset * unit_size, value_contents (newval),
 		TYPE_LENGTH (value_type (newval)));
       break;

@@ -3000,6 +3008,8 @@ value_primitive_field (struct value *arg1, int offset,
 {
   struct value *v;
   struct type *type;
+  struct gdbarch *arch = get_value_arch (arg1);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

   arg_type = check_typedef (arg_type);
   type = TYPE_FIELD_TYPE (arg_type, fieldno);
@@ -3078,7 +3088,8 @@ value_primitive_field (struct value *arg1, int offset,
   else
     {
       /* Plain old data member */
-      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
+      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) /
+	          (HOST_CHAR_BIT * unit_size);

       /* Lazy register values with offsets are not supported.  */
       if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
@@ -3091,7 +3102,7 @@ value_primitive_field (struct value *arg1, int offset,
 	  v = allocate_value (type);
 	  value_contents_copy_raw (v, value_embedded_offset (v),
 				   arg1, value_embedded_offset (arg1) + offset,
-				   TYPE_LENGTH (type));
+				   type_length_units (type));
 	}
       v->offset = (value_offset (arg1) + offset
 		   + value_embedded_offset (arg1));
@@ -3833,7 +3844,7 @@ value_fetch_lazy (struct value *val)
       if (TYPE_LENGTH (type))
 	read_value_memory (val, 0, value_stack (val),
 			   addr, value_contents_all_raw (val),
-			   TYPE_LENGTH (type));
+			   type_length_units (type));
     }
   else if (VALUE_LVAL (val) == lval_register)
     {
@@ -3892,7 +3903,7 @@ value_fetch_lazy (struct value *val)
       set_value_lazy (val, 0);
       value_contents_copy (val, value_embedded_offset (val),
 			   new_val, value_embedded_offset (new_val),
-			   TYPE_LENGTH (type));
+			   type_length_units (type));

       if (frame_debug)
 	{
diff --git a/gdb/value.h b/gdb/value.h
index 968882d..36aa860 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -571,8 +571,8 @@ extern int value_contents_eq (const struct value *val1, int offset1,
 			      const struct value *val2, int offset2,
 			      int length);

-/* Read LENGTH bytes of memory starting at MEMADDR into BUFFER, which
-   is (or will be copied to) VAL's contents buffer offset by
+/* Read LENGTH addressable memory units starting at MEMADDR into BUFFER,
+   which is (or will be copied to) VAL's contents buffer offset by
    EMBEDDED_OFFSET (that is, to &VAL->contents[EMBEDDED_OFFSET]).
    Marks value contents ranges as unavailable if the corresponding
    memory is likewise unavailable.  STACK indicates whether the memory
-- 
2.1.4

Re: [PATCH 4/5] Consider addressable memory unit size in various value functions

[Pedro Alves]

On 07/27/2015 11:05 PM, Simon Marchi wrote:
> -      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
> +      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) /
> +	          (HOST_CHAR_BIT * unit_size);

wrap in parens so alignment ends up correct (per gnu coding conventions),
and put / operator at beginning of line:

 offset += (TYPE_FIELD_BITPOS (arg_type, fieldno)
	    / (HOST_CHAR_BIT * unit_size));

OK with that change.

Thanks,
Pedro Alves

Re: [PATCH 4/5] Consider addressable memory unit size in various value functions

[Simon Marchi]

On 15-07-28 06:29 AM, Pedro Alves wrote:
> On 07/27/2015 11:05 PM, Simon Marchi wrote:
>> -      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
>> +      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) /
>> +	          (HOST_CHAR_BIT * unit_size);
> 
> wrap in parens so alignment ends up correct (per gnu coding conventions),
> and put / operator at beginning of line:
> 
>  offset += (TYPE_FIELD_BITPOS (arg_type, fieldno)
> 	    / (HOST_CHAR_BIT * unit_size));
> 
> OK with that change.
> 
> Thanks,
> Pedro Alves

Final version, with the formatting fixed:


From 3ae385afe150f2e001a1cc8fb14f4ba0ab94cdf2 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 28 Jul 2015 11:01:50 -0400
Subject: [PATCH] Consider addressable memory unit size in various value
 functions

This patch updates various value handling functions to make them
consider the addressable memory unit size of the current architecture.
This allows to correctly extract and print values on architectures whose
addressable memory unit is not 8 bits.

The patch doesn't cover all the code that would ideally need to be
adjusted, only the code paths that we happen to use, plus a few obvious
ones.  Specifically, those areas are not covered by this patch:

 - Management of unavailable bits
 - Bitfields
 - C++ stuff

Regression-tested on x86-64 Ubuntu 14.04.  I saw no related test result
change.

gdb/ChangeLog:

	* c-valprint.c (c_val_print_array): Consider addressable memory
	unit size.
	(c_val_print_ptr): Likewise.
	(c_val_print_int): Likewise.
	* findvar.c (read_frame_register_value): Likewise.
	* valarith.c (find_size_for_pointer_math): Likewise.
	(value_ptrdiff): Likewise.
	(value_subscripted_rvalue): Likewise.
	* valops.c (read_value_memory): Likewise (and rename variables).
	(value_assign): Likewise.
	(value_repeat): Likewise.
	(value_array): Likewise.
	(value_slice): Likewise.
	* valprint.c (generic_val_print_ptr): Likewise.
	(generic_val_print_enum): Likewise.
	(generic_val_print_bool): Likewise.
	(generic_val_print_int): Likewise.
	(generic_val_print_char): Likewise.
	(generic_val_print_float): Likewise.
	(generic_val_print_decfloat): Likewise.
	(generic_val_print_complex): Likewise.
	(val_print_scalar_formatted): Likewise.
	(val_print_array_elements): Likewise.
	* value.c (set_value_parent): Likewise.
	(value_contents_copy_raw): Likewise.
	(set_internalvar_component): Likewise.
	(value_primitive_field): Likewise.
	(value_fetch_lazy): Likewise.
	* value.h (read_value_memory): Update comment.
---
 gdb/ChangeLog    | 32 ++++++++++++++++++++++++++++++++
 gdb/c-valprint.c | 24 ++++++++++++++++++------
 gdb/findvar.c    |  4 ++--
 gdb/valarith.c   |  8 ++++----
 gdb/valops.c     | 36 ++++++++++++++++++++----------------
 gdb/valprint.c   | 53 ++++++++++++++++++++++++++++++++++++++++-------------
 gdb/value.c      | 39 +++++++++++++++++++++++++--------------
 gdb/value.h      |  4 ++--
 8 files changed, 143 insertions(+), 57 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 93e054b..f0220f5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,37 @@
 2015-07-28  Simon Marchi  <simon.marchi@ericsson.com>

+	* c-valprint.c (c_val_print_array): Consider addressable memory
+	unit size.
+	(c_val_print_ptr): Likewise.
+	(c_val_print_int): Likewise.
+	* findvar.c (read_frame_register_value): Likewise.
+	* valarith.c (find_size_for_pointer_math): Likewise.
+	(value_ptrdiff): Likewise.
+	(value_subscripted_rvalue): Likewise.
+	* valops.c (read_value_memory): Likewise (and rename variables).
+	(value_assign): Likewise.
+	(value_repeat): Likewise.
+	(value_array): Likewise.
+	(value_slice): Likewise.
+	* valprint.c (generic_val_print_ptr): Likewise.
+	(generic_val_print_enum): Likewise.
+	(generic_val_print_bool): Likewise.
+	(generic_val_print_int): Likewise.
+	(generic_val_print_char): Likewise.
+	(generic_val_print_float): Likewise.
+	(generic_val_print_decfloat): Likewise.
+	(generic_val_print_complex): Likewise.
+	(val_print_scalar_formatted): Likewise.
+	(val_print_array_elements): Likewise.
+	* value.c (set_value_parent): Likewise.
+	(value_contents_copy_raw): Likewise.
+	(set_internalvar_component): Likewise.
+	(value_primitive_field): Likewise.
+	(value_fetch_lazy): Likewise.
+	* value.h (read_value_memory): Update comment.
+
+2015-07-28  Simon Marchi  <simon.marchi@ericsson.com>
+
 	* value.c (get_value_arch): New function.
 	* value.h (get_value_arch): New declaration.

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 2009e95..0cf2d7d 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -236,6 +236,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 {
   struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
   struct type *elttype = check_typedef (unresolved_elttype);
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

   if (TYPE_LENGTH (type) > 0 && TYPE_LENGTH (unresolved_elttype) > 0)
     {
@@ -276,7 +278,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	      for (temp_len = 0;
 		   (temp_len < len
 		    && temp_len < options->print_max
-		    && extract_unsigned_integer (valaddr + embedded_offset
+		    && extract_unsigned_integer (valaddr
+						 + embedded_offset * unit_size
 						 + temp_len * eltlen,
 						 eltlen, byte_order) != 0);
 		   ++temp_len)
@@ -288,7 +291,8 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	      if (temp_len == options->print_max && temp_len < len)
 		{
 		  ULONGEST val
-		    = extract_unsigned_integer (valaddr + embedded_offset
+		    = extract_unsigned_integer (valaddr
+						+ embedded_offset * unit_size
 						+ temp_len * eltlen,
 						eltlen, byte_order);
 		  if (val != 0)
@@ -299,7 +303,7 @@ c_val_print_array (struct type *type, const gdb_byte *valaddr,
 	    }

 	  LA_PRINT_STRING (stream, unresolved_elttype,
-			   valaddr + embedded_offset, len,
+			   valaddr + embedded_offset * unit_size, len,
 			   NULL, force_ellipses, options);
 	  i = len;
 	}
@@ -342,6 +346,9 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
 		 const struct value *original_value,
 		 const struct value_print_options *options)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   if (options->format && options->format != 's')
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -363,7 +370,8 @@ c_val_print_ptr (struct type *type, const gdb_byte *valaddr,
     {
       struct type *unresolved_elttype = TYPE_TARGET_TYPE (type);
       struct type *elttype = check_typedef (unresolved_elttype);
-      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+      CORE_ADDR addr = unpack_pointer (type,
+				       valaddr + embedded_offset * unit_size);

       print_unpacked_pointer (type, elttype, unresolved_elttype, valaddr,
 			      embedded_offset, addr, stream, recurse, options);
@@ -430,6 +438,9 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
 		 struct ui_file *stream, const struct value *original_value,
 		 const struct value_print_options *options)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   if (options->format || options->output_format)
     {
       struct value_print_options opts = *options;
@@ -441,7 +452,7 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
     }
   else
     {
-      val_print_type_code_int (type, valaddr + embedded_offset,
+      val_print_type_code_int (type, valaddr + embedded_offset * unit_size,
 			       stream);
       /* C and C++ has no single byte int type, char is used
 	 instead.  Since we don't know whether the value is really
@@ -450,7 +461,8 @@ c_val_print_int (struct type *type, struct type *unresolved_type,
       if (c_textual_element_type (unresolved_type, options->format))
 	{
 	  fputs_filtered (" ", stream);
-	  LA_PRINT_CHAR (unpack_long (type, valaddr + embedded_offset),
+	  LA_PRINT_CHAR (unpack_long (type,
+				      valaddr + embedded_offset * unit_size),
 			 unresolved_type, stream);
 	}
     }
diff --git a/gdb/findvar.c b/gdb/findvar.c
index 2079b4b..83b4fca 100644
--- a/gdb/findvar.c
+++ b/gdb/findvar.c
@@ -662,7 +662,7 @@ read_frame_register_value (struct value *value, struct frame_info *frame)
   int offset = 0;
   int reg_offset = value_offset (value);
   int regnum = VALUE_REGNUM (value);
-  int len = TYPE_LENGTH (check_typedef (value_type (value)));
+  int len = type_length_units (check_typedef (value_type (value)));

   gdb_assert (VALUE_LVAL (value) == lval_register);

@@ -677,7 +677,7 @@ read_frame_register_value (struct value *value, struct frame_info *frame)
   while (len > 0)
     {
       struct value *regval = get_frame_register_value (frame, regnum);
-      int reg_len = TYPE_LENGTH (value_type (regval)) - reg_offset;
+      int reg_len = type_length_units (value_type (regval)) - reg_offset;

       /* If the register length is larger than the number of bytes
          remaining to copy, then only copy the appropriate bytes.  */
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 0162c10..3e349f2 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -54,7 +54,7 @@ find_size_for_pointer_math (struct type *ptr_type)
   gdb_assert (TYPE_CODE (ptr_type) == TYPE_CODE_PTR);
   ptr_target = check_typedef (TYPE_TARGET_TYPE (ptr_type));

-  sz = TYPE_LENGTH (ptr_target);
+  sz = type_length_units (ptr_target);
   if (sz == 0)
     {
       if (TYPE_CODE (ptr_type) == TYPE_CODE_VOID)
@@ -121,7 +121,7 @@ value_ptrdiff (struct value *arg1, struct value *arg2)
 	     "second argument is neither\n"
 	     "an integer nor a pointer of the same type."));

-  sz = TYPE_LENGTH (check_typedef (TYPE_TARGET_TYPE (type1)));
+  sz = type_length_units (check_typedef (TYPE_TARGET_TYPE (type1)));
   if (sz == 0)
     {
       warning (_("Type size unknown, assuming 1. "
@@ -192,12 +192,12 @@ value_subscripted_rvalue (struct value *array, LONGEST index, int lowerbound)
 {
   struct type *array_type = check_typedef (value_type (array));
   struct type *elt_type = check_typedef (TYPE_TARGET_TYPE (array_type));
-  unsigned int elt_size = TYPE_LENGTH (elt_type);
+  unsigned int elt_size = type_length_units (elt_type);
   unsigned int elt_offs = elt_size * longest_to_int (index - lowerbound);
   struct value *v;

   if (index < lowerbound || (!TYPE_ARRAY_UPPER_BOUND_IS_UNDEFINED (array_type)
-			     && elt_offs >= TYPE_LENGTH (array_type)))
+			     && elt_offs >= type_length_units (array_type)))
     error (_("no such vector element"));

   if (VALUE_LVAL (array) == lval_memory && value_lazy (array))
diff --git a/gdb/valops.c b/gdb/valops.c
index d68e9f3..dac0274 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -958,30 +958,33 @@ read_value_memory (struct value *val, int embedded_offset,
 		   int stack, CORE_ADDR memaddr,
 		   gdb_byte *buffer, size_t length)
 {
-  ULONGEST xfered = 0;
+  ULONGEST xfered_total = 0;
+  struct gdbarch *arch = get_value_arch (val);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

-  while (xfered < length)
+  while (xfered_total < length)
     {
       enum target_xfer_status status;
-      ULONGEST xfered_len;
+      ULONGEST xfered_partial;

       status = target_xfer_partial (current_target.beneath,
 				    TARGET_OBJECT_MEMORY, NULL,
-				    buffer + xfered, NULL,
-				    memaddr + xfered, length - xfered,
-				    &xfered_len);
+				    buffer + xfered_total * unit_size, NULL,
+				    memaddr + xfered_total,
+				    length - xfered_total,
+				    &xfered_partial);

       if (status == TARGET_XFER_OK)
 	/* nothing */;
       else if (status == TARGET_XFER_UNAVAILABLE)
-	mark_value_bytes_unavailable (val, embedded_offset + xfered,
-				      xfered_len);
+	mark_value_bytes_unavailable (val, embedded_offset + xfered_total,
+				      xfered_partial);
       else if (status == TARGET_XFER_EOF)
-	memory_error (TARGET_XFER_E_IO, memaddr + xfered);
+	memory_error (TARGET_XFER_E_IO, memaddr + xfered_total);
       else
-	memory_error (status, memaddr + xfered);
+	memory_error (status, memaddr + xfered_total);

-      xfered += xfered_len;
+      xfered_total += xfered_partial;
       QUIT;
     }
 }
@@ -1089,7 +1092,7 @@ value_assign (struct value *toval, struct value *fromval)
 	else
 	  {
 	    changed_addr = value_address (toval);
-	    changed_len = TYPE_LENGTH (type);
+	    changed_len = type_length_units (type);
 	    dest_buffer = value_contents (fromval);
 	  }

@@ -1280,7 +1283,7 @@ value_repeat (struct value *arg1, int count)

   read_value_memory (val, 0, value_stack (val), value_address (val),
 		     value_contents_all_raw (val),
-		     TYPE_LENGTH (value_enclosing_type (val)));
+		     type_length_units (value_enclosing_type (val)));

   return val;
 }
@@ -1606,10 +1609,11 @@ value_array (int lowbound, int highbound, struct value **elemvec)
     {
       error (_("bad array bounds (%d, %d)"), lowbound, highbound);
     }
-  typelength = TYPE_LENGTH (value_enclosing_type (elemvec[0]));
+  typelength = type_length_units (value_enclosing_type (elemvec[0]));
   for (idx = 1; idx < nelem; idx++)
     {
-      if (TYPE_LENGTH (value_enclosing_type (elemvec[idx])) != typelength)
+      if (type_length_units (value_enclosing_type (elemvec[idx]))
+	  != typelength)
 	{
 	  error (_("array elements must all be the same size"));
 	}
@@ -3812,7 +3816,7 @@ value_slice (struct value *array, int lowbound, int length)
       {
 	slice = allocate_value (slice_type);
 	value_contents_copy (slice, 0, array, offset,
-			     TYPE_LENGTH (slice_type));
+			     type_length_units (slice_type));
       }

     set_value_component_location (slice, array);
diff --git a/gdb/valprint.c b/gdb/valprint.c
index 8893830..713998c 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -433,6 +433,9 @@ generic_val_print_ptr (struct type *type, const gdb_byte *valaddr,
 		       const struct value *original_value,
 		       const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format && options->format != 's')
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -442,7 +445,8 @@ generic_val_print_ptr (struct type *type, const gdb_byte *valaddr,
     {
       struct type *unresolved_elttype = TYPE_TARGET_TYPE(type);
       struct type *elttype = check_typedef (unresolved_elttype);
-      CORE_ADDR addr = unpack_pointer (type, valaddr + embedded_offset);
+      CORE_ADDR addr = unpack_pointer (type,
+				       valaddr + embedded_offset * unit_size);

       print_unpacked_pointer (type, elttype, addr, stream, options);
     }
@@ -520,6 +524,8 @@ generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
   unsigned int i;
   unsigned int len;
   LONGEST val;
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);

   if (options->format)
     {
@@ -528,7 +534,7 @@ generic_val_print_enum (struct type *type, const gdb_byte *valaddr,
       return;
     }
   len = TYPE_NFIELDS (type);
-  val = unpack_long (type, valaddr + embedded_offset);
+  val = unpack_long (type, valaddr + embedded_offset * unit_size);
   for (i = 0; i < len; i++)
     {
       QUIT;
@@ -633,6 +639,8 @@ generic_val_print_bool (struct type *type, const gdb_byte *valaddr,
 			const struct generic_val_print_decorations *decorations)
 {
   LONGEST val;
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);

   if (options->format || options->output_format)
     {
@@ -644,7 +652,7 @@ generic_val_print_bool (struct type *type, const gdb_byte *valaddr,
     }
   else
     {
-      val = unpack_long (type, valaddr + embedded_offset);
+      val = unpack_long (type, valaddr + embedded_offset * unit_size);
       if (val == 0)
 	fputs_filtered (decorations->false_name, stream);
       else if (val == 1)
@@ -662,6 +670,9 @@ generic_val_print_int (struct type *type, const gdb_byte *valaddr,
 		       const struct value *original_value,
 		       const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format || options->output_format)
     {
       struct value_print_options opts = *options;
@@ -672,7 +683,8 @@ generic_val_print_int (struct type *type, const gdb_byte *valaddr,
 				  original_value, &opts, 0, stream);
     }
   else
-    val_print_type_code_int (type, valaddr + embedded_offset, stream);
+    val_print_type_code_int (type, valaddr + embedded_offset * unit_size,
+			     stream);
 }

 /* generic_val_print helper for TYPE_CODE_CHAR.  */
@@ -685,6 +697,8 @@ generic_val_print_char (struct type *type, struct type *unresolved_type,
 			const struct value_print_options *options)
 {
   LONGEST val;
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);

   if (options->format || options->output_format)
     {
@@ -697,7 +711,7 @@ generic_val_print_char (struct type *type, struct type *unresolved_type,
     }
   else
     {
-      val = unpack_long (type, valaddr + embedded_offset);
+      val = unpack_long (type, valaddr + embedded_offset * unit_size);
       if (TYPE_UNSIGNED (type))
 	fprintf_filtered (stream, "%u", (unsigned int) val);
       else
@@ -715,6 +729,9 @@ generic_val_print_float (struct type *type, const gdb_byte *valaddr,
 			 const struct value *original_value,
 			 const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format)
     {
       val_print_scalar_formatted (type, valaddr, embedded_offset,
@@ -722,7 +739,7 @@ generic_val_print_float (struct type *type, const gdb_byte *valaddr,
     }
   else
     {
-      print_floating (valaddr + embedded_offset, type, stream);
+      print_floating (valaddr + embedded_offset * unit_size, type, stream);
     }
 }

@@ -734,11 +751,15 @@ generic_val_print_decfloat (struct type *type, const gdb_byte *valaddr,
 			    const struct value *original_value,
 			    const struct value_print_options *options)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   if (options->format)
     val_print_scalar_formatted (type, valaddr, embedded_offset, original_value,
 				options, 0, stream);
   else
-    print_decimal_floating (valaddr + embedded_offset, type, stream);
+    print_decimal_floating (valaddr + embedded_offset * unit_size, type,
+			    stream);
 }

 /* generic_val_print helper for TYPE_CODE_COMPLEX.  */
@@ -751,22 +772,25 @@ generic_val_print_complex (struct type *type, const gdb_byte *valaddr,
 			   const struct generic_val_print_decorations
 			     *decorations)
 {
+  struct gdbarch *gdbarch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (gdbarch);
+
   fprintf_filtered (stream, "%s", decorations->complex_prefix);
   if (options->format)
     val_print_scalar_formatted (TYPE_TARGET_TYPE (type), valaddr,
 				embedded_offset, original_value, options, 0,
 				stream);
   else
-    print_floating (valaddr + embedded_offset, TYPE_TARGET_TYPE (type),
-		    stream);
+    print_floating (valaddr + embedded_offset * unit_size,
+		    TYPE_TARGET_TYPE (type), stream);
   fprintf_filtered (stream, "%s", decorations->complex_infix);
   if (options->format)
     val_print_scalar_formatted (TYPE_TARGET_TYPE (type), valaddr,
 				embedded_offset
-				+ TYPE_LENGTH (TYPE_TARGET_TYPE (type)),
+				+ type_length_units (TYPE_TARGET_TYPE (type)),
 				original_value, options, 0, stream);
   else
-    print_floating (valaddr + embedded_offset
+    print_floating (valaddr + embedded_offset * unit_size
 		    + TYPE_LENGTH (TYPE_TARGET_TYPE (type)),
 		    TYPE_TARGET_TYPE (type), stream);
   fprintf_filtered (stream, "%s", decorations->complex_suffix);
@@ -1150,6 +1174,9 @@ val_print_scalar_formatted (struct type *type,
 			    int size,
 			    struct ui_file *stream)
 {
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   gdb_assert (val != NULL);
   gdb_assert (valaddr == value_contents_for_printing_const (val));

@@ -1175,7 +1202,7 @@ val_print_scalar_formatted (struct type *type,
   else if (!value_bytes_available (val, embedded_offset, TYPE_LENGTH (type)))
     val_print_unavailable (stream);
   else
-    print_scalar_formatted (valaddr + embedded_offset, type,
+    print_scalar_formatted (valaddr + embedded_offset * unit_size, type,
 			    options, size, stream);
 }

@@ -1823,7 +1850,7 @@ val_print_array_elements (struct type *type,
   LONGEST low_pos, high_pos;

   elttype = TYPE_TARGET_TYPE (type);
-  eltlen = TYPE_LENGTH (check_typedef (elttype));
+  eltlen = type_length_units (check_typedef (elttype));
   index_type = TYPE_INDEX_TYPE (type);

   if (get_array_bounds (type, &low_bound, &high_bound))
diff --git a/gdb/value.c b/gdb/value.c
index 0d540d5..2f0b2cc 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1091,8 +1091,11 @@ set_value_parent (struct value *value, struct value *parent)
 gdb_byte *
 value_contents_raw (struct value *value)
 {
+  struct gdbarch *arch = get_value_arch (value);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
   allocate_value_contents (value);
-  return value->contents + value->embedded_offset;
+  return value->contents + value->embedded_offset * unit_size;
 }

 gdb_byte *
@@ -1242,7 +1245,7 @@ value_ranges_copy_adjusted (struct value *dst, int dst_bit_offset,
 			bit_length);
 }

-/* Copy LENGTH bytes of SRC value's (all) contents
+/* Copy LENGTH target addressable memory units of SRC value's (all) contents
    (value_contents_all) starting at SRC_OFFSET, into DST value's (all)
    contents, starting at DST_OFFSET.  If unavailable contents are
    being copied from SRC, the corresponding DST contents are marked
@@ -1257,8 +1260,9 @@ value_contents_copy_raw (struct value *dst, int dst_offset,
 			 struct value *src, int src_offset, int length)
 {
   range_s *r;
-  int i;
   int src_bit_offset, dst_bit_offset, bit_length;
+  struct gdbarch *arch = get_value_arch (src);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

   /* A lazy DST would make that this copy operation useless, since as
      soon as DST's contents were un-lazied (by a later value_contents
@@ -1275,14 +1279,14 @@ value_contents_copy_raw (struct value *dst, int dst_offset,
 					     TARGET_CHAR_BIT * length));

   /* Copy the data.  */
-  memcpy (value_contents_all_raw (dst) + dst_offset,
-	  value_contents_all_raw (src) + src_offset,
-	  length);
+  memcpy (value_contents_all_raw (dst) + dst_offset * unit_size,
+	  value_contents_all_raw (src) + src_offset * unit_size,
+	  length * unit_size);

   /* Copy the meta-data, adjusted.  */
-  src_bit_offset = src_offset * TARGET_CHAR_BIT;
-  dst_bit_offset = dst_offset * TARGET_CHAR_BIT;
-  bit_length = length * TARGET_CHAR_BIT;
+  src_bit_offset = src_offset * unit_size * HOST_CHAR_BIT;
+  dst_bit_offset = dst_offset * unit_size * HOST_CHAR_BIT;
+  bit_length = length * unit_size * HOST_CHAR_BIT;

   value_ranges_copy_adjusted (dst, dst_bit_offset,
 			      src, src_bit_offset,
@@ -2279,17 +2283,21 @@ set_internalvar_component (struct internalvar *var, int offset, int bitpos,
 			   int bitsize, struct value *newval)
 {
   gdb_byte *addr;
+  struct gdbarch *arch;
+  int unit_size;

   switch (var->kind)
     {
     case INTERNALVAR_VALUE:
       addr = value_contents_writeable (var->u.value);
+      arch = get_value_arch (var->u.value);
+      unit_size = gdbarch_addressable_memory_unit_size (arch);

       if (bitsize)
 	modify_field (value_type (var->u.value), addr + offset,
 		      value_as_long (newval), bitpos, bitsize);
       else
-	memcpy (addr + offset, value_contents (newval),
+	memcpy (addr + offset * unit_size, value_contents (newval),
 		TYPE_LENGTH (value_type (newval)));
       break;

@@ -3000,6 +3008,8 @@ value_primitive_field (struct value *arg1, int offset,
 {
   struct value *v;
   struct type *type;
+  struct gdbarch *arch = get_value_arch (arg1);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);

   arg_type = check_typedef (arg_type);
   type = TYPE_FIELD_TYPE (arg_type, fieldno);
@@ -3078,7 +3088,8 @@ value_primitive_field (struct value *arg1, int offset,
   else
     {
       /* Plain old data member */
-      offset += TYPE_FIELD_BITPOS (arg_type, fieldno) / 8;
+      offset += (TYPE_FIELD_BITPOS (arg_type, fieldno)
+	         / (HOST_CHAR_BIT * unit_size));

       /* Lazy register values with offsets are not supported.  */
       if (VALUE_LVAL (arg1) == lval_register && value_lazy (arg1))
@@ -3091,7 +3102,7 @@ value_primitive_field (struct value *arg1, int offset,
 	  v = allocate_value (type);
 	  value_contents_copy_raw (v, value_embedded_offset (v),
 				   arg1, value_embedded_offset (arg1) + offset,
-				   TYPE_LENGTH (type));
+				   type_length_units (type));
 	}
       v->offset = (value_offset (arg1) + offset
 		   + value_embedded_offset (arg1));
@@ -3833,7 +3844,7 @@ value_fetch_lazy (struct value *val)
       if (TYPE_LENGTH (type))
 	read_value_memory (val, 0, value_stack (val),
 			   addr, value_contents_all_raw (val),
-			   TYPE_LENGTH (type));
+			   type_length_units (type));
     }
   else if (VALUE_LVAL (val) == lval_register)
     {
@@ -3892,7 +3903,7 @@ value_fetch_lazy (struct value *val)
       set_value_lazy (val, 0);
       value_contents_copy (val, value_embedded_offset (val),
 			   new_val, value_embedded_offset (new_val),
-			   TYPE_LENGTH (type));
+			   type_length_units (type));

       if (frame_debug)
 	{
diff --git a/gdb/value.h b/gdb/value.h
index e25f52b..82deaf2 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -571,8 +571,8 @@ extern int value_contents_eq (const struct value *val1, int offset1,
 			      const struct value *val2, int offset2,
 			      int length);

-/* Read LENGTH bytes of memory starting at MEMADDR into BUFFER, which
-   is (or will be copied to) VAL's contents buffer offset by
+/* Read LENGTH addressable memory units starting at MEMADDR into BUFFER,
+   which is (or will be copied to) VAL's contents buffer offset by
    EMBEDDED_OFFSET (that is, to &VAL->contents[EMBEDDED_OFFSET]).
    Marks value contents ranges as unavailable if the corresponding
    memory is likewise unavailable.  STACK indicates whether the memory
-- 
2.1.4

[PATCH 5/5] Add new test internalvar.exp

[Simon Marchi]

I wrote this test While doing the work that lead to the previous patch,
so I thought I would contribute it upstream.  From what I can see, there
is no test currently to verify operations on internal variables (please
point me to it if I'm wrong).

gdb/testsuite/ChangeLog:

	* gdb.base/internalvar.exp: New file.
	* gdb.base/internalvar.c: New file.
---
 gdb/testsuite/gdb.base/internalvar.c   | 45 ++++++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.base/internalvar.exp | 42 +++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/internalvar.c
 create mode 100644 gdb/testsuite/gdb.base/internalvar.exp

diff --git a/gdb/testsuite/gdb.base/internalvar.c b/gdb/testsuite/gdb.base/internalvar.c
new file mode 100644
index 0000000..2aadc11
--- /dev/null
+++ b/gdb/testsuite/gdb.base/internalvar.c
@@ -0,0 +1,45 @@
+struct inner
+{
+  int a;
+  int b[2];
+};
+
+struct outer
+{
+  int x;
+  int y;
+
+  struct inner inner;
+
+  int z[2];
+};
+
+struct outer o;
+struct inner i;
+
+void
+break_here (void)
+{
+}
+
+int
+main (void)
+{
+  o.x = 0x1111;
+  o.y = 0x2222;
+
+  o.inner.a = 0x3333;
+  o.inner.b[0] = 0x4444;
+  o.inner.b[1] = 0x5555;
+
+  o.z[0] = 0x6666;
+  o.z[1] = 0x7777;
+
+  i.a = 0x8888;
+  i.b[0] = 0x9999;
+  i.b[1] = 0xaaaa;
+
+  break_here ();
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/internalvar.exp b/gdb/testsuite/gdb.base/internalvar.exp
new file mode 100644
index 0000000..701cc16
--- /dev/null
+++ b/gdb/testsuite/gdb.base/internalvar.exp
@@ -0,0 +1,42 @@
+# Copyright 2015 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+#
+# Test operations on internal variables.
+#
+
+standard_testfile
+
+if {[prepare_for_testing ${testfile}.exp ${testfile}]} {
+    return -1
+}
+
+runto break_here
+
+gdb_test_no_output "set \$a = 0x1234"
+gdb_test "p/x \$a" " = 0x1234"
+
+gdb_test_no_output "set \$s = o"
+gdb_test "p/x \$s" " = {x = 0x1111, y = 0x2222, inner = {a = 0x3333, b = \\{0x4444, 0x5555}}, z = \\{0x6666, 0x7777}}"
+gdb_test "p/x \$s.inner" " = {a = 0x3333, b = \\{0x4444, 0x5555}}"
+gdb_test "p/x \$s.inner.b\[1\]" " = 0x5555"
+
+gdb_test_no_output "set \$s.inner = i"
+gdb_test "p/x \$s" " = {x = 0x1111, y = 0x2222, inner = {a = 0x8888, b = \\{0x9999, 0xaaaa}}, z = \\{0x6666, 0x7777}}"
+
+gdb_test_no_output "set \$s.x = 0xffff"
+gdb_test_no_output "set \$s.inner.b\[1\] = 0xeeee"
+gdb_test_no_output "set \$s.z\[1\] = 0xdddd"
+gdb_test "p/x \$s" " = {x = 0xffff, y = 0x2222, inner = {a = 0x8888, b = \\{0x9999, 0xeeee}}, z = \\{0x6666, 0xdddd}}"
-- 
2.1.4

Re: [PATCH 5/5] Add new test internalvar.exp

[Pedro Alves]

On 07/16/2015 07:51 PM, Simon Marchi wrote:
> I wrote this test While doing the work that lead to the previous patch,
> so I thought I would contribute it upstream.  From what I can see, there
> is no test currently to verify operations on internal variables (please
> point me to it if I'm wrong).

Pedantically, it'd be better to call these convenience vars.

Convenience var specifics are tested in gdbvars.exp.  Likely we have this
more covered on the C++ tests.  Something like what you're testing, but
for bitfields, is done in bitfields.exp.  structs.exp is for infcalls, which IIRC
your target doesn't do.  I guess one could hack a bug in the value field access
code somewhere in value.c/valops.c/valprint.c, and the find the test that
has more failures.  :-)

In any case, more tests can't hurt.  :-)

AFAICS, this is really more about getting value field offsets and contents
right than convenience vars per se, which end up just exercising the
routine value paths.

You could put these in gdb.base/struct3.c|.exp, which if you look
at struct3.c, it's structure is similar, with an inner and outer struct too.
Otherwise, I'd suggest renaming the test in the value
direction -- value-fields.exp, value-units.exp, value-offsets.exp or
some such.

I'd also suggest making the describing comment be something like:

# Test accessing different fields of structures, inner structures,
# arrays, etc., and make sure GDB prints the right contents.  Handy to
# exercise the value machinery code that handles host vs target bytes/memory
# units.

Anyway, the test code itself looks good to me, with:

> ---
>  gdb/testsuite/gdb.base/internalvar.c   | 45 ++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.base/internalvar.exp | 42 +++++++++++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
>  create mode 100644 gdb/testsuite/gdb.base/internalvar.c
>  create mode 100644 gdb/testsuite/gdb.base/internalvar.exp
> 
> diff --git a/gdb/testsuite/gdb.base/internalvar.c b/gdb/testsuite/gdb.base/internalvar.c
> new file mode 100644
> index 0000000..2aadc11
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/internalvar.c
> @@ -0,0 +1,45 @@

Copyright header missing.

Thanks,
Pedro Alves

Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units

[Pedro Alves]

On 07/16/2015 07:51 PM, Simon Marchi wrote:
> This patch tries to clean up a bit the blur around the length field in
> struct type, regarding its use with architectures with non-8-bits
> addressable memory.  It clarifies that the field is expressed in bytes,
> which is what is the closest to the current reality.
> 
> It also introduces a new function to get the length of the type in
> addressable memory units.
> 

LGTM, with:

> gdb/ChangeLog:
> 
> 	* gdbtypes.c (type_length_units): New function.
> 	* gdbtypes.h (type_length_units): New declaration.
> 	(struct type): Update LENGTH's comment.

Write:

	(struct type) <length>: Update comment.

> +
>  /* Alloc a new type instance structure, fill it with some defaults,
>     and point it at OLDTYPE.  Allocate the new type instance from the
>     same place as OLDTYPE.  */
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index c166e48..83f85a6 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -780,31 +780,23 @@ struct type
>       check_typedef.  */
>    int instance_flags;
>  
> -  /* * Length of storage for a value of this type.  This is what
> -     sizeof(type) would return; use it for address arithmetic, memory
> -     reads and writes, etc.  This size includes padding.  For example,
> -     an i386 extended-precision floating point value really only
> -     occupies ten bytes, but most ABI's declare its size to be 12
> -     bytes, to preserve alignment.  A `struct type' representing such
> -     a floating-point type would have a `length' value of 12, even
> -     though the last two bytes are unused.
> -
> -     There's a bit of a host/target mess here, if you're concerned
> -     about machines whose bytes aren't eight bits long, or who don't
> -     have byte-addressed memory.  Various places pass this to memcpy
> -     and such, meaning it must be in units of host bytes.  Various
> -     other places expect they can calculate addresses by adding it
> -     and such, meaning it must be in units of target bytes.  For
> -     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
> -     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
> -
> -     One fix would be to make this field in bits (requiring that it
> -     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
> -     the other choice would be to make it consistently in units of
> -     HOST_CHAR_BIT.  However, this would still fail to address
> -     machines based on a ternary or decimal representation.  */
> +  /* * Length of storage for a value of this type.  The value is the
> +     expression in bytes of of what sizeof(type) would return.  This

Double "of of".  Please say "host bytes" to make this super clear.

> +     size includes padding.  For example, an i386 extended-precision
> +     floating point value really only occupies ten bytes, but most
> +     ABI's declare its size to be 12 bytes, to preserve alignment.
> +     A `struct type' representing such a floating-point type would
> +     have a `length' value of 12, even though the last two bytes are
> +     unused.
> +
> +     Since this field is expressed in bytes, its value is appropriate to

Likewise, "host bytes".

> +     pass to memcpy and such (it is assumed that GDB itself always runs
> +     on an 8-bits addressable architecture).  However, when using it for
> +     target address arithmetic (e.g. adding it to a target address), the
> +     type_length_units function should be used in order to get the length
> +     expressed in addressable memory units.  */

"target addressable memory units" while at it.

Likewise in the other patches.

>    
> -  unsigned length;
> +  unsigned int length;

Thanks,
Pedro Alves

Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units

[Simon Marchi]

On 15-07-24 07:26 AM, Pedro Alves wrote:
> On 07/16/2015 07:51 PM, Simon Marchi wrote:
>> This patch tries to clean up a bit the blur around the length field in
>> struct type, regarding its use with architectures with non-8-bits
>> addressable memory.  It clarifies that the field is expressed in bytes,
>> which is what is the closest to the current reality.
>>
>> It also introduces a new function to get the length of the type in
>> addressable memory units.
>>
> 
> LGTM, with:
> 
>> gdb/ChangeLog:
>>
>> 	* gdbtypes.c (type_length_units): New function.
>> 	* gdbtypes.h (type_length_units): New declaration.
>> 	(struct type): Update LENGTH's comment.
> 
> Write:
> 
> 	(struct type) <length>: Update comment.
> 
>> +
>>  /* Alloc a new type instance structure, fill it with some defaults,
>>     and point it at OLDTYPE.  Allocate the new type instance from the
>>     same place as OLDTYPE.  */
>> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
>> index c166e48..83f85a6 100644
>> --- a/gdb/gdbtypes.h
>> +++ b/gdb/gdbtypes.h
>> @@ -780,31 +780,23 @@ struct type
>>       check_typedef.  */
>>    int instance_flags;
>>  
>> -  /* * Length of storage for a value of this type.  This is what
>> -     sizeof(type) would return; use it for address arithmetic, memory
>> -     reads and writes, etc.  This size includes padding.  For example,
>> -     an i386 extended-precision floating point value really only
>> -     occupies ten bytes, but most ABI's declare its size to be 12
>> -     bytes, to preserve alignment.  A `struct type' representing such
>> -     a floating-point type would have a `length' value of 12, even
>> -     though the last two bytes are unused.
>> -
>> -     There's a bit of a host/target mess here, if you're concerned
>> -     about machines whose bytes aren't eight bits long, or who don't
>> -     have byte-addressed memory.  Various places pass this to memcpy
>> -     and such, meaning it must be in units of host bytes.  Various
>> -     other places expect they can calculate addresses by adding it
>> -     and such, meaning it must be in units of target bytes.  For
>> -     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
>> -     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
>> -
>> -     One fix would be to make this field in bits (requiring that it
>> -     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
>> -     the other choice would be to make it consistently in units of
>> -     HOST_CHAR_BIT.  However, this would still fail to address
>> -     machines based on a ternary or decimal representation.  */
>> +  /* * Length of storage for a value of this type.  The value is the
>> +     expression in bytes of of what sizeof(type) would return.  This
> 
> Double "of of".  Please say "host bytes" to make this super clear.
> 
>> +     size includes padding.  For example, an i386 extended-precision
>> +     floating point value really only occupies ten bytes, but most
>> +     ABI's declare its size to be 12 bytes, to preserve alignment.
>> +     A `struct type' representing such a floating-point type would
>> +     have a `length' value of 12, even though the last two bytes are
>> +     unused.
>> +
>> +     Since this field is expressed in bytes, its value is appropriate to
> 
> Likewise, "host bytes".
> 
>> +     pass to memcpy and such (it is assumed that GDB itself always runs
>> +     on an 8-bits addressable architecture).  However, when using it for
>> +     target address arithmetic (e.g. adding it to a target address), the
>> +     type_length_units function should be used in order to get the length
>> +     expressed in addressable memory units.  */
> 
> "target addressable memory units" while at it.
> 
> Likewise in the other patches.
> 
>>    
>> -  unsigned length;
>> +  unsigned int length;
> 
> Thanks,
> Pedro Alves

Thanks for the review.  Here is the updated version:


From bb292c235125038795185b325222dcd54bb9f36a Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Mon, 27 Jul 2015 17:17:49 -0400
Subject: [PATCH] Update comment for struct type's length field, introduce
 type_length_units

This patch tries to clean up a bit the blur around the length field in
struct type, regarding its use with architectures with non-8-bits
addressable memory.  It clarifies that the field is expressed in host
bytes, which is what is the closest to the current reality.

It also introduces a new function to get the length of the type in
target addressable memory units.

gdb/ChangeLog:

	* gdbtypes.c (type_length_units): New function.
	* gdbtypes.h (type_length_units): New declaration.
	(struct type) <length>: Update comment.
---
 gdb/gdbtypes.c | 11 +++++++++++
 gdb/gdbtypes.h | 47 ++++++++++++++++++++++-------------------------
 2 files changed, 33 insertions(+), 25 deletions(-)

diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c
index 3f1f3fb..e6a9547 100644
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -252,6 +252,17 @@ get_target_type (struct type *type)
   return type;
 }

+/* See gdbtypes.h.  */
+
+unsigned int
+type_length_units (struct type *type)
+{
+  struct gdbarch *arch = get_type_arch (type);
+  int unit_size = gdbarch_addressable_memory_unit_size (arch);
+
+  return TYPE_LENGTH (type) / unit_size;
+}
+
 /* Alloc a new type instance structure, fill it with some defaults,
    and point it at OLDTYPE.  Allocate the new type instance from the
    same place as OLDTYPE.  */
diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
index c166e48..f270855 100644
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -780,31 +780,23 @@ struct type
      check_typedef.  */
   int instance_flags;

-  /* * Length of storage for a value of this type.  This is what
-     sizeof(type) would return; use it for address arithmetic, memory
-     reads and writes, etc.  This size includes padding.  For example,
-     an i386 extended-precision floating point value really only
-     occupies ten bytes, but most ABI's declare its size to be 12
-     bytes, to preserve alignment.  A `struct type' representing such
-     a floating-point type would have a `length' value of 12, even
-     though the last two bytes are unused.
-
-     There's a bit of a host/target mess here, if you're concerned
-     about machines whose bytes aren't eight bits long, or who don't
-     have byte-addressed memory.  Various places pass this to memcpy
-     and such, meaning it must be in units of host bytes.  Various
-     other places expect they can calculate addresses by adding it
-     and such, meaning it must be in units of target bytes.  For
-     some DSP targets, in which HOST_CHAR_BIT will (presumably) be 8
-     and TARGET_CHAR_BIT will be (say) 32, this is a problem.
-
-     One fix would be to make this field in bits (requiring that it
-     always be a multiple of HOST_CHAR_BIT and TARGET_CHAR_BIT) ---
-     the other choice would be to make it consistently in units of
-     HOST_CHAR_BIT.  However, this would still fail to address
-     machines based on a ternary or decimal representation.  */
-
-  unsigned length;
+  /* * Length of storage for a value of this type.  The value is the
+     expression in host bytes of what sizeof(type) would return.  This
+     size includes padding.  For example, an i386 extended-precision
+     floating point value really only occupies ten bytes, but most
+     ABI's declare its size to be 12 bytes, to preserve alignment.
+     A `struct type' representing such a floating-point type would
+     have a `length' value of 12, even though the last two bytes are
+     unused.
+
+     Since this field is expressed in host bytes, its value is appropriate
+     to pass to memcpy and such (it is assumed that GDB itself always runs
+     on an 8-bits addressable architecture).  However, when using it for
+     target address arithmetic (e.g. adding it to a target address), the
+     type_length_units function should be used in order to get the length
+     expressed in target addressable memory units.  */
+
+  unsigned int length;

   /* * Core type, shared by a group of qualified types.  */

@@ -1659,6 +1651,11 @@ extern struct gdbarch *get_type_arch (const struct type *);

 extern struct type *get_target_type (struct type *type);

+/* Return the equivalent of TYPE_LENGTH, but in number of target
+   addressable memory units of the associated gdbarch instead of bytes.  */
+
+extern unsigned int type_length_units (struct type *type);
+
 /* * Helper function to construct objfile-owned types.  */

 extern struct type *init_type (enum type_code, int, int, const char *,
-- 
2.1.4

Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units

[Pedro Alves]

On 07/27/2015 10:37 PM, Simon Marchi wrote:

> Thanks for the review.  Here is the updated version:

Great, LGTM.  Please push.

Thanks,
Pedro Alves

Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units

[Simon Marchi]

On 15-07-28 06:19 AM, Pedro Alves wrote:
> On 07/27/2015 10:37 PM, Simon Marchi wrote:
> 
>> Thanks for the review.  Here is the updated version:
> 
> Great, LGTM.  Please push.
> 
> Thanks,
> Pedro Alves

Ok, I pushed patches 1-4.  I'll take some time later to better integrate
the test (patch 5) with what is already there.

Thanks!

Simon