bug.h: work around GCC PR82365 in BUG()
authorArnd Bergmann <arnd@arndb.de>
Wed, 21 Feb 2018 22:45:54 +0000 (14:45 -0800)
committerLinus Torvalds <torvalds@linux-foundation.org>
Wed, 21 Feb 2018 23:35:43 +0000 (15:35 -0800)
Looking at functions with large stack frames across all architectures
led me discovering that BUG() suffers from the same problem as
fortify_panic(), which I've added a workaround for already.

In short, variables that go out of scope by calling a noreturn function
or __builtin_unreachable() keep using stack space in functions
afterwards.

A workaround that was identified is to insert an empty assembler
statement just before calling the function that doesn't return.  I'm
adding a macro "barrier_before_unreachable()" to document this, and
insert calls to that in all instances of BUG() that currently suffer
from this problem.

The files that saw the largest change from this had these frame sizes
before, and much less with my patch:

  fs/ext4/inode.c:82:1: warning: the frame size of 1672 bytes is larger than 800 bytes [-Wframe-larger-than=]
  fs/ext4/namei.c:434:1: warning: the frame size of 904 bytes is larger than 800 bytes [-Wframe-larger-than=]
  fs/ext4/super.c:2279:1: warning: the frame size of 1160 bytes is larger than 800 bytes [-Wframe-larger-than=]
  fs/ext4/xattr.c:146:1: warning: the frame size of 1168 bytes is larger than 800 bytes [-Wframe-larger-than=]
  fs/f2fs/inode.c:152:1: warning: the frame size of 1424 bytes is larger than 800 bytes [-Wframe-larger-than=]
  net/netfilter/ipvs/ip_vs_core.c:1195:1: warning: the frame size of 1068 bytes is larger than 800 bytes [-Wframe-larger-than=]
  net/netfilter/ipvs/ip_vs_core.c:395:1: warning: the frame size of 1084 bytes is larger than 800 bytes [-Wframe-larger-than=]
  net/netfilter/ipvs/ip_vs_ftp.c:298:1: warning: the frame size of 928 bytes is larger than 800 bytes [-Wframe-larger-than=]
  net/netfilter/ipvs/ip_vs_ftp.c:418:1: warning: the frame size of 908 bytes is larger than 800 bytes [-Wframe-larger-than=]
  net/netfilter/ipvs/ip_vs_lblcr.c:718:1: warning: the frame size of 960 bytes is larger than 800 bytes [-Wframe-larger-than=]
  drivers/net/xen-netback/netback.c:1500:1: warning: the frame size of 1088 bytes is larger than 800 bytes [-Wframe-larger-than=]

In case of ARC and CRIS, it turns out that the BUG() implementation
actually does return (or at least the compiler thinks it does),
resulting in lots of warnings about uninitialized variable use and
leaving noreturn functions, such as:

  block/cfq-iosched.c: In function 'cfq_async_queue_prio':
  block/cfq-iosched.c:3804:1: error: control reaches end of non-void function [-Werror=return-type]
  include/linux/dmaengine.h: In function 'dma_maxpq':
  include/linux/dmaengine.h:1123:1: error: control reaches end of non-void function [-Werror=return-type]

This makes them call __builtin_trap() instead, which should normally
dump the stack and kill the current process, like some of the other
architectures already do.

I tried adding barrier_before_unreachable() to panic() and
fortify_panic() as well, but that had very little effect, so I'm not
submitting that patch.

Vineet said:

: For ARC, it is double win.
:
: 1. Fixes 3 -Wreturn-type warnings
:
: | ../net/core/ethtool.c:311:1: warning: control reaches end of non-void function
: [-Wreturn-type]
: | ../kernel/sched/core.c:3246:1: warning: control reaches end of non-void function
: [-Wreturn-type]
: | ../include/linux/sunrpc/svc_xprt.h:180:1: warning: control reaches end of
: non-void function [-Wreturn-type]
:
: 2.  bloat-o-meter reports code size improvements as gcc elides the
:    generated code for stack return.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
Link: http://lkml.kernel.org/r/20171219114112.939391-1-arnd@arndb.de
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Vineet Gupta <vgupta@synopsys.com> [arch/arc]
Tested-by: Vineet Gupta <vgupta@synopsys.com> [arch/arc]
Cc: Mikael Starvik <starvik@axis.com>
Cc: Jesper Nilsson <jesper.nilsson@axis.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
arch/arc/include/asm/bug.h
arch/cris/include/arch-v10/arch/bug.h
arch/ia64/include/asm/bug.h
arch/m68k/include/asm/bug.h
arch/sparc/include/asm/bug.h
include/asm-generic/bug.h
include/linux/compiler-gcc.h
include/linux/compiler.h

index ea022d47896cef2761bbb8a147844c506f358621..21ec82466d62c89922566ec3bf32551c2dce003b 100644 (file)
@@ -23,7 +23,8 @@ void die(const char *str, struct pt_regs *regs, unsigned long address);
 
 #define BUG()  do {                                                            \
        pr_warn("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
-       dump_stack();                                                           \
+       barrier_before_unreachable();                                           \
+       __builtin_trap();                                                       \
 } while (0)
 
 #define HAVE_ARCH_BUG
