This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>, <gdb-patches at sourceware dot org>
- Date: Mon, 27 Jul 2015 17:37:39 -0400
- Subject: Re: [PATCH 1/5] Update comment for struct type's length field, introduce type_length_units
- Authentication-results: sourceware.org; auth=none
- References: <1437072684-26565-1-git-send-email-simon dot marchi at ericsson dot com> <55B220F4 dot 60706 at redhat dot com>
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