[1/5] Update comment for struct type's length field, introduce type_length_units
Commit Message
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(-)
Comments
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
@@ -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. */
@@ -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 *,