Re: linux-next: build failure after merge of the akpm-current tree

[Andrew Morton]

On Fri, 6 May 2016 14:58:10 +1000 Stephen Rothwell <sfr@xxxxxxxxxxxxxxxx> 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@xxxxxxxx>
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@xxxxxxxx: fix older gcc warnings]
  (12243652.bxSxEgjgfk@wuerfel">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: 1780465.XdtPJpi8Tt@wuerfel">http://lkml.kernel.org/r/1780465.XdtPJpi8Tt@wuerfel
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
Reviewed-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx>
Tested-by: Josh Poimboeuf <jpoimboe@xxxxxxxxxx> # on gcc-5.3
Tested-by: Quinn Tran <quinn.tran@xxxxxxxxxx>
Cc: Martin Jambor <mjambor@xxxxxxx>
Cc: "Martin K. Petersen" <martin.petersen@xxxxxxxxxx>
Cc: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
Cc: Denys Vlasenko <dvlasenk@xxxxxxxxxx>
Cc: Thomas Graf <tgraf@xxxxxxx>
Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
Cc: David Rientjes <rientjes@xxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Himanshu Madhani <himanshu.madhani@xxxxxxxxxx>
Cc: Jan Hubicka <hubicka@xxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---

 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
_