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

Daniel Jacobowitz
2003-01-29T23:14:30Z

On Wed, Jan 29, 2003 at 03:48:29PM -0700, Kevin Buettner wrote:
> 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.

It won't actually create a problem if you give them different main
types; the main type is purely internal to the type system, and is
never searched directly.

That said, while I'm not really happy with your approach either, I
think it is more correct and less fragile in general.

> 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.)

My only comment is that I'd rather you check in replace_type that there
are no types on the variant ring with different space qualifiers, and
internal_error if there are. How's that sound?

--
Daniel Jacobowitz
MontaVista Software Debian GNU/Linux Developer