diff mbox series

Fix ICE in cp_var_mod_type_p

Message ID 20181119161558.isihgrgla2lp2nnl@kam.mff.cuni.cz
State New
Headers show
Series Fix ICE in cp_var_mod_type_p | expand

Commit Message

Jan Hubicka Nov. 19, 2018, 4:15 p.m. UTC
Hi,
enable-checking compiler crashes in free_lang_data because verify_type
is called too early (after we free data in types but before we clear the
langhooks) and it ends up calling cp_var_mod_type_p which ICEs.

This is fixed by moving type checking after hooks updates.  It would be
also possible to move hook update into free_lang_data_in_cgraph but it
seems to me that it is better to keep such a global cahgne in the
toplevel function (free_lang_data).

lto-bootstrapped/retested x86_64-linux, OK?

Honza
	* tree.c (free_lang_data_in_cgraph): Add argument fld; break out
	type checking to...
	(free_lang_data) ... here; update call of free_lang_data_in_cgraph.

Comments

Richard Biener Nov. 20, 2018, 8:08 a.m. UTC | #1
On Mon, 19 Nov 2018, Jan Hubicka wrote:

> Hi,
> enable-checking compiler crashes in free_lang_data because verify_type
> is called too early (after we free data in types but before we clear the
> langhooks) and it ends up calling cp_var_mod_type_p which ICEs.
> 
> This is fixed by moving type checking after hooks updates.  It would be
> also possible to move hook update into free_lang_data_in_cgraph but it
> seems to me that it is better to keep such a global cahgne in the
> toplevel function (free_lang_data).
> 
> lto-bootstrapped/retested x86_64-linux, OK?

OK.  I remember I had a similar patch that did unconditionally
verify_type (of course failing galore...)

Richard.

