This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PR 34472: Fix use-after-free() and VEC iteration bugs in ipa-struct-reorg
- From: Olga Golovanevsky <OLGA at il dot ibm dot com>
- To: "Diego Novillo" <dnovillo at google dot com>
- Cc: Richard Sandiford <rsandifo at nildram dot co dot uk>, gcc-patches at gcc dot gnu dot org
- Date: Thu, 24 Jan 2008 15:05:11 +0200
- Subject: Re: PR 34472: Fix use-after-free() and VEC iteration bugs in ipa-struct-reorg
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,