On 02/07/2018 04:01 PM, Andrew Morton wrote: > On Wed, 20 Dec 2017 12:29:02 -0800 Vineet Gupta <Vineet.Gupta1 at synopsys.com> wrote: > >> On 12/20/2017 12:12 PM, Arnd Bergmann wrote: >>>> Sorry, I didn't realize we are missing the stack trace now which you removed >>>> from the patch - why ? Did u intend to reduce inline generated code for the >>>> stack dump calls - which sounds like a great idea. But it would only work >>>> for the synchronous abort() but not when builtin translates to actual trap >>>> inducing instruction. >>> >>> I assumed that the trap instruction would trigger the register and >>> stack dump, as it does on all other architectures. >> >> Only if __builtin_trap() translated to that instruction. Otherwise we need to do >> this inside abort() >> >>> The most common >>> way this is handled is to have one instruction that is known to trap, >>> and use that to trigger a BUG(), and have __builtin_trap() issue >>> that instruction as well. >> >> Good point. So we'll need ARC specific abort anyways. >> >> >>> You might also want to implement CONFIG_DEBUG_BUGVERBOSE >>> support to attach further data to it. >> >> OK I'll take a look ! >> >>> How about overriding abort() with the same instruction that >>> __builtin_trap() inserts on newer compilers then? That should >>> make the behavior consistent. >> >> Yeah this is a great point ! Will do > > Didn't do ;) > > Is Arnd's patch good to merge or do we need a fixup? > > > From: Arnd Bergmann <arnd at arndb.de> > Subject: bug.h: work around GCC PR82365 in BUG() > > 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://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365 > Link: http://lkml.kernel.org/r/20171219114112.939391-1-arnd at arndb.de > Signed-off-by: Arnd Bergmann <arnd at arndb.de> > Tested-by: Vineet Gupta <vgupta at synopsys.com> > Cc: Mikael Starvik <starvik at axis.com> > Cc: Jesper Nilsson <jesper.nilsson at axis.com> > Cc: Tony Luck <tony.luck at intel.com> > Cc: Fenghua Yu <fenghua.yu at intel.com> > Cc: Geert Uytterhoeven <geert at linux-m68k.org> > Cc: "David S. Miller" <davem at davemloft.net> > Cc: Christopher Li <sparse at chrisli.org> > Cc: Thomas Gleixner <tglx at linutronix.de> > Cc: Peter Zijlstra <peterz at infradead.org> > Cc: Kees Cook <keescook at chromium.org> > Cc: Ingo Molnar <mingo at kernel.org> > Cc: Josh Poimboeuf <jpoimboe at redhat.com> > Cc: Will Deacon <will.deacon at arm.com> > Cc: "Steven Rostedt (VMware)" <rostedt at goodmis.org> > Cc: Mark Rutland <mark.rutland at arm.com> > Signed-off-by: Andrew Morton <akpm at linux-foundation.org> > --- > > 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 -puN arch/arc/include/asm/bug.h~bugh-work-around-gcc-pr82365-in-bug arch/arc/include/asm/bug.h > --- a/arch/arc/include/asm/bug.h~bugh-work-around-gcc-pr82365-in-bug > +++ a/arch/arc/include/asm/bug.h > @@ -23,7 +23,8 @@ void die(const char *str, struct pt_regs > > #define BUG() do { \ > pr_warn("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__, __func__); \ > - dump_stack(); \ > + barrier_before_unreachable(); \ > + __builtin_trap(); \ > } while (0) > 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. Acked-by: Vineet Gupta <vgupta at synopsys.com> # for arch/arc Tested-by: Vineet Gupta <vgupta at synopsys.com> # for arch/arc
