Atsushi Nemoto wrote:
> On Wed, 02 Aug 2006 12:21:11 +0200, Franck Bui-Huu <vagabon.xyz@xxxxxxxxx> wrote:
>> does something like this on top of this patch make you feel better ?
>>
>> -- >8 --
>>
>> diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
>> index 4ceddfa..8a9db45 100644
>> --- a/arch/mips/kernel/process.c
>> +++ b/arch/mips/kernel/process.c
>> @@ -480,7 +480,13 @@ unsigned long unwind_stack(struct task_s
>> return 0;
>>
>> if (leaf)
>> - pc = regs->regs[31];
>> + /*
>> + * For some extreme cases, get_frame_info() can
>> + * consider wrongly a nested function as a leaf
>> + * one. In that cases avoid to return always the
>> + * same value.
>> + */
>> + pc = pc != regs->regs[31] ? regs->regs[31] : 0;
>
> Yes, it should be safe. But still I'm not sure unwind_stack() should
> take "regs" as its argument...
>
does this updated patch make you really happy ? If so I'll resend the whole
updated patchset.
-- >8 --
Subject: Improve unwind_stack()
This patch allows unwind_stack() to return ra for leaf function.
But it tries to detects cases where get_frame_info() wrongly
consider nested function as a leaf one.
It also pass 'unsinged long *sp' instead of 'unsigned long **sp'
as second parameter. The code looks cleaner.
Signed-off-by: Franck Bui-Huu <vagabon.xyz@xxxxxxxxx>
---
arch/mips/kernel/process.c | 35 ++++++++++++++++++++++-------------
arch/mips/kernel/traps.c | 24 ++++++++++++------------
2 files changed, 34 insertions(+), 25 deletions(-)
diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index 309bfa4..951bf9c 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -448,15 +448,16 @@ #endif
}
#ifdef CONFIG_KALLSYMS
-/* used by show_frametrace() */
-unsigned long unwind_stack(struct task_struct *task,
- unsigned long **sp, unsigned long pc)
+/* used by show_backtrace() */
+unsigned long unwind_stack(struct task_struct *task, unsigned long *sp,
+ unsigned long pc, unsigned long ra)
{
unsigned long stack_page;
struct mips_frame_info info;
char *modname;
char namebuf[KSYM_NAME_LEN + 1];
unsigned long size, ofs;
+ int leaf;
stack_page = (unsigned long)task_stack_page(task);
if (!stack_page)
@@ -469,18 +470,26 @@ unsigned long unwind_stack(struct task_s
info.func = (void *)(pc - ofs);
info.func_size = ofs; /* analyze from start to ofs */
- if (get_frame_info(&info)) {
- /* leaf or unknown */
- *sp += info.frame_size / sizeof(long);
+ leaf = get_frame_info(&info);
+ if (leaf < 0)
return 0;
- }
- if ((unsigned long)*sp < stack_page ||
- (unsigned long)*sp + info.frame_size / sizeof(long) >
- stack_page + THREAD_SIZE - 32)
+
+ if (*sp < stack_page ||
+ *sp + info.frame_size > stack_page + THREAD_SIZE - 32)
return 0;
- pc = (*sp)[info.pc_offset];
- *sp += info.frame_size / sizeof(long);
- return pc;
+ if (leaf)
+ /*
+ * For some extreme cases, get_frame_info() can
+ * consider wrongly a nested function as a leaf
+ * one. In that cases avoid to return always the
+ * same value.
+ */
+ pc = pc != ra ? ra : 0;
+ else
+ pc = ((unsigned long *)(*sp))[info.pc_offset];
+
+ *sp += info.frame_size;
+ return __kernel_text_address(pc) ? pc : 0;
}
#endif
diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 303f008..ab77034 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -74,8 +74,9 @@ void (*board_ejtag_handler_setup)(void);
void (*board_bind_eic_interrupt)(int irq, int regset);
-static void show_raw_backtrace(unsigned long *sp)
+static void show_raw_backtrace(unsigned long reg29)
{
+ unsigned long *sp = (unsigned long *)reg29;
unsigned long addr;
printk("Call Trace:");
@@ -99,30 +100,29 @@ static int __init set_raw_show_trace(cha
}
__setup("raw_show_trace", set_raw_show_trace);
-extern unsigned long unwind_stack(struct task_struct *task,
- unsigned long **sp, unsigned long pc);
+extern unsigned long unwind_stack(struct task_struct *task, unsigned long *sp,
+ unsigned long pc, unsigned long ra);
+
static void show_backtrace(struct task_struct *task, struct pt_regs *regs)
{
- unsigned long *sp = (long *)regs->regs[29];
+ unsigned long sp = regs->regs[29];
+ unsigned long ra = regs->regs[31];
unsigned long pc = regs->cp0_epc;
- int top = 1;
if (raw_show_trace || !__kernel_text_address(pc)) {
show_raw_backtrace(sp);
return;
}
printk("Call Trace:\n");
- while (__kernel_text_address(pc)) {
+ do {
print_ip_sym(pc);
- pc = unwind_stack(task, &sp, pc);
- if (top && pc == 0)
- pc = regs->regs[31]; /* leaf? */
- top = 0;
- }
+ pc = unwind_stack(task, &sp, pc, ra);
+ ra = 0;
+ } while (pc);
printk("\n");
}
#else
-#define show_backtrace(task, r) show_raw_backtrace((long *)(r)->regs[29]);
+#define show_backtrace(task, r) show_raw_backtrace((r)->regs[29]);
#endif
/*
--
1.4.2.rc2
