Hi Andi, Andrew, Amit,
The attached patch fixes the .cfi directives for the common_interrupt
path for opteron. It seems that the existing .cfi directives in this
path only work by accident.
I spent yesterday decoding a stack by hand and looking at the
dwarf unwind data using "readelf -wF". I found that the existing
.cfi directives describe registers sharing the same stack addresses
(not a good thing).
This patch makes the unwind data make sense and makes gdb/kgdb more
likely to produce a useful stack traces.
Jim Houston - Concurrent Computer Corp.
--
diff -urN -X dontdiff linux-2.6.4-rc2-mm1.orig/arch/x86_64/kernel/entry.S linux-2.6.4-rc2-mm1/arch/x86_64/kernel/entry.S
--- linux-2.6.4-rc2-mm1.orig/arch/x86_64/kernel/entry.S 2004-03-10 17:06:03.000000000 -0500
+++ linux-2.6.4-rc2-mm1/arch/x86_64/kernel/entry.S 2004-03-17 09:19:30.000000000 -0500
@@ -402,9 +402,9 @@
/* 0(%rsp): interrupt number */
.macro interrupt func
CFI_STARTPROC simple
- CFI_DEF_CFA rsp,(SS-ORIG_RAX)
- CFI_OFFSET rsp,(RSP-SS)
- CFI_OFFSET rip,(RIP-SS)
+ CFI_DEF_CFA rsp,(SS-RDI)
+ CFI_REL_OFFSET rsp,(RSP-ORIG_RAX)
+ CFI_REL_OFFSET rip,(RIP-ORIG_RAX)
cld
#ifdef CONFIG_DEBUG_INFO
SAVE_ALL
diff -urN -X dontdiff linux-2.6.4-rc2-mm1.orig/include/asm-x86_64/calling.h linux-2.6.4-rc2-mm1/include/asm-x86_64/calling.h
--- linux-2.6.4-rc2-mm1.orig/include/asm-x86_64/calling.h 2004-03-10 17:05:42.000000000 -0500
+++ linux-2.6.4-rc2-mm1/include/asm-x86_64/calling.h 2004-03-17 09:19:30.000000000 -0500
@@ -35,26 +35,26 @@
subq $9*8+\addskip,%rsp
CFI_ADJUST_CFA_OFFSET 9*8+\addskip
movq %rdi,8*8(%rsp)
- CFI_OFFSET rdi,8*8-(9*8+\addskip)
+ CFI_REL_OFFSET rdi,8*8
movq %rsi,7*8(%rsp)
- CFI_OFFSET rsi,7*8-(9*8+\addskip)
+ CFI_REL_OFFSET rsi,7*8
movq %rdx,6*8(%rsp)
- CFI_OFFSET rdx,6*8-(9*8+\addskip)
+ CFI_REL_OFFSET rdx,6*8
.if \norcx
.else
movq %rcx,5*8(%rsp)
- CFI_OFFSET rcx,5*8-(9*8+\addskip)
+ CFI_REL_OFFSET rcx,5*8
.endif
movq %rax,4*8(%rsp)
- CFI_OFFSET rax,4*8-(9*8+\addskip)
+ CFI_REL_OFFSET rax,4*8
movq %r8,3*8(%rsp)
- CFI_OFFSET r8,3*8-(9*8+\addskip)
+ CFI_REL_OFFSET r8,3*8
movq %r9,2*8(%rsp)
- CFI_OFFSET r9,2*8-(9*8+\addskip)
+ CFI_REL_OFFSET r9,2*8
movq %r10,1*8(%rsp)
- CFI_OFFSET r10,1*8-(9*8+\addskip)
+ CFI_REL_OFFSET r10,1*8
movq %r11,(%rsp)
- CFI_OFFSET r11,-(9*8+\addskip)
+ CFI_REL_OFFSET r11,
.endm
#define ARG_SKIP 9*8
@@ -100,17 +100,17 @@
subq $REST_SKIP,%rsp
CFI_ADJUST_CFA_OFFSET REST_SKIP
movq %rbx,5*8(%rsp)
- CFI_OFFSET rbx,5*8-(REST_SKIP)
+ CFI_REL_OFFSET rbx,5*8
movq %rbp,4*8(%rsp)
- CFI_OFFSET rbp,4*8-(REST_SKIP)
+ CFI_REL_OFFSET rbp,4*8
movq %r12,3*8(%rsp)
- CFI_OFFSET r12,3*8-(REST_SKIP)
+ CFI_REL_OFFSET r12,3*8
movq %r13,2*8(%rsp)
- CFI_OFFSET r13,2*8-(REST_SKIP)
+ CFI_REL_OFFSET r13,2*8
movq %r14,1*8(%rsp)
- CFI_OFFSET r14,1*8-(REST_SKIP)
+ CFI_REL_OFFSET r14,1*8
movq %r15,(%rsp)
- CFI_OFFSET r15,0*8-(REST_SKIP)
+ CFI_REL_OFFSET r15,0*8
.endm
.macro RESTORE_REST
On 17 Mar 2004 16:37:15 -0500
Jim Houston <[email protected]> wrote:
>
> Hi Andi, Andrew, Amit,
>
> The attached patch fixes the .cfi directives for the common_interrupt
> path for opteron. It seems that the existing .cfi directives in this
> path only work by accident.
>
> I spent yesterday decoding a stack by hand and looking at the
> dwarf unwind data using "readelf -wF". I found that the existing
> .cfi directives describe registers sharing the same stack addresses
> (not a good thing).
>
> This patch makes the unwind data make sense and makes gdb/kgdb more
> likely to produce a useful stack traces.
Thanks. I applied it. The calling.h part gave rejects, but I applied it
by hand. It would be nice if you could check in the final kernel if I didn't
make a mistake.
-Andi
On Wed, 2004-03-17 at 18:53, Andi Kleen wrote:
On 17 Mar 2004 16:37:15 -0500
> Jim Houston <[email protected]> wrote:
> > The attached patch fixes the .cfi directives for the common_interrupt
> > path for opteron. It seems that the existing .cfi directives in this
> > path only work by accident.
> >
> > This patch makes the unwind data make sense and makes gdb/kgdb more
> > likely to produce a useful stack traces.
>
> Thanks. I applied it. The calling.h part gave rejects, but I applied it
> by hand. It would be nice if you could check in the final kernel if I didn't
> make a mistake.
>
Hi Andi, Andrew, Amit,
The attached patch is updated to work with linux-2.6.5-rc1.
Jim Houston - Concurrent Computer Corp.
--
diff -urN linux-2.6.5-rc1-mm2.orig/arch/x86_64/kernel/entry.S linux-2.6.5-rc1-mm2/arch/x86_64/kernel/entry.S
--- linux-2.6.5-rc1-mm2.orig/arch/x86_64/kernel/entry.S 2004-03-18 09:37:13.830453136 -0500
+++ linux-2.6.5-rc1-mm2/arch/x86_64/kernel/entry.S 2004-03-18 09:37:29.237110968 -0500
@@ -403,9 +403,9 @@
/* 0(%rsp): interrupt number */
.macro interrupt func
CFI_STARTPROC simple
- CFI_DEF_CFA rsp,(SS-ORIG_RAX)
- CFI_OFFSET rsp,(RSP-SS)
- CFI_OFFSET rip,(RIP-SS)
+ CFI_DEF_CFA rsp,(SS-RDI)
+ CFI_REL_OFFSET rsp,(RSP-ORIG_RAX)
+ CFI_REL_OFFSET rip,(RIP-ORIG_RAX)
cld
#ifdef CONFIG_DEBUG_INFO
SAVE_ALL
diff -urN linux-2.6.5-rc1-mm2.orig/include/asm-x86_64/calling.h linux-2.6.5-rc1-mm2/include/asm-x86_64/calling.h
--- linux-2.6.5-rc1-mm2.orig/include/asm-x86_64/calling.h 2004-03-18 09:36:21.635387992 -0500
+++ linux-2.6.5-rc1-mm2/include/asm-x86_64/calling.h 2004-03-18 09:36:05.978768160 -0500
@@ -35,28 +35,28 @@
subq $9*8+\addskip,%rsp
CFI_ADJUST_CFA_OFFSET 9*8+\addskip
movq %rdi,8*8(%rsp)
- CFI_OFFSET rdi,8*8-(9*8+\addskip)
+ CFI_REL_OFFSET rdi,8*8
movq %rsi,7*8(%rsp)
- CFI_OFFSET rsi,7*8-(9*8+\addskip)
+ CFI_REL_OFFSET rsi,7*8
movq %rdx,6*8(%rsp)
- CFI_OFFSET rdx,6*8-(9*8+\addskip)
+ CFI_REL_OFFSET rdx,6*8
.if \norcx
.else
movq %rcx,5*8(%rsp)
- CFI_OFFSET rcx,5*8-(9*8+\addskip)
+ CFI_REL_OFFSET rcx,5*8
.endif
movq %rax,4*8(%rsp)
- CFI_OFFSET rax,4*8-(9*8+\addskip)
+ CFI_REL_OFFSET rax,4*8
.if \nor891011
.else
movq %r8,3*8(%rsp)
- CFI_OFFSET r8,3*8-(9*8+\addskip)
+ CFI_REL_OFFSET r8,3*8
movq %r9,2*8(%rsp)
- CFI_OFFSET r9,2*8-(9*8+\addskip)
+ CFI_REL_OFFSET r9,2*8
movq %r10,1*8(%rsp)
- CFI_OFFSET r10,1*8-(9*8+\addskip)
+ CFI_REL_OFFSET r10,1*8
movq %r11,(%rsp)
- CFI_OFFSET r11,-(9*8+\addskip)
+ CFI_REL_OFFSET r11,
.endif
.endm
@@ -109,17 +109,17 @@
subq $REST_SKIP,%rsp
CFI_ADJUST_CFA_OFFSET REST_SKIP
movq %rbx,5*8(%rsp)
- CFI_OFFSET rbx,5*8-(REST_SKIP)
+ CFI_REL_OFFSET rbx,5*8
movq %rbp,4*8(%rsp)
- CFI_OFFSET rbp,4*8-(REST_SKIP)
+ CFI_REL_OFFSET rbp,4*8
movq %r12,3*8(%rsp)
- CFI_OFFSET r12,3*8-(REST_SKIP)
+ CFI_REL_OFFSET r12,3*8
movq %r13,2*8(%rsp)
- CFI_OFFSET r13,2*8-(REST_SKIP)
+ CFI_REL_OFFSET r13,2*8
movq %r14,1*8(%rsp)
- CFI_OFFSET r14,1*8-(REST_SKIP)
+ CFI_REL_OFFSET r14,1*8
movq %r15,(%rsp)
- CFI_OFFSET r15,0*8-(REST_SKIP)
+ CFI_REL_OFFSET r15,0*8
.endm
.macro RESTORE_REST
Thanks. Checked into kgdb.sourceforge.net cvs tree
-Amit
On Thursday 18 Mar 2004 10:21 pm, Jim Houston wrote:
> On Wed, 2004-03-17 at 18:53, Andi Kleen wrote:
> On 17 Mar 2004 16:37:15 -0500
>
> > Jim Houston <[email protected]> wrote:
> > > The attached patch fixes the .cfi directives for the common_interrupt
> > > path for opteron. It seems that the existing .cfi directives in this
> > > path only work by accident.
> > >
> > > This patch makes the unwind data make sense and makes gdb/kgdb more
> > > likely to produce a useful stack traces.
> >
> > Thanks. I applied it. The calling.h part gave rejects, but I applied it
> > by hand. It would be nice if you could check in the final kernel if I
> > didn't make a mistake.
>
> Hi Andi, Andrew, Amit,
>
> The attached patch is updated to work with linux-2.6.5-rc1.
>
> Jim Houston - Concurrent Computer Corp.
>
> --
>
> diff -urN linux-2.6.5-rc1-mm2.orig/arch/x86_64/kernel/entry.S
> linux-2.6.5-rc1-mm2/arch/x86_64/kernel/entry.S ---
> linux-2.6.5-rc1-mm2.orig/arch/x86_64/kernel/entry.S 2004-03-18
> 09:37:13.830453136 -0500 +++
> linux-2.6.5-rc1-mm2/arch/x86_64/kernel/entry.S 2004-03-18
> 09:37:29.237110968 -0500 @@ -403,9 +403,9 @@
> /* 0(%rsp): interrupt number */
> .macro interrupt func
> CFI_STARTPROC simple
> - CFI_DEF_CFA rsp,(SS-ORIG_RAX)
> - CFI_OFFSET rsp,(RSP-SS)
> - CFI_OFFSET rip,(RIP-SS)
> + CFI_DEF_CFA rsp,(SS-RDI)
> + CFI_REL_OFFSET rsp,(RSP-ORIG_RAX)
> + CFI_REL_OFFSET rip,(RIP-ORIG_RAX)
> cld
> #ifdef CONFIG_DEBUG_INFO
> SAVE_ALL
> diff -urN linux-2.6.5-rc1-mm2.orig/include/asm-x86_64/calling.h
> linux-2.6.5-rc1-mm2/include/asm-x86_64/calling.h ---
> linux-2.6.5-rc1-mm2.orig/include/asm-x86_64/calling.h 2004-03-18
> 09:36:21.635387992 -0500 +++
> linux-2.6.5-rc1-mm2/include/asm-x86_64/calling.h 2004-03-18
> 09:36:05.978768160 -0500 @@ -35,28 +35,28 @@
> subq $9*8+\addskip,%rsp
> CFI_ADJUST_CFA_OFFSET 9*8+\addskip
> movq %rdi,8*8(%rsp)
> - CFI_OFFSET rdi,8*8-(9*8+\addskip)
> + CFI_REL_OFFSET rdi,8*8
> movq %rsi,7*8(%rsp)
> - CFI_OFFSET rsi,7*8-(9*8+\addskip)
> + CFI_REL_OFFSET rsi,7*8
> movq %rdx,6*8(%rsp)
> - CFI_OFFSET rdx,6*8-(9*8+\addskip)
> + CFI_REL_OFFSET rdx,6*8
> .if \norcx
> .else
> movq %rcx,5*8(%rsp)
> - CFI_OFFSET rcx,5*8-(9*8+\addskip)
> + CFI_REL_OFFSET rcx,5*8
> .endif
> movq %rax,4*8(%rsp)
> - CFI_OFFSET rax,4*8-(9*8+\addskip)
> + CFI_REL_OFFSET rax,4*8
> .if \nor891011
> .else
> movq %r8,3*8(%rsp)
> - CFI_OFFSET r8,3*8-(9*8+\addskip)
> + CFI_REL_OFFSET r8,3*8
> movq %r9,2*8(%rsp)
> - CFI_OFFSET r9,2*8-(9*8+\addskip)
> + CFI_REL_OFFSET r9,2*8
> movq %r10,1*8(%rsp)
> - CFI_OFFSET r10,1*8-(9*8+\addskip)
> + CFI_REL_OFFSET r10,1*8
> movq %r11,(%rsp)
> - CFI_OFFSET r11,-(9*8+\addskip)
> + CFI_REL_OFFSET r11,
> .endif
> .endm
>
> @@ -109,17 +109,17 @@
> subq $REST_SKIP,%rsp
> CFI_ADJUST_CFA_OFFSET REST_SKIP
> movq %rbx,5*8(%rsp)
> - CFI_OFFSET rbx,5*8-(REST_SKIP)
> + CFI_REL_OFFSET rbx,5*8
> movq %rbp,4*8(%rsp)
> - CFI_OFFSET rbp,4*8-(REST_SKIP)
> + CFI_REL_OFFSET rbp,4*8
> movq %r12,3*8(%rsp)
> - CFI_OFFSET r12,3*8-(REST_SKIP)
> + CFI_REL_OFFSET r12,3*8
> movq %r13,2*8(%rsp)
> - CFI_OFFSET r13,2*8-(REST_SKIP)
> + CFI_REL_OFFSET r13,2*8
> movq %r14,1*8(%rsp)
> - CFI_OFFSET r14,1*8-(REST_SKIP)
> + CFI_REL_OFFSET r14,1*8
> movq %r15,(%rsp)
> - CFI_OFFSET r15,0*8-(REST_SKIP)
> + CFI_REL_OFFSET r15,0*8
> .endm
>
> .macro RESTORE_REST
On Fri, 19 Mar 2004 18:47:43 +0530
"Amit S. Kale" <[email protected]> wrote:
> Thanks. Checked into kgdb.sourceforge.net cvs tree
It's not very useful because that tree still has the broken
"interrupt threads" support.
-Andi
On Saturday 20 Mar 2004 12:53 am, Andi Kleen wrote:
> On Fri, 19 Mar 2004 18:47:43 +0530
>
> "Amit S. Kale" <[email protected]> wrote:
> > Thanks. Checked into kgdb.sourceforge.net cvs tree
>
> It's not very useful because that tree still has the broken
> "interrupt threads" support.
Does it show interrupt threads or not?
-Amit
This patch adds call frame debug record generation for entry.S frames. If used
with KGDB and gdb 6.0 it allows correct "back trace" (bt) through interrupt,
system call and trap frames. Frames that point to user space are tied off,
those that point to the kernel unwind to the kernel. It also ties off the
initial frame in head.S and the idle process stacks for cpus other than cpu0
(which actually goes back to head.S). The patch adds one (1) instruction to the
executable kernel (a few lines to the debug space, however).
The patch requires a gdb that handles CFI expressions, although it takes pains
to work around a bug in gdb 6.0's expression analizer (the work around is also
compatable with a correctly working expression analizer as can be found in the
CVS version of gdb). AFAIK gdb 6.0 is the first gdb to handle CFI expressions
so you must use it to make this patch useful.
Use of earlier gdb's with this patch will not be affected (either for good or bad).
--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml
George Anzinger <[email protected]> writes:
> This patch adds call frame debug record generation for entry.S frames.
[...]
Sorry, but that's quite ugly and will be hard to maintain (kinda like
maintaining an own assembler on your own) I think it would be far
better to require recent binutils for DEBUG_INFO builds and use the
.cfi_* mnemonics. They make dwarf2 code *much* simpler and cleaner.
Overall I think it's a good idea to add full dwarf2 annotation to
the i386 kernel, but not without assembler please.
-Andi
Andi Kleen wrote:
> George Anzinger <[email protected]> writes:
>
>
>>This patch adds call frame debug record generation for entry.S frames.
>
>
> [...]
>
> Sorry, but that's quite ugly and will be hard to maintain (kinda like
> maintaining an own assembler on your own) I think it would be far
> better to require recent binutils for DEBUG_INFO builds and use the
> .cfi_* mnemonics. They make dwarf2 code *much* simpler and cleaner.
>
> Overall I think it's a good idea to add full dwarf2 annotation to
> the i386 kernel, but not without assembler please.
Hi Andi,
I just knew you would say that :).
I think I have said before that the current .cfi support in the assembler is not
up to the job. In fact gdb 6.0 also has a nasty bug that this code works
around. The main issue is the ability to use the dwarf2 cfi expression to build
a call frame that determines if the interrupt/ trap frame returns to user space
or to the kernel. I think (I confess I have not tried) this may be doable with
the .cfi escape op code, but I suspect the result would be just as ugly as this
patch is. You would have to roll your own .uleb128 and .sleb128 numbers, for
example. Also, you would need to be able to define labels in the dwarf code
(or intuit how var the assembler is going to put your target and use that offset).
The long and short of it is, to do it at all, you need to have a fair knowledge
of dwarf2. Once you get to that, I suspect one way is as good as another.
At this point, the code works with kgdb, which, itself is not in the kernel. I
welcome any one who wants to help do it correctly.
--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml
> The long and short of it is, to do it at all, you need to have a fair
> knowledge of dwarf2. Once you get to that, I suspect one way is as good as
> another.
Did you contact the gdb and binutils maintainers about the problems?
Maybe it can be easily fixed.
-Andi
Andi Kleen wrote:
>>The long and short of it is, to do it at all, you need to have a fair
>>knowledge of dwarf2. Once you get to that, I suspect one way is as good as
>>another.
>
>
> Did you contact the gdb and binutils maintainers about the problems?
> Maybe it can be easily fixed.
>
I mentioned it to Daniel Jacobowitz.
The problem is what is needed is access to the full dwarf2 expression code.
Actually only a small sub set is needed here, but I suspect they would only do
the whole thing, and it is rather rich. I only implemented about 20% of the
opcodes.
For example, the way gdb knows that "this is the bottom of the stack" is for the
CFI address to come back as zero. Normally this is a stack address. An
expression is needed to get zero, and, at least in interrupt / trap handling,
the expression needs to be conditional. So, either a new language is invented
or access is provided to the dwarf2 language, or an abstracted version of it.
The ladder is what I did. I provided the dwarf2 opcodes with macros that
wrapped the required boiler plate around them. I set it up the way C does, i.e.
as a separate block of asm code, rather than intermixed with the assembly thing
(which would require relocs to the debug space and back as well as additional
boiler plate). This is artifact of how I figured out how to translate the
dwarf2 spec to real code, i.e. I looked at what C was doing.
The thing is, we are talking assembly code here. That means that just about
anything is possible WRT the call frame.
If I had any sway over what the binutils folks do, I would argue for allowing
dwarf2 code intermixed with inline asm in the C asm() code. At the moment this
is very hard (impossible) to do.
An example of what I would like to be able to do is to build a call frame for
the out of line part of the spin lock. It would be a very simple frame that
would just say it was called from the inline part of the spin lock.
As second example is to properly describe the "switch frame" used for context
switching. Currently x86 requires frame pointers to cover this, i.e. with frame
pointers off, gdb can not unwind tasks that are not active, even with dwarf2
frame stuff.
--
George Anzinger [email protected]
High-res-timers: http://sourceforge.net/projects/high-res-timers/
Preemption patch: http://www.kernel.org/pub/linux/kernel/people/rml