* linux-next: build failure after merge of the akpm-current tree @ 2016-05-06 4:58 Stephen Rothwell 2016-05-06 5:44 ` Andrew Morton 0 siblings, 1 reply; 15+ messages in thread From: Stephen Rothwell @ 2016-05-06 4:58 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-next, linux-kernel, Arnd Bergmann Hi Andrew, After merging the akpm-current tree, today's linux-next build (and a few earlier ones) (powerpc allnoconfig (and many others)) failed like this: lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') lib/vsprintf.c:160:2: error: initializer element is not constant and more ... basically any big endian build of code using __swabxx or cpu_to_lexx in initailisers (in the above case cpu_to_le16()). Caused (presumably) by commit eeee46ed3cda ("byteswap: try to avoid __builtin_constant_p gcc bug") This is a buyild using gcc 4.6.3. I have revreted that commit for today to see if it fixes the overnight builds. -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: linux-next: build failure after merge of the akpm-current tree 2016-05-06 4:58 linux-next: build failure after merge of the akpm-current tree Stephen Rothwell @ 2016-05-06 5:44 ` Andrew Morton 2016-05-06 6:09 ` Stephen Rothwell 2016-06-15 9:33 ` linux-next: build failure after merge of the akpm-current tree Paul Bolle 0 siblings, 2 replies; 15+ messages in thread From: Andrew Morton @ 2016-05-06 5:44 UTC (permalink / raw) To: Stephen Rothwell; +Cc: linux-next, linux-kernel, Arnd Bergmann On Fri, 6 May 2016 14:58:10 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > Hi Andrew, > > After merging the akpm-current tree, today's linux-next build (and a > few earlier ones) (powerpc allnoconfig (and many others)) failed like > this: > > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > lib/vsprintf.c:160:2: error: initializer element is not constant > > and more ... basically any big endian build of code using __swabxx > or cpu_to_lexx in initailisers (in the above case cpu_to_le16()). > > Caused (presumably) by commit > > eeee46ed3cda ("byteswap: try to avoid __builtin_constant_p gcc bug") > > This is a buyild using gcc 4.6.3. > > I have revreted that commit for today to see if it fixes the overnight > builds. hm, it works for me. powerpc allnoconfig, after setting CONFIG_PPC64=y. With this: From: Arnd Bergmann <arnd@arndb.de> Subject: byteswap: try to avoid __builtin_constant_p gcc bug This is another attempt to avoid a regression in wwn_to_u64() after that started using get_unaligned_be64(), which in turn ran into a bug on gcc-4.9 through 6.1. The regression got introduced due to the combination of two separate workarounds (e3bde9568d99 ("include/linux/unaligned: force inlining of byteswap operations") and ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")) that each try to sidestep distinct problems with gcc behavior (code growth and increased stack usage). Unfortunately after both have been applied, a more serious gcc bug has been uncovered, leading to incorrect object code that discards part of a function and causes undefined behavior. As part of this problem is how __builtin_constant_p gets evaluated on an argument passed by reference into an inline function, this avoids the use of __builtin_constant_p() for all architectures that set CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not set ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably do not suffer from the problem in the qla2xxx driver, but they might still run into it elsewhere. Both of the original workarounds were only merged in the 4.6 kernel, and the bug that is fixed by this patch should only appear if both are there, so we probably don't need to backport the fix. On the other hand, it works by simplifying the code path and should not have any negative effects. [arnd@arndb.de: fix older gcc warnings] (http://lkml.kernel.org/r/12243652.bxSxEgjgfk@wuerfel) Link: https://lkml.org/lkml/headers/2016/4/12/1103 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66122 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232 Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 Fixes: e3bde9568d99 ("include/linux/unaligned: force inlining of byteswap operations") Fixes: ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") Link: http://lkml.kernel.org/r/1780465.XdtPJpi8Tt@wuerfel Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com> Tested-by: Josh Poimboeuf <jpoimboe@redhat.com> # on gcc-5.3 Tested-by: Quinn Tran <quinn.tran@qlogic.com> Cc: Martin Jambor <mjambor@suse.cz> Cc: "Martin K. Petersen" <martin.petersen@oracle.com> Cc: James Bottomley <James.Bottomley@hansenpartnership.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: Thomas Graf <tgraf@suug.ch> Cc: Peter Zijlstra <peterz@infradead.org> Cc: David Rientjes <rientjes@google.com> Cc: Ingo Molnar <mingo@kernel.org> Cc: Himanshu Madhani <himanshu.madhani@qlogic.com> Cc: Jan Hubicka <hubicka@ucw.cz> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- include/uapi/linux/swab.h | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff -puN include/uapi/linux/swab.h~byteswap-try-to-avoid-__builtin_constant_p-gcc-bug-v2 include/uapi/linux/swab.h --- a/include/uapi/linux/swab.h~byteswap-try-to-avoid-__builtin_constant_p-gcc-bug-v2 +++ a/include/uapi/linux/swab.h @@ -45,9 +45,7 @@ static inline __attribute_const__ __u16 __fswab16(__u16 val) { -#ifdef __HAVE_BUILTIN_BSWAP16__ - return __builtin_bswap16(val); -#elif defined (__arch_swab16) +#if defined (__arch_swab16) return __arch_swab16(val); #else return ___constant_swab16(val); @@ -56,9 +54,7 @@ static inline __attribute_const__ __u16 static inline __attribute_const__ __u32 __fswab32(__u32 val) { -#ifdef __HAVE_BUILTIN_BSWAP32__ - return __builtin_bswap32(val); -#elif defined(__arch_swab32) +#if defined(__arch_swab32) return __arch_swab32(val); #else return ___constant_swab32(val); @@ -67,9 +63,7 @@ static inline __attribute_const__ __u32 static inline __attribute_const__ __u64 __fswab64(__u64 val) { -#ifdef __HAVE_BUILTIN_BSWAP64__ - return __builtin_bswap64(val); -#elif defined (__arch_swab64) +#if defined (__arch_swab64) return __arch_swab64(val); #elif defined(__SWAB_64_THRU_32__) __u32 h = val >> 32; @@ -102,28 +96,40 @@ static inline __attribute_const__ __u32 * __swab16 - return a byteswapped 16-bit value * @x: value to byteswap */ +#ifdef __HAVE_BUILTIN_BSWAP16__ +#define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) +#else #define __swab16(x) \ (__builtin_constant_p((__u16)(x)) ? \ ___constant_swab16(x) : \ __fswab16(x)) +#endif /** * __swab32 - return a byteswapped 32-bit value * @x: value to byteswap */ +#ifdef __HAVE_BUILTIN_BSWAP32__ +#define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) +#else #define __swab32(x) \ (__builtin_constant_p((__u32)(x)) ? \ ___constant_swab32(x) : \ __fswab32(x)) +#endif /** * __swab64 - return a byteswapped 64-bit value * @x: value to byteswap */ +#ifdef __HAVE_BUILTIN_BSWAP64__ +#define __swab64(x) (__u64)__builtin_bswap64((__u64)(x)) +#else #define __swab64(x) \ (__builtin_constant_p((__u64)(x)) ? \ ___constant_swab64(x) : \ __fswab64(x)) +#endif /** * __swahw32 - return a word-swapped 32-bit value _ ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: linux-next: build failure after merge of the akpm-current tree 2016-05-06 5:44 ` Andrew Morton @ 2016-05-06 6:09 ` Stephen Rothwell 2016-05-06 6:55 ` Michael Ellerman 2016-06-15 9:33 ` linux-next: build failure after merge of the akpm-current tree Paul Bolle 1 sibling, 1 reply; 15+ messages in thread From: Stephen Rothwell @ 2016-05-06 6:09 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-next, linux-kernel, Arnd Bergmann Hi Andrew, On Thu, 5 May 2016 22:44:29 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > > On Fri, 6 May 2016 14:58:10 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > After merging the akpm-current tree, today's linux-next build (and a > > few earlier ones) (powerpc allnoconfig (and many others)) failed like > > this: > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > and more ... basically any big endian build of code using __swabxx > > or cpu_to_lexx in initailisers (in the above case cpu_to_le16()). > > > > Caused (presumably) by commit > > > > eeee46ed3cda ("byteswap: try to avoid __builtin_constant_p gcc bug") > > > > This is a buyild using gcc 4.6.3. > > > > I have revreted that commit for today to see if it fixes the overnight > > builds. > > hm, it works for me. powerpc allnoconfig, after setting CONFIG_PPC64=y. Interesting ... mine also works with gcc 5.2.0. I do 32 and 64 bit allnoconfig powerpc builds before I release linux-next and they work, but the 32 bit one fails overnight when gcc 4.6.3 is used :-( (we don't do a 64 bit allnoconfig overnight as it requires playing with the generated .config). Basically all the powerpc builds were failing with gcc 4.6.3 since next-20160502. The overnight powerpc allnoconfig build has just succeeded for today's linux-next. -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: linux-next: build failure after merge of the akpm-current tree 2016-05-06 6:09 ` Stephen Rothwell @ 2016-05-06 6:55 ` Michael Ellerman 2016-05-06 14:22 ` [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() Josh Poimboeuf 0 siblings, 1 reply; 15+ messages in thread From: Michael Ellerman @ 2016-05-06 6:55 UTC (permalink / raw) To: Stephen Rothwell, Andrew Morton; +Cc: linux-next, linux-kernel, Arnd Bergmann On Fri, 2016-05-06 at 16:09 +1000, Stephen Rothwell wrote: > Hi Andrew, > > On Thu, 5 May 2016 22:44:29 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > > > > On Fri, 6 May 2016 14:58:10 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > > After merging the akpm-current tree, today's linux-next build (and a > > > few earlier ones) (powerpc allnoconfig (and many others)) failed like > > > this: > > > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > > > and more ... basically any big endian build of code using __swabxx > > > or cpu_to_lexx in initailisers (in the above case cpu_to_le16()). > > > > > > Caused (presumably) by commit > > > > > > eeee46ed3cda ("byteswap: try to avoid __builtin_constant_p gcc bug") > > > > > > This is a buyild using gcc 4.6.3. > > > > > > I have revreted that commit for today to see if it fixes the overnight > > > builds. > > > > hm, it works for me. powerpc allnoconfig, after setting CONFIG_PPC64=y. > > Interesting ... mine also works with gcc 5.2.0. I do 32 and 64 bit > allnoconfig powerpc builds before I release linux-next and they work, > but the 32 bit one fails overnight when gcc 4.6.3 is used :-( (we don't > do a 64 bit allnoconfig overnight as it requires playing with the > generated .config). Basically all the powerpc builds were failing with > gcc 4.6.3 since next-20160502. There's a powerpc-5.3 compiler in kisskb now, I've been testing it for a while and it seems OK, you can start using that as well of or instead if you like for linux-next builds. cheers ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-06 6:55 ` Michael Ellerman @ 2016-05-06 14:22 ` Josh Poimboeuf 2016-05-09 0:59 ` Stephen Rothwell 0 siblings, 1 reply; 15+ messages in thread From: Josh Poimboeuf @ 2016-05-06 14:22 UTC (permalink / raw) To: Michael Ellerman Cc: Stephen Rothwell, Andrew Morton, linux-next, linux-kernel, Arnd Bergmann, Linus Torvalds On Fri, May 06, 2016 at 04:55:12PM +1000, Michael Ellerman wrote: > On Fri, 2016-05-06 at 16:09 +1000, Stephen Rothwell wrote: > > Hi Andrew, > > > > On Thu, 5 May 2016 22:44:29 -0700 Andrew Morton <akpm@linux-foundation.org> wrote: > > > > > > On Fri, 6 May 2016 14:58:10 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > > > > After merging the akpm-current tree, today's linux-next build (and a > > > > few earlier ones) (powerpc allnoconfig (and many others)) failed like > > > > this: > > > > > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > > > > > and more ... basically any big endian build of code using __swabxx > > > > or cpu_to_lexx in initailisers (in the above case cpu_to_le16()). > > > > > > > > Caused (presumably) by commit > > > > > > > > eeee46ed3cda ("byteswap: try to avoid __builtin_constant_p gcc bug") > > > > > > > > This is a buyild using gcc 4.6.3. > > > > > > > > I have revreted that commit for today to see if it fixes the overnight > > > > builds. > > > > > > hm, it works for me. powerpc allnoconfig, after setting CONFIG_PPC64=y. > > > > Interesting ... mine also works with gcc 5.2.0. I do 32 and 64 bit > > allnoconfig powerpc builds before I release linux-next and they work, > > but the 32 bit one fails overnight when gcc 4.6.3 is used :-( (we don't > > do a 64 bit allnoconfig overnight as it requires playing with the > > generated .config). Basically all the powerpc builds were failing with > > gcc 4.6.3 since next-20160502. > > There's a powerpc-5.3 compiler in kisskb now, I've been testing it for a while > and it seems OK, you can start using that as well of or instead if you like for > linux-next builds. I've also seen no problems on powerpc with 4.4 and 4.8. I suspect it's specific to gcc 4.6. Stephen, can you confirm this patch fixes it? ---- From: Josh Poimboeuf <jpoimboe@redhat.com> Subject: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() gcc support for __builtin_bswap16() was supposedly added for powerpc in gcc 4.6, and was then later added for other architectures in gcc 4.8. However, Stephen Rothwell reported that attempting to use it on powerpc in gcc 4.6 fails with: lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') lib/vsprintf.c:160:2: error: initializer element is not constant I'm not entirely sure what those errors mean, but I don't see them on gcc 4.8. So let's consider gcc 4.8 to be the official starting point for __builtin_bswap16(). Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug") Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> --- include/linux/compiler-gcc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index eeae401..3d5202e 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -246,7 +246,7 @@ #define __HAVE_BUILTIN_BSWAP32__ #define __HAVE_BUILTIN_BSWAP64__ #endif -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600) +#if GCC_VERSION >= 40800 #define __HAVE_BUILTIN_BSWAP16__ #endif #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ -- 2.4.11 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-06 14:22 ` [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() Josh Poimboeuf @ 2016-05-09 0:59 ` Stephen Rothwell 2016-05-09 5:39 ` Sedat Dilek 2016-05-09 6:33 ` Stephen Rothwell 0 siblings, 2 replies; 15+ messages in thread From: Stephen Rothwell @ 2016-05-09 0:59 UTC (permalink / raw) To: Josh Poimboeuf Cc: Michael Ellerman, Andrew Morton, linux-next, linux-kernel, Arnd Bergmann, Linus Torvalds Hi Josh, On Fri, 6 May 2016 09:22:25 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > I've also seen no problems on powerpc with 4.4 and 4.8. I suspect it's > specific to gcc 4.6. Stephen, can you confirm this patch fixes it? That will obviously fix the problem for us (since it will effectively restore the code to what it was before the other commit for our gcc 4.6.3 builds and we have not seen it in other builds). I will add this patch to linux-next today. And since "byteswap: try to avoid __builtin_constant_p gcc bug" is not in Linus' tree, hopefully we can have this fix applied soon. > From: Josh Poimboeuf <jpoimboe@redhat.com> > Subject: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() > > gcc support for __builtin_bswap16() was supposedly added for powerpc in > gcc 4.6, and was then later added for other architectures in gcc 4.8. > > However, Stephen Rothwell reported that attempting to use it on powerpc > in gcc 4.6 fails with: > > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > lib/vsprintf.c:160:2: error: initializer element is not constant > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > lib/vsprintf.c:160:2: error: initializer element is not constant > > I'm not entirely sure what those errors mean, but I don't see them on > gcc 4.8. So let's consider gcc 4.8 to be the official starting point > for __builtin_bswap16(). > > Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug") > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > --- > include/linux/compiler-gcc.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > index eeae401..3d5202e 100644 > --- a/include/linux/compiler-gcc.h > +++ b/include/linux/compiler-gcc.h > @@ -246,7 +246,7 @@ > #define __HAVE_BUILTIN_BSWAP32__ > #define __HAVE_BUILTIN_BSWAP64__ > #endif > -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600) > +#if GCC_VERSION >= 40800 > #define __HAVE_BUILTIN_BSWAP16__ > #endif > #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ > -- > 2.4.11 -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-09 0:59 ` Stephen Rothwell @ 2016-05-09 5:39 ` Sedat Dilek 2016-05-09 8:31 ` Stephen Rothwell 2016-05-09 6:33 ` Stephen Rothwell 1 sibling, 1 reply; 15+ messages in thread From: Sedat Dilek @ 2016-05-09 5:39 UTC (permalink / raw) To: Stephen Rothwell Cc: Josh Poimboeuf, Michael Ellerman, Andrew Morton, linux-next, linux-kernel, Arnd Bergmann, Linus Torvalds On 5/9/16, Stephen Rothwell <sfr@canb.auug.org.au> wrote: > Hi Josh, > > On Fri, 6 May 2016 09:22:25 -0500 Josh Poimboeuf <jpoimboe@redhat.com> > wrote: >> >> I've also seen no problems on powerpc with 4.4 and 4.8. I suspect it's >> specific to gcc 4.6. Stephen, can you confirm this patch fixes it? > > That will obviously fix the problem for us (since it will effectively > restore the code to what it was before the other commit for our gcc > 4.6.3 builds and we have not seen it in other builds). I will add this > patch to linux-next today. > > And since "byteswap: try to avoid __builtin_constant_p gcc bug" is not > in Linus' tree, hopefully we can have this fix applied soon. > FYI, this patch is in Linus tree (v4.6-rc7 has it). - Sedat - [1] http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7322dd755e7dd34bc5359aa27abeed1687e0f628 >> From: Josh Poimboeuf <jpoimboe@redhat.com> >> Subject: [PATCH] compiler-gcc: require gcc 4.8 for powerpc >> __builtin_bswap16() >> >> gcc support for __builtin_bswap16() was supposedly added for powerpc in >> gcc 4.6, and was then later added for other architectures in gcc 4.8. >> >> However, Stephen Rothwell reported that attempting to use it on powerpc >> in gcc 4.6 fails with: >> >> lib/vsprintf.c:160:2: error: initializer element is not constant >> lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') >> lib/vsprintf.c:160:2: error: initializer element is not constant >> lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') >> lib/vsprintf.c:160:2: error: initializer element is not constant >> lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') >> lib/vsprintf.c:160:2: error: initializer element is not constant >> lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') >> lib/vsprintf.c:160:2: error: initializer element is not constant >> >> I'm not entirely sure what those errors mean, but I don't see them on >> gcc 4.8. So let's consider gcc 4.8 to be the official starting point >> for __builtin_bswap16(). >> >> Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc >> bug") >> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> >> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> >> --- >> include/linux/compiler-gcc.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h >> index eeae401..3d5202e 100644 >> --- a/include/linux/compiler-gcc.h >> +++ b/include/linux/compiler-gcc.h >> @@ -246,7 +246,7 @@ >> #define __HAVE_BUILTIN_BSWAP32__ >> #define __HAVE_BUILTIN_BSWAP64__ >> #endif >> -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= >> 40600) >> +#if GCC_VERSION >= 40800 >> #define __HAVE_BUILTIN_BSWAP16__ >> #endif >> #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ >> -- >> 2.4.11 > > -- > Cheers, > Stephen Rothwell > -- > To unsubscribe from this list: send the line "unsubscribe linux-next" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-09 5:39 ` Sedat Dilek @ 2016-05-09 8:31 ` Stephen Rothwell 0 siblings, 0 replies; 15+ messages in thread From: Stephen Rothwell @ 2016-05-09 8:31 UTC (permalink / raw) To: Sedat Dilek Cc: Josh Poimboeuf, Michael Ellerman, Andrew Morton, linux-next, linux-kernel, Arnd Bergmann, Linus Torvalds Hi Sedat, On Mon, 9 May 2016 07:39:24 +0200 Sedat Dilek <sedat.dilek@gmail.com> wrote: > > On 5/9/16, Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > > And since "byteswap: try to avoid __builtin_constant_p gcc bug" is not > > in Linus' tree, hopefully we can have this fix applied soon. > > FYI, this patch is in Linus tree (v4.6-rc7 has it). Yes, the "not" was a typo. -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-09 0:59 ` Stephen Rothwell 2016-05-09 5:39 ` Sedat Dilek @ 2016-05-09 6:33 ` Stephen Rothwell 2016-05-09 9:34 ` Michael Ellerman 1 sibling, 1 reply; 15+ messages in thread From: Stephen Rothwell @ 2016-05-09 6:33 UTC (permalink / raw) To: Josh Poimboeuf Cc: Michael Ellerman, Andrew Morton, linux-next, linux-kernel, Arnd Bergmann, Linus Torvalds Hi all, On Mon, 9 May 2016 10:59:27 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > On Fri, 6 May 2016 09:22:25 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > I've also seen no problems on powerpc with 4.4 and 4.8. I suspect it's > > specific to gcc 4.6. Stephen, can you confirm this patch fixes it? > > That will obviously fix the problem for us (since it will effectively > restore the code to what it was before the other commit for our gcc > 4.6.3 builds and we have not seen it in other builds). I will add this > patch to linux-next today. > > And since "byteswap: try to avoid __builtin_constant_p gcc bug" is not > in Linus' tree, hopefully we can have this fix applied soon. > > > From: Josh Poimboeuf <jpoimboe@redhat.com> > > Subject: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() > > > > gcc support for __builtin_bswap16() was supposedly added for powerpc in > > gcc 4.6, and was then later added for other architectures in gcc 4.8. > > > > However, Stephen Rothwell reported that attempting to use it on powerpc > > in gcc 4.6 fails with: > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > I'm not entirely sure what those errors mean, but I don't see them on > > gcc 4.8. So let's consider gcc 4.8 to be the official starting point > > for __builtin_bswap16(). > > > > Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug") > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > --- > > include/linux/compiler-gcc.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > > index eeae401..3d5202e 100644 > > --- a/include/linux/compiler-gcc.h > > +++ b/include/linux/compiler-gcc.h > > @@ -246,7 +246,7 @@ > > #define __HAVE_BUILTIN_BSWAP32__ > > #define __HAVE_BUILTIN_BSWAP64__ > > #endif > > -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600) > > +#if GCC_VERSION >= 40800 > > #define __HAVE_BUILTIN_BSWAP16__ > > #endif > > #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ > > -- > > 2.4.11 Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> Michael, do you want to pass the fix patch on, or will I submit it directly to Linus? -- Cheers, Stephen Rothwell ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-09 6:33 ` Stephen Rothwell @ 2016-05-09 9:34 ` Michael Ellerman 2016-05-09 10:16 ` Arnd Bergmann 0 siblings, 1 reply; 15+ messages in thread From: Michael Ellerman @ 2016-05-09 9:34 UTC (permalink / raw) To: Stephen Rothwell, Josh Poimboeuf Cc: Andrew Morton, linux-next, linux-kernel, Arnd Bergmann, Linus Torvalds On Mon, 2016-05-09 at 16:33 +1000, Stephen Rothwell wrote: > On Mon, 9 May 2016 10:59:27 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > On Fri, 6 May 2016 09:22:25 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > > > I've also seen no problems on powerpc with 4.4 and 4.8. I suspect it's > > > specific to gcc 4.6. Stephen, can you confirm this patch fixes it? > > > > That will obviously fix the problem for us (since it will effectively > > restore the code to what it was before the other commit for our gcc > > 4.6.3 builds and we have not seen it in other builds). I will add this > > patch to linux-next today. > > > > And since "byteswap: try to avoid __builtin_constant_p gcc bug" is not > > in Linus' tree, hopefully we can have this fix applied soon. > > > From: Josh Poimboeuf <jpoimboe@redhat.com> > > > Subject: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() > > > > > > gcc support for __builtin_bswap16() was supposedly added for powerpc in > > > gcc 4.6, and was then later added for other architectures in gcc 4.8. > > > > > > However, Stephen Rothwell reported that attempting to use it on powerpc > > > in gcc 4.6 fails with: > > > > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') > > > lib/vsprintf.c:160:2: error: initializer element is not constant > > > > > > I'm not entirely sure what those errors mean, but I don't see them on > > > gcc 4.8. So let's consider gcc 4.8 to be the official starting point > > > for __builtin_bswap16(). > > > > > > Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug") > > > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> > > > Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> > > > --- > > > include/linux/compiler-gcc.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > > > index eeae401..3d5202e 100644 > > > --- a/include/linux/compiler-gcc.h > > > +++ b/include/linux/compiler-gcc.h > > > @@ -246,7 +246,7 @@ > > > #define __HAVE_BUILTIN_BSWAP32__ > > > #define __HAVE_BUILTIN_BSWAP64__ > > > #endif > > > -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600) > > > +#if GCC_VERSION >= 40800 > > > #define __HAVE_BUILTIN_BSWAP16__ > > > #endif > > > #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ > > > -- > > > 2.4.11 > > Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> > > Michael, do you want to pass the fix patch on, or will I submit it > directly to Linus? I'm happy for you to send it, I haven't actually hit the bug myself. cheers ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-09 9:34 ` Michael Ellerman @ 2016-05-09 10:16 ` Arnd Bergmann 2016-05-09 10:36 ` Stephen Rothwell 0 siblings, 1 reply; 15+ messages in thread From: Arnd Bergmann @ 2016-05-09 10:16 UTC (permalink / raw) To: Michael Ellerman Cc: Stephen Rothwell, Josh Poimboeuf, Andrew Morton, linux-next, linux-kernel, Linus Torvalds On Monday 09 May 2016 19:34:53 Michael Ellerman wrote: > On Mon, 2016-05-09 at 16:33 +1000, Stephen Rothwell wrote: > > On Mon, 9 May 2016 10:59:27 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote: > > > On Fri, 6 May 2016 09:22:25 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote: > > > > diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h > > > > index eeae401..3d5202e 100644 > > > > --- a/include/linux/compiler-gcc.h > > > > +++ b/include/linux/compiler-gcc.h > > > > @@ -246,7 +246,7 @@ > > > > #define __HAVE_BUILTIN_BSWAP32__ > > > > #define __HAVE_BUILTIN_BSWAP64__ > > > > #endif > > > > -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600) > > > > +#if GCC_VERSION >= 40800 > > > > #define __HAVE_BUILTIN_BSWAP16__ > > > > #endif > > > > #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ > > > > Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> Acked-by: Arnd Bergmann <arnd@arndb.de> > > Michael, do you want to pass the fix patch on, or will I submit it > > directly to Linus? > > I'm happy for you to send it, I haven't actually hit the bug myself. I found the commit in gcc-4.8 that replaced the powerpc-specific implementation of __builtin_bswap16 with an architecture-independent one. Apparently the powerpc version (gcc-4.6 and 4.7) just mapped to the lhbrx/sthbrx instructions, so it ended up not being a constant, though the intent of the patch was mainly to add support for the builtin to x86. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52624 has the patch that went into gcc-4.8 and more information. Arnd ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() 2016-05-09 10:16 ` Arnd Bergmann @ 2016-05-09 10:36 ` Stephen Rothwell 0 siblings, 0 replies; 15+ messages in thread From: Stephen Rothwell @ 2016-05-09 10:36 UTC (permalink / raw) To: Linus Torvalds Cc: Arnd Bergmann, Michael Ellerman, Josh Poimboeuf, Andrew Morton, linux-next, linux-kernel, Sedat Dilek From: Josh Poimboeuf <jpoimboe@redhat.com> Date: Fri, 6 May 2016 09:22:25 -0500 gcc support for __builtin_bswap16() was supposedly added for powerpc in gcc 4.6, and was then later added for other architectures in gcc 4.8. However, Stephen Rothwell reported that attempting to use it on powerpc in gcc 4.6 fails with: lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[0]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[1]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[2]') lib/vsprintf.c:160:2: error: initializer element is not constant lib/vsprintf.c:160:2: error: (near initialization for 'decpair[3]') lib/vsprintf.c:160:2: error: initializer element is not constant I'm not entirely sure what those errors mean, but I don't see them on gcc 4.8. So let's consider gcc 4.8 to be the official starting point for __builtin_bswap16(). Arnd Bergmann adds: "I found the commit in gcc-4.8 that replaced the powerpc-specific implementation of __builtin_bswap16 with an architecture-independent one. Apparently the powerpc version (gcc-4.6 and 4.7) just mapped to the lhbrx/sthbrx instructions, so it ended up not being a constant, though the intent of the patch was mainly to add support for the builtin to x86. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52624 has the patch that went into gcc-4.8 and more information." Fixes: 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug") Reported-by: Stephen Rothwell <sfr@canb.auug.org.au> Tested-by: Stephen Rothwell <sfr@canb.auug.org.au> Acked-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com> Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au> --- include/linux/compiler-gcc.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index eeae401..3d5202e 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -246,7 +246,7 @@ #define __HAVE_BUILTIN_BSWAP32__ #define __HAVE_BUILTIN_BSWAP64__ #endif -#if GCC_VERSION >= 40800 || (defined(__powerpc__) && GCC_VERSION >= 40600) +#if GCC_VERSION >= 40800 #define __HAVE_BUILTIN_BSWAP16__ #endif #endif /* CONFIG_ARCH_USE_BUILTIN_BSWAP */ -- 2.4.11 -- Cheers, Stephen Rothwell ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: linux-next: build failure after merge of the akpm-current tree 2016-05-06 5:44 ` Andrew Morton 2016-05-06 6:09 ` Stephen Rothwell @ 2016-06-15 9:33 ` Paul Bolle 2016-06-15 14:03 ` Josh Poimboeuf 1 sibling, 1 reply; 15+ messages in thread From: Paul Bolle @ 2016-06-15 9:33 UTC (permalink / raw) To: Andrew Morton, Arnd Bergmann; +Cc: Stephen Rothwell, linux-next, linux-kernel On do, 2016-05-05 at 22:44 -0700, Andrew Morton wrote: > From: Arnd Bergmann <arnd@arndb.de> > Subject: byteswap: try to avoid __builtin_constant_p gcc bug > > This is another attempt to avoid a regression in wwn_to_u64() after that > started using get_unaligned_be64(), which in turn ran into a bug on > gcc-4.9 through 6.1. > > The regression got introduced due to the combination of two separate > workarounds (e3bde9568d99 ("include/linux/unaligned: force inlining of > byteswap operations") and ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 > for wwn access")) that each try to sidestep distinct problems with gcc > behavior (code growth and increased stack usage). Unfortunately after > both have been applied, a more serious gcc bug has been uncovered, leading > to incorrect object code that discards part of a function and causes > undefined behavior. > > As part of this problem is how __builtin_constant_p gets evaluated on an > argument passed by reference into an inline function, this avoids the use > of __builtin_constant_p() for all architectures that set > CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not set > ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably do not suffer > from the problem in the qla2xxx driver, but they might still run into it > elsewhere. > > Both of the original workarounds were only merged in the 4.6 kernel, and > the bug that is fixed by this patch should only appear if both are there, > so we probably don't need to backport the fix. On the other hand, it > works by simplifying the code path and should not have any negative > effects. > > [arnd@arndb.de: fix older gcc warnings] > (http://lkml.kernel.org/r/12243652.bxSxEgjgfk@wuerfel) > Link: https://lkml.org/lkml/headers/2016/4/12/1103 > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66122 > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232 > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 > Fixes: e3bde9568d99 ("include/linux/unaligned: force inlining of byteswap operations") > Fixes: ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") > Link: http://lkml.kernel.org/r/1780465.XdtPJpi8Tt@wuerfel > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com> > Tested-by: Josh Poimboeuf <jpoimboe@redhat.com> # on gcc-5.3 > Tested-by: Quinn Tran <quinn.tran@qlogic.com> > Cc: Martin Jambor <mjambor@suse.cz> > Cc: "Martin K. Petersen" <martin.petersen@oracle.com> > Cc: James Bottomley <James.Bottomley@hansenpartnership.com> > Cc: Denys Vlasenko <dvlasenk@redhat.com> > Cc: Thomas Graf <tgraf@suug.ch> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: David Rientjes <rientjes@google.com> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Himanshu Madhani <himanshu.madhani@qlogic.com> > Cc: Jan Hubicka <hubicka@ucw.cz> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> This became commit 7322dd755e7d ("byteswap: try to avoid __builtin_constant_p gcc bug"). That commit was included in v4.6-rc7. Ever since that rc I see this warning when building for x86_64: net/netfilter/ipvs/ip_vs_sync.c: In function ‘ip_vs_proc_sync_conn’: net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘opt.init_seq’ may be used uninitialized in this function [-Wmaybe-uninitialized] struct ip_vs_sync_conn_options opt; ^ net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘opt.delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘opt.previous_delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘*((void *)&opt+12).init_seq’ may be used uninitialized in this function [-Wmaybe-uninitialized] net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘*((void *)&opt+12).delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘*((void *)&opt+12).previous_delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] (It doesn't show up when building for 32 bits x86. Perhaps there's an int/long mismatch somewhere in the related code.) Anyone else seeing this? It looks like a false positive. I can make it disappear by making sure ip_vs_proc_seqopt() isn't inlined. But that's not something I dare to put into a patch for a false positive. Anyone sitting on a better fix? Paul Bolle ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: linux-next: build failure after merge of the akpm-current tree 2016-06-15 9:33 ` linux-next: build failure after merge of the akpm-current tree Paul Bolle @ 2016-06-15 14:03 ` Josh Poimboeuf 2016-06-15 21:12 ` Paul Bolle 0 siblings, 1 reply; 15+ messages in thread From: Josh Poimboeuf @ 2016-06-15 14:03 UTC (permalink / raw) To: Paul Bolle Cc: Andrew Morton, Arnd Bergmann, Stephen Rothwell, linux-next, linux-kernel On Wed, Jun 15, 2016 at 11:33:32AM +0200, Paul Bolle wrote: > On do, 2016-05-05 at 22:44 -0700, Andrew Morton wrote: > > From: Arnd Bergmann <arnd@arndb.de> > > Subject: byteswap: try to avoid __builtin_constant_p gcc bug > > > > This is another attempt to avoid a regression in wwn_to_u64() after that > > started using get_unaligned_be64(), which in turn ran into a bug on > > gcc-4.9 through 6.1. > > > > The regression got introduced due to the combination of two separate > > workarounds (e3bde9568d99 ("include/linux/unaligned: force inlining of > > byteswap operations") and ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 > > for wwn access")) that each try to sidestep distinct problems with gcc > > behavior (code growth and increased stack usage). Unfortunately after > > both have been applied, a more serious gcc bug has been uncovered, leading > > to incorrect object code that discards part of a function and causes > > undefined behavior. > > > > As part of this problem is how __builtin_constant_p gets evaluated on an > > argument passed by reference into an inline function, this avoids the use > > of __builtin_constant_p() for all architectures that set > > CONFIG_ARCH_USE_BUILTIN_BSWAP. Most architectures do not set > > ARCH_SUPPORTS_OPTIMIZED_INLINING, which means they probably do not suffer > > from the problem in the qla2xxx driver, but they might still run into it > > elsewhere. > > > > Both of the original workarounds were only merged in the 4.6 kernel, and > > the bug that is fixed by this patch should only appear if both are there, > > so we probably don't need to backport the fix. On the other hand, it > > works by simplifying the code path and should not have any negative > > effects. > > > > [arnd@arndb.de: fix older gcc warnings] > > (http://lkml.kernel.org/r/12243652.bxSxEgjgfk@wuerfel) > > Link: https://lkml.org/lkml/headers/2016/4/12/1103 > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66122 > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70232 > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646 > > Fixes: e3bde9568d99 ("include/linux/unaligned: force inlining of byteswap operations") > > Fixes: ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access") > > Link: http://lkml.kernel.org/r/1780465.XdtPJpi8Tt@wuerfel > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > > Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com> > > Tested-by: Josh Poimboeuf <jpoimboe@redhat.com> # on gcc-5.3 > > Tested-by: Quinn Tran <quinn.tran@qlogic.com> > > Cc: Martin Jambor <mjambor@suse.cz> > > Cc: "Martin K. Petersen" <martin.petersen@oracle.com> > > Cc: James Bottomley <James.Bottomley@hansenpartnership.com> > > Cc: Denys Vlasenko <dvlasenk@redhat.com> > > Cc: Thomas Graf <tgraf@suug.ch> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: David Rientjes <rientjes@google.com> > > Cc: Ingo Molnar <mingo@kernel.org> > > Cc: Himanshu Madhani <himanshu.madhani@qlogic.com> > > Cc: Jan Hubicka <hubicka@ucw.cz> > > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> > > This became commit 7322dd755e7d ("byteswap: try to avoid > __builtin_constant_p gcc bug"). That commit was included in v4.6-rc7. > Ever since that rc I see this warning when building for x86_64: > net/netfilter/ipvs/ip_vs_sync.c: In function ‘ip_vs_proc_sync_conn’: > net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘opt.init_seq’ may be used uninitialized in this function [-Wmaybe-uninitialized] > struct ip_vs_sync_conn_options opt; > ^ > net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘opt.delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] > net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘opt.previous_delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] > net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘*((void *)&opt+12).init_seq’ may be used uninitialized in this function [-Wmaybe-uninitialized] > net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘*((void *)&opt+12).delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] > net/netfilter/ipvs/ip_vs_sync.c:1069:33: warning: ‘*((void *)&opt+12).previous_delta’ may be used uninitialized in this function [-Wmaybe-uninitialized] > > (It doesn't show up when building for 32 bits x86. Perhaps there's an > int/long mismatch somewhere in the related code.) > > Anyone else seeing this? > > It looks like a false positive. I can make it disappear by making sure > ip_vs_proc_seqopt() isn't inlined. But that's not something I dare to > put into a patch for a false positive. Anyone sitting on a better fix? Hi Paul, I agree it looks like a false positive, though the code is a bit convoluted, so I'm not surprised that gcc might get confused. How about initializing opt to 0? diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c index 803001a..f45496c 100644 --- a/net/netfilter/ipvs/ip_vs_sync.c +++ b/net/netfilter/ipvs/ip_vs_sync.c @@ -1066,7 +1066,7 @@ static int ip_vs_proc_str(__u8 *p, unsigned int plen, unsigned int *data_len, */ static inline int ip_vs_proc_sync_conn(struct netns_ipvs *ipvs, __u8 *p, __u8 *msg_end) { - struct ip_vs_sync_conn_options opt; + struct ip_vs_sync_conn_options opt = {0}; union ip_vs_sync_conn *s; struct ip_vs_protocol *pp; struct ip_vs_conn_param param; -- Josh ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: linux-next: build failure after merge of the akpm-current tree 2016-06-15 14:03 ` Josh Poimboeuf @ 2016-06-15 21:12 ` Paul Bolle 0 siblings, 0 replies; 15+ messages in thread From: Paul Bolle @ 2016-06-15 21:12 UTC (permalink / raw) To: Josh Poimboeuf Cc: Andrew Morton, Arnd Bergmann, Stephen Rothwell, linux-next, linux-kernel Hi Josh, On wo, 2016-06-15 at 09:03 -0500, Josh Poimboeuf wrote: > I agree it looks like a false positive, though the code is a bit > convoluted, so I'm not surprised that gcc might get confused. Agree. > How about initializing opt to 0? I'm unconvinced. Especially since this only shows up in 64 bits builds. > --- a/net/netfilter/ipvs/ip_vs_sync.c > +++ b/net/netfilter/ipvs/ip_vs_sync.c > static inline int ip_vs_proc_sync_conn(struct netns_ipvs *ipvs, __u8 > - struct ip_vs_sync_conn_options opt; > + struct ip_vs_sync_conn_options opt = {0}; You forgot to append /* I give up */ here. More common would be to use /* silence GCC */ Thanks, Paul Bolle ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-06-15 21:12 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-05-06 4:58 linux-next: build failure after merge of the akpm-current tree Stephen Rothwell 2016-05-06 5:44 ` Andrew Morton 2016-05-06 6:09 ` Stephen Rothwell 2016-05-06 6:55 ` Michael Ellerman 2016-05-06 14:22 ` [PATCH] compiler-gcc: require gcc 4.8 for powerpc __builtin_bswap16() Josh Poimboeuf 2016-05-09 0:59 ` Stephen Rothwell 2016-05-09 5:39 ` Sedat Dilek 2016-05-09 8:31 ` Stephen Rothwell 2016-05-09 6:33 ` Stephen Rothwell 2016-05-09 9:34 ` Michael Ellerman 2016-05-09 10:16 ` Arnd Bergmann 2016-05-09 10:36 ` Stephen Rothwell 2016-06-15 9:33 ` linux-next: build failure after merge of the akpm-current tree Paul Bolle 2016-06-15 14:03 ` Josh Poimboeuf 2016-06-15 21:12 ` Paul Bolle
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).