This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Re: Regression: field type preservation: 7.0 -> 7.0.1+HEAD
- From: Joel Brobecker <brobecker at adacore dot com>
- To: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- Cc: gdb-patches at sourceware dot org, gdb at sourceware dot org, Vladimir Prus <vladimir at codesourcery dot com>
- Date: Sun, 3 Jan 2010 08:57:17 +0400
- Subject: Re: [patch] Re: Regression: field type preservation: 7.0 -> 7.0.1+HEAD
- References: <20100101184505.GA18391@host0.dyn.jankratochvil.net> <201001021308.19130.vladimir@codesourcery.com> <20100102203022.GA8372@host0.dyn.jankratochvil.net>
> gdb/
> 2010-01-02 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * value.c (value_primitive_field): Remove one check_typedef call.
> Move bitpos and container_bitsize initialization after
> allocate_value_lazy. New comment before accessing TYPE_LENGTH.
Interesting. This patch allowed me to figure out why the initial patch
sent by Vladimir was in fact working when it looks like it could not
possibly work :).
Personally, I'm not a big fan of the approach you took in this case.
You rely on a side-effect of the various allocate_* routines, but
this is only because these routines call check_typedef. In particular,
there is something interesting in allocate_value_lazy. A variable is
declared...
struct type *atype = check_typedef (type);
... but then never used. There is no available info as to why this is,
as this is here since day 1 of the public CVS repo, but it's now clear
to me that this helps fixing that length - a type length that does not
appear to be used at all in the rest of the routine!
In my opinion, I agree with Daniel's comment that it is unusual to
call check_typedef without storing the function result. But this
seems to be less awkward than relying on an undocumented side-effect
of a function that uses an undocumented side-effect of a check_typedef,
especially since the function in question does not really need that
side-effect to be applied at the time the function is called!
So, my proposal, if the other maintainers agree, is to document
the side-effect of check_typedef (sets the typedef TYPE_LENGTH)
as this appears to be a fully-intended behavior, and then do:
> - type = check_typedef (type);
> + /* Call check_typedef on our type to make sure that, if TYPE
> + is a TYPE_CODE_TYPEDEF, its length is set to the length
> + of the target type instead of zero. However, we do not
> + replace the typedef type by the target type, because we want
> + to keep the typedef in order to be able to print the type
> + description correctly. */
> + check_typedef (type);
> gdb/testsuite/
> 2010-01-02 Jan Kratochvil <jan.kratochvil@redhat.com>
>
> * gdb.mi/var-cmd.c (do_bitfield_tests): Change "V.sharable" type to
> "uint_for_mi_testing".
This part is OK.
--
Joel