Re: [patch RFC] Re: Notes on a frame_unwind_address_in_block problem

[Mark Kettenis <mark dot kettenis at xs4all dot nl>]

> Date: Mon, 1 Jan 2007 15:02:48 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Mon, Jan 01, 2007 at 08:54:08PM +0100, Mark Kettenis wrote:
> > Well, I really can't say I like it.  The problem is that it's been
> > several months since we last discussed this problem, so I'll have to
> > start to think again from scratch :(.  Isn't it just a matter of
> 
> Yeah, sorry about that.  Anyway, I'm happy to discuss alternatives;
> I don't like it much either.

I have thought about this for a bit now, and I think I have found a
way out.  But let me first start by being a bit more specific about
why I think your change is so bad:

The basic principle of unwinding is that you get information about
THIS_FRAME from NEXT_FRAME; you always walk the frame chain in one
direction.  This means that the only requirement for calling
frame_unwind_xxx functions is that you have a NEXT_FRAME.  Your change
introduces a function that breaks this rule, by requiring THIS_FRAME
to be there, walking the frame chain in the other direction.  And it
didn't take you very long to hit the problem with that: infinite
recursion.

Now the problem we're facing is that frame_unwind_address_in_block()
(frame_func_unwind() is nothing but a fancy wrapper around that call)
cannot be implemented reliably without knowledge about THIS_FRAME; it
needs to know whether THIS_FRAME could be a fake frame set up by the
kernel in order to determine whether it is a good idea to adjust the
unwound PC or not.  In our current implementation that information is
carried by the frame type.

The solution I think, is to pass this information explicitly to
frame_unwind_address_in_block(), i.e. we change it's prototype from:

  CORE_ADDR frame_unwind_address_in_block (struct frame_info *next_frame);

into

  CORE_ADDR frame_unwind_address_in_block (struct frame_info *next_frame,
                                           enum frame_type *this_type);

Of course get_frame_address_in_block() doesn't need this extra
argument, and would be implemented as:

CORE_ADDR
get_frame_address_in_block (struct frame_info *this_frame)
{
  return frame_unwind_address_in_block (this_frame->next,
                                        get_frame_type (this_frame));
}


If you think a bit further (almost) all cases where we currently call
frame_unwind_address_in_block() in sniffers, we really need to specify
THIS_FRAME's type explicitly.  So signal frame sniffers would need to
do call frame_unwind_address_in_block (next_frame, SIGTRAMP_FRAME).
Doing so in the dwarf2_signal_frame_this_id() would fix the bug we're
trying to fix.

Mark