Message-ID: <20141121095037.2f5a4d4f@gandalf.local.home>
Date:	Fri, 21 Nov 2014 09:50:37 -0500
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Ingo Molnar <mingo@...nel.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>, stable@...r.kernel.org
Subject: [GIT PULL] ftrace/x86: Add frames pointers to trampoline as
 necessary


Linus,

While testing function triggers, I noticed that the stack trace trigger
for functions was missing the function that caused the trigger as
well as the parent function that called the triggered function. I use
this feature a lot and never noticed this before. Then I realized that
the difference was that I had CONFIG_FRAME_POINTERS enabled, which I
don't on the production machines I'm debugging.

When frame pointers are enabled, the stack trace code will ignore anything
that isn't found via the frame pointer. As the function trace trampoline
does not set up frame pointers, it is skipped. When fentry is used,
(gcc -pg --mfentry) it is called before the function stack frame is set
up, thus we also lose the parent pointer. These are quite useful for
debugging live kernels.

This goes back to 3.7 where the stacktrace trigger was introduced.
Before that, the function stack trace option had this issue, but since
it was before fentry was introduced, it only lost the traced function.
But that wasn't an issue as the option required the function itself
to be traced, and that showed the missing function.

This is cherry-picked from my 3.19 queue. After some thought, I figured
it should go into 3.18 and stable.

Please pull the latest trace-fixes-v3.18-rc5 tree, which can be found at:


  git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
trace-fixes-v3.18-rc5

Tag SHA1: 7c58faf742b3dba437a84f40e994ac490cdad86a
Head SHA1: ec86b2aedbcb69ea4f8eae0d5e1af9321810b2f8


Steven Rostedt (Red Hat) (1):
      ftrace/x86: Add frames pointers to trampoline as necessary

----
 arch/x86/kernel/mcount_64.S | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
---------------------------
commit ec86b2aedbcb69ea4f8eae0d5e1af9321810b2f8
Author: Steven Rostedt (Red Hat) <rostedt@...dmis.org>
Date:   Tue Nov 18 19:13:25 2014 -0500

    ftrace/x86: Add frames pointers to trampoline as necessary
    
    When CONFIG_FRAME_POINTERS are enabled, it is required that the
    ftrace_caller and ftrace_regs_caller trampolines set up frame pointers
    otherwise a stack trace from a function call wont print the functions
    that called the trampoline. This is due to a check in
    __save_stack_address():
    
     #ifdef CONFIG_FRAME_POINTER
    	if (!reliable)
    		return;
     #endif
    
    The "reliable" variable is only set if the function address is equal to
    contents of the address before the address the frame pointer register
    points to. If the frame pointer is not set up for the ftrace caller
    then this will fail the reliable test. It will miss the function that
    called the trampoline. Worse yet, if fentry is used (gcc 4.6 and
    beyond), it will also miss the parent, as the fentry is called before
    the stack frame is set up. That means the bp frame pointer points
    to the stack of just before the parent function was called.
    
    Link: http://lkml.kernel.org/r/20141119034829.355440340@goodmis.org
    
    Cc: Ingo Molnar <mingo@...hat.com>
    Cc: "H. Peter Anvin" <hpa@...or.com>
    Cc: x86@...nel.org
    Cc: stable@...r.kernel.org # 3.7+
    Acked-by: Thomas Gleixner <tglx@...utronix.de>
    Signed-off-by: Steven Rostedt <rostedt@...dmis.org>

diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/mcount_64.S
index c73aecf10d34..d23132a9ef1c 100644
--- a/arch/x86/kernel/mcount_64.S
+++ b/arch/x86/kernel/mcount_64.S
@@ -45,14 +45,51 @@ END(function_hook)
 #endif
 .endm
 
+#ifdef CONFIG_FRAME_POINTER
+/*
+ * Stack traces will stop at the ftrace trampoline if the frame pointer
+ * is not set up properly. If fentry is used, we need to save a frame
+ * pointer for the parent as well as the function traced, because the
+ * fentry is called before the stack frame is set up, where as mcount
+ * is called afterward.
+ */
+.macro create_frame parent rip
+#ifdef CC_USING_FENTRY
+	pushq \parent
+	pushq %rbp
+	movq %rsp, %rbp
+#endif
+	pushq \rip
+	pushq %rbp
+	movq %rsp, %rbp
+.endm
+
+.macro restore_frame
+#ifdef CC_USING_FENTRY
+	addq $16, %rsp
+#endif
+	popq %rbp
+	addq $8, %rsp
+.endm
+#else
+.macro create_frame parent rip
+.endm
+.macro restore_frame
+.endm
+#endif /* CONFIG_FRAME_POINTER */
+
 ENTRY(ftrace_caller)
 	ftrace_caller_setup
 	/* regs go into 4th parameter (but make it NULL) */
 	movq $0, %rcx
 
+	create_frame %rsi, %rdi
+
 GLOBAL(ftrace_call)
 	call ftrace_stub
 
+	restore_frame
+
 	MCOUNT_RESTORE_FRAME
 ftrace_return:
 
@@ -96,9 +133,13 @@ ENTRY(ftrace_regs_caller)
 	/* regs go into 4th parameter */
 	leaq (%rsp), %rcx
 
+	create_frame %rsi, %rdi
+
 GLOBAL(ftrace_regs_call)
 	call ftrace_stub
 
+	restore_frame
+
 	/* Copy flags back to SS, to restore them */
 	movq EFLAGS(%rsp), %rax
 	movq %rax, SS(%rsp)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists