Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Chuck Ebbert]

In-Reply-To: <200608060805.06821.ak@suse.de>

On Sun, 6 Aug 2006 08:05:06 +0200, Andi Kleen wrote:

> Hmm, actually I applied it but then I had doubts it actually 
> works -- I think you don't need _stext but the code before
> the first call in head. Since head.S doesn't do a call
> that's probably start_kernel

But head.S does do a call (on i386 but not x86_64 AFAICT):

| #ifdef CONFIG_SMP
|        movb ready, %cl
|        movb $1, ready
|        cmpb $0,%cl
|        je 1f                   # the first CPU calls start_kernel
|                                # all other CPUs call initialize_secondary
|        call initialize_secondary
|        jmp L6
| 1:
| #endif /* CONFIG_SMP */
|        call start_kernel
| L6:
|        jmp L6                  # main should never return here, but
|                                # just in case, we know what happens.

And the backtraces I saw ended up at L6:

| DWARF2 unwinder stuck at 0xc0100210

System.map on i386 SMP says:

| c0100210 t L6

-- 
Chuck

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Andi Kleen]

> And the backtraces I saw ended up at L6:
> 
> | DWARF2 unwinder stuck at 0xc0100210
> 
> System.map on i386 SMP says:
> 
> | c0100210 t L6


Yes that's the problem. If you check for <= stext/_stext then the unwinder
won't catch the L6 (which is above it) and report a "stuck" again

-Andi

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Chuck Ebbert]

In-Reply-To: <200608061604.40452.ak@suse.de>

On Sun, 6 Aug 2006 16:04:40 +0200, Andi Kleen wrote:

> > And the backtraces I saw ended up at L6:
> > 
> > | DWARF2 unwinder stuck at 0xc0100210
> > 
> > System.map on i386 SMP says:
> > 
> > | c0100210 t L6
> 
> 
> Yes that's the problem. If you check for <= stext/_stext then the unwinder
> won't catch the L6 (which is above it) and report a "stuck" again

Maybe I'm being dense here, but:

c0100210 t L6
c0100212 t check_x87
c010023a t setup_idt
c0100257 t rp_sidt
c0100264 t ignore_int
c0100298 T stext
c0100298 T _stext

It looks like L6 is before _stext to me.

-- 
Chuck

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Jan Beulich]

>> Yes that's the problem. If you check for <= stext/_stext then the unwinder
>> won't catch the L6 (which is above it) and report a "stuck" again
>
>Maybe I'm being dense here, but:
>
>c0100210 t L6
>c0100212 t check_x87
>c010023a t setup_idt
>c0100257 t rp_sidt
>c0100264 t ignore_int
>c0100298 T stext
>c0100298 T _stext
>
>It looks like L6 is before _stext to me.

So it would seem to me. Nevertheless, in my opinion the proper fix is to annotate the call site
(in head.S) to specify a zero EIP as return address (which denotes the bottom of a frame).

Jan

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Andi Kleen]

On Monday 07 August 2006 10:00, Jan Beulich wrote:
> >> Yes that's the problem. If you check for <= stext/_stext then the unwinder
> >> won't catch the L6 (which is above it) and report a "stuck" again
> >
> >Maybe I'm being dense here, but:
> >
> >c0100210 t L6
> >c0100212 t check_x87
> >c010023a t setup_idt
> >c0100257 t rp_sidt
> >c0100264 t ignore_int
> >c0100298 T stext
> >c0100298 T _stext
> >
> >It looks like L6 is before _stext to me.
> 
> So it would seem to me. Nevertheless, in my opinion the proper fix is to annotate the call site
> (in head.S) to specify a zero EIP as return address (which denotes the bottom of a frame).

Can you please send a patch to do that?

That seems to be missing in some other places too, e.g. i386 sysenter path, x86-64 kernel_thread,
more?

-Andi

 

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

