From: Steven Rostedt <rostedt@goodmis.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@elte.hu>, "H. Peter Anvin" <hpa@zytor.com>,
LKML <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Heiko Carstens <heiko.carstens@de.ibm.com>,
feng.tang@intel.com, Peter Zijlstra <peterz@infradead.org>,
Frederic Weisbecker <fweisbec@gmail.com>,
David Daney <ddaney@caviumnetworks.com>,
Andrew Haley <aph@redhat.com>,
Richard Guenther <richard.guenther@gmail.com>,
jakub@redhat.com, gcc <gcc@gcc.gnu.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
linux-kbuild <linux-kbuild@vger.kernel.org>,
Sam Ravnborg <sam@ravnborg.org>
Subject: Re: [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect GCC messing with mcount prologue
Date: Fri, 20 Nov 2009 05:33:00 -0000 [thread overview]
Message-ID: <1258695154.22249.1014.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <1258694593.22249.1012.camel@gandalf.stny.rr.com>
This touches the Makefile scripts. I forgot to CC kbuild and Sam.
-- Steve
On Fri, 2009-11-20 at 00:23 -0500, Steven Rostedt wrote:
> Ingo,
>
> Not sure if this is too much for this late in the -rc game, but it finds
> the gcc bug at build time, and we don't need to disable function graph
> tracer for all i386 builds.
>
> This is built on my last urgent repo pull request.
>
> Please pull the latest tip/tracing/urgent-2 tree, which can be found at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
> tip/tracing/urgent-2
>
>
> Steven Rostedt (1):
> tracing/x86: Add check to detect GCC messing with mcount prologue
>
> ----
> kernel/trace/Kconfig | 1 -
> scripts/Makefile.build | 25 +++++++++++++++-
> scripts/recordmcount.pl | 74 +++++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 95 insertions(+), 5 deletions(-)
> ---------------------------
> commit c7715fb611c69ac4b7f722a891de08b206fb7686
> Author: Steven Rostedt <srostedt@redhat.com>
> Date: Thu Nov 19 23:41:02 2009 -0500
>
> tracing/x86: Add check to detect GCC messing with mcount prologue
>
> Latest versions of GCC create a funny prologue for some functions.
> Instead of the typical:
>
> push %ebp
> mov %esp,%ebp
> and $0xffffffe0,%esp
> [...]
> call mcount
>
> GCC may try to align the stack before setting up the frame pointer
> register:
>
> push %edi
> lea 0x8(%esp),%edi
> and $0xffffffe0,%esp
> pushl -0x4(%edi)
> push %ebp
> mov %esp,%ebp
> [...]
> call mcount
>
> This crazy code places a copy of the return address into the
> frame pointer. The function graph tracer uses this pointer to
> save and replace the return address of the calling function to jump
> to the function graph tracer's return handler, which will put back
> the return address. But instead instead of the typical return:
>
> mov %ebp,%esp
> pop %ebp
> ret
>
> The return of the function performs:
>
> lea -0x8(%edi),%esp
> pop %edi
> ret
>
> The function graph tracer return handler will not be called at the exit
> of the function, but the parent function will call it. Because we missed
> the return of the child function, the handler will replace the parent's
> return address with that of the child. Obviously this will cause a crash
> (Note, there is code to detect this case and safely panic the kernel).
>
> The kicker is that this happens to just a handful of functions.
> And only with certain gcc options.
>
> Compiling with: -march=pentium-mmx
> will cause the problem to appear. But if you were to change
> pentium-mmx to i686 or add -mtune=generic, then the problem goes away.
>
> I first saw this problem when compiling with optimize for size.
> But it seems that various other options may cause this issue to arise.
>
> Instead of completely disabling the function graph tracer for i386 builds
> this patch adds a check to recordmcount.pl to make sure that all
> functions that contain a call to mcount start with "push %ebp".
> If not, it will fail the compile and print out the nasty warning:
>
> CC kernel/time/timer_stats.o
>
> ********************************************************
> Your version of GCC breaks the function graph tracer
> Please disable CONFIG_FUNCTION_GRAPH_TRACER
> Failed function was "timer_stats_update_stats"
> ********************************************************
>
> The script recordmcount.pl is given a new parameter "do_check". If
> this is negative, the script will only perform this check without
> creating the mcount caller section. This will be executed for x86_32
> when CONFIG_FUNCTION_GRAPH_TRACER is enabled and CONFIG_DYNAMIC_FTRACE
> is not.
>
> If the arch is x86_32 and $do_check is greater than 1, it will perform
> the check while processing the mcount callers. If $do_check is 0, then
> no check will be performed. This is for non x86_32 archs and when
> compiling without CONFIG_FUNCTION_GRAPH_TRACER enabled, even on x86_32.
>
> Reported-by: Thomas Gleixner <tglx@linutronix.de>
> LKML-Reference: <alpine.LFD.2.00.0911191423190.24119@localhost.localdomain>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
>
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index b416512..cd39064 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -143,7 +143,6 @@ config FUNCTION_GRAPH_TRACER
> bool "Kernel Function Graph Tracer"
> depends on HAVE_FUNCTION_GRAPH_TRACER
> depends on FUNCTION_TRACER
> - depends on !X86_32 || !CC_OPTIMIZE_FOR_SIZE
> default y
> help
> Enable the kernel to trace a function at both its return
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 341b589..3b897f2 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -206,10 +206,33 @@ cmd_modversions = \
> endif
>
> ifdef CONFIG_FTRACE_MCOUNT_RECORD
> +
> + ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + ifdef CONFIG_X86_32
> + rm_do_check = 1
> + else
> + rm_do_check = 0
> + endif
> + else
> + rm_do_check = 0
> + endif
> +
> cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
> "$(if $(CONFIG_64BIT),64,32)" \
> "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" "$(MV)" \
> - "$(if $(part-of-module),1,0)" "$(@)";
> + "$(if $(part-of-module),1,0)" "$(rm_do_check)" "$(@)";
> +
> +else
> +
> + ifdef CONFIG_X86_32
> + ifdef CONFIG_FUNCTION_GRAPH_TRACER
> + cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
> + "$(if $(CONFIG_64BIT),64,32)" \
> + "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" "$(MV)" \
> + "$(if $(part-of-module),1,0)" "-1" "$(@)";
> + endif
> + endif
> +
> endif
>
> define rule_cc_o_c
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 090d300..164a9d5 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -99,14 +99,14 @@ $P =~ s@.*/@@g;
>
> my $V = '0.1';
>
> -if ($#ARGV < 7) {
> - print "usage: $P arch bits objdump objcopy cc ld nm rm mv is_module inputfile\n";
> +if ($#ARGV < 11) {
> + print "usage: $P arch bits objdump objcopy cc ld nm rm mv is_module do_check inputfile\n";
> print "version: $V\n";
> exit(1);
> }
>
> my ($arch, $bits, $objdump, $objcopy, $cc,
> - $ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
> + $ld, $nm, $rm, $mv, $is_module, $do_check, $inputfile) = @ARGV;
>
> # This file refers to mcount and shouldn't be ftraced, so lets' ignore it
> if ($inputfile eq "kernel/trace/ftrace.o") {
> @@ -129,6 +129,60 @@ $nm = "nm" if ((length $nm) == 0);
> $rm = "rm" if ((length $rm) == 0);
> $mv = "mv" if ((length $mv) == 0);
>
> +# gcc can do stupid things with the stack pointer on x86_32.
> +# It may pass a copy of the return address to mcount, which will
> +# break the function graph tracer. If this happens then we need
> +# to flag it and break the build.
> +#
> +# For x86_32, the parameter do_check will be negative if we only
> +# want to perform the check, and positive if we should od the check.
> +# If function graph tracing is not enabled, do_check will be zero.
> +#
> +
> +my $check_next_line = 0;
> +my $line_failed = 0;
> +my $last_function;
> +
> +sub test_x86_32_prologue
> +{
> + if ($check_next_line) {
> + if (!/push\s*\%ebp/) {
> + $line_failed = 1;
> + }
> + }
> +
> + if ($line_failed && /mcount/) {
> + print STDERR "\n********************************************************\n";
> + print STDERR " Your version of GCC breaks the function graph tracer\n";
> + print STDERR " Please disable CONFIG_FUNCTION_GRAPH_TRACER\n";
> + print STDERR " Failed function was \"$last_function\"\n";
> + print STDERR "********************************************************\n\n";
> + exit -1;
> + }
> + $check_next_line = 0;
> +
> + # check the first line after a function starts for
> + # push %ebp
> + if (/^[0-9a-fA-F]+\s+<([a-zA-Z_].*)>:$/) {
> + $last_function = $1;
> + $check_next_line = 1;
> + $line_failed = 0;
> + }
> +}
> +
> +if ($do_check < 0) {
> + # Only perform the check and quit
> + open(IN, "$objdump -dr $inputfile|") || die "error running $objdump";
> +
> + while (<IN>) {
> + test_x86_32_prologue;
> + }
> + close (IN);
> + exit 0;
> +}
> +
> +my $check = 0;
> +
> #print STDERR "running: $P '$arch' '$objdump' '$objcopy' '$cc' '$ld' " .
> # "'$nm' '$rm' '$mv' '$inputfile'\n";
>
> @@ -153,6 +207,12 @@ if ($arch eq "x86") {
> }
> }
>
> +if ($arch eq "i386") {
> + if ($do_check) {
> + $check = 1;
> + }
> +}
> +
> #
> # We base the defaults off of i386, the other archs may
> # feel free to change them in the below if statements.
> @@ -381,6 +441,14 @@ my $text;
> my $read_headers = 1;
>
> while (<IN>) {
> +
> + # x86_32 may need to test the start of every function to see
> + # if GCC did not mess up the mcount prologue. All functions must
> + # start with push %ebp, otherwise it is broken.
> + if ($check) {
> + test_x86_32_prologue;
> + }
> +
> # is it a section?
> if (/$section_regex/) {
> $read_headers = 0;
>
next prev parent reply other threads:[~2009-11-20 5:33 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <alpine.LFD.2.00.0911181933540.24119@localhost.localdomain>
[not found] ` <tip-887a29f59b93cf54e21814869a4ab6e80b6fa623@git.kernel.org>
[not found] ` <20091119072040.GA23579@elte.hu>
[not found] ` <alpine.LFD.2.00.0911191053390.24119@localhost.localdomain>
[not found] ` <alpine.LFD.2.00.0911191423190.24119@localhost.localdomain>
2009-11-19 15:39 ` BUG: GCC-4.4.x changes the function frame on some functions Thomas Gleixner
2009-11-19 15:45 ` Andrew Haley
2009-11-19 15:55 ` H. Peter Anvin
2009-11-19 15:59 ` Richard Guenther
2009-11-19 16:03 ` Steven Rostedt
2009-11-19 16:13 ` H. Peter Anvin
2009-11-19 16:20 ` Frederic Weisbecker
2009-11-19 16:07 ` Thomas Gleixner
2009-11-19 16:19 ` Andrew Haley
2009-11-19 16:45 ` Thomas Gleixner
2009-11-19 16:13 ` Steven Rostedt
2009-11-19 15:46 ` H. Peter Anvin
2009-11-19 15:51 ` Richard Guenther
2009-11-19 15:53 ` Richard Guenther
2009-11-19 17:38 ` Andi Kleen
2009-11-19 17:41 ` Linus Torvalds
2009-11-19 17:52 ` Thomas Gleixner
2009-11-19 18:00 ` Steven Rostedt
2009-11-19 18:04 ` Richard Guenther
2009-11-19 18:23 ` Andrew Haley
2009-11-19 18:42 ` Linus Torvalds
2009-11-19 18:45 ` Linus Torvalds
2009-11-19 18:56 ` Linus Torvalds
2009-11-19 19:03 ` Thomas Gleixner
2009-11-23 9:17 ` Jakub Jelinek
2009-11-23 9:53 ` Thomas Gleixner
2009-11-19 19:12 ` David Daney
2009-11-19 19:29 ` Steven Rostedt
2009-11-19 19:47 ` Frederic Weisbecker
2009-11-19 19:55 ` Kai Tietz
2009-11-19 20:06 ` Frederic Weisbecker
2009-11-19 20:06 ` Steven Rostedt
2009-11-19 20:18 ` Steven Rostedt
2009-11-19 20:29 ` Frederic Weisbecker
2009-11-19 20:26 ` Frederic Weisbecker
2009-11-19 20:38 ` Linus Torvalds
2009-11-19 20:45 ` Steven Rostedt
2009-11-19 19:51 ` H. Peter Anvin
2009-11-19 20:08 ` Linus Torvalds
2009-11-19 21:13 ` Jeff Law
2009-11-19 20:11 ` Steven Rostedt
2009-11-19 21:07 ` Jeff Law
2009-11-19 18:34 ` Thomas Gleixner
2009-11-19 18:40 ` Linus Torvalds
2009-11-19 18:48 ` Ingo Molnar
2009-11-19 19:08 ` Steven Rostedt
2009-11-19 19:51 ` Ingo Molnar
2009-11-20 9:58 ` [PATCH] gcc mcount-nofp was " Andi Kleen
2009-11-20 12:35 ` Steven Rostedt
2009-11-20 19:08 ` H. Peter Anvin
2009-11-19 20:37 ` Thomas Gleixner
2009-11-19 18:22 ` Andrew Haley
2009-11-19 18:34 ` Steven Rostedt
2009-11-19 18:37 ` Andrew Pinski
2009-11-19 18:38 ` Andrew Haley
2009-11-19 18:38 ` H. Peter Anvin
2009-11-19 18:40 ` Thomas Gleixner
2009-11-20 5:24 ` [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect GCC messing with mcount prologue Steven Rostedt
2009-11-20 5:33 ` Steven Rostedt [this message]
2009-11-20 17:02 ` Steven Rostedt
2009-11-20 17:15 ` H. Peter Anvin
2009-11-20 19:36 ` Andrew Haley
2009-11-20 19:48 ` Steven Rostedt
2009-11-20 19:50 ` H. Peter Anvin
2009-11-22 9:39 ` H.J. Lu
2009-11-22 17:21 ` Andrew Haley
2009-11-22 23:31 ` H.J. Lu
2009-11-24 14:43 ` Andrew Haley
2009-11-24 14:56 ` Thomas Gleixner
2009-11-24 15:06 ` Jakub Jelinek
2009-11-24 15:32 ` Andrew Haley
2009-11-24 15:36 ` Jakub Jelinek
2009-11-24 15:48 ` Andrew Haley
2009-11-24 16:39 ` H. Peter Anvin
2009-11-24 17:12 ` Andrew Haley
2009-11-24 17:30 ` Steven Rostedt
2009-11-25 20:05 ` H. Peter Anvin
2009-11-24 19:56 ` H. Peter Anvin
2009-11-25 15:29 ` Thomas Gleixner
2009-11-25 15:45 ` Ingo Molnar
2009-11-25 15:53 ` Thomas Gleixner
2009-11-25 16:26 ` Ingo Molnar
2009-11-25 16:45 ` Jakub Jelinek
2009-11-25 20:13 ` H. Peter Anvin
2009-11-25 21:01 ` Andrew Haley
2009-11-22 9:06 ` Ingo Molnar
2009-11-24 17:05 Ross Ridge
2009-11-24 17:11 ` Andrew Haley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1258695154.22249.1014.camel@gandalf.stny.rr.com \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=aph@redhat.com \
--cc=ddaney@caviumnetworks.com \
--cc=feng.tang@intel.com \
--cc=fweisbec@gmail.com \
--cc=gcc@gcc.gnu.org \
--cc=heiko.carstens@de.ibm.com \
--cc=hpa@zytor.com \
--cc=jakub@redhat.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=richard.guenther@gmail.com \
--cc=sam@ravnborg.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).