Message ID | 20130219223007.GO1215@tucnak.zalov.cz |
---|---|
State | New |
Headers | show |
On Tue, 19 Feb 2013, Jakub Jelinek wrote: > Hi! > > On the following patch gcc ICEs because malloc memory is corrupted. > The problem is that const_val array is allocated at the start of the pass, > but during the execution of ccp some new SSA_NAMEs are created > (update_call_from_tree if I remember well). > > The following patch fixes that by making const_val a vector instead of > array, and growing it when needed. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Ick. This happens from calling fold_stmt during substitute_and_fold, a place where increasing the lattice isn't useful. So I'd prefer the same fix as present in VRP for this issue, simply return VARYING for out-of-bound accesses. Same issue is latent for copyprop. I'm testing the following. Richard. 2013-02-20 Richard Biener <rguenther@suse.de> Jakub Jelinek <jakub@redhat.com> PR tree-optimization/56396 * tree-ssa-ccp.c (n_const_val): New static variable. (get_value): Return NULL for SSA names we don't have a lattice entry for. (ccp_initialize): Initialize n_const_val. * tree-ssa-copy.c (n_copy_of): New static variable. (init_copy_prop): Initialize n_copy_of. (get_value): Return NULL_TREE for SSA names we don't have a lattice entry for. * gcc.dg/pr56396.c: New testcase. Index: gcc/tree-ssa-ccp.c =================================================================== *** gcc/tree-ssa-ccp.c (revision 196167) --- gcc/tree-ssa-ccp.c (working copy) *************** typedef struct prop_value_d prop_value_t *** 162,167 **** --- 162,168 ---- memory reference used to store (i.e., the LHS of the assignment doing the store). */ static prop_value_t *const_val; + static unsigned n_const_val; static void canonicalize_float_value (prop_value_t *); static bool ccp_fold_stmt (gimple_stmt_iterator *); *************** get_value (tree var) *** 295,301 **** { prop_value_t *val; ! if (const_val == NULL) return NULL; val = &const_val[SSA_NAME_VERSION (var)]; --- 296,303 ---- { prop_value_t *val; ! if (const_val == NULL ! || SSA_NAME_VERSION (var) >= n_const_val) return NULL; val = &const_val[SSA_NAME_VERSION (var)]; *************** ccp_initialize (void) *** 713,719 **** { basic_block bb; ! const_val = XCNEWVEC (prop_value_t, num_ssa_names); /* Initialize simulation flags for PHI nodes and statements. */ FOR_EACH_BB (bb) --- 715,722 ---- { basic_block bb; ! n_const_val = num_ssa_names; ! const_val = XCNEWVEC (prop_value_t, n_const_val); /* Initialize simulation flags for PHI nodes and statements. */ FOR_EACH_BB (bb) Index: gcc/tree-ssa-copy.c =================================================================== *** gcc/tree-ssa-copy.c (revision 196167) --- gcc/tree-ssa-copy.c (working copy) *************** struct prop_value_d { *** 280,285 **** --- 280,286 ---- typedef struct prop_value_d prop_value_t; static prop_value_t *copy_of; + static unsigned n_copy_of; /* Return true if this statement may generate a useful copy. */ *************** init_copy_prop (void) *** 664,670 **** { basic_block bb; ! copy_of = XCNEWVEC (prop_value_t, num_ssa_names); FOR_EACH_BB (bb) { --- 665,672 ---- { basic_block bb; ! n_copy_of = num_ssa_names; ! copy_of = XCNEWVEC (prop_value_t, n_copy_of); FOR_EACH_BB (bb) { *************** init_copy_prop (void) *** 728,734 **** static tree get_value (tree name) { ! tree val = copy_of[SSA_NAME_VERSION (name)].value; if (val && val != name) return val; return NULL_TREE; --- 730,739 ---- static tree get_value (tree name) { ! tree val; ! if (SSA_NAME_VERSION (name) >= n_copy_of) ! return NULL_TREE; ! val = copy_of[SSA_NAME_VERSION (name)].value; if (val && val != name) return val; return NULL_TREE; Index: gcc/testsuite/gcc.dg/pr56396.c =================================================================== *** gcc/testsuite/gcc.dg/pr56396.c (revision 0) --- gcc/testsuite/gcc.dg/pr56396.c (working copy) *************** *** 0 **** --- 1,22 ---- + /* PR tree-optimization/56396 */ + /* { dg-do compile } */ + /* { dg-options "-O2 -fpic -g" } */ + + struct S { char *s; int z; }; + struct T { int t; } *c, u; + void bar (int, const char *); + + inline void * + foo (void *x, char *y, int z) + { + struct S s; + char b[256]; + s.s = b; + s.z = __builtin___sprintf_chk (s.s, 1, __builtin_object_size (s.s, 2), "Require"); + if (s.z < 0) + bar (u.t | c->t, "rls"); + if (foo (x, s.s, s.z)) + { + } + return (void *) 0; + }
On Wed, Feb 20, 2013 at 10:52:52AM +0100, Richard Biener wrote: > *************** get_value (tree var) > *** 295,301 **** > { > prop_value_t *val; > > ! if (const_val == NULL) > return NULL; > > val = &const_val[SSA_NAME_VERSION (var)]; > --- 296,303 ---- > { > prop_value_t *val; > > ! if (const_val == NULL > ! || SSA_NAME_VERSION (var) >= n_const_val) > return NULL; You could just use if (SSA_NAME_VERSION (var) >= n_const_val) test here if upon free (const_val); you'd set n_const_val back to 0. Jakub
--- gcc/tree-ssa-ccp.c.jj 2013-01-11 09:02:48.000000000 +0100 +++ gcc/tree-ssa-ccp.c 2013-02-19 20:04:21.194681227 +0100 @@ -155,13 +155,13 @@ struct prop_value_d { typedef struct prop_value_d prop_value_t; -/* Array of propagated constant values. After propagation, +/* Vector of propagated constant values. After propagation, CONST_VAL[I].VALUE holds the constant value for SSA_NAME(I). If the constant is held in an SSA name representing a memory store (i.e., a VDEF), CONST_VAL[I].MEM_REF will contain the actual memory reference used to store (i.e., the LHS of the assignment doing the store). */ -static prop_value_t *const_val; +static vec<prop_value_t> const_val; static void canonicalize_float_value (prop_value_t *); static bool ccp_fold_stmt (gimple_stmt_iterator *); @@ -295,9 +295,12 @@ get_value (tree var) { prop_value_t *val; - if (const_val == NULL) + if (!const_val.exists ()) return NULL; + if (SSA_NAME_VERSION (var) >= const_val.length ()) + const_val.safe_grow_cleared (SSA_NAME_VERSION (var) + 32); + val = &const_val[SSA_NAME_VERSION (var)]; if (val->lattice_val == UNINITIALIZED) *val = get_default_value (var); @@ -333,8 +336,12 @@ get_constant_value (tree var) static inline void set_value_varying (tree var) { - prop_value_t *val = &const_val[SSA_NAME_VERSION (var)]; + prop_value_t *val; + if (SSA_NAME_VERSION (var) >= const_val.length ()) + const_val.safe_grow_cleared (SSA_NAME_VERSION (var) + 32); + + val = &const_val[SSA_NAME_VERSION (var)]; val->lattice_val = VARYING; val->value = NULL_TREE; val->mask = double_int_minus_one; @@ -428,7 +435,12 @@ static bool set_lattice_value (tree var, prop_value_t new_val) { /* We can deal with old UNINITIALIZED values just fine here. */ - prop_value_t *old_val = &const_val[SSA_NAME_VERSION (var)]; + prop_value_t *old_val; + + if (SSA_NAME_VERSION (var) >= const_val.length ()) + const_val.safe_grow_cleared (SSA_NAME_VERSION (var) + 32); + + old_val = &const_val[SSA_NAME_VERSION (var)]; canonicalize_float_value (&new_val); @@ -713,7 +725,7 @@ ccp_initialize (void) { basic_block bb; - const_val = XCNEWVEC (prop_value_t, num_ssa_names); + const_val.create (num_ssa_names); /* Initialize simulation flags for PHI nodes and statements. */ FOR_EACH_BB (bb) @@ -774,6 +786,9 @@ static void do_dbg_cnt (void) { unsigned i; + + if (num_ssa_names >= const_val.length ()) + const_val.safe_grow_cleared (num_ssa_names); for (i = 0; i < num_ssa_names; i++) { if (!dbg_cnt (ccp)) @@ -829,9 +844,8 @@ ccp_finalize (void) something_changed = substitute_and_fold (get_constant_value, ccp_fold_stmt, true); - free (const_val); - const_val = NULL; - return something_changed;; + const_val.release (); + return something_changed; } --- gcc/testsuite/gcc.dg/pr56396.c.jj 2013-02-19 20:02:48.663057957 +0100 +++ gcc/testsuite/gcc.dg/pr56396.c 2013-02-19 19:59:00.000000000 +0100 @@ -0,0 +1,22 @@ +/* PR tree-optimization/56396 */ +/* { dg-do compile } */ +/* { dg-options "-O2 -fpic -g" } */ + +struct S { char *s; int z; }; +struct T { int t; } *c, u; +void bar (int, const char *); + +inline void * +foo (void *x, char *y, int z) +{ + struct S s; + char b[256]; + s.s = b; + s.z = __builtin___sprintf_chk (s.s, 1, __builtin_object_size (s.s, 2), "Require"); + if (s.z < 0) + bar (u.t | c->t, "rls"); + if (foo (x, s.s, s.z)) + { + } + return (void *) 0; +}