[PATCHv2 0/2] oprofile, x86: allow backtrace for 32bit apps under 64bit kernel

[PATCHv2 0/2] oprofile, x86: allow backtrace for 32bit apps under 64bit kernel

[Jiri Olsa]

hi,

I'm sending reworked version of the support for backtrace of ia32
applications under 64bit kernels.

updates are based on comments by Robert Richter

v2:
	- using struct stack_frame for 64bit processes
	- using struct stack_frame_ia32 for compat processes
	- added inline function x86_backtrace_32 which is
	  just empty stub for !CONFIG_COMPAT
	- I did not figure out a smart way to share dump code between
	  32 and 64 bits. I'm currently under impression the code is
	  so far easy and light enough to live separately and mixing
	  the code would come with more complexity.


attached patches:
- 1/2 using struct stack_frame for 64bit processes dump
- 2/2 adding backtrace dump for 32bit process in compat mode

please let me know what you think

thanks,
jirka
---
 arch/x86/oprofile/backtrace.c |   70 ++++++++++++++++++++++++++++++++++------
 1 files changed, 59 insertions(+), 11 deletions(-)

[PATCHv2 1/2] oprofile, x86: using struct stack_frame for 64bit processes dump

[Jiri Olsa]

Removing unnecessary struct frame_head and replacing it with
struct stack_frame.

The struct stack_frame is already defined and used in other places
in kernel, so there's no reason to define new structure.

wbr,
jirka


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 arch/x86/oprofile/backtrace.c |   19 +++++++------------
 1 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c
index 3855096..d640a86 100644
--- a/arch/x86/oprofile/backtrace.c
+++ b/arch/x86/oprofile/backtrace.c
@@ -48,35 +48,30 @@ static struct stacktrace_ops backtrace_ops = {
 	.walk_stack	= print_context_stack,
 };
 
