From patchwork Sun Dec 29 01:06:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 37114 Received: (qmail 49179 invoked by alias); 29 Dec 2019 01:06:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 49170 invoked by uid 89); 29 Dec 2019 01:06:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-22.2 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.1 spammy=H*r:150 X-HELO: EUR03-AM5-obe.outbound.protection.outlook.com Received: from mail-oln040092070077.outbound.protection.outlook.com (HELO EUR03-AM5-obe.outbound.protection.outlook.com) (40.92.70.77) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 29 Dec 2019 01:06:33 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=deMt3AxVPC/nPyaUeaPmbl4RUM1d+Fd+OK9/psBOVPUKUd2PxQYNZ4WBndf+IzKr/iIv8Iezo/g6roO4uB6HIu4x6jZuisoHnMbgyz1an8/rWMGlGwy7xS1ANN3U0I6tpFJpdYYPrQwFJOagiIAiV3H/QmoCz+DAk0Fa9joMZzJFeq89uFdjYvnNpwj+eQQFHXCe9BSL8/J76qgD2gEQJcIQc9nwlSYOkEAaxkdvOvoAecdBDUig2OGuST9mpZfn3TZS8yC2U9Gvqwian43zacQgO14ZeWrXRTcOnJXHEgaJJntGfUQKPPWb6wylKzlK8chIfPG9o423S92Gzu829A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=7oEVEarBHCZoJXJVt4SIaZ0PL8RqptZFuL1/xp6yBqs=; b=NQuvCsz0YpHU3iQymAkz9d+bPzJ0DQCE1dIuMFbYvU6my6+N4ChQEUcj5Ngw1V2K+uQ+QTglHLkNlK7MVLMBc3VWqfCL5ItrWVptu01rh7leKoddb8HAs6Ccsy+E9c8NcwoNkesCP2e3VzoCV3mWt7D2/BPg0nlO2zVI4Dn8IIQRWM4ikRE1gcn5HrEVTCPs+2rSRrD+ugP6l2bT9kQ490quEGhaONQGKr5KUB2TQZ3L8HU1fc6peaZbfMH21RVGhoasFzrgRbvIzSDodlG8KbxJbnKKVNRW9hmAKDDeiGBebC4o4YBR7i4VqqXpqaTYrNB7TpCbZfnEq0mZHKwD8A== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from AM5EUR03FT056.eop-EUR03.prod.protection.outlook.com (10.152.16.54) by AM5EUR03HT005.eop-EUR03.prod.protection.outlook.com (10.152.16.154) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2581.11; Sun, 29 Dec 2019 01:06:30 +0000 Received: from AM0PR08MB3714.eurprd08.prod.outlook.com (10.152.16.60) by AM5EUR03FT056.mail.protection.outlook.com (10.152.17.224) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2581.11 via Frontend Transport; Sun, 29 Dec 2019 01:06:30 +0000 Received: from AM0PR08MB3714.eurprd08.prod.outlook.com ([fe80::8dd1:fb18:6271:f769]) by AM0PR08MB3714.eurprd08.prod.outlook.com ([fe80::8dd1:fb18:6271:f769%7]) with mapi id 15.20.2581.007; Sun, 29 Dec 2019 01:06:30 +0000 Received: from [192.168.1.101] (146.60.252.106) by AM0PR10CA0037.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:20b:150::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2581.11 via Frontend Transport; Sun, 29 Dec 2019 01:06:29 +0000 From: Bernd Edlinger To: Andrew Burgess CC: Christian Biesinger , "gdb-patches@sourceware.org" Subject: Re: [PATCH 3/3] gdb: Better frame tracking for inline frames Date: Sun, 29 Dec 2019 01:06:30 +0000 Message-ID: References: <20191228232310.GO3865@embecosm.com> In-Reply-To: <20191228232310.GO3865@embecosm.com> x-microsoft-original-message-id: x-ms-exchange-transport-forked: True MIME-Version: 1.0 On 12/29/19 12:23 AM, Andrew Burgess wrote: > * Bernd Edlinger [2019-12-28 10:02:40 +0000]: > >> On 12/26/19 11:33 PM, Andrew Burgess wrote: >>> * Christian Biesinger [2019-12-26 07:24:39 +0100]: >>> >>>> On Mon, Dec 23, 2019, 02:51 Andrew Burgess >>>> 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 >>> >> From c000bc2a96b471953cac91c09985551115d66600 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger 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", "", -- 1.9.1