r
And add proper CFI annotation to it which was previously
impossible. This prevents "stuck" messages by the dwarf2 unwinder
when reaching the top of a kernel stack.
Cc: [email protected]
Signed-off-by: Andi Kleen <[email protected]>
---
arch/i386/kernel/entry.S | 15 +++++++++++++++
arch/i386/kernel/process.c | 9 ---------
2 files changed, 15 insertions(+), 9 deletions(-)
Index: linux/arch/i386/kernel/entry.S
===================================================================
--- linux.orig/arch/i386/kernel/entry.S
+++ linux/arch/i386/kernel/entry.S
@@ -946,6 +946,21 @@ ENTRY(arch_unwind_init_running)
ENDPROC(arch_unwind_init_running)
#endif
+ENTRY(kernel_thread_helper)
+ CFI_STARTPROC
+ movl %edx,%eax
+ CFI_REGISTER edx,eax
+ push %edx
+ CFI_ADJUST_CFA_OFFSET 4
+ CFI_REL_OFFSET edx,0
+ call *%ebx
+ push %eax
+ CFI_ADJUST_CFA_OFFSET 4
+ CFI_REL_OFFSET eax,0
+ call do_exit
+ CFI_ENDPROC
+ENDPROC(kernel_thread_helper)
+
.section .rodata,"a"
#include "syscall_table.S"
Index: linux/arch/i386/kernel/process.c
===================================================================
--- linux.orig/arch/i386/kernel/process.c
+++ linux/arch/i386/kernel/process.c
@@ -321,15 +321,6 @@ void show_regs(struct pt_regs * regs)
* the "args".
*/
extern void kernel_thread_helper(void);
-__asm__(".section .text\n"
- ".align 4\n"
- "kernel_thread_helper:\n\t"
- "movl %edx,%eax\n\t"
- "pushl %edx\n\t"
- "call *%ebx\n\t"
- "pushl %eax\n\t"
- "call do_exit\n"
- ".previous");
/*
* Create a kernel thread
>And add proper CFI annotation to it which was previously
>impossible. This prevents "stuck" messages by the dwarf2 unwinder
>when reaching the top of a kernel stack.
>+ENTRY(kernel_thread_helper)
>+ CFI_STARTPROC
>+ movl %edx,%eax
>+ CFI_REGISTER edx,eax
This is pointless, as %eax will be clobbered by the callee of the
subsequent call.
>+ push %edx
>+ CFI_ADJUST_CFA_OFFSET 4
>+ CFI_REL_OFFSET edx,0
This likewise is pointless, as the argument is owned by the callee.
>+ call *%ebx
>+ push %eax
>+ CFI_ADJUST_CFA_OFFSET 4
>+ CFI_REL_OFFSET eax,0
And this too - the value now in %eax has no relation with the
value known by the caller of this routine (which doesn't expect
any return from here anyway).
>+ call do_exit
>+ CFI_ENDPROC
>+ENDPROC(kernel_thread_helper)
Jan
> And this too - the value now in %eax has no relation with the
> value known by the caller of this routine (which doesn't expect
> any return from here anyway).
Ok, but somehow it needs to be annotiated so that the unwinder stops
and doesn't fall back. Can you please send a replacement patch that
does this correctly?
Thanks,
-Andi
>> And this too - the value now in %eax has no relation with the
>> value known by the caller of this routine (which doesn't expect
>> any return from here anyway).
>
>Ok, but somehow it needs to be annotiated so that the unwinder stops
>and doesn't fall back. Can you please send a replacement patch that
>does this correctly?
I would do it this way (untested):
ENTRY(kernel_thread_helper)
CFI_STARTPROC
movl %edx,%eax
pushl %edx
CFI_ADJUST_CFA_OFFSET 4
call *%ebx
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
call do_exit
CFI_ENDPROC
ENDPROC(kernel_thread_helper)
(i.e. tracking the stack pointer movement, but not the register values
other than the return address)
Jan
> ENTRY(kernel_thread_helper)
> CFI_STARTPROC
> movl %edx,%eax
> pushl %edx
> CFI_ADJUST_CFA_OFFSET 4
> call *%ebx
> pushl %eax
> CFI_ADJUST_CFA_OFFSET 4
> call do_exit
> CFI_ENDPROC
> ENDPROC(kernel_thread_helper)
>
> (i.e. tracking the stack pointer movement, but not the register values
> other than the return address)
Done thanks.
-Andi
>> And this too - the value now in %eax has no relation with the
>> value known by the caller of this routine (which doesn't expect
>> any return from here anyway).
>
>Ok, but somehow it needs to be annotiated so that the unwinder stops
>and doesn't fall back. Can you please send a replacement patch that
>does this correctly?
Actually, with the previous attempt we still didn't achieve the
original
goal of terminating the frame chain at kernel_thread_helper. Thus
another try (the seemingly odd extra push can be avoided once we
start using CFI expressions, which the unwinder currently doesn't
support):
ENTRY(kernel_thread_helper)
CFI_STARTPROC
pushl $0 # fake return address
movl %edx,%eax
pushl %edx
CFI_ADJUST_CFA_OFFSET 4
call *%ebx
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
call do_exit
CFI_ENDPROC
ENDPROC(kernel_thread_helper)
Jan
On Monday 14 August 2006 14:36, Jan Beulich wrote:
> >> And this too - the value now in %eax has no relation with the
> >> value known by the caller of this routine (which doesn't expect
> >> any return from here anyway).
> >
> >Ok, but somehow it needs to be annotiated so that the unwinder stops
> >and doesn't fall back. Can you please send a replacement patch that
> >does this correctly?
>
> Actually, with the previous attempt we still didn't achieve the
> original
> goal of terminating the frame chain at kernel_thread_helper. Thus
> another try (the seemingly odd extra push can be avoided once we
> start using CFI expressions, which the unwinder currently doesn't
> support):
I added the extra push thanks (hope there weren't more changes)
-Andi
Just like done in the x86-64 patch that I just sent, I'd recommend
moving
the push added yesterday outside of the CFI-covered region (so that
in the unlikely event of being caught at the push there won't be an
ill
assumption that a [fake] return address is already on the stack, or
that
there is a return address at all):
ENTRY(kernel_thread_helper)
pushl $0 # fake return address
CFI_STARTPROC
movl %edx,%eax
pushl %edx
CFI_ADJUST_CFA_OFFSET 4
call *%ebx
pushl %eax
CFI_ADJUST_CFA_OFFSET 4
call do_exit
CFI_ENDPROC
ENDPROC(kernel_thread_helper)
Jan
On Tue, 15 Aug 2006 12:36:44 +0200
"Jan Beulich" <[email protected]> wrote:
> Just like done in the x86-64 patch that I just sent, I'd recommend
> moving
> the push added yesterday outside of the CFI-covered region (so that
> in the unlikely event of being caught at the push there won't be an
> ill
> assumption that a [fake] return address is already on the stack, or
> that
> there is a return address at all):
Done.
-Andi