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

[Bernd Edlinger <bernd.edlinger@hotmail.de>]

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



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
>