Re: [PATCH 3/3] gdb: Better frame tracking for inline frames

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Bernd Edlinger <bernd.edlinger@hotmail.de> [2019-12-28 10:02:40 +0000]:

> On 12/26/19 11:33 PM, Andrew Burgess wrote:
> > * Christian Biesinger <cbiesinger@google.com> [2019-12-26 07:24:39 +0100]:
> > 
> >> On Mon, Dec 23, 2019, 02:51 Andrew Burgess <andrew.burgess@embecosm.com>
> >> wrote:
> >> 
> >> > This commit improves GDB's handling of inline functions when there are
> >> > more than one inline function in a stack, so for example if we have a
> >> > stack like:
> >> >
> >> >    main -> aaa -> bbb -> ccc -> ddd
> >> >
> >> > And aaa, bbb, and ccc are all inline within main GDB should (when
> >> > given sufficient debug information) be able to step from main through
> >> > aaa, bbb, and ccc.  Unfortunately, this currently doesn't work, here's
> >> > an example session:
> >> >
> >> >   (gdb) start
> >> >   Temporary breakpoint 1 at 0x4003b0: file test.c, line 38.
> >> >   Starting program: /project/gdb/tests/inline/test
> >> >
> >> >   Temporary breakpoint 1, main () at test.c:38
> >> >   38      global_var = 0;
> >> >   (gdb) step
> >> >   39      return aaa () + 1;
> >> >   (gdb) step
> >> >   aaa () at test.c:39
> >> >   39      return aaa () + 1;
> >> >   (gdb) step
> >> >   bbb () at test.c:39
> >> >   39      return aaa () + 1;
> >> >   (gdb) step
> >> >   ccc () at test.c:39
> >> >   39      return aaa () + 1;
> >> >   (gdb) step
> >> >   ddd () at test.c:32
> >> >   32      return global_var;
> >> >   (gdb) bt
> >> >   #0  ddd () at test.c:32
> >> >   #1  0x00000000004003c1 in ccc () at test.c:39
> >> >   #2  bbb () at test.c:26
> >> >   #3  aaa () at test.c:14
> >> >   #4  main () at test.c:39
> >> >
> >> 
> >> How come only #1 is printing the address?
> > 
> > Well.....
> > 
> > For inline frames the sal returned by find_frame_sal has a symtab and
> > line set, but doesn't have the pc or end fields set.
> > 
> > If we then look at frame_show_address it seems specifically designed
> > to return false for precisely the case we're discussing.
> > 
> > I agree with you that it seems odd that we only get the address for #1
> > in this case.  I considered this patch:
> > 
> > ## START ##
> > 
> > diff --git a/gdb/stack.c b/gdb/stack.c
> > index 22820524871..ce85a1183f0 100644
> > --- a/gdb/stack.c
> > +++ b/gdb/stack.c
> > @@ -327,7 +327,7 @@ frame_show_address (struct frame_info *frame,
> >         gdb_assert (inline_skipped_frames (inferior_thread ()) > 0);
> >        else
> >         gdb_assert (get_frame_type (get_next_frame (frame)) == INLINE_FRAME);
> > -      return false;
> > +      return frame_relative_level (frame) > 0;
> >      }
> >  
> >    return get_frame_pc (frame) != sal.pc;
> > 
> > ## END ##
> > 
> > The addresses are printed more now, there's a few test failures that
> > would need to be checked first.
> > 
> 
> Hmm....
> 
> In the case of inline functions the call stack would behave odd
> with this patch.
> 
> Since the inline frames all share the same PC, the value is redundant,
> still different source line location is presented for each.
> Also when stepping in the inner frame, all inlined frames would
> also change the PC, so you get the impression that the inlined function
> is now called from a slightly different place.
> 
> It might be more useful to add an info to the call stack,
> that a frame was inlined instead?
> 
> #0  iii () at dw2-inline-many-frames.c:145
> #1  inlined in hhh () at dw2-inline-many-frames.c:115
> #2  inlined in ggg () at dw2-inline-many-frames.c:77
> #3  inlined in fff () at dw2-inline-many-frames.c:92
> #4  0x0000000000401226 in eee () at dw2-inline-many-frames.c:155
> #5  0x0000000000401209 in ddd () at dw2-inline-many-frames.c:135
> #6  inlined in ccc () at dw2-inline-many-frames.c:84
> #7  inlined in bbb () at dw2-inline-many-frames.c:108
> #8  inlined in aaa () at dw2-inline-many-frames.c:60
> #9  inlined in main () at dw2-inline-many-frames.c:125

I really want to like this idea, as I agree showing the address feels
less useful, and somehow marking the inline nature of things feels
like a good thing, but I'm put off by this:

  #9  inlined in main () ...

The 'inlined in main' is telling us about frame #8, right?  And this
feels weird too.

Not sure I have any better suggestions though.

Thanks,
Andrew



> 
> 
> 
> Bernd.
> 
> > Ideally I would like to keep changing this behaviour separate from
> > this patch series, but I do think this might be something we should
> > consider changing.
> > 
> > Thanks,
> > Andrew
> > 
>