[mark.kettenis@xs4all.nl: Re: [FYI] Inlining support, rough patch]

[Daniel Jacobowitz <drow at false dot org>]

For the record.

----- Forwarded message from Mark Kettenis <mark.kettenis@xs4all.nl> -----

Date: Fri, 25 Jul 2008 16:46:41 +0200 (CEST)
From: Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: [FYI] Inlining support, rough patch
To: drow@false.org
CC: mark.kettenis@xs4all.nl

> Date: Fri, 18 Jul 2008 09:03:08 -0400
> From: Daniel Jacobowitz <drow@false.org>
> 
> Thanks for looking at this.  I'll turn this conversation into internals
> documentation later, no matter what version we end up with.
> 
> On Fri, Jul 18, 2008 at 01:53:01AM +0200, Mark Kettenis wrote:
> > I can see why it would be desirable to have inlined functions appear
> > in a backtrace.  And I don't necessarily disagree with doing this
> > through inlined frames.  While they're not real stack frames, they're
> > not too different from frames we create for leaf calls that don't have
> > a stack frame of their own.  The important question is what the frame
> > ID for such a frame should be.  It seems you use the stack address and
> > entry point of the function in which the code was inlined.  As a
> > consequence you need to extend the frame ID to distinguish it from
> > that surrounding frame.  I think the correct thing to do is to use the
> > code address (low address or entry point) of the inlined function and
> > perhaps the stack pointer at that point to construct the frame ID.
> 
> I don't think I turned unwinding upside down; in fact I think the
> unwinder changes are fairly unintrusive, but that may be because I saw
> the earlier versions of this patch...  The only substantive change is
> the short-circuit test in get_prev_frame_1 to accomodate the
> upside-down ID construction, which is itself isolated to inline
> frames.  Are there other specific changes that you're worried about,
> or is it just the ID construction?

It's the ID construction that I'm worried about.  It is the very core
of the unwinding code.  I really think your diff violates the most
fundamental principle of this bit of code and in that way, makes it
much harder to understand it.

> As for those IDs, I think they're necessary.  I'd be delighted to be
> wrong, so I'll explain the new fields and see if we can find a way to
> be rid of them.

Oh, I agree the IDs are necessary.  But they should be constructed
from the next (inner) frame.  I think this essentially means that the
unwinder that constructs them for dwarf2 needs to be part of the
dwarf2 unwinder.

> > The inline_depth field is necessary because more than one function
> can begin at a given instruction.  For example:
> 
> volatile int x;
> 
> int inner (void)
> {
>   return x;
> }
> 
> int mid (void)
> {
>   return inner();
> }
> 
> int outer (void)
> {
>   return mid();
> }
> 
> No code will be generated for mid that doesn't come from inner, so
> there's no code address that can be used to distinguish them.

Ok, yes I see.  So it needs to be part of 'struct frame_id'.

> The separate block address and code address, and the use of the
> enclosing function's stack address, are necessary because of
> frame_id_stack_eq.  Given two frame IDs, there are several places we
> must be able to find out if they belong to the same real stack frame.

Could you explain why?  Is this because when we're stopped in an
inlined function, you want to give the user access to local variables
in the function in which that code was inlined?  I'm not sure that's
the right thing to do.

> > I've just eliminated the separate block_addr; we can use code_addr
> instead, with a few changes elsewhere - including, happily, the
> elimination of frame_id_stack_eq.  That removes the requirement
> for the inlined frame's ID to have the same stack address as the
> calling frame, though it still needs a stable stack address.
> 
> > I think this is doable if the inline unwinder is integrated with the
> > dwarf2 unwinder, or at least shares code with it.  That way the
> > generic frame unwinding code can stay much the way it is.  These
> > frames probably need to be marked and the unwinding code probably
> > needs a way to skip them.  But that will only have a fairly limited
> > impact.
> 
> That would make inlining dependent on .debug_frame.  But I don't see
> why it should be.  We get inlining from .debug_info instead, and
> conceptually there's no reason we couldn't get it from another debug
> format.

The different sections are just a way to organize things, they're not
really different debug formats.

> While inlined frames no longer need to have the same stack address as
> their caller, in my latest version, they do still need a stable stack
> address.  The only ways I see to get one are to unwind to the caller
> and ask it, or to hard-code and enforce a requirement that there be
> .debug_frame and ask the dwarf2 unwinder directly.  The second seems
> like a special case of the first, so I'm missing the advantage.

Well, it keeps the generic code straightforward and easier to
understand.  I think that's a majot benefit.

Mark


----- End forwarded message -----

-- 
Daniel Jacobowitz
CodeSourcery