jeremie.salvu...@free.fr writes:

> While hacking on gengtype with Basile, we noticed a strange piece of code at 
> line 2539 in gcc/gengtype.c r162692
>
> static void
> output_type_enum (outf_p of, type_p s)
> {
>   if (s->kind == TYPE_PARAM_STRUCT && s->u.s.line.file != NULL) /* Strange 
> code @@*/
>     {
>       oprintf (of, ", gt_e_");
>       output_mangled_typename (of, s);
>     }
>   else if (UNION_OR_STRUCT_P (s) && s->u.s.line.file != NULL)
>     {
>       oprintf (of, ", gt_ggc_e_");
>       output_mangled_typename (of, s);
>     }
>   else
>     oprintf (of, ", gt_types_enum_last");
> }
>
> We think that the enum type_kind discriminates fields union in struct type. 
> So for TYPE_PARAM_STRUCT we believe that 
> the param_struct field of union u inside struct type is used. If this is 
> true, the test s->u.s.line.file != NULL is meaningless when s->kind == 
> TYPE_PARAM_STRUCT, it should be s->u.param_struct.line.file != NULL instead 
> in our opinion.

I agree that this is wrong.

> However, the existing code appears to work but we don't understand why.

That one is fairly easy.  If you look at the generated code, you will
see that those values are only used to pass to gt_pch_note_object.  From
there they will eventually be passed to either ggc_pch_count_object or
ggc_pch_alloc_object.  The default page allocator ignores this type.
The zone allocator does use the type, but nobody uses that allocator.
And even if you do use the zone allocator, it will work correctly if
perhaps suboptimally as long as it always gets the same type for a given
struct, which I believe will happen.

You should send in a tested patch to fix that problem (and nothing
else).

Ian

Reply via email to