From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Andrew Burgess <andrew.burgess@embecosm.com>
Cc: Christian Biesinger <cbiesinger@google.com>,
"gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re: [PATCH 3/3] gdb: Better frame tracking for inline frames
Date: Sun, 29 Dec 2019 01:06:00 -0000 [thread overview]
Message-ID: <AM0PR08MB3714B103ECF99BC1B8DCFF67E4240@AM0PR08MB3714.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <20191228232310.GO3865@embecosm.com>
[-- 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
next prev parent reply other threads:[~2019-12-29 1:06 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-28 10:02 Bernd Edlinger
2019-12-28 23:23 ` Andrew Burgess
2019-12-29 1:06 ` Bernd Edlinger [this message]
-- strict thread matches above, loose matches on Subject: below --
2019-12-23 1:51 [PATCH 0/3] Improve inline frame debug experience Andrew Burgess
2019-12-23 1:51 ` [PATCH 3/3] gdb: Better frame tracking for inline frames Andrew Burgess
2019-12-26 7:25 ` Christian Biesinger via gdb-patches
2019-12-26 23:33 ` Andrew Burgess
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM0PR08MB3714B103ECF99BC1B8DCFF67E4240@AM0PR08MB3714.eurprd08.prod.outlook.com \
--to=bernd.edlinger@hotmail.de \
--cc=andrew.burgess@embecosm.com \
--cc=cbiesinger@google.com \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).