| Message ID | 6D39441BF12EF246A7ABCE6654B0235380C0761B@HHMAIL01.hh.imgtec.org |
|---|---|
| State | New |
| Headers | show |
On Thu, Apr 20, 2017 at 12:49:49PM +0000, Matthew Fortune wrote: > Hi Jeff, > > I missed a load of test failures while on vacation and just noticed > that the fix you did for a potentially uninitialized variable warning > is overwriting the stack and breaking MSA in MIPS. > > I guess you may have intended to set the length appropriately and > only zero the elements that would not be set in the loop: > > memset (&orig_perm[nelt], 0, MAX_VECT_LEN - nelt); > > but I switched it to just zero initialise the whole array for > simplicity in the patch below. > > I thought I'd check with you before committing. Obviously I'd like to > apply this to GCC 7 branch as well. As it is just 16, it is not a big deal either way, memsetting everything might be even faster. If it would be bigger than that, the MAX_VECT_LEN - nelt case would be better. > gcc/ > * config/mips/mips.c (mips_expand_vec_perm_const): Re-fix > uninitialized variable warning to avoid buffer overrun. Ok, thanks, even for GCC 7 branch. Jakub
Jakub Jelinek <jakub@redhat.com> writes: > On Thu, Apr 20, 2017 at 12:49:49PM +0000, Matthew Fortune wrote: > > Hi Jeff, > > > > I missed a load of test failures while on vacation and just noticed > > that the fix you did for a potentially uninitialized variable warning > > is overwriting the stack and breaking MSA in MIPS. > > > > I guess you may have intended to set the length appropriately and only > > zero the elements that would not be set in the loop: > > > > memset (&orig_perm[nelt], 0, MAX_VECT_LEN - nelt); > > > > but I switched it to just zero initialise the whole array for > > simplicity in the patch below. > > > > I thought I'd check with you before committing. Obviously I'd like to > > apply this to GCC 7 branch as well. > > As it is just 16, it is not a big deal either way, memsetting everything > might be even faster. If it would be bigger than that, the MAX_VECT_LEN > - nelt case would be better. > > > gcc/ > > * config/mips/mips.c (mips_expand_vec_perm_const): Re-fix > > uninitialized variable warning to avoid buffer overrun. > > Ok, thanks, even for GCC 7 branch. Thanks Jakub. Committed on trunk and gcc-7-branch. Matthew
diff --git a/gcc/ChangeLog b/gcc/ChangeLog index c908048..80d3436 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2017-04-20 Matthew Fortune <matthew.fortune@imgtec.com> + + * config/mips/mips.c (mips_expand_vec_perm_const): Re-fix + uninitialized variable warning to avoid buffer overrun. + 2017-04-20 Alexander Monakov <amonakov@ispras.ru> PR other/71250 diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c index b35fba7..6bfd86a 100644 --- a/gcc/config/mips/mips.c +++ b/gcc/config/mips/mips.c @@ -21358,7 +21358,7 @@ mips_expand_vec_perm_const (rtx operands[4]) /* This is overly conservative, but ensures we don't get an uninitialized warning on ORIG_PERM. */ - memset (&orig_perm[nelt], 0, MAX_VECT_LEN); + memset (orig_perm, 0, MAX_VECT_LEN); for (i = which = 0; i < nelt; ++i) { rtx e = XVECEXP (sel, 0, i);