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

  On 12/29/19 12:23 AM, Andrew Burgess wrote:
> * 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 () ...
> 

Yes, I think #9 means aaa() is inlined into main () at dw2-inline-many-frames.c:125
#8 means bbb() is inlined into aaa() at dw2-inline-many-frames.c:60
#7 means ccc() is inlined into bbb() at dw2-inline-many-frames.c:108
but #6 is wrong, I just put this together by hand...
it should be 0x0000000000401061 in ccc () at dw2-inline-many-frames.c:84
since this is the PC after the call to ddd ().


I tried a patch as attached and it produces this stack trace:

(gdb) bt
#0  kkk () at dw2-inline-many-frames.c:101
#1  0x0000000000401157 in jjj () at dw2-inline-many-frames.c:68
#2  0x0000000000401167 in iii () at dw2-inline-many-frames.c:145
#3  inlined in hhh () at dw2-inline-many-frames.c:115
#4  inlined in ggg () at dw2-inline-many-frames.c:77
#5  inlined in fff () at dw2-inline-many-frames.c:92
#6  0x0000000000401177 in eee () at dw2-inline-many-frames.c:155
#7  0x0000000000401187 in ddd () at dw2-inline-many-frames.c:135
#8  0x0000000000401061 in ccc () at dw2-inline-many-frames.c:84
#9  inlined in bbb () at dw2-inline-many-frames.c:108
#10 inlined in aaa () at dw2-inline-many-frames.c:60
#11 inlined in main () at dw2-inline-many-frames.c:125


Frames where no inlined is printed, mean called from addr in ... at ...


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

Yes, maybe.  It's probably also okay to leave this as is.


Thanks,
Bernd.

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