Patchwork Fix PR rtl-optimization/52629

login
register
mail settings
Submitter Eric Botcazou
Date March 26, 2012, 8:42 a.m.
Message ID <201203261042.13711.ebotcazou@adacore.com>
Download mbox | patch
Permalink /patch/148652/
State New
Headers show

Comments

Eric Botcazou - March 26, 2012, 8:42 a.m.
This is an out-of-bounds access in count_spilled_pseudo: we can enter the 
function for a pseudo without hard reg but we immediately use the hard reg 
number as an index:

static void
count_spilled_pseudo (int spilled, int spilled_nregs, int reg)
{
  int freq = REG_FREQ (reg);
  int r = reg_renumber[reg];
  int nregs = hard_regno_nregs[r][PSEUDO_REGNO_MODE (reg)];

The negative R case is only checked a few lines below:

  /* Ignore spilled pseudo-registers which can be here only if IRA is
     used.  */
  if ((ira_conflicts_p && r < 0)
      || REGNO_REG_SET_P (&spilled_pseudos, reg)
      || spilled + spilled_nregs <= r || r + nregs <= spilled)
    return;


It turns out that count_pseudo contains very similar (and correct) code so the 
patch just mimics that code.

Bootstrapped/regtested on x86_64-suse-linux, applied on mainline.  Should it be 
applied to the release branches as well?


2012-03-26  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/52629
	* reload1.c (count_pseudo): Short-circuit common case.
	(count_spilled_pseudo): Return early for pseudos without hard regs.
	Assert that the pseudo has got a hard reg before manipulating it.
Eric Botcazou - March 27, 2012, 9 p.m.
> Bootstrapped/regtested on x86_64-suse-linux, applied on mainline.  Should
> it be applied to the release branches as well?
>
>
> 2012-03-26  Eric Botcazou  <ebotcazou@adacore.com>
>
> 	PR rtl-optimization/52629
> 	* reload1.c (count_pseudo): Short-circuit common case.
> 	(count_spilled_pseudo): Return early for pseudos without hard regs.
> 	Assert that the pseudo has got a hard reg before manipulating it.

RMs, what's your decision?
Richard Guenther - March 28, 2012, 8:05 a.m.
On Tue, 27 Mar 2012, Eric Botcazou wrote:

> > Bootstrapped/regtested on x86_64-suse-linux, applied on mainline.  Should
> > it be applied to the release branches as well?
> >
> >
> > 2012-03-26  Eric Botcazou  <ebotcazou@adacore.com>
> >
> > 	PR rtl-optimization/52629
> > 	* reload1.c (count_pseudo): Short-circuit common case.
> > 	(count_spilled_pseudo): Return early for pseudos without hard regs.
> > 	Assert that the pseudo has got a hard reg before manipulating it.
> 
> RMs, what's your decision?

What's the impact of this bug?  If it is a wrong-code or ice-on-valid
bug then it's ok for the 4.7 branch.

Richard.
Eric Botcazou - March 28, 2012, 9:17 a.m.
> What's the impact of this bug?  If it is a wrong-code or ice-on-valid
> bug then it's ok for the 4.7 branch.

Neither, it's an out-of-bounds memory access in reload1.c with apparently no 
visible effects as it has been introduced with IRA (4.4 series).

Patch

Index: reload1.c
===================================================================
--- reload1.c	(revision 185570)
+++ reload1.c	(working copy)
@@ -1746,11 +1746,12 @@  count_pseudo (int reg)
   int r = reg_renumber[reg];
   int nregs;
 
+  /* Ignore spilled pseudo-registers which can be here only if IRA is used.  */
+  if (ira_conflicts_p && r < 0)
+    return;
+
   if (REGNO_REG_SET_P (&pseudos_counted, reg)
-      || REGNO_REG_SET_P (&spilled_pseudos, reg)
-      /* Ignore spilled pseudo-registers which can be here only if IRA
-	 is used.  */
-      || (ira_conflicts_p && r < 0))
+      || REGNO_REG_SET_P (&spilled_pseudos, reg))
     return;
 
   SET_REGNO_REG_SET (&pseudos_counted, reg);
@@ -1827,12 +1828,17 @@  count_spilled_pseudo (int spilled, int s
 {
   int freq = REG_FREQ (reg);
   int r = reg_renumber[reg];
-  int nregs = hard_regno_nregs[r][PSEUDO_REGNO_MODE (reg)];
+  int nregs;
+
+  /* Ignore spilled pseudo-registers which can be here only if IRA is used.  */
+  if (ira_conflicts_p && r < 0)
+    return;
+
+  gcc_assert (r >= 0);
+
+  nregs = hard_regno_nregs[r][PSEUDO_REGNO_MODE (reg)];
 
-  /* Ignore spilled pseudo-registers which can be here only if IRA is
-     used.  */
-  if ((ira_conflicts_p && r < 0)
-      || REGNO_REG_SET_P (&spilled_pseudos, reg)
+  if (REGNO_REG_SET_P (&spilled_pseudos, reg)
       || spilled + spilled_nregs <= r || r + nregs <= spilled)
     return;