Re: [FYI] Inlining support, rough patch

[Daniel Jacobowitz <drow at false dot org>]

Resending to list.

On Fri, Jul 25, 2008 at 04:46:41PM +0200, Mark Kettenis wrote:
> > 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.

I don't understand what you mean when you say this makes the generic
code any harder to understand.  Can you point to lines for me?

All of the code relating to inline ID construction is in
inline-frame.c.  Here's the only related bit in frame.c:

+  /* If we are unwinding from an inline frame, all of the below tests
+     were already performed when we unwound from the next non-inline
+     frame.  We must skip them, since we can not get THIS_FRAME's ID
+     until we have unwound all the way down to the previous non-inline
+     frame.  */
+  if (get_frame_type (this_frame) == INLINE_FRAME)
+    return get_prev_frame_raw (this_frame);

Inlining is definitely a special case, yes.  I'm happy to write
internals documentation and more comments.  I'll do that whatever we
decide.

> > 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.

No, we don't do that.  You use the frame chain normally - go "up" to
get them.

It's mostly in inferior control, dealing with the problem that
function calls nest more completely than inlined functions.  For
instance, when we are handling "next" and enter a function call we
unwind the function's caller's frame ID and compare it to the ID of
the frame where we were previously stepping.  If the function call was
the first instruction of an inlined function, then the frame ID will
have changed - we have to recognize what's happened so that we can
correctly skip over the function call.

We can do this by saving the non-inilned frame ID instead of peering
at the internal structure of frame IDs, though.

> > 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.

But they're used independently.  Sometimes we have to ignore
.debug_frame, either because it's missing or because of a bug in a
deployed compiler - both of these have happened to me in the past
year.  I don't want to make inlining entwined with the use of the
dwarf2 frame unwinder if I can avoid it.

There's another approach we could take.  We could delay creation of
the inlined frames until after the normal sniffer has run.  This
requires stitching them in afterwards and adjusting frame->level.
I've tried this; it was much more intrusive and complicated
than the current version.  That's a variant of the fix Jan made here,
on top of my original (broken) merge of this code to HEAD:

http://sourceware.org/ml/gdb-patches/2008-06/msg00379.html

Search for frame-unwind.c and struct inline_cache.

If you can think of a way to do this that doesn't involve complicating
the generic unwind machinery - exactly what we're both trying to avoid
- I'll give it another shot.

> > 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.

I'm just having trouble understanding how this would help.

-- 
Daniel Jacobowitz
CodeSourcery