linux-next.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* 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  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  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  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).