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

[Olga Golovanevsky <OLGA at il dot ibm dot com>]

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?

Olga

>
> Richard
>
>
> 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.
>
> Index: gcc/ipa-struct-reorg.c
> ===================================================================
> --- gcc/ipa-struct-reorg.c   2007-12-28 14:37:34.000000000 +0000
> +++ gcc/ipa-struct-reorg.c   2007-12-28 14:50:12.000000000 +0000
> @@ -3069,26 +3069,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;
>  }
> @@ -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++;
> +    }
>  }
>
>  /* We exclude from non-field accesses of the structure
> @@ -3854,7 +3860,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)
> @@ -3865,6 +3872,8 @@ exclude_cold_structs (void)
>       }
>     remove_structure (i);
>        }
> +    else
> +      i++;
>  }
>
>  /* This function decomposes original structure into substructures,