mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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>
Subject: [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect GCC messing with mcount prologue
Date: Fri, 20 Nov 2009 00:23:13 -0500	[thread overview]
Message-ID: <1258694593.22249.1012.camel@gandalf.stny.rr.com> (raw)
In-Reply-To: <alpine.LFD.2.00.0911191423190.24119@localhost.localdomain>


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;



  parent reply	other threads:[~2009-11-20  5:23 UTC|newest]

Thread overview: 93+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <200911172214.nAHMEBh2023787@imap1.linux-foundation.org>
2009-11-18 19:30 ` [patch for 2.6.32? 1/3] hrtimers: remove the "timer_stats_active" check when setting the start info Thomas Gleixner
2009-11-18 20:24   ` [tip:timers/urgent] hrtimer: Fix /proc/timer_list regression tip-bot for Feng Tang
2009-11-19  7:20     ` Ingo Molnar
2009-11-19 10:05       ` Thomas Gleixner
2009-11-19 14:30         ` BUG: function graph tracer function frame assumptions Thomas Gleixner
2009-11-19 15:37           ` BUG: GCC-4.4.x changes the function frame on some functions Thomas Gleixner
2009-11-19 15:44             ` Andrew Haley
2009-11-19 15:54               ` H. Peter Anvin
2009-11-19 15:57                 ` Richard Guenther
2009-11-19 16:02               ` Steven Rostedt
2009-11-19 16:11                 ` H. Peter Anvin
2009-11-19 16:19                 ` Frederic Weisbecker
2009-11-19 16:06               ` Thomas Gleixner
2009-11-19 16:17                 ` Andrew Haley
2009-11-19 16:43                   ` Thomas Gleixner
2009-11-19 16:12               ` Steven Rostedt
2009-11-19 15:45             ` H. Peter Anvin
2009-11-19 15:49               ` Richard Guenther
2009-11-19 15:52                 ` Richard Guenther
2009-11-19 17:37                 ` Andi Kleen
2009-11-19 17:39             ` Linus Torvalds
2009-11-19 17:51               ` Thomas Gleixner
2009-11-19 17:59               ` Steven Rostedt
2009-11-19 18:03                 ` Richard Guenther
2009-11-19 18:22                   ` Andrew Haley
2009-11-19 18:41                     ` Linus Torvalds
2009-11-19 18:43                       ` Linus Torvalds
2009-11-19 18:54                         ` Linus Torvalds
2009-11-19 19:01                           ` Thomas Gleixner
2009-11-23  9:16                             ` Jakub Jelinek
2009-11-23  9:51                               ` Thomas Gleixner
2009-11-19 19:10                           ` David Daney
2009-11-19 19:28                             ` Steven Rostedt
2009-11-19 19:46                               ` Frederic Weisbecker
2009-11-19 19:54                                 ` Kai Tietz
2009-11-19 20:05                                   ` Frederic Weisbecker
2009-11-19 20:05                                 ` Steven Rostedt
2009-11-19 20:17                                   ` Steven Rostedt
2009-11-19 20:28                                     ` Frederic Weisbecker
2009-11-19 20:25                                   ` Frederic Weisbecker
2009-11-19 20:36                                     ` Linus Torvalds
2009-11-19 20:44                                       ` Steven Rostedt
2009-11-19 19:50                               ` H. Peter Anvin
2009-11-19 20:06                                 ` Linus Torvalds
2009-11-19 21:12                                   ` Jeff Law
2009-11-19 20:10                                 ` Steven Rostedt
2009-11-19 21:05                                 ` Jeff Law
2009-11-19 18:31                   ` Thomas Gleixner
2009-11-19 18:38                   ` Linus Torvalds
2009-11-19 18:47                     ` Ingo Molnar
2009-11-19 19:06                       ` Steven Rostedt
2009-11-19 19:50                         ` Ingo Molnar
2009-11-20  9:57                         ` [PATCH] gcc mcount-nofp was " Andi Kleen
2009-11-20 12:34                           ` Steven Rostedt
2009-11-20 19:06                             ` H. Peter Anvin
2009-11-19 20:36                     ` Thomas Gleixner
2009-11-19 18:20             ` Andrew Haley
2009-11-19 18:33               ` Steven Rostedt
2009-11-19 18:36                 ` Andrew Pinski
2009-11-19 18:36                 ` Andrew Haley
2009-11-19 18:37                 ` H. Peter Anvin
2009-11-19 18:39               ` Thomas Gleixner
2009-11-20  5:23           ` Steven Rostedt [this message]
2009-11-20  5:32             ` [PATCH][GIT PULL][v2.6.32] tracing/x86: Add check to detect GCC messing with mcount prologue Steven Rostedt
2009-11-20 17:00             ` Steven Rostedt
2009-11-20 17:13               ` H. Peter Anvin
2009-11-20 19:35               ` Andrew Haley
2009-11-20 19:46                 ` Steven Rostedt
2009-11-20 19:49                   ` H. Peter Anvin
2009-11-22  9:38                 ` H.J. Lu
2009-11-22 17:20                   ` Andrew Haley
2009-11-22 23:30                     ` H.J. Lu
2009-11-24 14:43                       ` Andrew Haley
2009-11-24 14:55                         ` 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:46                                 ` Andrew Haley
2009-11-24 16:38                                   ` 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:55                                       ` H. Peter Anvin
2009-11-25 15:29                             ` Thomas Gleixner
2009-11-25 15:44                               ` Ingo Molnar
2009-11-25 15:53                                 ` Thomas Gleixner
2009-11-25 16:25                                   ` Ingo Molnar
2009-11-25 16:44                                 ` Jakub Jelinek
2009-11-25 20:12                                   ` H. Peter Anvin
2009-11-25 21:00                                     ` Andrew Haley
2009-11-22  9:05               ` Ingo Molnar
2009-11-20 10:30   ` [tip:timers/urgent] hrtimer: Fix /proc/timer_list regression tip-bot for Feng Tang
2009-11-20 14:19     ` Heiko Carstens

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=1258694593.22249.1012.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-kernel@vger.kernel.org \
    --cc=mingo@elte.hu \
    --cc=peterz@infradead.org \
    --cc=richard.guenther@gmail.com \
    --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

all inboxes | Powered by JetHome®