On Tue, Jul 8, 2014 at 2:25 AM, Jan Hubicka <hubi...@ucw.cz> wrote:
>> > Index: stor-layout.c
>> > ===================================================================
>> > --- stor-layout.c (revision 212098)
>> > +++ stor-layout.c (working copy)
>> > @@ -2065,7 +2065,7 @@ void
>> > finish_builtin_struct (tree type, const char *name, tree fields,
>> > tree align_type)
>> > {
>> > - tree tail, next;
>> > + tree tail, next, variant;
>> >
>> > for (tail = NULL_TREE; fields; tail = fields, fields = next)
>> > {
>> > @@ -2074,6 +2074,10 @@ finish_builtin_struct (tree type, const
>> > DECL_CHAIN (fields) = tail;
>> > }
>> > TYPE_FIELDS (type) = tail;
>> > + for (variant = TYPE_MAIN_VARIANT (type);
>> > + variant != 0;
>> > + variant = TYPE_NEXT_VARIANT (variant))
>> > + TYPE_FIELDS (variant) = tail;
>>
>> I think that's a bogus place to fix that. Instead the caller should
>> use build_variant_type_copy. Especially that the fixup above
>> depends on all variants being added before finish_builtin_struct
>> is called.
>>
>> Please revert the above.
>
> Sorry, I missed this email at the airport. Will test revert overnight.
>
> I do not understand how you propose to fix it.
>
> The variant is produced before the builtin structure is finished and it thus
> have FIELDS and SIZE NULL (as the pointer to struct itself is field of the
> struct):
>
> /* function_info pointer pointer */
> fn_info_ptr_type = build_pointer_type
> (build_qualified_type (fn_info_ptr_type, TYPE_QUAL_CONST));
> field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE,
> fn_info_ptr_type);
> DECL_CHAIN (field) = fields;
> fields = field;
>
> finish_builtin_struct (type, "__gcov_info", fields, NULL_TREE);
>
> which leads to build_variant_type_copy.
>
> Once the struct is finished, we need to update the variants somewhere. Same
> loop walking the variants appears in finalize_type_size that is transitively
> called by finish_builtin_struct:
>
> /* Also layout any other variants of the type. */
> if (TYPE_NEXT_VARIANT (type)
> || type != TYPE_MAIN_VARIANT (type))
> {
> tree variant;
> /* Record layout info of this variant. */
> tree size = TYPE_SIZE (type);
> tree size_unit = TYPE_SIZE_UNIT (type);
> unsigned int align = TYPE_ALIGN (type);
> unsigned int user_align = TYPE_USER_ALIGN (type);
> enum machine_mode mode = TYPE_MODE (type);
>
> /* Copy it into all variants. */
> for (variant = TYPE_MAIN_VARIANT (type);
> variant != 0;
> variant = TYPE_NEXT_VARIANT (variant))
> {
> TYPE_SIZE (variant) = size;
> TYPE_SIZE_UNIT (variant) = size_unit;
> TYPE_ALIGN (variant) = align;
> TYPE_USER_ALIGN (variant) = user_align;
> SET_TYPE_MODE (variant, mode);
> }
> }
>
> Difference to my hunk is that the code works when called on non-main variant,
> but I think it makes sense to always finish main variant of builtin structs.
> (in fact I do not see why one would finalize size on non-main variants given
> that the sizes must match either)
So the issue seems to be:
gcov_info_type = lang_hooks.types.make_type (RECORD_TYPE);
gcov_fn_info_type = lang_hooks.types.make_type (RECORD_TYPE);
gcov_fn_info_ptr_type = build_pointer_type
(build_qualified_type (gcov_fn_info_type, TYPE_QUAL_CONST));
build_fn_info_type (gcov_fn_info_type, n_counters, gcov_info_type);
build_info_type (gcov_info_type, gcov_fn_info_ptr_type);
that __gcov_info has a member of type const __gcov_info * and that
rather than using the equivalent of
struct __gcov_info;
typedef const __gcov_info *gcov_fn_info_ptr_type;
struct __gcov_info {
...
gcov_fn_info_ptr_type x;
};
we build the variant of the yet incomplete struct and complete
it later.
Sth like
Index: coverage.c
===================================================================
--- coverage.c (revision 210965)
+++ coverage.c (working copy)
@@ -1078,9 +1078,10 @@
/* Build the info and fn_info types. These are mutually recursive. */
gcov_info_type = lang_hooks.types.make_type (RECORD_TYPE);
gcov_fn_info_type = lang_hooks.types.make_type (RECORD_TYPE);
+ build_fn_info_type (gcov_fn_info_type, n_counters, gcov_info_type);
+ gcov_info_type = lang_hooks.types.make_type (RECORD_TYPE);
gcov_fn_info_ptr_type = build_pointer_type
(build_qualified_type (gcov_fn_info_type, TYPE_QUAL_CONST));
- build_fn_info_type (gcov_fn_info_type, n_counters, gcov_info_type);
build_info_type (gcov_info_type, gcov_fn_info_ptr_type);
/* Build the gcov info var, this is referred to in its own
should fix it by delaying the variant build for one case after the record
has been completed and retaining the incomplete declaration for
the pointer type.
Richard.
> What would be correct fix then?
> Honza
>
>
>>
>> Thanks,
>> Richard.
>>
>> > if (align_type)
>> > {
