[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20170316133201.16b9a73a@gandalf.local.home>
Date: Thu, 16 Mar 2017 13:32:01 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: x86@...nel.org, Andy Lutomirski <luto@...capital.net>,
Pavel Machek <pavel@....cz>,
kernel list <linux-kernel@...r.kernel.org>,
Ingo Molnar <mingo@...nel.org>,
Andrew Lutomirski <luto@...nel.org>,
Borislav Petkov <bp@...en8.de>,
Brian Gerst <brgerst@...il.com>,
Denys Vlasenko <dvlasenk@...hat.com>,
Peter Anvin <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] x86: mostly disable '-maccumulate-outgoing-args'
On Thu, 16 Mar 2017 10:42:08 -0500
Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
> ---
> arch/x86/Makefile | 29 +++++++++++++++++++++++++----
> arch/x86/Makefile_32.cpu | 18 ------------------
> arch/x86/kernel/ftrace.c | 6 ++++++
> scripts/Kbuild.include | 4 ++++
> 4 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> index 2d44933..fa45989b 100644
> --- a/arch/x86/Makefile
> +++ b/arch/x86/Makefile
> @@ -120,10 +120,6 @@ else
> # -funit-at-a-time shrinks the kernel .text considerably
> # unfortunately it makes reading oopses harder.
> KBUILD_CFLAGS += $(call cc-option,-funit-at-a-time)
> -
> - # this works around some issues with generating unwind tables in older gccs
> - # newer gccs do it by default
> - KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> endif
>
> ifdef CONFIG_X86_X32
> @@ -147,6 +143,31 @@ ifeq ($(CONFIG_KMEMCHECK),y)
> KBUILD_CFLAGS += $(call cc-option,-fno-builtin-memcpy)
> endif
>
> +# If the function graph tracer is used with mcount instead of fentry,
> +# '-maccumulate-outgoing-args' is needed to prevent gcc bug
"to prevent a gcc bug"
> +# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=42109
> +ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + ifndef CONFIG_HAVE_FENTRY
> + ACCUMULATE_OUTGOING_ARGS := 1
> + else
> + ifeq ($(call cc-option, -mfentry),)
Hmm, the blank entry makes me nervous. I wonder if it would be better
if we had ifneq ($(call cc-option-yn, -mfentry),y)
Unfortunately, there's one of each in the existing kernel, so there is
really no precedence.
> + ACCUMULATE_OUTGOING_ARGS := 1
> + endif
> + endif
> +endif
> +
> +# Jump labels need '-maccumulate-outgoing-args' for gcc < 4.5.2 to prevent
Can we make a test instead? I hate testing versions, and things get
backported all the time. We usually like to have a test case instead of
relying on versions. Not to mention, a newer gcc may one day break.
-- Steve
> +# gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=46226
> +ifdef CONFIG_JUMP_LABEL
> + ifneq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> + ACCUMULATE_OUTGOING_ARGS = $(call cc-if-fullversion, -lt, 040502, 1)
> + endif
> +endif
> +
> +ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
> + KBUILD_CFLAGS += -maccumulate-outgoing-args
> +endif
> +
> # Stackpointer is addressed different for 32 bit and 64 bit x86
> sp-$(CONFIG_X86_32) := esp
> sp-$(CONFIG_X86_64) := rsp
> diff --git a/arch/x86/Makefile_32.cpu b/arch/x86/Makefile_32.cpu
> index 6647ed4..a45eb15 100644
> --- a/arch/x86/Makefile_32.cpu
> +++ b/arch/x86/Makefile_32.cpu
> @@ -45,24 +45,6 @@ cflags-$(CONFIG_MGEODE_LX) += $(call cc-option,-march=geode,-march=pentium-mmx)
> # cpu entries
> cflags-$(CONFIG_X86_GENERIC) += $(call tune,generic,$(call tune,i686))
>
> -# Work around the pentium-mmx code generator madness of gcc4.4.x which
> -# does stack alignment by generating horrible code _before_ the mcount
> -# prologue (push %ebp, mov %esp, %ebp) which breaks the function graph
> -# tracer assumptions. For i686, generic, core2 this is set by the
> -# compiler anyway
> -ifeq ($(CONFIG_FUNCTION_GRAPH_TRACER), y)
> -ADD_ACCUMULATE_OUTGOING_ARGS := y
> -endif
> -
> -# Work around to a bug with asm goto with first implementations of it
> -# in gcc causing gcc to mess up the push and pop of the stack in some
> -# uses of asm goto.
> -ifeq ($(CONFIG_JUMP_LABEL), y)
> -ADD_ACCUMULATE_OUTGOING_ARGS := y
> -endif
> -
> -cflags-$(ADD_ACCUMULATE_OUTGOING_ARGS) += $(call cc-option,-maccumulate-outgoing-args)
> -
> # Bug fix for binutils: this option is required in order to keep
> # binutils from generating NOPL instructions against our will.
> ifneq ($(CONFIG_X86_P6_NOP),y)
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 8f3d9cf..59f9b46 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -29,6 +29,12 @@
> #include <asm/ftrace.h>
> #include <asm/nops.h>
>
> +#if defined(CONFIG_FUNCTION_GRAPH_TRACER) && \
> + !defined(CC_USING_FENTRY) && \
> + !defined(CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE)
> +# error Your compiler does not support function graph tracing
> +#endif
> +
> #ifdef CONFIG_DYNAMIC_FTRACE
>
> int ftrace_arch_code_modify_prepare(void)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index d6ca649..afe3fd3 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -148,6 +148,10 @@ cc-fullversion = $(shell $(CONFIG_SHELL) \
> # Usage: EXTRA_CFLAGS += $(call cc-ifversion, -lt, 0402, -O1)
> cc-ifversion = $(shell [ $(cc-version) $(1) $(2) ] && echo $(3) || echo $(4))
>
> +# cc-if-fullversion
> +# Usage: EXTRA_CFLAGS += $(call cc-if-fullversion, -lt, 040502, -O1)
> +cc-if-fullversion = $(shell [ $(cc-fullversion) $(1) $(2) ] && echo $(3) || echo $(4))
> +
> # cc-ldoption
> # Usage: ldflags += $(call cc-ldoption, -Wl$(comma)--hash-style=both)
> cc-ldoption = $(call try-run,\
Powered by blists - more mailing lists