This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] More thorough checking in reg_fits_class_p
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Richard Earnshaw <rearnsha at arm dot com>
- Cc: Jim MacArthur <jim dot macarthur at arm dot com>, "gcc-patches\ at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Mon, 30 Apr 2012 15:07:53 +0100
- Subject: Re: [patch] More thorough checking in reg_fits_class_p
- References: <4F994B99.1010606@arm.com> <4F9E9772.8080700@arm.com>
Richard Earnshaw <rearnsha@arm.com> writes:
> On 26/04/12 14:20, Jim MacArthur wrote:
>> The current code in reg_fits_class_p appears to be incorrect; since
>> offset may be negative, it's necessary to check both ends of the range
>> otherwise an array overrun or underrun may occur when calling
>> in_hard_reg_set_p. in_hard_reg_set_p should also be checked for each
>> register in the range of regno .. regno+offset.
>>
>> A negative offset can occur on a big-endian target. For example, on
>> AArch64, subreg_regno_offset (0, DImode, 0, TImode) returns -1.
>>
>> We discovered this problem while developing unrelated code for
>> big-endian support in the AArch64 back end.
>>
>> I've tested this with an x86 bootstrap which shows no errors, and with
>> our own AArch64 back end.
>>
>> Ok for trunk?
>>
>> gcc/Changelog entry:
>>
>> 2012-04-26 Jim MacArthur<jim.macarthur@arm.com>
>> * recog.c (reg_fits_class_p): Check every register between regno and
>> regno+offset is in the hard register set.
>>
>
> OK.
>
> R.
>
>>
>> reg-fits-class-9
>>
>>
>> diff --git a/gcc/recog.c b/gcc/recog.c
>> index 8fb96a0..825bfb1 100644
>> --- a/gcc/recog.c
>> +++ b/gcc/recog.c
>> @@ -2759,14 +2759,28 @@ bool
>> reg_fits_class_p (const_rtx operand, reg_class_t cl, int offset,
>> enum machine_mode mode)
>> {
>> - int regno = REGNO (operand);
>> + unsigned int regno = REGNO (operand);
>>
>> if (cl == NO_REGS)
>> return false;
>>
>> - return (HARD_REGISTER_NUM_P (regno)
>> - && in_hard_reg_set_p (reg_class_contents[(int) cl],
>> - mode, regno + offset));
>> + /* We should not assume all registers in the range regno to regno + offset are
>> + valid just because each end of the range is. */
>> + if (HARD_REGISTER_NUM_P (regno) && HARD_REGISTER_NUM_P (regno + offset))
>> + {
>> + unsigned int i;
>> +
>> + unsigned int start = MIN (regno, regno + offset);
>> + unsigned int end = MAX (regno, regno + offset);
>> + for (i = start; i <= end; i++)
>> + {
>> + if (!in_hard_reg_set_p (reg_class_contents[(int) cl],
>> + mode, i))
>> + return false;
>> + }
This doesn't look right to me. We should still only need to check
in_hard_reg_set_p for one register number. I'd have expected
something like:
return (HARD_REGISTER_NUM_P (regno)
&& HARD_REGISTER_NUM_P (regno + offset)
&& in_hard_reg_set_p (reg_class_contents[(int) cl],
mode, regno + offset));
instead.
Richard