This is the mail archive of the gdb@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]

[RFC] Using values to handle unwinding


On Wed, Oct 17, 2007 at 12:03:50PM -0400, Daniel Jacobowitz wrote:
> This is the patch I referenced yesterday on gdb-patches; the
> background is here:
>   http://sourceware.org/ml/gdb-patches/2007-10/msg00432.html

And the thread I'm replying to starts here for more:
  http://sourceware.org/ml/gdb/2007-10/msg00124.html

I have finished polishing these patches.  I will post (to gdb-patches,
in a moment) patches for lazy register values, prev_register returning
a value, and passing this_frame instead of next_frame to basically
everything except the gdbarch unwind_pc method.  unwind_dummy_id
becomes just dummy_id, et cetera.

These patches break the build for every unconverted target.  I updated
x86_64-* and i386-*, and all the platform-independent unwinders
(tramp, trad, dummy, dwarf2, sentinel).  Later in this mail are some
detailed conversion instructions.

I also wrote some gdbint documentation, for both new and existing
frame unwinding details.  It's not complete but it's a step in the
right direction.

One addition to the instructions: unless you have to do something
particularly strange to a file, I think a single-line changelog entry
is reasonable.  I want to improve GDB, not the size of its ChangeLog
:-) And writing the changelogs for this patch set took quite a long
time.

I'd love comments on the patches, the overall approach, and how to
proceed.  Ideally, we check this in (breaking many targets), update
each target completely as someone needs that target, and make sure all
targets are updated by the next release of GDB.  I personally use
amd64, i386, arm, mips, m68k, and powerpc; so I'm pretty likely to
update all of those (mechanically).  I'll do the laggards before the
next release too, but not right this minute.  I would appreciate
assistance with other targets :-)

Thoughts?

===

Conversion notes:

Here's how you convert an unwinder.  Start by finding a prev_register
routine.  They're pretty much all named *_prev_register so you can
grep for "_prev_register ".  Update it to have the new signature.
After you're done converting it, be sure to convert the matching
this_id routine too!  The signature of this_id has changed (next_frame
-> this_frame), but in a way that will not cause compilation errors.
So never do a prev_register routine without the matching this_id.
They come in pairs pretty much everywhere.

frame_unwind_append_sniffer has gone away.  It has been replaced by
frame_unwind_append_unwinder, which takes a different signature
(matching the pre-existing frame_unwind_prepend_unwinder).  And both
prepended and appended sniffers now take next_frame, just like
prev_register and this_id.  During execution of the sniffer, the
unwinder being sniffed for is installed in the current frame.  This
lets simple interfaces like get_frame_func and
get_frame_address_in_block behave as expected.  Which means the exact
same routines can be called from the sniffer as from prev_value.

What this means during convertion is that you need to add the
sniffer to the "struct frame_unwind" and call frame_unwind_append_unwinder
instead of frame_unwind_append_sniffer.  The type of the sniffer has
also changed.  Sniffers which just "return 1" (every architecture
has one) can use default_frame_sniffer.

There are not as many frame base unwinders, but keep an eye out for
those too - they have the same problem as this_id, and the frame base
sniffers have to take this_frame also.  Similarly, the init function
in struct tramp_frame now takes this_frame.  I did a lot of searching
files for next_frame.  A few gdbarch methods still use next_frame, but
most functions with an argument named next_frame are due for an
update.

For each affected routine, rename the next_frame argument to this_frame.
Be sure to update the description in the function comment, and any
other comments inside the function.  Check every use of this_frame;
some examples are in the table below.  If it is passed to other
miscellaneous target-specific functions, then you need to convert
those (add them to your worklist, as it were...) and be sure to
update all their callers too.

I found it best to do an entire architecture at a time, e.g. amd64*.c
+ i386*.c all in one go.  I started with amd64-tdep.c.  The only
nonstandard routine was called from amd64_sigtramp_frame_cache:
tdep->sigcontext_addr.  So from there I had to go out and do
all of the amd64 and i386 OS ports and the i386 sigtramp unwinder
too.

You have to be careful when using get_prev_frame in an unwinder
(generally don't do it - the prev frame requires this frame) and
similarly for get_next_frame (it returns NULL instead of the sentinel
frame, so generally don't do this either).  Let me know if you run
into any problems with having the wrong frame and not being able
to change all the affected functions.

For instance, before I converted sniffers to take this_frame, I ran
into i386_linux_sigcontext_addr.  That calls
i386_linux_sigtramp_start, which was particularly tricky because I had
not converted the sigtramp_p routine to take this_frame, which was
itself tricky because I haven't converted sniffers to take this_frame
being sniffed.  So I had to revisit the design at that point.  Until
I did, I deliberately left the call untouched:

  pc = i386_linux_sigtramp_start (next_frame);

Since I'd renamed next_frame -> this_frame in
i386_linux_sigcontext_addr already, this clearly won't compile -
that's a flag to come back and look at it later.  It's not practical
to test all platforms in advance for this sort of conversion,
but the majority of unwinders are in tdep files so we can compile
them easily (--enable-targets=...).  I added a /* FIXME */
while I was working on it, just for good measure, and
periodically scanned my diff for added FIXMEs.

Then some of the files I'd already touched contained additional
unwinders (e.g. amd64obsd_trapframe_sniffer) so I went back
and did those too.

Here's some example conversions:

NEXT_FRAME				THIS_FRAME
----------				----------
frame_unwind_register			get_frame_register
frame_unwind_register_unsigned		get_frame_register_unsigned
frame_unwind_register_signed		get_frame_register_signed
frame_pc_unwind				get_frame_pc
frame_func_unwind			get_frame_func
						(lose the extra argument)
frame_unwind_address_in_block		get_frame_address_in_block
						(lose the extra argument)
get_frame_memory_unsigned		*unchanged*
						(just rename the argument)
safe_frame_unwind_memory		*unchanged*
						(ditto, ignore the name...)

Old register kind			New function
-----------------			------------
not_lval				frame_unwind_got_constant
    [For values no larger than a ULONGEST.
     Calculation of the value may be guarded by if (valuep).
     We always calculate the value in this new interface.  That's
     the majority of calls, so this is not a noticeable loss.]
not_lval				frame_unwind_got_address
    [If the port does complicated address to pointer conversion
     and you have a CORE_ADDR, you'll need this instead.]
lval_register				frame_unwind_got_register
lval_memory				frame_unwind_got_memory


-- 
Daniel Jacobowitz
CodeSourcery


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