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

Bernd Edlinger
2019-12-29T01:06:36Z

[-- Attachment #1: Type: text/plain, Size: 6010 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-bt-print-inlined-instead-of-the-PC-for-inline-frames.patch --]
[-- Type: text/x-patch; name="0001-bt-print-inlined-instead-of-the-PC-for-inline-frames.patch", Size: 1027 bytes --]

From c000bc2a96b471953cac91c09985551115d66600 Mon Sep 17 00:00:00 2001
From: Bernd Edlinger <bernd.edlinger@hotmail.de>
Date: Sun, 29 Dec 2019 01:31:49 +0100
Subject: [PATCH] bt: print "inlined" instead of the PC for inline frames

---
gdb/stack.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gdb/stack.c b/gdb/stack.c
index fed4824..e39c073 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -1333,10 +1333,15 @@ print_frame (const frame_print_options &fp_opts,
if (opts.addressprint)
if (!sal.symtab
|| frame_show_address (frame, sal)
+ || (sal.line != 0 && sal.pc == 0 && sal.end == 0
+ && frame_relative_level (frame) > 0)
|| print_what == LOC_AND_ADDRESS)
{
annotate_frame_address ();
- if (pc_p)
+ if (sal.line != 0 && sal.pc == 0 && sal.end == 0
+ && frame_relative_level (frame) > 0)
+ uiout->text ("inlined");
+ else if (pc_p)
print_pc (uiout, gdbarch, frame, pc);
else
uiout->field_string ("addr", "<unavailable>",
--
1.9.1