Richard Earnshaw <rearn...@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.macart...@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