>> So it would seem to me. Nevertheless, in my opinion the proper fix is
to annotate the call site
>> (in head.S) to specify a zero EIP as return address (which denotes
the bottom of a frame).
>
>Can you please send a patch to do that?
>
>That seems to be missing in some other places too, e.g. i386 sysenter
path, x86-64 kernel_thread,
>more?

Attaching both an i386 version (boot/idle thread only, you did
kernel_thread already)
and an x86-64 one (boot/idle and kernel_thread). The i386 sysenter path
is a different
thing, there we have an actual caller (though outside of the kernel),
which I'd like to
continue to reflect/catch through arch_unw_user_mode().

Jan

[-- Attachment #2: linux-2.6.18-rc4-unwind-x86_64-term.patch --]
[-- Type: text/plain, Size: 1143 bytes --]

Add kernel thread stack frame termination for properly stopping stack
unwinds.

One open question: Should these added pushes perhaps be made
conditional upon CONFIG_STACK_UNWIND or CONFIG_UNWIND_INFO?

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- linux-2.6.18-rc4/arch/x86_64/kernel/entry.S	2006-08-15 11:29:41.000000000 +0200
+++ 2.6.18-rc4-unwind-x86_64-term/arch/x86_64/kernel/entry.S	2006-08-15 10:15:40.000000000 +0200
@@ -973,6 +973,8 @@ ENTRY(kernel_thread)
 ENDPROC(kernel_thread)
 	
 child_rip:
+	pushq $0		# fake return address
+	CFI_STARTPROC
 	/*
 	 * Here we are in the child and the registers are set as they were
 	 * at kernel_thread() invocation in the parent.
@@ -983,6 +985,7 @@ child_rip:
 	# exit
 	xorl %edi, %edi
 	call do_exit
+	CFI_ENDPROC
 ENDPROC(child_rip)
 
 /*
--- linux-2.6.18-rc4/arch/x86_64/kernel/head.S	2006-06-18 03:49:35.000000000 +0200
+++ 2.6.18-rc4-unwind-x86_64-term/arch/x86_64/kernel/head.S	2006-08-15 11:05:13.000000000 +0200
@@ -191,6 +191,7 @@ startup_64:
 	 * jump
 	 */
 	movq	initial_code(%rip),%rax
+	pushq	$0		# fake return address
 	jmp	*%rax
 
 	/* SMP bootup changes these two */

[-- Attachment #3: linux-2.6.18-rc4-unwind-i386-term.patch --]
[-- Type: text/plain, Size: 1156 bytes --]

Add boot/idle kernel thread stack frame termination for properly
stopping stack unwinds.

One open question: Should this added push perhaps be made conditional
upon CONFIG_STACK_UNWIND or CONFIG_UNWIND_INFO?

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- linux-2.6.18-rc4/arch/i386/kernel/head.S	2006-08-15 11:32:08.000000000 +0200
+++ 2.6.18-rc4-unwind-i386-term/arch/i386/kernel/head.S	2006-08-15 11:06:03.000000000 +0200
@@ -317,20 +317,14 @@ is386:	movl $2,%ecx		# set MP
 	movl %eax,%gs
 	lldt %ax
 	cld			# gcc2 wants the direction flag cleared at all times
+	pushl %eax		# fake return address
 #ifdef CONFIG_SMP
 	movb ready, %cl
 	movb $1, ready
-	cmpb $0,%cl
-	je 1f			# the first CPU calls start_kernel
-				# all other CPUs call initialize_secondary
-	call initialize_secondary
-	jmp L6
-1:
+	cmpb $0,%cl		# the first CPU calls start_kernel
+	jne initialize_secondary # all other CPUs call initialize_secondary
 #endif /* CONFIG_SMP */
-	call start_kernel
-L6:
-	jmp L6			# main should never return here, but
-				# just in case, we know what happens.
+	jmp start_kernel
 
 /*
  * We depend on ET to be correct. This checks for 287/387.

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Andi Kleen]

On Tue, 15 Aug 2006 12:33:59 +0200
"Jan Beulich" <jbeulich@novell.com> wrote:

> >> So it would seem to me. Nevertheless, in my opinion the proper fix is
> to annotate the call site
> >> (in head.S) to specify a zero EIP as return address (which denotes
> the bottom of a frame).
> >
> >Can you please send a patch to do that?
> >
> >That seems to be missing in some other places too, e.g. i386 sysenter
> path, x86-64 kernel_thread,
> >more?
> 
> Attaching both an i386 version (boot/idle thread only, you did
> kernel_thread already)
> and an x86-64 one (boot/idle and kernel_thread). The i386 sysenter path
> is a different
> thing,

Ok added thanks.

Re One open question: Should this added push perhaps be made conditional
upon CONFIG_STACK_UNWIND or CONFIG_UNWIND_INFO?

I don't think that's needed because they are all slow paths.

 there we have an actual caller (though outside of the kernel),
> which I'd like to
> continue to reflect/catch through arch_unw_user_mode().

Ok, but does it work now? I thought it didn't.
I've also seen a stuck on the x86-64 sysenter path on x86-64.

-Andi 

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Jan Beulich]

>> which I'd like to
>> continue to reflect/catch through arch_unw_user_mode().
>
>Ok, but does it work now? I thought it didn't.
>I've also seen a stuck on the x86-64 sysenter path on x86-64.

That's the next thing for me to look into.

Jan

Re: [patch] i386: fix one case of stuck dwarf2 unwinder

[Chuck Ebbert]

In-Reply-To: <200608060430.06935.ak@suse.de>

On Sun, 6 Aug 2006 04:30:06 +0200, Andi Kleen wrote:
> 
> > +extern void stext(void); /* real start of kernel text */
> 
> Can't you use _stext[] from asm/sections.h?

OK.


[patch] i386: fix one case of stuck dwarf2 unwinder

When the dwarf2 unwinder does its thing, sometimes it ends up in
kernel startup code in head.S.  Changing arch_unw_user_mode() to
treat that case as if it were user mode is the easy fix.

Signed-off-by: Chuck Ebbert <76306.1226@compuserve.com>

--- 2.6.18-rc3-32.orig/include/asm-i386/unwind.h
+++ 2.6.18-rc3-32/include/asm-i386/unwind.h
@@ -13,6 +13,7 @@
 #include <asm/fixmap.h>
 #include <asm/ptrace.h>
 #include <asm/uaccess.h>
+#include <asm/sections.h>
 
 struct unwind_frame_info
 {
@@ -71,13 +72,14 @@ extern asmlinkage int arch_unwind_init_r
                                                                           void *arg),
                                                void *arg);
 
+/* check if unwind has reached either user mode or kernel startup code */
 static inline int arch_unw_user_mode(const struct unwind_frame_info *info)
 {
 #if 0 /* This can only work when selector register and EFLAGS saves/restores
          are properly annotated (and tracked in UNW_REGISTER_INFO). */
 	return user_mode_vm(&info->regs);
 #else
-	return info->regs.eip < PAGE_OFFSET
+	return info->regs.eip < (unsigned long)_stext
 	       || (info->regs.eip >= __fix_to_virt(FIX_VDSO)
 	            && info->regs.eip < __fix_to_virt(FIX_VDSO) + PAGE_SIZE)
 	       || info->regs.esp < PAGE_OFFSET;
-- 
Chuck

Re: [patch] i386: fix one case of stuck dwarf2 unwinder II

[Andi Kleen]

On Sunday 06 August 2006 07:00, Chuck Ebbert wrote:
> In-Reply-To: <200608060430.06935.ak@suse.de>
> 
> On Sun, 6 Aug 2006 04:30:06 +0200, Andi Kleen wrote:
> > 
> > > +extern void stext(void); /* real start of kernel text */
> > 
> > Can't you use _stext[] from asm/sections.h?
> 
> OK.

Hmm, actually I applied it but then I had doubts it actually 
works -- I think you don't need _stext but the code before
the first call in head. Since head.S doesn't do a call
that's probably start_kernel

Can you please resubmit a patch that does this properly?

-Andi