From patchwork Tue Mar 6 16:26:38 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: fix memory corruption bug in tm_region_init Date: Tue, 06 Mar 2012 06:26:38 -0000 From: Aldy Hernandez X-Patchwork-Id: 144959 Message-Id: <4F563ABE.7090501@redhat.com> To: Jakub Jelinek Cc: Richard Henderson , gcc-patches , Torvald Riegel On 03/06/12 10:20, Jakub Jelinek wrote: > On Tue, Mar 06, 2012 at 08:04:12AM -0800, Richard Henderson wrote: >> On 03/06/12 07:55, Aldy Hernandez wrote: >>> + bb_regions = VEC_alloc (tm_region_p, heap, last_basic_block); >>> + VEC_reserve (tm_region_p, heap, bb_regions, last_basic_block); >>> + for (i = 0; i< last_basic_block; ++i) >>> + VEC_quick_insert (tm_region_p, bb_regions, i, NULL); >> >> The reserve is redundant, since you already did that in the alloc. >> You're looking for VEC_safe_grow_cleared here instead of that loop. I got confused because reload1.c had a similar loop which I based mine off of (grow_reg_equivs). >> >> Otherwise ok. > > And VEC_safe_grow_cleared will do even the VEC_alloc if bb_regions is > NULL before this or you just set it to NULL before VEC_safe_grow_cleared. > > Jakub Yes, I just noticed as I was testing the patch below. Committing to 4.7 branch and mainline. Thanks guys. * trans-mem.c: New typedef for tm_region_p. Define vector types for tm_region_p. (tm_region_init): Replace region_worklist to a vector called bb_regions. Index: trans-mem.c =================================================================== --- trans-mem.c (revision 184949) +++ trans-mem.c (working copy) @@ -1757,6 +1757,10 @@ struct tm_region bitmap irr_blocks; }; +typedef struct tm_region *tm_region_p; +DEF_VEC_P (tm_region_p); +DEF_VEC_ALLOC_P (tm_region_p, heap); + /* True if there are pending edge statements to be committed for the current function being scanned in the tmmark pass. */ bool pending_edge_inserts_p; @@ -1858,7 +1862,7 @@ tm_region_init (struct tm_region *region VEC(basic_block, heap) *queue = NULL; bitmap visited_blocks = BITMAP_ALLOC (NULL); struct tm_region *old_region; - struct tm_region **region_worklist; + VEC(tm_region_p, heap) *bb_regions = NULL; all_tm_regions = region; bb = single_succ (ENTRY_BLOCK_PTR); @@ -1866,17 +1870,15 @@ tm_region_init (struct tm_region *region /* We could store this information in bb->aux, but we may get called through get_all_tm_blocks() from another pass that may be already using bb->aux. */ - region_worklist = - (struct tm_region **) xcalloc (sizeof (struct tm_region *), - last_basic_block + NUM_FIXED_BLOCKS); + VEC_safe_grow_cleared (tm_region_p, heap, bb_regions, last_basic_block); VEC_safe_push (basic_block, heap, queue, bb); - region_worklist[bb->index] = region; + VEC_replace (tm_region_p, bb_regions, bb->index, region); do { bb = VEC_pop (basic_block, queue); - region = region_worklist[bb->index]; - region_worklist[bb->index] = NULL; + region = VEC_index (tm_region_p, bb_regions, bb->index); + VEC_replace (tm_region_p, bb_regions, bb->index, NULL); /* Record exit and irrevocable blocks. */ region = tm_region_init_1 (region, bb); @@ -1898,15 +1900,15 @@ tm_region_init (struct tm_region *region the entry block of the new region is associated with this region. Other successors are still part of the old region. */ if (old_region != region && e->dest != region->entry_block) - region_worklist[e->dest->index] = old_region; + VEC_replace (tm_region_p, bb_regions, e->dest->index, old_region); else - region_worklist[e->dest->index] = region; + VEC_replace (tm_region_p, bb_regions, e->dest->index, region); } } while (!VEC_empty (basic_block, queue)); VEC_free (basic_block, heap, queue); BITMAP_FREE (visited_blocks); - free (region_worklist); + VEC_free (tm_region_p, heap, bb_regions); } /* The "gate" function for all transactional memory expansion and optimization