Re: PR 34472: Fix use-after-free() and VEC iteration bugs in ipa-struct-reorg

[Richard Sandiford <rsandifo at nildram dot co dot uk>]

"Richard Guenther" <richard.guenther@gmail.com> writes:
> On Jan 24, 2008 2:05 PM, Olga Golovanevsky <OLGA@il.ibm.com> wrote:
>> Richard Sandiford wrote on 10/01/2008 13:25:55:
>>
>> > The PR is about existing testsuite FAILs that only show up on some
>> > host/target combinations.
>>
>> After the Andreas' patch that solves execution failures
>>
>> http://gcc.gnu.org/ml/gcc-patches/2008-01/msg01081.html
>>
>> this patch is ok for me with the comments below and Eric's comment.
>>
>> Ok for mainline?
>
> Ok.

Thanks.

>> > gcc/
>> >    PR rtl-optimization/34472.
>> >    * ipa-struct-reorg.c (safe_cond_expr_check): Change the DATA
>> >    parameter to a "bool *" and set *DATA to false if there is
>> >    an unsafe access.  Do not delete the structure here.
>> >    (check_cond_exprs): Delete it here instead.  Do not increase I
>> >    when removing a structure.
>> >    (exclude_cold_structs): Do not increase I when removing a structure.
>>
>> You can unite two functions with the same change:
>>
>>     (check_cond_exprs, exclude_cold_structs): Do not increase I when
>>     removing a structure
...
>> > @@ -3542,9 +3540,17 @@ check_cond_exprs (void)
>> >    d_str str;
>> >    unsigned i;
>> >
>> > -  for (i = 0; VEC_iterate (structure, structures, i, str); i++)
>> > -    if (str->accs)
>> > -      htab_traverse (str->accs, safe_cond_expr_check, &i);
>> > +  i = 0;
>> > +  while (VEC_iterate (structure, structures, i, str))
>> > +    {
>> > +      bool safe_p = true;
>>
>> empty line here.
>>
>> > +      if (str->accs)
>> > +   htab_traverse (str->accs, safe_cond_expr_check, &safe_p);
>> > +      if (!safe_p)
>> > +   remove_structure (i);
>> > +      else
>> > +   i++;
>> > +    }
>> >  }

OK, here's what I applied.

Richard


gcc/
	PR tree-optimization/34472
	* ipa-struct-reorg.c (safe_cond_expr_check): Change the DATA
	parameter to a "bool *" and set *DATA to false if there is
	an unsafe access.  Do not delete the structure here.
	(check_cond_exprs): Delete it here instead.
	(check_cond_exprs, exclude_cold_structs): Do not increase
	I when removing a structure.

Index: gcc/ipa-struct-reorg.c
===================================================================
--- gcc/ipa-struct-reorg.c	2008-01-24 17:44:35.000000000 +0000
+++ gcc/ipa-struct-reorg.c	2008-01-24 17:44:45.000000000 +0000
@@ -3074,26 +3074,24 @@ dump_accs (d_str str)
 }
 
 /* This function checks whether an access statement, pointed by SLOT,
-   is a condition we are capable to transform. If not, it removes
-   the structure with index, represented by DATA, from the vector
-   of structures.  */
+   is a condition we are capable to transform.  It returns false if not,
+   setting bool *DATA to false.  */
  
 static int
 safe_cond_expr_check (void **slot, void *data)
 {
   struct access_site *acc = *(struct access_site **) slot;
 
-  if (TREE_CODE (acc->stmt) == COND_EXPR)
+  if (TREE_CODE (acc->stmt) == COND_EXPR
+      && !is_safe_cond_expr (acc->stmt))
     {
-      if (!is_safe_cond_expr (acc->stmt))
+      if (dump_file)
 	{
-	  if (dump_file)
-	    {
-	      fprintf (dump_file, "\nUnsafe conditional statement ");
-	      print_generic_stmt (dump_file, acc->stmt, 0);
-	    }
-	  remove_structure (*(unsigned *) data);
+	  fprintf (dump_file, "\nUnsafe conditional statement ");
+	  print_generic_stmt (dump_file, acc->stmt, 0);
 	}
+      *(bool *) data = false;
+      return 0;
     }
   return 1;
 }
@@ -3547,9 +3545,18 @@ check_cond_exprs (void)
   d_str str;
   unsigned i;
 
-  for (i = 0; VEC_iterate (structure, structures, i, str); i++)
-    if (str->accs)
-      htab_traverse (str->accs, safe_cond_expr_check, &i);
+  i = 0;
+  while (VEC_iterate (structure, structures, i, str))
+    {
+      bool safe_p = true;
+
+      if (str->accs)
+	htab_traverse (str->accs, safe_cond_expr_check, &safe_p);
+      if (!safe_p)
+	remove_structure (i);
+      else
+	i++;
+    }
 }
 
 /* We exclude from non-field accesses of the structure 
@@ -3859,7 +3866,8 @@ exclude_cold_structs (void)
     sum_counts (str, &hotest);
 
   /* Remove cold structures from structures vector.  */
-  for (i = 0; VEC_iterate (structure, structures, i, str); i++)
+  i = 0;
+  while (VEC_iterate (structure, structures, i, str))
     if (str->count * 100 < (hotest * STRUCT_REORG_COLD_STRUCT_RATIO))
       {
 	if (dump_file)
@@ -3870,6 +3878,8 @@ exclude_cold_structs (void)
 	  }
 	remove_structure (i);
       }
+    else
+      i++;
 }
 
 /* This function decomposes original structure into substructures,