On Thu, 6 Feb 2014, Jakub Jelinek wrote: > Hi! > > This patch fixes a double free on simd-lane access if > get_vectype_for_scalar_type fails (the new dr is already in datarefs[i] > and if we free_data_ref it, it will be free_data_ref'ed again at the end of > the failed vectorization), fixes handling of clobbers (plugs a memory leak > due to missed free_data_ref and more importantly, if we goto again, dr > would be kept at the old dr and thus we'd basically remove all the remaining > drs in a loop) and plugs a memory leak with simd_lane_access. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok. > PS, just looking at the patch now again, in the light of PR59594 > the reordering of datarefs before goto again looks also wrong, we IMHO have > to perform a vector ordered remove in that case, ok to handle it as a > follow-up? Yeah... Thanks, Richard. > 2014-02-06 Jakub Jelinek <ja...@redhat.com> > > PR middle-end/59150 > * tree-vect-data-refs.c (vect_analyze_data_refs): For clobbers, call > free_data_ref on the dr first, and before goto again also set dr > to the next dr. For simd_lane_access, free old datarefs[i] before > overwriting it. For get_vectype_for_scalar_type failure, don't > free_data_ref if simd_lane_access. > > --- gcc/tree-vect-data-refs.c.jj 2014-02-05 15:28:10.000000000 +0100 > +++ gcc/tree-vect-data-refs.c 2014-02-05 18:30:53.323659288 +0100 > @@ -3297,12 +3297,13 @@ again: > clobber stmts during vectorization. */ > if (gimple_clobber_p (stmt)) > { > + free_data_ref (dr); > if (i == datarefs.length () - 1) > { > datarefs.pop (); > break; > } > - datarefs[i] = datarefs.pop (); > + datarefs[i] = dr = datarefs.pop (); > goto again; > } > > @@ -3643,13 +3644,14 @@ again: > if (simd_lane_access) > { > STMT_VINFO_SIMD_LANE_ACCESS_P (stmt_info) = true; > + free_data_ref (datarefs[i]); > datarefs[i] = dr; > } > > /* Set vectype for STMT. */ > scalar_type = TREE_TYPE (DR_REF (dr)); > - STMT_VINFO_VECTYPE (stmt_info) = > - get_vectype_for_scalar_type (scalar_type); > + STMT_VINFO_VECTYPE (stmt_info) > + = get_vectype_for_scalar_type (scalar_type); > if (!STMT_VINFO_VECTYPE (stmt_info)) > { > if (dump_enabled_p ()) > @@ -3669,7 +3671,8 @@ again: > if (gather || simd_lane_access) > { > STMT_VINFO_DATA_REF (stmt_info) = NULL; > - free_data_ref (dr); > + if (gather) > + free_data_ref (dr); > } > return false; > } > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer