public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	<x86@kernel.org>, <stable@vger.kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>
Subject: [for-next][PATCH 02/18] ftrace/x86: Add frames pointers to trampoline as necessary
Date: Wed, 19 Nov 2014 18:26:34 -0500	[thread overview]
Message-ID: <20141119232709.587971857@goodmis.org> (raw)
In-Reply-To: 20141119232632.737569526@goodmis.org

[-- Attachment #1: 0002-ftrace-x86-Add-frames-pointers-to-trampoline-as-nece.patch --]
[-- Type: text/plain, Size: 2881 bytes --]

From: "Steven Rostedt (Red Hat)" <rostedt@goodmis.org>

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@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: stable@vger.kernel.org # 3.7+
Acked-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 arch/x86/kernel/mcount_64.S | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/x86/kernel/mcount_64.S b/arch/x86/kernel/mcount_64.S
index 42f0cdd20baf..35a793fa4bba 100644
--- a/arch/x86/kernel/mcount_64.S
+++ b/arch/x86/kernel/mcount_64.S
@@ -47,14 +47,51 @@ GLOBAL(\trace_label)
 #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 ftrace_caller_op_ptr
 	/* 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
 
 	/*
@@ -105,9 +142,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)
-- 
2.1.1



  parent reply	other threads:[~2014-11-19 23:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-19 23:26 [for-next][PATCH 00/18] tracing: Fixes and trace-seq updates Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 01/18] tracing: Fix race of function probes counting Steven Rostedt
2014-11-19 23:26 ` Steven Rostedt [this message]
2014-11-19 23:26 ` [for-next][PATCH 03/18] ftrace/x86/extable: Add is_ftrace_trampoline() function Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 04/18] x86/kvm/tracing: Use helper function trace_seq_buffer_ptr() Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 05/18] RAS/tracing: Use trace_seq_buffer_ptr() helper instead of open coded Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 06/18] tracing: Fix trace_seq_bitmask() to start at current position Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 07/18] tracing: Add trace_seq_has_overflowed() and trace_handle_return() Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 08/18] blktrace/tracing: Use trace_seq_has_overflowed() helper function Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 09/18] ring-buffer: Remove check of trace_seq_{puts,printf}() return values Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 10/18] tracing: Have branch tracer use trace_handle_return() helper function Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 11/18] tracing: Have function_graph use trace_seq_has_overflowed() Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 12/18] kprobes/tracing: Use trace_seq_has_overflowed() for overflow checks Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 13/18] tracing: Do not check return values of trace_seq_p*() for mmio tracer Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 14/18] tracing/probes: Do not use return value of trace_seq_printf() Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 15/18] tracing/uprobes: Do not use return values " Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 16/18] tracing: Do not use return values of trace_seq_printf() in syscall tracing Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 17/18] tracing: Remove return values of most trace_seq_*() functions Steven Rostedt
2014-11-19 23:26 ` [for-next][PATCH 18/18] tracing: Fix return value of ftrace_raw_output_prep() Steven Rostedt

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=20141119232709.587971857@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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