Patchwork Fix PR57441 (memory error in SLSR)

login
register
mail settings
Submitter William J. Schmidt
Date May 29, 2013, 2:50 p.m.
Message ID <1369839046.3473.7.camel@gnopaine>
Download mbox | patch
Permalink /patch/247277/
State New
Headers show

Comments

William J. Schmidt - May 29, 2013, 2:50 p.m.
Hi,

Prior to adding conditional candidates to SLSR, the size of the
increment vector was bounded by the number of related candidates, and
the vector was malloc'd to be that size.  With conditional candidates,
there can be an additional increment for each related PHI operand,
causing potential overflow and heap corruption.  This patch removes the
no longer valid assumption and allocates a fixed-size vector.

Bootstrap and regtest in progress on powerpc64-linux-gnu.  Ok for trunk
if everything checks out?

Thanks,
Bill


2013-05-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>

	* gimple-ssa-strength-reduction.c (analyze_candidates_and_replace):
	Don't limit size of incr_vec to number of candidates.
Richard Guenther - May 29, 2013, 3:07 p.m.
On Wed, May 29, 2013 at 4:50 PM, Bill Schmidt
<wschmidt@linux.vnet.ibm.com> wrote:
> Hi,
>
> Prior to adding conditional candidates to SLSR, the size of the
> increment vector was bounded by the number of related candidates, and
> the vector was malloc'd to be that size.  With conditional candidates,
> there can be an additional increment for each related PHI operand,
> causing potential overflow and heap corruption.  This patch removes the
> no longer valid assumption and allocates a fixed-size vector.
>
> Bootstrap and regtest in progress on powerpc64-linux-gnu.  Ok for trunk
> if everything checks out?

Ok with referencing the PR in the changelog and adding the testcase.

Thanks,
Richard.

> Thanks,
> Bill
>
>
> 2013-05-29  Bill Schmidt  <wschmidt@linux.vnet.ibm.com>
>
>         * gimple-ssa-strength-reduction.c (analyze_candidates_and_replace):
>         Don't limit size of incr_vec to number of candidates.
>
>
> Index: gcc/gimple-ssa-strength-reduction.c
> ===================================================================
> --- gcc/gimple-ssa-strength-reduction.c (revision 199406)
> +++ gcc/gimple-ssa-strength-reduction.c (working copy)
> @@ -3361,7 +3361,6 @@ analyze_candidates_and_replace (void)
>          less expensive to calculate than the replaced statements.  */
>        else
>         {
> -         int length;
>           enum machine_mode mode;
>           bool speed;
>
> @@ -3372,14 +3371,11 @@ analyze_candidates_and_replace (void)
>
>           /* If all candidates have already been replaced under other
>              interpretations, nothing remains to be done.  */
> -         length = count_candidates (c);
> -         if (!length)
> +         if (!count_candidates (c))
>             continue;
> -         if (length > MAX_INCR_VEC_LEN)
> -           length = MAX_INCR_VEC_LEN;
>
>           /* Construct an array of increments for this candidate chain.  */
> -         incr_vec = XNEWVEC (incr_info, length);
> +         incr_vec = XNEWVEC (incr_info, MAX_INCR_VEC_LEN);
>           incr_vec_len = 0;
>           record_increments (c);
>
>
>

Patch

Index: gcc/gimple-ssa-strength-reduction.c
===================================================================
--- gcc/gimple-ssa-strength-reduction.c	(revision 199406)
+++ gcc/gimple-ssa-strength-reduction.c	(working copy)
@@ -3361,7 +3361,6 @@  analyze_candidates_and_replace (void)
 	 less expensive to calculate than the replaced statements.  */
       else
 	{
-	  int length;
 	  enum machine_mode mode;
 	  bool speed;
 
@@ -3372,14 +3371,11 @@  analyze_candidates_and_replace (void)
 
 	  /* If all candidates have already been replaced under other
 	     interpretations, nothing remains to be done.  */
-	  length = count_candidates (c);
-	  if (!length)
+	  if (!count_candidates (c))
 	    continue;
-	  if (length > MAX_INCR_VEC_LEN)
-	    length = MAX_INCR_VEC_LEN;
 
 	  /* Construct an array of increments for this candidate chain.  */
-	  incr_vec = XNEWVEC (incr_info, length);
+	  incr_vec = XNEWVEC (incr_info, MAX_INCR_VEC_LEN);
 	  incr_vec_len = 0;
 	  record_increments (c);