Re: [RFC] Smarter stack traces using the frame pointer

[Stephen Rothwell]

On 04 Nov 2003 14:13:49 -0800 Adam Litke <agl@xxxxxxxxxx> wrote:
>
> I was working on this for the mjb tree but I thought I'd throw it out
> here in case anyone else finds it interesting.  This simple change to
> the stack trace code uses the frame pointer to do the trace instead of
> assuming any kernel address on the stack is a return address.  It makes
> for much more readable stack traces.

I was asked by a colleague to do the same thing and came up with this
alternative version which, I think, gets it mostly right - I am not
sure about the stack trace from an OOPS.

Don't bother to tell me this is a bit of a hack - I know :-)

Patch relative to 2.6.0-test9.
-- 
Cheers,
Stephen Rothwell                    sfr@xxxxxxxxxxxxxxxx
http://www.canb.auug.org.au/~sfr/

diff -ruN 2.6.0-test9/arch/i386/kernel/process.c 2.6.0-test9-sfr.1/arch/i386/kernel/process.c
--- 2.6.0-test9/arch/i386/kernel/process.c	2003-10-09 10:22:41.000000000 +1000
+++ 2.6.0-test9-sfr.1/arch/i386/kernel/process.c	2003-10-28 17:00:40.000000000 +1100
@@ -243,7 +243,7 @@
 		".previous			\n"
 		: "=r" (cr4): "0" (0));
 	printk("CR0: %08lx CR2: %08lx CR3: %08lx CR4: %08lx\n", cr0, cr2, cr3, cr4);
-	show_trace(NULL, &regs->esp);
+	show_trace(&regs->esp, (unsigned long *)regs->ebp);
 }
 
 /*
diff -ruN 2.6.0-test9/arch/i386/kernel/traps.c 2.6.0-test9-sfr.1/arch/i386/kernel/traps.c
--- 2.6.0-test9/arch/i386/kernel/traps.c	2003-10-20 11:35:54.000000000 +1000
+++ 2.6.0-test9-sfr.1/arch/i386/kernel/traps.c	2003-10-29 15:45:50.000000000 +1100
@@ -91,9 +91,11 @@
 asmlinkage void spurious_interrupt_bug(void);
 asmlinkage void machine_check(void);
 
+#define DECLARE_EBP	register unsigned long *ebp asm("ebp")
+
 static int kstack_depth_to_print = 24;
 
-void show_trace(struct task_struct *task, unsigned long * stack)
+void show_trace(unsigned long *stack, unsigned long *frame)
 {
 	unsigned long addr;
 
@@ -106,10 +108,24 @@
 #endif
 	while (!kstack_end(stack)) {
 		addr = *stack++;
-		if (kernel_text_address(addr)) {
+		if (
+#ifdef CONFIG_FRAME_POINTER
+			/*
+			 * The plus 2 is because the return address
+			 * is immediately above where the frame pointer 
+			 * points and we incremented the stack pointer
+			 * above ...
+			 */
+			(!frame || (stack == (frame + 2))) &&
+#endif
+			kernel_text_address(addr)) {
 			printk(" [<%08lx>] ", addr);
 			print_symbol("%s\n", addr);
 		}
+#ifdef CONFIG_FRAME_POINTER
+		if (frame && (stack == (frame + 2)))
+			frame = (unsigned long *)*frame;
+#endif
 	}
 	printk("\n");
 }
@@ -121,19 +137,45 @@
 	/* User space on another CPU? */
 	if ((esp ^ (unsigned long)tsk->thread_info) & (PAGE_MASK<<1))
 		return;
-	show_trace(tsk, (unsigned long *)esp);
+	/*
+	 * switch_to pushes the frame pointer just before saving
+	 * the stack pointer.
+	 */
+	show_trace((unsigned long *)esp, (unsigned long *)*(unsigned long *)esp);
 }
 
 void show_stack(struct task_struct *task, unsigned long *esp)
 {
 	unsigned long *stack;
 	int i;
+	DECLARE_EBP;
+	unsigned long *frame;
 
 	if (esp == NULL) {
-		if (task)
+		if (task && (task != current)) {
 			esp = (unsigned long*)task->thread.esp;
-		else
+			/*
+			 * switch_to pushes the frame pointer just before
+			 * saving the stack pointer.
+			 */
+			frame = (unsigned long *)*esp;
+		} else {
 			esp = (unsigned long *)&esp;
+			frame = ebp;
+#ifdef CONFIG_FRAME_POINTER
+			/* we want the enclosing frame ... */
+			frame = (unsigned long *)*ebp;
+#endif
+		}
+	} else {
+		/*
+		 * The only place that calls show_stack with task == NULL
+		 * and esp != NULL is show_registers below, where esp
+		 * points at the esp field of a pt_regs structure on the
+		 * stack.  So this gets us the saved ebp value.
+		 */
+		frame = ((struct pt_regs *)((unsigned char *)esp -
+				offsetof(struct pt_regs, esp)))->ebp;
 	}
 
 	stack = esp;
@@ -145,7 +187,7 @@
 		printk("%08lx ", *stack++);
 	}
 	printk("\n");
-	show_trace(task, esp);
+	show_trace(esp, frame);
 }
 
 /*
@@ -154,8 +196,9 @@
 void dump_stack(void)
 {
 	unsigned long stack;
+	DECLARE_EBP;
 
-	show_trace(current, &stack);
+	show_trace(&stack, ebp);
 }
 
 EXPORT_SYMBOL(dump_stack);
diff -ruN 2.6.0-test9/include/asm-i386/processor.h 2.6.0-test9-sfr.1/include/asm-i386/processor.h
--- 2.6.0-test9/include/asm-i386/processor.h	2003-10-20 11:36:07.000000000 +1000
+++ 2.6.0-test9-sfr.1/include/asm-i386/processor.h	2003-10-28 16:57:42.000000000 +1100
@@ -492,7 +492,7 @@
 extern int kernel_thread(int (*fn)(void *), void * arg, unsigned long flags);
 
 extern unsigned long thread_saved_pc(struct task_struct *tsk);
-void show_trace(struct task_struct *task, unsigned long *stack);
+void show_trace(unsigned long *stack, unsigned long *frame);
 
 unsigned long get_wchan(struct task_struct *p);
 #define KSTK_EIP(tsk)	(((unsigned long *)(4096+(unsigned long)(tsk)->thread_info))[1019])
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/