This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Support Dwarf3 DW_CFA_val_* expressions


> From: Alexandre Oliva <aoliva@redhat.com>
> Date: Tue, 07 Mar 2006 14:46:00 -0300
> 
> On Mar  7, 2006, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> 
> >> From: Alexandre Oliva <aoliva@redhat.com>
> >> Date: Tue, 07 Mar 2006 02:31:27 -0300
> >> 
> >> On Mar  4, 2006, Daniel Jacobowitz <drow@false.org> wrote:
> >> 
> >> > On Sat, Mar 04, 2006 at 09:01:43AM -0300, Alexandre Oliva wrote:
> >> >> > It's also good to have a few testcases,
> >> >> > but I don't completely understand the testcases :(
> >> >> 
> >> >> You're not alone in that feeling, I assure you :-)
> >> 
> >> > Like Mark, I am happier with the code than the testcases.  In fact,
> >> > I'd rather not have the tests at all than have something quite
> >> > so overkill.
> 
> >> Fair enough...  Those tests uncovered a problem with the way our
> >> dwarf2 code identifies frames, though: it uses the function start
> >> address and the CFA to identify a stack frame, whereas it should use
> >> the PC and the CFA.
> 
> > Nope, using the function address is the correct thing to do.
> > Otherwise you'll end up with a different frame ID if you single-step
> > to the next instruction for example.
> 
> So I see :-(  Sorry for posting the patch before testing was even
> started.

No problem.

> >> In some corner cases, such as the one exercised by the i386
> >> testcase, if you're on the second instruction of the region covered
> >> by the hand-generated unwind info and ask for a backtrace, GDB
> >> complains that the stack is corrupt, but the only problem is that
> >> the dwarf2 machinery gets confused by the same-CFA, same-function
> >> pair of stack frames.  This is fixed in the revised patch below.
> 
> > That fix is wrong!  Either your hand-generated unwind info is wrong
> > (or incomplete) or GDB needs to do something more clever (like perhaps
> > using the address of the first instruction in the region covered by
> > the unwind info).
> 
> Using the address of the first instruction in the region wouldn't work
> either.  The hand-generated unwind info arranges for _L_mutex_lock_31
> on i386 to seem like it calls itself, for some reason I don't quite
> understand.  Jakub says the backtrace we get after my change is
> correct, whereas *without* the patch we get this:
> 
> 0x080486bc in _L_mutex_lock_31 ()
> 1: x/i $pc  0x80486bc <_L_mutex_lock_31>:       lea    0x8(%ebp),%ecx
> (gdb) where
> #0  0x080486bc in _L_mutex_lock_31 ()
> #1  0x080486c4 in _L_mutex_lock_31 ()
> Previous frame identical to this frame (corrupt stack?)
> (gdb) disass foo
> Dump of assembler code for function foo:
> 0x08048540 <foo+0>:     push   %ebp
> 0x08048541 <foo+1>:     mov    %esp,%ebp
> 0x08048543 <foo+3>:     sub    $0x108,%esp
> 0x08048549 <foo+9>:     mov    %ebx,0xfffffff8(%ebp)
> 0x0804854c <foo+12>:    lea    0xfffffef8(%ebp),%ebx
> 0x08048552 <foo+18>:    mov    %esi,0xfffffffc(%ebp)
> 0x08048555 <foo+21>:    mov    0x8(%ebp),%esi
> 0x08048558 <foo+24>:    test   %esi,%esi
> 0x0804855a <foo+26>:    jne    0x80486bc <_L_mutex_lock_31>
> 0x08048560 <foo+32>:    mov    0xfffffff8(%ebp),%ebx
> 0x08048563 <foo+35>:    mov    0xfffffffc(%ebp),%esi
> 0x08048566 <foo+38>:    mov    %ebp,%esp
> 0x08048568 <foo+40>:    pop    %ebp
> 0x08048569 <foo+41>:    ret
> End of assembler dump.
> (gdb) disass
> Dump of assembler code for function _L_mutex_lock_31:
> 0x080486bc <_L_mutex_lock_31+0>:        lea    0x8(%ebp),%ecx
> 0x080486bf <_L_mutex_lock_31+3>:        call   0x8048690 <bar>
> 0x080486c4 <_L_mutex_lock_31+8>:        jmp    0x8048560 <foo+32>
> 0x080486c9 <_L_mutex_lock_31+13>:       nop
> 0x080486ca <_L_mutex_lock_31+14>:       nop
> 0x080486cb <_L_mutex_lock_31+15>:       nop
> End of assembler dump.
> 
> > In any case, the frame ID should only change when
> > we really enter a different frame.
> 
> My understanding is that the intention *is* to represent debug info as
> an entry into a new frame, but I don't quite understand why it would
> be correct to have two entries for the same function, as in the stack
> trace below (generated by the incorrectly-patched GDB).

Hmm, that unwind info is a lie then, but I can see that it doesn't
really make a difference for handling exceptions.  You'll only notice
the problems if you actually start interpreting the frames.

Unwinding with the porgram counter annywhere in _L_mutex_lock_31
should just land us directly at foo+32.  

Roland, any reason why that wouldn't work?

> (gdb) si
> 0x080486bc in _L_mutex_lock_31 ()
> 1: x/i $pc  0x80486bc <_L_mutex_lock_31>:       lea    0x8(%ebp),%ecx
> (gdb) where
> #0  0x080486bc in _L_mutex_lock_31 ()
> #1  0x080486c4 in _L_mutex_lock_31 ()
> #2  0x08048560 in foo (x=3)
>     at ../../../gdb/testsuite/gdb.dwarf2/cfa-val-expr-1.c:91
> #3  0x08048582 in fn2 ()
>     at ../../../gdb/testsuite/gdb.dwarf2/cfa-val-expr-1.c:237
> #4  0x08048598 in fn1 ()
>     at ../../../gdb/testsuite/gdb.dwarf2/cfa-val-expr-1.c:244
> #5  0x080485fb in main ()
>     at ../../../gdb/testsuite/gdb.dwarf2/cfa-val-expr-1.c:252
> 
> 
> Roland, do you happen to know what the point is of the two stack
> frames for _L_mutex_lock_31?  Could we perhaps do away with them, so
> as to avoid the problem entirely, given that GDB is apparently doing
> the right thing?
> 
> 
> As an aside, I don't quite see that GDB's approach to identifying
> stack frames by using the stack pointer and the function entry point
> is entirely correct for architectures in which recursive functions
> don't necessarily change the stack pointer: consider an architecture
> with register windows that use a separate stack, like IA64, with the
> further assumption that one wouldn't need to save the return address
> on the regular stack because the register window machinery would take
> care of saving it.

Well, I simplified things a bit; actually a frame ID can have a third
"special" address in addition to the code and stack addresses.
Unsurprisingly on ia64, this third address is used to store the
backing store pointer.

> > Perhaps you can explain in more detail what the problem is?
> 
> Is the above enough?
> 
> >> > And you're not correct that older GCCs could run them
> >> > far enough to test; they'll fail to build because of the use of
> >> > __attribute__((cleanup))
> >> 
> >> Oops, indeed, sorry about that.  The testcases must definitely be
> >> amended if they're to remain.  Should they?
> 
> > I think the tests need to be simplified.
> 
> Tricky.  The very point of the tests is to test the complex CFA
> expressions.  I could easily remove all of the cleanup and run-time
> unwinding stuff (is this what you meant?), but taking out the hairy
> bits would render the test pointless.

Sorry yes, I meant removing the cleanup and run-time unwinding stuff.
You indicated those were not necessary for the GDB tests, they would
just confuse people trying to understand what is happening, and make
the test dependend on particular GCC versions.

However, since the test doesn't seem to be helpful for GDB at all
(except perhaps for testing how GDB reacts to somewhat bogus debug
info), it's probably be a good idea to have a test that actually tests
these new DWARF3 expressions.  Since I don't think it would be
possible to write a test that runs on all ISA's an i386-specific test
in testsuite/gdb.arch would probably be best.

Anyway, feel free to commit the code that adds support for the new
DWARF3 expressions (the origional one, not the "corrected" one ;).

Mark


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]