> Honza
> 	* tree.c (free_lang_data_in_cgraph): Add argument fld; break out
> 	type checking to...
> 	(free_lang_data) ... here; update call of free_lang_data_in_cgraph.
> Index: tree.c
> ===================================================================
> --- tree.c	(revision 266235)
> +++ tree.c	(working copy)
> @@ -6014,44 +6014,38 @@ assign_assembler_name_if_needed (tree t)
>     been set up.  */
>  
>  static void
> -free_lang_data_in_cgraph (void)
> +free_lang_data_in_cgraph (struct free_lang_data_d *fld)
>  {
>    struct cgraph_node *n;
>    varpool_node *v;
> -  struct free_lang_data_d fld;
>    tree t;
>    unsigned i;
>    alias_pair *p;
>  
>    /* Find decls and types in the body of every function in the callgraph.  */
>    FOR_EACH_FUNCTION (n)
> -    find_decls_types_in_node (n, &fld);
> +    find_decls_types_in_node (n, fld);
>  
>    FOR_EACH_VEC_SAFE_ELT (alias_pairs, i, p)
> -    find_decls_types (p->decl, &fld);
> +    find_decls_types (p->decl, fld);
>  
>    /* Find decls and types in every varpool symbol.  */
>    FOR_EACH_VARIABLE (v)
> -    find_decls_types_in_var (v, &fld);
> +    find_decls_types_in_var (v, fld);
>  
>    /* Set the assembler name on every decl found.  We need to do this
>       now because free_lang_data_in_decl will invalidate data needed
>       for mangling.  This breaks mangling on interdependent decls.  */
> -  FOR_EACH_VEC_ELT (fld.decls, i, t)
> +  FOR_EACH_VEC_ELT (fld->decls, i, t)
>      assign_assembler_name_if_needed (t);
>  
>    /* Traverse every decl found freeing its language data.  */
> -  FOR_EACH_VEC_ELT (fld.decls, i, t)
> -    free_lang_data_in_decl (t, &fld);
> +  FOR_EACH_VEC_ELT (fld->decls, i, t)
> +    free_lang_data_in_decl (t, fld);
>  
>    /* Traverse every type found freeing its language data.  */
> -  FOR_EACH_VEC_ELT (fld.types, i, t)
> -    free_lang_data_in_type (t, &fld);
> -  if (flag_checking)
> -    {
> -      FOR_EACH_VEC_ELT (fld.types, i, t)
> -	verify_type (t);
> -    }
> +  FOR_EACH_VEC_ELT (fld->types, i, t)
> +    free_lang_data_in_type (t, fld);
>  }
>  
>  
> @@ -6061,6 +6055,7 @@ static unsigned
>  free_lang_data (void)
>  {
>    unsigned i;
> +  struct free_lang_data_d fld;
>  
>    /* If we are the LTO frontend we have freed lang-specific data already.  */
>    if (in_lto_p
> @@ -6081,7 +6076,7 @@ free_lang_data (void)
>  
>    /* Traverse the IL resetting language specific information for
>       operands, expressions, etc.  */
> -  free_lang_data_in_cgraph ();
> +  free_lang_data_in_cgraph (&fld);
>  
>    /* Create gimple variants for common types.  */
>    for (unsigned i = 0;
> @@ -6102,6 +6097,15 @@ free_lang_data (void)
>  
>    lang_hooks.tree_inlining.var_mod_type_p = hook_bool_tree_tree_false;
>  
> +  if (flag_checking)
> +    {
> +      int i;
> +      tree t;
> +
> +      FOR_EACH_VEC_ELT (fld.types, i, t)
> +	verify_type (t);
> +    }
> +
>    /* We do not want the default decl_assembler_name implementation,
>       rather if we have fixed everything we want a wrapper around it
>       asserting that all non-local symbols already got their assembler
> Index: testsuite/g++.dg/torture/pr87997.C
> ===================================================================
> --- testsuite/g++.dg/torture/pr87997.C	(nonexistent)
> +++ testsuite/g++.dg/torture/pr87997.C	(working copy)
> @@ -0,0 +1,35 @@
> +/* { dg-do compile } */
> +template <typename, typename> struct a;
> +template <template <typename> class b, typename c, typename f, typename... d>
> +struct a<b<f, d...>, c> {
> +  using e = b<c>;
> +};
> +template <typename f> class h {
> +public:
> +  typedef f g;
> +};
> +template <typename j, typename c> using k = typename a<j, c>::e;
> +template <typename j> struct l { template <typename f> using m = k<j, f>; };
> +template <typename j> struct n {
> +  typedef typename j::g o;
> +  template <typename f> struct p {
> +    typedef typename l<j>::template m<f> other;
> +  };
> +};
> +template <typename f, typename j> struct F {
> +  typedef typename n<j>::template p<f>::other q;
> +};
> +template <typename f, typename j = h<f>> class r {
> +public:
> +  typename n<typename F<f, j>::q>::o operator[](long);
> +  f *t() noexcept;
> +};
> +class s {
> +  void m_fn2();
> +  r<int (s::*)()> u;
> +};
> +void s::m_fn2() try {
> +  for (int i;;)
> +    (this->*u[i])();
> +} catch (...) {
> +}
> 
>
diff mbox series

Patch

Index: tree.c
===================================================================
--- tree.c	(revision 266235)
+++ tree.c	(working copy)
@@ -6014,44 +6014,38 @@  assign_assembler_name_if_needed (tree t)
    been set up.  */
 
 static void
-free_lang_data_in_cgraph (void)
+free_lang_data_in_cgraph (struct free_lang_data_d *fld)
 {
   struct cgraph_node *n;
   varpool_node *v;
-  struct free_lang_data_d fld;
   tree t;
   unsigned i;
   alias_pair *p;
 
   /* Find decls and types in the body of every function in the callgraph.  */
   FOR_EACH_FUNCTION (n)
-    find_decls_types_in_node (n, &fld);
+    find_decls_types_in_node (n, fld);
 
   FOR_EACH_VEC_SAFE_ELT (alias_pairs, i, p)
-    find_decls_types (p->decl, &fld);
+    find_decls_types (p->decl, fld);
 
   /* Find decls and types in every varpool symbol.  */
   FOR_EACH_VARIABLE (v)
-    find_decls_types_in_var (v, &fld);
+    find_decls_types_in_var (v, fld);
 
   /* Set the assembler name on every decl found.  We need to do this
      now because free_lang_data_in_decl will invalidate data needed
      for mangling.  This breaks mangling on interdependent decls.  */
-  FOR_EACH_VEC_ELT (fld.decls, i, t)
+  FOR_EACH_VEC_ELT (fld->decls, i, t)
     assign_assembler_name_if_needed (t);
 
   /* Traverse every decl found freeing its language data.  */
-  FOR_EACH_VEC_ELT (fld.decls, i, t)
-    free_lang_data_in_decl (t, &fld);
+  FOR_EACH_VEC_ELT (fld->decls, i, t)
+    free_lang_data_in_decl (t, fld);
 
   /* Traverse every type found freeing its language data.  */
-  FOR_EACH_VEC_ELT (fld.types, i, t)
-    free_lang_data_in_type (t, &fld);
-  if (flag_checking)
-    {
-      FOR_EACH_VEC_ELT (fld.types, i, t)
-	verify_type (t);
-    }
+  FOR_EACH_VEC_ELT (fld->types, i, t)
+    free_lang_data_in_type (t, fld);
 }
 
 
@@ -6061,6 +6055,7 @@  static unsigned
 free_lang_data (void)
 {
   unsigned i;
+  struct free_lang_data_d fld;
 
   /* If we are the LTO frontend we have freed lang-specific data already.  */
   if (in_lto_p
@@ -6081,7 +6076,7 @@  free_lang_data (void)
 
   /* Traverse the IL resetting language specific information for
      operands, expressions, etc.  */
-  free_lang_data_in_cgraph ();
+  free_lang_data_in_cgraph (&fld);
 
   /* Create gimple variants for common types.  */
   for (unsigned i = 0;
@@ -6102,6 +6097,15 @@  free_lang_data (void)
 
   lang_hooks.tree_inlining.var_mod_type_p = hook_bool_tree_tree_false;
 
+  if (flag_checking)
+    {
+      int i;
+      tree t;
+
+      FOR_EACH_VEC_ELT (fld.types, i, t)
+	verify_type (t);
+    }
+
   /* We do not want the default decl_assembler_name implementation,
      rather if we have fixed everything we want a wrapper around it
      asserting that all non-local symbols already got their assembler
Index: testsuite/g++.dg/torture/pr87997.C
===================================================================
--- testsuite/g++.dg/torture/pr87997.C	(nonexistent)
+++ testsuite/g++.dg/torture/pr87997.C	(working copy)
@@ -0,0 +1,35 @@ 
+/* { dg-do compile } */
+template <typename, typename> struct a;
+template <template <typename> class b, typename c, typename f, typename... d>
+struct a<b<f, d...>, c> {
+  using e = b<c>;
+};
+template <typename f> class h {
+public:
+  typedef f g;
+};
+template <typename j, typename c> using k = typename a<j, c>::e;
+template <typename j> struct l { template <typename f> using m = k<j, f>; };
+template <typename j> struct n {
+  typedef typename j::g o;
+  template <typename f> struct p {
+    typedef typename l<j>::template m<f> other;
+  };
+};
+template <typename f, typename j> struct F {
+  typedef typename n<j>::template p<f>::other q;
+};
+template <typename f, typename j = h<f>> class r {
+public:
+  typename n<typename F<f, j>::q>::o operator[](long);
+  f *t() noexcept;
+};
+class s {
+  void m_fn2();
+  r<int (s::*)()> u;
+};
+void s::m_fn2() try {
+  for (int i;;)
+    (this->*u[i])();
+} catch (...) {
+}