[RFC] Move ``length'' from struct main_type to struct type

Kevin Buettner kevinb@redhat.com
Wed Jan 29 22:48:00 GMT 2003


A while back, I introduced a change to dwarf2read.c in
read_tag_pointer_type() which was supposed to create a pointer type
variant of a (potentially) different length:

  if (TYPE_LENGTH (type) != byte_size || addr_class != DW_ADDR_none)
    {
      if (ADDRESS_CLASS_TYPE_FLAGS_P ())
	{
	  int type_flags;

	  type_flags = ADDRESS_CLASS_TYPE_FLAGS (byte_size, addr_class);
	  gdb_assert ((type_flags & ~TYPE_FLAG_ADDRESS_CLASS_ALL) == 0);
	  type = make_type_with_address_space (type, type_flags);
	}
    ...
    }

  TYPE_LENGTH (type) = byte_size;

However, this code doesn't work correctly.  As it stands now, the type
length is shared between all type variants (which differ only in
qualifiers).  This means that as soon as the above TYPE_LENGTH
assignment is performed, all type variants end up getting the same
length.  I should note that when I initially developed this code, I
did so on a branch which did not yet implement this sharing via struct
main_type.

The patch below corrects this problem by moving the length field from
struct main_type to struct type.  I am not entirely happy with this
approach, but the other approaches I've considered are even less
palatable.

E.g, another approach that I considered would be to create a new type
which has a different main_type that varies only in the length field. 
The problem with this is that the names end up being the same, and it
seems to me that there will be problems with searching and finding the
right type when the user specifies it by name.

Certainly, if anyone can think of a better approach, I'd be happy to
hear about it.

Those of you reviewing this patch should carefully consider the
comment in replace_type().  I don't think the problem that I mention
there will actually arise since replace_type() is only called by symbol
readers which don't know how to (and indeed can't) create variants of
different sizes, so the situation described in the replace_type() comment
should never (at the moment anyway) arise.

Comments?  (I'll wait at least a week before checking this one in.)

	* gdbtypes.h (struct main_type): Move ``length'' field from here...
	(struct type): ...to here.
	(TYPE_LENGTH): Adjust to reflect different location of ``length''
	field.
	* gdbtypes.c (make_qualified_type): Set length on newly created type.
	(replace_type): Set length on all type variants for a given type.

Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.69
diff -u -p -r1.69 gdbtypes.c
--- gdbtypes.c	17 Jan 2003 19:12:18 -0000	1.69
+++ gdbtypes.c	29 Jan 2003 22:14:40 -0000
@@ -469,6 +469,9 @@ make_qualified_type (struct type *type, 
   /* Now set the instance flags and return the new type.  */
   TYPE_INSTANCE_FLAGS (ntype) = new_flags;
 
+  /* Set length of new type to that of the original type.  */
+  TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+
   return ntype;
 }
 
@@ -556,9 +559,23 @@ make_cv_type (int cnst, int voltl, struc
 void
 replace_type (struct type *ntype, struct type *type)
 {
-  struct type *cv_chain, *as_chain, *ptr, *ref;
+  struct type *chain;
 
   *TYPE_MAIN_TYPE (ntype) = *TYPE_MAIN_TYPE (type);
+
+  /* The type length is not a part of the main type.  Update it for each
+     type on the variant chain.  Note that this is not correct for variants
+     on the chain which are supposed to have a length different than other
+     variants on the chain, but that's one of the drawbacks of constructing
+     types this way.  We *could* only change the lengths which are the same
+     as NTYPE's original length, but that doesn't really help because we
+     don't know what the new lengths should be for the types that don't
+     match.  */
+  chain = ntype;
+  do {
+    TYPE_LENGTH (ntype) = TYPE_LENGTH (type);
+    chain = TYPE_CHAIN (chain);
+  } while (ntype != chain);
 
   /* Assert that the two types have equivalent instance qualifiers.
      This should be true for at least all of our debug readers.  */
Index: gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.42
diff -u -p -r1.42 gdbtypes.h
--- gdbtypes.h	19 Jan 2003 04:06:45 -0000	1.42
+++ gdbtypes.h	29 Jan 2003 22:14:41 -0000
@@ -297,32 +297,6 @@ struct main_type
 
   char *tag_name;
 
-  /* 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;
-
   /* FIXME, these should probably be restricted to a Fortran-specific
      field in some fashion.  */
 #define BOUND_CANNOT_BE_DETERMINED   5
@@ -489,15 +463,42 @@ struct type
   struct type *reference_type;
 
   /* Variant chain.  This points to a type that differs from this one only
-     in qualifiers.  Currently, the possible qualifiers are const, volatile,
-     code-space, and data-space.  The variants are linked in a circular
-     ring and share MAIN_TYPE.  */
+     in qualifiers and length.  Currently, the possible qualifiers are
+     const, volatile, code-space, data-space, and address class.  The
+     length may differ only when one of the address class flags are set.
+     The variants are linked in a circular ring and share MAIN_TYPE.  */
   struct type *chain;
 
   /* Flags specific to this instance of the type, indicating where
      on the ring we are.  */
   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;
+
   /* Core type, shared by a group of qualified types.  */
   struct main_type *main_type;
 };
@@ -758,7 +759,7 @@ extern void allocate_cplus_struct_type (
    But check_typedef does set the TYPE_LENGTH of the TYPEDEF type,
    so you only have to call check_typedef once.  Since allocate_value
    calls check_typedef, TYPE_LENGTH (VALUE_TYPE (X)) is safe.  */
-#define TYPE_LENGTH(thistype) TYPE_MAIN_TYPE(thistype)->length
+#define TYPE_LENGTH(thistype) (thistype)->length
 #define TYPE_OBJFILE(thistype) TYPE_MAIN_TYPE(thistype)->objfile
 #define TYPE_FLAGS(thistype) TYPE_MAIN_TYPE(thistype)->flags
 /* Note that TYPE_CODE can be TYPE_CODE_TYPEDEF, so if you want the real



More information about the Gdb-patches mailing list