index 905afeacfedf53de44b3ba08487b04b46141e564..06da9d49152a0fd78ef1bac764df62aa38e35c83 100644 (file)
@@ -44,18 +44,25 @@ struct bug_frame {
  * not be used like this with newer versions of gcc.
  */
 #define BUG()                                                          \
+do {                                                                   \
        __asm__ __volatile__ ("clear.d [" __stringify(BUG_MAGIC) "]\n\t"\
                              "movu.w " __stringify(__LINE__) ",$r0\n\t"\
                              "jump 0f\n\t"                             \
                              ".section .rodata\n"                      \
                              "0:\t.string \"" __FILE__ "\"\n\t"        \
-                             ".previous")
+                             ".previous");                             \
+       unreachable();                                                  \
+} while (0)
 #endif
 
 #else
 
 /* This just causes an oops. */
-#define BUG() (*(int *)0 = 0)
+#define BUG()                                                          \
+do {                                                                   \
+       barrier_before_unreachable();                                   \
+       __builtin_trap();                                               \
+} while (0)
 
 #endif
 
index bd3eeb8d1cfa379a56456a3fd27909dbeaf2b55b..66b37a5327654f87a40fc277fad53d927875c355 100644 (file)
@@ -4,7 +4,11 @@
 
 #ifdef CONFIG_BUG
 #define ia64_abort()   __builtin_trap()
-#define BUG() do { printk("kernel BUG at %s:%d!\n", __FILE__, __LINE__); ia64_abort(); } while (0)
+#define BUG() do {                                             \
+       printk("kernel BUG at %s:%d!\n", __FILE__, __LINE__);   \
+       barrier_before_unreachable();                           \
+       ia64_abort();                                           \
+} while (0)
 
 /* should this BUG be made generic? */
 #define HAVE_ARCH_BUG
index b7e2bf1ba4a60d1e139aedf50c7a82fc0b0235cd..275dca1435bf9f64202b0a9249b0a07138105c82 100644 (file)
@@ -8,16 +8,19 @@
 #ifndef CONFIG_SUN3
 #define BUG() do { \
        pr_crit("kernel BUG at %s:%d!\n", __FILE__, __LINE__); \
+       barrier_before_unreachable(); \
        __builtin_trap(); \
 } while (0)
 #else
 #define BUG() do { \
        pr_crit("kernel BUG at %s:%d!\n", __FILE__, __LINE__); \
+       barrier_before_unreachable(); \
        panic("BUG!"); \
 } while (0)
 #endif
 #else
 #define BUG() do { \
+       barrier_before_unreachable(); \
        __builtin_trap(); \
 } while (0)
 #endif
index 6f17528356b2f71c8a71764a0ac2715a3398a33f..ea53e418f6c045763ff030a1dc2fa73d524c38b8 100644 (file)
@@ -9,10 +9,14 @@
 void do_BUG(const char *file, int line);
 #define BUG() do {                                     \
        do_BUG(__FILE__, __LINE__);                     \
+       barrier_before_unreachable();                   \
        __builtin_trap();                               \
 } while (0)
 #else
-#define BUG()          __builtin_trap()
+#define BUG() do {                                     \
+       barrier_before_unreachable();                   \
+       __builtin_trap();                               \
+} while (0)
 #endif
 
 #define HAVE_ARCH_BUG
index 963b755d19b0329e9e15b238db5f62074d615c51..a7613e1b0c87a4a38635b6fe99333afde18238b6 100644 (file)
@@ -52,6 +52,7 @@ struct bug_entry {
 #ifndef HAVE_ARCH_BUG
 #define BUG() do { \
        printk("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \
+       barrier_before_unreachable(); \
        panic("BUG!"); \
 } while (0)
 #endif
index 73bc63e0a1c4b664f233f176c7694fa8e54e34aa..901c1ccb3374aaeba42ab3cda3ed4c3e46d75085 100644 (file)
 #endif
 #endif
 
+/*
+ * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
+ * confuse the stack allocation in gcc, leading to overly large stack
+ * frames, see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
+ *
+ * Adding an empty inline assembly before it works around the problem
+ */
+#define barrier_before_unreachable() asm volatile("")
+
 /*
  * Mark a position in code as unreachable.  This can be used to
  * suppress control flow warnings after asm blocks that transfer
  * unreleased.  Really, we need to have autoconf for the kernel.
  */
 #define unreachable() \
-       do { annotate_unreachable(); __builtin_unreachable(); } while (0)
+       do {                                    \
+               annotate_unreachable();         \
+               barrier_before_unreachable();   \
+               __builtin_unreachable();        \
+       } while (0)
 
 /* Mark a function definition as prohibited from being cloned. */
 #define __noclone      __attribute__((__noclone__, __optimize__("no-tracer")))
index e835fc0423eccf7f66cca38c9dcbfa4f6d065ce3..ab4711c63601495347eee07382a692a4b1cee3ab 100644 (file)
@@ -86,6 +86,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 # define barrier_data(ptr) barrier()
 #endif
 
+/* workaround for GCC PR82365 if needed */
+#ifndef barrier_before_unreachable
+# define barrier_before_unreachable() do { } while (0)
+#endif
+
 /* Unreachable code */
 #ifdef CONFIG_STACK_VALIDATION
 /*