Messages in this thread | | | Date | Thu, 16 Mar 2017 13:32:01 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH] x86: mostly disable '-maccumulate-outgoing-args' |
| |
On Thu, 16 Mar 2017 10:42:08 -0500 Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.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,\
| |