-struct frame_head {
-	struct frame_head *bp;
-	unsigned long ret;
-} __attribute__((packed));
-
-static struct frame_head *dump_user_backtrace(struct frame_head *head)
+static struct stack_frame *dump_user_backtrace(struct stack_frame *head)
 {
-	struct frame_head bufhead[2];
+	struct stack_frame bufhead[2];
 
-	/* Also check accessibility of one struct frame_head beyond */
+	/* Also check accessibility of one struct stack_frame beyond */
 	if (!access_ok(VERIFY_READ, head, sizeof(bufhead)))
 		return NULL;
 	if (__copy_from_user_inatomic(bufhead, head, sizeof(bufhead)))
 		return NULL;
 
-	oprofile_add_trace(bufhead[0].ret);
+	oprofile_add_trace(bufhead[0].return_address);
 
 	/* frame pointers should strictly progress back up the stack
 	 * (towards higher addresses) */
-	if (head >= bufhead[0].bp)
+	if (head >= bufhead[0].next_frame)
 		return NULL;
 
-	return bufhead[0].bp;
+	return bufhead[0].next_frame;
 }
 
 void
 x86_backtrace(struct pt_regs * const regs, unsigned int depth)
 {
-	struct frame_head *head = (struct frame_head *)frame_pointer(regs);
+	struct stack_frame *head = (struct stack_frame *)frame_pointer(regs);
 
 	if (!user_mode_vm(regs)) {
 		unsigned long stack = kernel_stack_pointer(regs);
-- 
1.7.1

[PATCHv2 2/2] oprofile, x86: adding backtrace dump for 32bit process in compat mode

[Jiri Olsa]

This patch implements the oprofile backtrace  generation for 32 bit
applications running in the 64bit environment (compat mode).

With this change it's possible to get backtrace for 32bits applications
under the 64bits environment using oprofile's callgraph options.

opcontrol --setup -c ...
opreport -l -cg ...

wbr,
jirka


Signed-off-by: Jiri Olsa <jolsa@redhat.com>
---
 arch/x86/oprofile/backtrace.c |   53 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/arch/x86/oprofile/backtrace.c b/arch/x86/oprofile/backtrace.c
index d640a86..2d49d4e 100644
--- a/arch/x86/oprofile/backtrace.c
+++ b/arch/x86/oprofile/backtrace.c
@@ -14,6 +14,7 @@
 #include <asm/ptrace.h>
 #include <asm/uaccess.h>
 #include <asm/stacktrace.h>
+#include <linux/compat.h>
 
 static void backtrace_warning_symbol(void *data, char *msg,
 				     unsigned long symbol)
@@ -48,6 +49,55 @@ static struct stacktrace_ops backtrace_ops = {
 	.walk_stack	= print_context_stack,
 };
 
+#ifdef CONFIG_COMPAT
+static struct stack_frame_ia32 *
+dump_user_backtrace_32(struct stack_frame_ia32 *head)
+{
+	struct stack_frame_ia32 bufhead[2];
+	struct stack_frame_ia32 *fp;
+
+	/* Also check accessibility of one struct frame_head beyond */
+	if (!access_ok(VERIFY_READ, head, sizeof(bufhead)))
+		return NULL;
+	if (__copy_from_user_inatomic(bufhead, head, sizeof(bufhead)))
+		return NULL;
+
+	fp = (struct stack_frame_ia32 *) compat_ptr(bufhead[0].next_frame);
+
+	oprofile_add_trace(bufhead[0].return_address);
+
+	/* frame pointers should strictly progress back up the stack
+	* (towards higher addresses) */
+	if (head >= fp)
+		return NULL;
+
+	return fp;
+}
+
+static inline int
+x86_backtrace_32(struct pt_regs * const regs, unsigned int depth)
+{
+	struct stack_frame_ia32 *head;
+
+	/* User process is 32-bit */
+	if (!current || !test_thread_flag(TIF_IA32))
+		return 0;
+
+	head = (struct stack_frame_ia32 *) regs->bp;
+	while (depth-- && head)
+		head = dump_user_backtrace_32(head);
+
+	return 1;
+}
+
+#else
+static inline int
+x86_backtrace_32(struct pt_regs * const regs, unsigned int depth)
+{
+	return 0;
+}
+#endif /* CONFIG_COMPAT */
+
 static struct stack_frame *dump_user_backtrace(struct stack_frame *head)
 {
 	struct stack_frame bufhead[2];
@@ -81,6 +131,9 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth)
 		return;
 	}
 
+	if (x86_backtrace_32(regs, depth))
+		return;
+
 	while (depth-- && head)
 		head = dump_user_backtrace(head);
 }
-- 
1.7.1

Re: [PATCHv2 0/2] oprofile, x86: allow backtrace for 32bit apps under 64bit kernel

[Robert Richter]

On 29.09.10 10:46:45, Jiri Olsa wrote:
> I'm sending reworked version of the support for backtrace of ia32
> applications under 64bit kernels.
> 
> updates are based on comments by Robert Richter
> 
> v2:
> 	- using struct stack_frame for 64bit processes
> 	- using struct stack_frame_ia32 for compat processes
> 	- added inline function x86_backtrace_32 which is
> 	  just empty stub for !CONFIG_COMPAT
> 	- I did not figure out a smart way to share dump code between
> 	  32 and 64 bits. I'm currently under impression the code is
> 	  so far easy and light enough to live separately and mixing
> 	  the code would come with more complexity.
> 
> 
> attached patches:
> - 1/2 using struct stack_frame for 64bit processes dump
> - 2/2 adding backtrace dump for 32bit process in compat mode
> 
> please let me know what you think
> 
> thanks,
> jirka
> ---
>  arch/x86/oprofile/backtrace.c |   70 ++++++++++++++++++++++++++++++++++------
>  1 files changed, 59 insertions(+), 11 deletions(-)

Applied to oprofile/core. Thanks Jirka.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center