On Thu, 21 Mar 2019, Jakub Jelinek wrote:

> Hi!
> 
> As mentioned in the PR, if e.g. build_aligned_type creates some specially
> aligned variant of some TYPE_MAIN_VARIANT, that type doesn't appear in the
> IL during free_lang_data and later on we call build_aligned_type with the
> same arguments as before, we'll get the previously created aligned variant
> which has not been free_lang_data processed and during LTO streaming we ICE
> because of that.
> 
> The following patch prunes unmarked types from the type variant chain, so
> that we don't find them later on and instead create new type variants from
> the free_lang_data processed types.
> 
> Another change is just to be sure, marking TYPE_CANONICAL if it is not
> already marked, so that we don't get weird behavior if we'd remove it.
> In the usual case TYPE_CANONICAL will be the TYPE_MAIN_VARIANT type we mark
> already anyway.
> 
> The rest of the changes is to make sure the new types we create during
> free_lang_data get marked in the pset, so that we will not try to purge
> them.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

LGTM - any reason you only sometimes conditionalize add_tree_to_fld_list
on the fld->pset.add () result?  IMHO consistency should win over
micro-optimizing the case you know the type will not be in the set.

Thanks,
Richard.

> 2019-03-20  Jan Hubicka  <hubi...@ucw.cz>
>           Jakub Jelinek  <ja...@redhat.com>
> 
>       PR lto/89692
>       * tree.c (fld_type_variant): Call fld->pset.add.
>       (fld_incomplete_type_of): Likewise and don't call add_tree_to_fld_list
>       if it returns true.
>       (fld_process_array_type): Likewise.
>       (free_lang_data_in_type): Purge non-marked types from TYPE_NEXT_VARIANT
>       list.
>       (find_decls_types_r): Call fld_worklist_push for TYPE_CANONICAL (t).
> 
>       * g++.dg/other/pr89692.C: New test.
> 
> --- gcc/tree.c.jj     2019-03-20 12:24:56.443322228 +0100
> +++ gcc/tree.c        2019-03-20 20:16:12.277091827 +0100
> @@ -5215,6 +5215,7 @@ fld_type_variant (tree first, tree t, st
>    if (inner_type)
>      TREE_TYPE (v) = inner_type;
>    gcc_checking_assert (fld_type_variant_equal_p (t,v, inner_type));
> +  fld->pset.add (v);
>    add_tree_to_fld_list (v, fld);
>    return v;
>  }
> @@ -5253,7 +5254,8 @@ fld_process_array_type (tree t, tree t2,
>        array = build_array_type_1 (t2, TYPE_DOMAIN (t),
>                                 TYPE_TYPELESS_STORAGE (t), false);
>        TYPE_CANONICAL (array) = TYPE_CANONICAL (t);
> -      add_tree_to_fld_list (array, fld);
> +      if (!fld->pset.add (array))
> +     add_tree_to_fld_list (array, fld);
>      }
>    return array;
>  }
> @@ -5298,7 +5300,8 @@ fld_incomplete_type_of (tree t, struct f
>                                               TYPE_REF_CAN_ALIAS_ALL (t));
>         gcc_assert (TYPE_CANONICAL (t2) != t2
>                     && TYPE_CANONICAL (t2) == TYPE_CANONICAL (TREE_TYPE (t)));
> -       add_tree_to_fld_list (first, fld);
> +       if (!fld->pset.add (first))
> +         add_tree_to_fld_list (first, fld);
>         return fld_type_variant (first, t, fld);
>       }
>        return t;
> @@ -5321,6 +5324,7 @@ fld_incomplete_type_of (tree t, struct f
>         copy = build_distinct_type_copy (t);
>  
>         /* It is possible that type was not seen by free_lang_data yet.  */
> +       fld->pset.add (copy);
>         add_tree_to_fld_list (copy, fld);
>         TYPE_SIZE (copy) = NULL;
>         TYPE_USER_ALIGN (copy) = 0;
> @@ -5445,6 +5449,18 @@ free_lang_data_in_type (tree type, struc
>  
>    TYPE_NEEDS_CONSTRUCTING (type) = 0;
>  
> +  /* Purge non-marked variants from the variants chain, so that they
> +     don't reappear in the IL after free_lang_data.  */
> +  while (TYPE_NEXT_VARIANT (type)
> +      && !fld->pset.contains (TYPE_NEXT_VARIANT (type)))
> +    {
> +      tree t = TYPE_NEXT_VARIANT (type);
> +      TYPE_NEXT_VARIANT (type) = TYPE_NEXT_VARIANT (t);
> +      /* Turn the removed types into distinct types.  */
> +      TYPE_MAIN_VARIANT (t) = t;
> +      TYPE_NEXT_VARIANT (t) = NULL_TREE;
> +    }
> +
>    if (TREE_CODE (type) == FUNCTION_TYPE)
>      {
>        TREE_TYPE (type) = fld_simplified_type (TREE_TYPE (type), fld);
> @@ -5464,6 +5480,7 @@ free_lang_data_in_type (tree type, struc
>                         & ~TYPE_QUAL_CONST
>                         & ~TYPE_QUAL_VOLATILE;
>             TREE_VALUE (p) = build_qualified_type (arg_type, quals);
> +           fld->pset.add (TREE_VALUE (p));
>             free_lang_data_in_type (TREE_VALUE (p), fld);
>           }
>         /* C++ FE uses TREE_PURPOSE to store initial values.  */
> @@ -5886,8 +5903,7 @@ find_decls_types_r (tree *tp, int *ws, v
>           ctx = BLOCK_SUPERCONTEXT (ctx);
>         fld_worklist_push (ctx, fld);
>       }
> -      /* Do not walk TYPE_CANONICAL.  We do not stream it and thus do not
> -      and want not to reach unused types this way.  */
> +      fld_worklist_push (TYPE_CANONICAL (t), fld);
>  
>        if (RECORD_OR_UNION_TYPE_P (t) && TYPE_BINFO (t))
>       {
> --- gcc/testsuite/g++.dg/other/pr89692.C.jj   2019-03-20 20:20:10.233278697 
> +0100
> +++ gcc/testsuite/g++.dg/other/pr89692.C      2019-03-20 20:19:53.193551778 
> +0100
> @@ -0,0 +1,20 @@
> +// PR lto/89692
> +// { dg-do compile }
> +// { dg-require-effective-target lto }
> +// { dg-options "-flto -O2" }
> +
> +struct S {
> +  short int a, b;
> +  unsigned char c : 1;
> +};
> +
> +bool
> +foo (void)
> +{
> +  unsigned char d[sizeof (S)] = { 0 };
> +  S e;
> +
> +  __builtin_memcpy (&e, d, sizeof (d));
> +
> +  return e.c == d[0];
> +}
> 
>       Jakub
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Reply via email to