[Patch, IRA] Fix a function accessing beyond end-of-array

[Patch, IRA] Fix a function accessing beyond end-of-array

[Hari Sandanagobalane]

Hello,
I discussed this problem with Vlad in 
http://gcc.gnu.org/ml/gcc/2011-05/msg00131.html. I propose the following 
patch to fix it. Okay to commit?

Revised the ChangeLog.

Thanks
Hari

ChangeLog:
         * ira.c (clarify_prohibited_class_mode_regs): Prevent the 
function from accessing beyond the end of REGNO_REG_CLASS array by 
stopping the loop early.

Patch:
Index: gcc/ira.c
===================================================================
--- gcc/ira.c   (revision 173654)
+++ gcc/ira.c   (working copy)
@@ -1422,6 +1422,12 @@
            if (TEST_HARD_REG_BIT 
(ira_prohibited_class_mode_regs[cl][j], hard_regno))
              continue;
            nregs = hard_regno_nregs[hard_regno][j];
+          if (hard_regno + nregs >= FIRST_PSEUDO_REGISTER)
+            {
+              SET_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j],
+                                hard_regno);
+               continue;
+            }
            pclass = ira_pressure_class_translate[REGNO_REG_CLASS 
(hard_regno)];
            for (nregs-- ;nregs >= 0; nregs--)
              if (((enum reg_class) pclass

Re: [Patch, IRA] Fix a function accessing beyond end-of-array

[Eric Botcazou]

>          * ira.c (clarify_prohibited_class_mode_regs): Prevent the
> function from accessing beyond the end of REGNO_REG_CLASS array by
> stopping the loop early.

Just fine (the ChangeLog only of course, the patch is Vlad's), thanks.

-- 
Eric Botcazou

Re: [Patch, IRA] Fix a function accessing beyond end-of-array

[Vladimir Makarov]

On 05/11/2011 10:07 AM, Hari Sandanagobalane wrote:
> Hello,
> I discussed this problem with Vlad in 
> http://gcc.gnu.org/ml/gcc/2011-05/msg00131.html. I propose the 
> following patch to fix it. Okay to commit?
>
> Revised the ChangeLog.
>
> Thanks
> Hari
>
> ChangeLog:
>         * ira.c (clarify_prohibited_class_mode_regs): Prevent the 
> function from accessing beyond the end of REGNO_REG_CLASS array by 
> stopping the loop early.
>
> Patch:
> Index: gcc/ira.c
> ===================================================================
> --- gcc/ira.c   (revision 173654)
> +++ gcc/ira.c   (working copy)
> @@ -1422,6 +1422,12 @@
>            if (TEST_HARD_REG_BIT 
> (ira_prohibited_class_mode_regs[cl][j], hard_regno))
>              continue;
>            nregs = hard_regno_nregs[hard_regno][j];
> +          if (hard_regno + nregs >= FIRST_PSEUDO_REGISTER)

I think it should be '>' not '>='

With this change, the patch is ok.  Thanks for the patch.

> +            {
> +              SET_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j],
> +                                hard_regno);
> +               continue;
> +            }
>            pclass = ira_pressure_class_translate[REGNO_REG_CLASS 
> (hard_regno)];
>            for (nregs-- ;nregs >= 0; nregs--)
>              if (((enum reg_class) pclass

Re: [Patch, IRA] Fix a function accessing beyond end-of-array

[Hari Sandanagobalane]

On 11/05/11 19:32, Vladimir Makarov wrote:
> On 05/11/2011 10:07 AM, Hari Sandanagobalane wrote:
>> Hello,
>> I discussed this problem with Vlad in
>> http://gcc.gnu.org/ml/gcc/2011-05/msg00131.html. I propose the
>> following patch to fix it. Okay to commit?
>>
>> Revised the ChangeLog.
>>
>> Thanks
>> Hari
>>
>> ChangeLog:
>>          * ira.c (clarify_prohibited_class_mode_regs): Prevent the
>> function from accessing beyond the end of REGNO_REG_CLASS array by
>> stopping the loop early.
>>
>> Patch:
>> Index: gcc/ira.c
>> ===================================================================
>> --- gcc/ira.c   (revision 173654)
>> +++ gcc/ira.c   (working copy)
>> @@ -1422,6 +1422,12 @@
>>             if (TEST_HARD_REG_BIT
>> (ira_prohibited_class_mode_regs[cl][j], hard_regno))
>>               continue;
>>             nregs = hard_regno_nregs[hard_regno][j];
>> +          if (hard_regno + nregs>= FIRST_PSEUDO_REGISTER)
>
> I think it should be '>' not'>='

Vlad,

The REGNO_REG_CLASS is generally an array of size FIRST_PSEUDO_REGISTER. 
So, the indexes go from 0 to FIRST_PSEUDO_REGISTER-1. I think the ">=" 
condition is fine in that case. Do you agree?

Cheers
Hari

>
> With this change, the patch is ok.  Thanks for the patch.
>
>> +            {
>> +              SET_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j],
>> +                                hard_regno);
>> +               continue;
>> +            }
>>             pclass = ira_pressure_class_translate[REGNO_REG_CLASS
>> (hard_regno)];
>>             for (nregs-- ;nregs>= 0; nregs--)
>>               if (((enum reg_class) pclass
>

Re: [Patch, IRA] Fix a function accessing beyond end-of-array

[Jakub Jelinek]

On Thu, May 12, 2011 at 10:09:42AM +0100, Hari Sandanagobalane wrote:
> The REGNO_REG_CLASS is generally an array of size
> FIRST_PSEUDO_REGISTER. So, the indexes go from 0 to
> FIRST_PSEUDO_REGISTER-1.

That is true.

> I think the ">=" condition is fine in that
> case. Do you agree?

That is wrong.  It is perfectly fine to handle hard reg with
regno (FIRST_PSEUDO_REGISTER - 1) if it has nregs 1, or
hard reg with regno (FIRST_PSEUDO_REGISTER - 2) if it has nregs 1 or 2.
The following loop starts with nregs--, therefore for
hard_regno + nregs == FIRST_PSEUDO_REGISTER at the place of your test
REGNO_REG_CLASS will be used in the loop for hard_regno + nregs - 1
down to hard_regno + 0.

> >
> >With this change, the patch is ok.  Thanks for the patch.
> >
> >>+            {
> >>+              SET_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j],
> >>+                                hard_regno);
> >>+               continue;
> >>+            }
> >>            pclass = ira_pressure_class_translate[REGNO_REG_CLASS
> >>(hard_regno)];
> >>            for (nregs-- ;nregs>= 0; nregs--)
> >>              if (((enum reg_class) pclass
> >

	Jakub

Re: [Patch, IRA] Fix a function accessing beyond end-of-array

[Hari Sandanagobalane]

On 12/05/11 10:18, Jakub Jelinek wrote:
> On Thu, May 12, 2011 at 10:09:42AM +0100, Hari Sandanagobalane wrote:
>> The REGNO_REG_CLASS is generally an array of size
>> FIRST_PSEUDO_REGISTER. So, the indexes go from 0 to
>> FIRST_PSEUDO_REGISTER-1.
>
> That is true.
>
>> I think the ">=" condition is fine in that
>> case. Do you agree?
>
> That is wrong.  It is perfectly fine to handle hard reg with
> regno (FIRST_PSEUDO_REGISTER - 1) if it has nregs 1, or
> hard reg with regno (FIRST_PSEUDO_REGISTER - 2) if it has nregs 1 or 2.
> The following loop starts with nregs--, therefore for
> hard_regno + nregs == FIRST_PSEUDO_REGISTER at the place of your test
> REGNO_REG_CLASS will be used in the loop for hard_regno + nregs - 1
> down to hard_regno + 0.

Ooh yes. You are right. Thanks for that!

I have now committed the revised patch to mainline as revision 173699.

Cheers
Hari

>
>>>
>>> With this change, the patch is ok.  Thanks for the patch.
>>>
>>>> +            {
>>>> +              SET_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j],
>>>> +                                hard_regno);
>>>> +               continue;
>>>> +            }
>>>>             pclass = ira_pressure_class_translate[REGNO_REG_CLASS
>>>> (hard_regno)];
>>>>             for (nregs-- ;nregs>= 0; nregs--)
>>>>               if (((enum reg_class) pclass
>>>
>
> 	Jakub

[Patch, IRA] Fix a function accessing beyond end-of-array

[Hari Sandanagobalane]

Hello,
I discussed this problem with Vlad in 
http://gcc.gnu.org/ml/gcc/2011-05/msg00131.html. I propose the following 
patch to fix it. Okay to commit?

Thanks
Hari

ChangeLog:
        * ira.c (clarify_prohibited_class_mode_regs): It was running 
beyond the end of REGNO_REG_CLASS array. Fixed.

Patch:
Index: gcc/ira.c
===================================================================
--- gcc/ira.c   (revision 173654)
+++ gcc/ira.c   (working copy)
@@ -1422,6 +1422,12 @@
           if (TEST_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j], 
hard_regno))
             continue;
           nregs = hard_regno_nregs[hard_regno][j];
+          if (hard_regno + nregs >= FIRST_PSEUDO_REGISTER)
+            {
+              SET_HARD_REG_BIT (ira_prohibited_class_mode_regs[cl][j],
+                                hard_regno);
+               continue;
+            }
           pclass = ira_pressure_class_translate[REGNO_REG_CLASS 
(hard_regno)];
           for (nregs-- ;nregs >= 0; nregs--)
             if (((enum reg_class) pclass

Re: [Patch, IRA] Fix a function accessing beyond end-of-array

[Eric Botcazou]

>         * ira.c (clarify_prohibited_class_mode_regs): It was running
> beyond the end of REGNO_REG_CLASS array. Fixed.

This isn't a valid ChangeLog entry.  It must not state the "why" (it was 
running beyond the end of REGNO_REG_CLASS array), only the "what".  So you 
need to describe how you're fixing the problem, e.g. "Do not access the array 
if ..." or "Stop the loop if ...".  See the numerous examples in ChangeLog.

-- 
Eric Botcazou