| Message ID | BANLkTinvrJ5MAZABDmdmQk6UZHDKJQo1+A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jun 28, 2011 at 04:06:11PM +0400, Dmitry Eremin-Solenikov wrote: > For the reference I'm providing the patch in the attachment. > > Russell, will you agree to merge at least this partial solution, so > that gdb chokes, > but doesn't go to indefinite recursion? If you'd agree, I'll submit it > with proper > headers and sign-off. It looks like it does the right thing, but I think we need someone who knows how the debug info is supposed to work comment. Adding Catalin to the thread to see whether Catalin has any comments; Catalin added the original unwinder annotations.
On Tue, Jun 28, 2011 at 04:06:11PM +0400, Dmitry Eremin-Solenikov wrote: > On 6/28/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > I did some checks. It seems, the problem isn't related to unwinder. At least > it looks like kernel has all necessary unwinding subops. It looks like the > problem is really related to the lack of necessary .cfi information. At least > when i added .cfi_startproc/.cfi_endproc annotations to entry-armv.S code, > gdb stopped decoding backtrace with the "previous frame identical to this frame" > error. Unfortunately I don't have enough knowledge to add .cfi annotations to > irq handlers. I think it may have stopped decoding because of some information it reads from the stack doesn't look sane. But I wonder whether we could get it looping again depending on the register values in the interrupted context. > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > index e8d8856..d77f9d7 100644 > --- a/arch/arm/kernel/entry-armv.S > +++ b/arch/arm/kernel/entry-armv.S > @@ -28,6 +28,7 @@ > #include "entry-header.S" > #include <asm/entry-macro-multi.S> > > + .cfi_sections .debug_frame > /* > * Interrupt handling. Preserves r7, r8, r9 > */ > @@ -113,6 +114,7 @@ ENDPROC(__und_invalid) > > .macro svc_entry, stack_hole=0 > UNWIND(.fnstart ) > + .cfi_startproc > UNWIND(.save {r0 - pc} ) > sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4) Could you add some directives like below in the svc_entry macro (after "sub sp...", not sure if it matters) and check whether gdb behaves better: .cfi_def_cfa_offset S_PC .cfi_offset 14, -4 Thanks.
On Tue, Jun 28, 2011 at 03:20:45PM +0100, Catalin Marinas wrote: > On Tue, Jun 28, 2011 at 04:06:11PM +0400, Dmitry Eremin-Solenikov wrote: > > On 6/28/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > > I did some checks. It seems, the problem isn't related to unwinder. At least > > it looks like kernel has all necessary unwinding subops. It looks like the > > problem is really related to the lack of necessary .cfi information. At least > > when i added .cfi_startproc/.cfi_endproc annotations to entry-armv.S code, > > gdb stopped decoding backtrace with the "previous frame identical to this frame" > > error. Unfortunately I don't have enough knowledge to add .cfi annotations to > > irq handlers. > > I think it may have stopped decoding because of some information it > reads from the stack doesn't look sane. But I wonder whether we could > get it looping again depending on the register values in the interrupted > context. > > > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S > > index e8d8856..d77f9d7 100644 > > --- a/arch/arm/kernel/entry-armv.S > > +++ b/arch/arm/kernel/entry-armv.S > > @@ -28,6 +28,7 @@ > > #include "entry-header.S" > > #include <asm/entry-macro-multi.S> > > > > + .cfi_sections .debug_frame > > /* > > * Interrupt handling. Preserves r7, r8, r9 > > */ > > @@ -113,6 +114,7 @@ ENDPROC(__und_invalid) > > > > .macro svc_entry, stack_hole=0 > > UNWIND(.fnstart ) > > + .cfi_startproc > > UNWIND(.save {r0 - pc} ) > > sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4) > > Could you add some directives like below in the svc_entry macro (after > "sub sp...", not sure if it matters) and check whether gdb behaves > better: > > .cfi_def_cfa_offset S_PC > .cfi_offset 14, -4 Actually since the return address is in S_PC (which maybe gdb assumes it would be the saved LR), this is probably not be correct. After SVC entry, we have he following structure on the stack: ORIG_r0 CPSR <--- assuming this is the Call Frame Address (SP+S_PC+4) PC <--- CFA - 4 LR <--- don't care SP <--- CFA - 12 ... So we tell gdb about this with something like below (untested): .cfi_def_cfa_offset S_PC + 4 .cfi_offset 14, -4 .cfi_offset 13, -12
On Tue, Jun 28, 2011 at 03:30:14PM +0100, Catalin Marinas wrote: > Actually since the return address is in S_PC (which maybe gdb assumes it > would be the saved LR), this is probably not be correct. After SVC > entry, we have he following structure on the stack: > > ORIG_r0 > CPSR > <--- assuming this is the Call Frame Address (SP+S_PC+4) > PC <--- CFA - 4 > LR <--- don't care > SP <--- CFA - 12 > ... If I'm reading this correctly, it's not correct. parent SP --> parent context stack [possible empty word] ORIG_r0 parent context CPSR parent context PC parent context LR parent context SP ... new SP --> R0 That empty word may or may not be present if the parent SP is aligned to a 64-bit boundary.
On Tue, Jun 28, 2011 at 03:37:59PM +0100, Russell King - ARM Linux wrote: > On Tue, Jun 28, 2011 at 03:30:14PM +0100, Catalin Marinas wrote: > > Actually since the return address is in S_PC (which maybe gdb assumes it > > would be the saved LR), this is probably not be correct. After SVC > > entry, we have he following structure on the stack: > > > > ORIG_r0 > > CPSR > > <--- assuming this is the Call Frame Address (SP+S_PC+4) > > PC <--- CFA - 4 > > LR <--- don't care > > SP <--- CFA - 12 > > ... > > If I'm reading this correctly, it's not correct. > > parent SP --> parent context stack > [possible empty word] > ORIG_r0 > parent context CPSR > parent context PC > parent context LR > parent context SP > ... > new SP --> R0 > > That empty word may or may not be present if the parent SP is aligned to > a 64-bit boundary. But it shouldn't matter if we tell gdb that the previous SP (parent) is stored in the current stack at CFA - 12. It calculates CFA by adding S_PC+4 to the current SP, in which case the possible empty word doesn't matter. But please note that I don't have any gdb experience, so we need someone else to confirm.
On 6/28/11, Catalin Marinas <catalin.marinas@arm.com> wrote: > On Tue, Jun 28, 2011 at 03:20:45PM +0100, Catalin Marinas wrote: >> On Tue, Jun 28, 2011 at 04:06:11PM +0400, Dmitry Eremin-Solenikov wrote: >> > On 6/28/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >> > I did some checks. It seems, the problem isn't related to unwinder. At >> > least >> > it looks like kernel has all necessary unwinding subops. It looks like >> > the >> > problem is really related to the lack of necessary .cfi information. At >> > least >> > when i added .cfi_startproc/.cfi_endproc annotations to entry-armv.S >> > code, >> > gdb stopped decoding backtrace with the "previous frame identical to >> > this frame" >> > error. Unfortunately I don't have enough knowledge to add .cfi >> > annotations to >> > irq handlers. >> >> I think it may have stopped decoding because of some information it >> reads from the stack doesn't look sane. But I wonder whether we could >> get it looping again depending on the register values in the interrupted >> context. >> >> > diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S >> > index e8d8856..d77f9d7 100644 >> > --- a/arch/arm/kernel/entry-armv.S >> > +++ b/arch/arm/kernel/entry-armv.S >> > @@ -28,6 +28,7 @@ >> > #include "entry-header.S" >> > #include <asm/entry-macro-multi.S> >> > >> > + .cfi_sections .debug_frame >> > /* >> > * Interrupt handling. Preserves r7, r8, r9 >> > */ >> > @@ -113,6 +114,7 @@ ENDPROC(__und_invalid) >> > >> > .macro svc_entry, stack_hole=0 >> > UNWIND(.fnstart ) >> > + .cfi_startproc >> > UNWIND(.save {r0 - pc} ) >> > sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4) >> >> Could you add some directives like below in the svc_entry macro (after >> "sub sp...", not sure if it matters) and check whether gdb behaves >> better: >> >> .cfi_def_cfa_offset S_PC >> .cfi_offset 14, -4 > > Actually since the return address is in S_PC (which maybe gdb assumes it > would be the saved LR), this is probably not be correct. After SVC > entry, we have he following structure on the stack: > > ORIG_r0 > CPSR > <--- assuming this is the Call Frame Address (SP+S_PC+4) > PC <--- CFA - 4 > LR <--- don't care > SP <--- CFA - 12 > ... > > > So we tell gdb about this with something like below (untested): > > .cfi_def_cfa_offset S_PC + 4 > .cfi_offset 14, -4 > .cfi_offset 13, -12 This brings "unknown CFA rule" gdb exception, but it seems I got your idea.
On 6/28/11, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote: > On 6/28/11, Catalin Marinas <catalin.marinas@arm.com> wrote: >> On Tue, Jun 28, 2011 at 03:20:45PM +0100, Catalin Marinas wrote: >>> On Tue, Jun 28, 2011 at 04:06:11PM +0400, Dmitry Eremin-Solenikov wrote: >>> > On 6/28/11, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: >>> > I did some checks. It seems, the problem isn't related to unwinder. At >>> > least >>> > it looks like kernel has all necessary unwinding subops. It looks like >>> > the >>> > problem is really related to the lack of necessary .cfi information. >>> > At >>> > least >>> > when i added .cfi_startproc/.cfi_endproc annotations to entry-armv.S >>> > code, >>> > gdb stopped decoding backtrace with the "previous frame identical to >>> > this frame" >>> > error. Unfortunately I don't have enough knowledge to add .cfi >>> > annotations to >>> > irq handlers. >>> >>> I think it may have stopped decoding because of some information it >>> reads from the stack doesn't look sane. But I wonder whether we could >>> get it looping again depending on the register values in the interrupted >>> context. >>> >>> > diff --git a/arch/arm/kernel/entry-armv.S >>> > b/arch/arm/kernel/entry-armv.S >>> > index e8d8856..d77f9d7 100644 >>> > --- a/arch/arm/kernel/entry-armv.S >>> > +++ b/arch/arm/kernel/entry-armv.S >>> > @@ -28,6 +28,7 @@ >>> > #include "entry-header.S" >>> > #include <asm/entry-macro-multi.S> >>> > >>> > + .cfi_sections .debug_frame >>> > /* >>> > * Interrupt handling. Preserves r7, r8, r9 >>> > */ >>> > @@ -113,6 +114,7 @@ ENDPROC(__und_invalid) >>> > >>> > .macro svc_entry, stack_hole=0 >>> > UNWIND(.fnstart ) >>> > + .cfi_startproc >>> > UNWIND(.save {r0 - pc} ) >>> > sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4) >>> >>> Could you add some directives like below in the svc_entry macro (after >>> "sub sp...", not sure if it matters) and check whether gdb behaves >>> better: >>> >>> .cfi_def_cfa_offset S_PC >>> .cfi_offset 14, -4 >> >> Actually since the return address is in S_PC (which maybe gdb assumes it >> would be the saved LR), this is probably not be correct. After SVC >> entry, we have he following structure on the stack: >> >> ORIG_r0 >> CPSR >> <--- assuming this is the Call Frame Address (SP+S_PC+4) >> PC <--- CFA - 4 >> LR <--- don't care >> SP <--- CFA - 12 >> ... >> >> >> So we tell gdb about this with something like below (untested): >> >> .cfi_def_cfa_offset S_PC + 4 >> .cfi_offset 14, -4 >> .cfi_offset 13, -12 > > This brings "unknown CFA rule" gdb exception, but it seems I got your idea. No, this seems to work, it was my fault. I got more or less reasonable backtrace now.
On Tue, Jun 28, 2011 at 03:54:11PM +0100, Dmitry Eremin-Solenikov wrote: > On 6/28/11, Dmitry Eremin-Solenikov <dbaryshkov@gmail.com> wrote: > > On 6/28/11, Catalin Marinas <catalin.marinas@arm.com> wrote: > >> Actually since the return address is in S_PC (which maybe gdb assumes it > >> would be the saved LR), this is probably not be correct. After SVC > >> entry, we have he following structure on the stack: > >> > >> ORIG_r0 > >> CPSR > >> <--- assuming this is the Call Frame Address (SP+S_PC+4) > >> PC <--- CFA - 4 > >> LR <--- don't care > >> SP <--- CFA - 12 > >> ... > >> > >> > >> So we tell gdb about this with something like below (untested): > >> > >> .cfi_def_cfa_offset S_PC + 4 > >> .cfi_offset 14, -4 > >> .cfi_offset 13, -12 > > > > This brings "unknown CFA rule" gdb exception, but it seems I got your idea. > > No, this seems to work, it was my fault. I got more or less reasonable > backtrace now. Does gdb manage to get into the parent stack frame? BTW, are you compiling with FRAME_POINTER enabled? In this case you would need to set some offset for the FP register (11). If you don't mind missing the first part in the parent context, maybe something like below: .cfi_def_cfa_offset S_PC .cfi_offset 14, -4 .cfi_offset 13, -8 .cfi_offset 11, -16
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S index e8d8856..d77f9d7 100644 --- a/arch/arm/kernel/entry-armv.S +++ b/arch/arm/kernel/entry-armv.S @@ -28,6 +28,7 @@ #include "entry-header.S" #include <asm/entry-macro-multi.S> + .cfi_sections .debug_frame /* * Interrupt handling. Preserves r7, r8, r9 */ @@ -113,6 +114,7 @@ ENDPROC(__und_invalid) .macro svc_entry, stack_hole=0 UNWIND(.fnstart ) + .cfi_startproc UNWIND(.save {r0 - pc} ) sub sp, sp, #(S_FRAME_SIZE + \stack_hole - 4) #ifdef CONFIG_THUMB2_KERNEL @@ -347,6 +349,7 @@ ENDPROC(__pabt_svc) .macro usr_entry UNWIND(.fnstart ) UNWIND(.cantunwind ) @ don't unwind the user space + .cfi_startproc sub sp, sp, #S_FRAME_SIZE ARM( stmib sp, {r1 - r12} ) THUMB( stmia sp, {r0 - r12} ) @@ -427,6 +430,7 @@ __dabt_usr: mov r2, sp adr lr, BSYM(ret_from_exception) b do_DataAbort + .cfi_endproc UNWIND(.fnend ) ENDPROC(__dabt_usr) @@ -454,6 +458,7 @@ __irq_usr: mov why, #0 b ret_to_user + .cfi_endproc UNWIND(.fnend ) ENDPROC(__irq_usr) @@ -496,6 +501,7 @@ __und_usr: #else b __und_usr_unknown #endif + .cfi_endproc UNWIND(.fnend ) ENDPROC(__und_usr) @@ -691,6 +697,7 @@ __pabt_usr: enable_irq @ Enable interrupts mov r2, sp @ regs bl do_PrefetchAbort @ call abort handler + .cfi_endproc UNWIND(.fnend ) /* fall through */ /* @@ -699,9 +706,11 @@ __pabt_usr: ENTRY(ret_from_exception) UNWIND(.fnstart ) UNWIND(.cantunwind ) + .cfi_startproc get_thread_info tsk mov why, #0 b ret_to_user + .cfi_endproc UNWIND(.fnend ) ENDPROC(__pabt_usr) ENDPROC(ret_from_exception) diff --git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S index 051166c..5ed13ae 100644 --- a/arch/arm/kernel/entry-header.S +++ b/arch/arm/kernel/entry-header.S @@ -86,6 +86,7 @@ #else ldmia sp, {r0 - pc}^ @ load r0 - pc, cpsr #endif + .cfi_endproc .endm .macro restore_user_regs, fast = 0, offset = 0