[PATCH] bug.h: Work around GCC PR82365 in BUG()
Vineet Gupta
Vineet.Gupta1 at synopsys.com
Tue Dec 19 08:57:10 PST 2017
On 12/19/2017 03:41 AM, Arnd Bergmann wrote:
> 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.
>
> Link: https://urldefense.proofpoint.com/v2/url?u=https-3A__gcc.gnu.org_bugzilla_show-5Fbug.cgi-3Fid-3D82365&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=c14YS-cH-kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=3Iu4HWDn1cXkYBpSFh5I80IzDKJi33hs5DbfGM-b3mI&s=sTrcyN5ej_ION8hJvF9eGLUZYwdlwI50vXUp3MK-XWY&e=
> Signed-off-by: Arnd Bergmann <arnd at arndb.de>
> ---
> The name barrier_before_unreachable() is a bit suboptimal here,
> as it fails to describe the fact that it is needed for both
> __builtin_unreachable() and for calling noreturn functions. Any other
> suggestions would be welcome here.
> ---
> arch/arc/include/asm/bug.h | 3 ++-
> arch/cris/include/arch-v10/arch/bug.h | 11 +++++++++--
> arch/ia64/include/asm/bug.h | 6 +++++-
> arch/m68k/include/asm/bug.h | 3 +++
> arch/sparc/include/asm/bug.h | 6 +++++-
> include/asm-generic/bug.h | 1 +
> include/linux/compiler-gcc.h | 15 ++++++++++++++-
> include/linux/compiler.h | 5 +++++
> 8 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arc/include/asm/bug.h b/arch/arc/include/asm/bug.h
> index ea022d47896c..21ec82466d62 100644
> --- a/arch/arc/include/asm/bug.h
> +++ b/arch/arc/include/asm/bug.h
> @@ -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
> diff --git a/arch/cris/include/arch-v10/arch/bug.h b/arch/cris/include/arch-v10/arch/bug.h
> index 905afeacfedf..06da9d49152a 100644
> --- a/arch/cris/include/arch-v10/arch/bug.h
> +++ b/arch/cris/include/arch-v10/arch/bug.h
> @@ -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(); \
I suppose BUG() implies "dead end" like semantics - which ARC was lacking before ?
> +} while (0)
>
> #endif
>
> diff --git a/arch/ia64/include/asm/bug.h b/arch/ia64/include/asm/bug.h
> index bd3eeb8d1cfa..66b37a532765 100644
> --- a/arch/ia64/include/asm/bug.h
> +++ b/arch/ia64/include/asm/bug.h
> @@ -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
> diff --git a/arch/m68k/include/asm/bug.h b/arch/m68k/include/asm/bug.h
> index b7e2bf1ba4a6..275dca1435bf 100644
> --- a/arch/m68k/include/asm/bug.h
> +++ b/arch/m68k/include/asm/bug.h
> @@ -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
> diff --git a/arch/sparc/include/asm/bug.h b/arch/sparc/include/asm/bug.h
> index 6f17528356b2..ea53e418f6c0 100644
> --- a/arch/sparc/include/asm/bug.h
> +++ b/arch/sparc/include/asm/bug.h
> @@ -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
> diff --git a/include/asm-generic/bug.h b/include/asm-generic/bug.h
> index 963b755d19b0..a7613e1b0c87 100644
> --- a/include/asm-generic/bug.h
> +++ b/include/asm-generic/bug.h
> @@ -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
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 5d595cfdb2c4..66cfdad68f7e 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -205,6 +205,15 @@
> #endif
>
> /*
> + * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
> + * confuse the stack allocation in gcc, leading to overly large stack
> + * frames, see https://urldefense.proofpoint.com/v2/url?u=https-3A__gcc.gnu.org_bugzilla_show-5Fbug.cgi-3Fid-3D82365&d=DwIBAg&c=DPL6_X_6JkXFx7AXWqB0tg&r=c14YS-cH-kdhTOW89KozFhBtBJgs1zXscZojEZQ0THs&m=3Iu4HWDn1cXkYBpSFh5I80IzDKJi33hs5DbfGM-b3mI&s=sTrcyN5ej_ION8hJvF9eGLUZYwdlwI50vXUp3MK-XWY&e=
> + *
> + * 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
> * control elsewhere.
> @@ -214,7 +223,11 @@
> * 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")))
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 52e611ab9a6c..97847f2f86cf 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -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
> /*
More information about the linux-snps-arc
mailing list