From patchwork Thu Mar 5 18:01:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Bernd Edlinger X-Patchwork-Id: 38434 Received: (qmail 102345 invoked by alias); 5 Mar 2020 18:01:51 -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 102328 invoked by uid 89); 5 Mar 2020 18:01:50 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-21.7 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:200, 0*, H*RU:sk:AM4PR07, H*f:sk:cover.1 X-HELO: EUR05-VI1-obe.outbound.protection.outlook.com Received: from mail-vi1eur05olkn2090.outbound.protection.outlook.com (HELO EUR05-VI1-obe.outbound.protection.outlook.com) (40.92.90.90) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Thu, 05 Mar 2020 18:01:48 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=F7dfRkVX0BKH+mSnasyHQlBXnbo13ikSihzSCHTksQbxyqmPSgpwj0wIGSryYHdpJRpWtZz80aSDAhD3ctXCEpzT+IJf7OA1R11nCuJcBCtcT8zrqaNa/Bm6gYA1fcI0wkzmd7MlsGda950S9VHGxaP6Gs8xE2KtAD7a5H3Ofqb00WypufHsuCfhhEk7O31QFL0ztpDIjCuGbtrQkE1sbTqJZfK48n/7A5ZeuOQJarziePjY1MWvjWlHeHZ9odlD4HusC9YH2qU7tWZ4egM861kDNQUG6vheZpmQerGDSaCThMP4K59BsvQqebvtJh03CMD3pliOWw0FaObU3zqzDw== 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=jU8piyzRLAsm4YlvQ2bUHlcaPBE7LMdwPslT2DttNSQ=; b=d9EepLFHjK+F6YW3l0VhXSDBnG/5u+pIN1JpJksiZoar5XoAXgy8gIjffMFdPTrXUPV+axGHsiL1lPFmvtlt9muff/7SwUm/CqX3aAf0w0ZJ7ne151mgXmms28Lnz5dCLI90M27XOpxRMZv+g/1ittoJanLZecnlhoxGTzE1zt92wOXqBtFpuY4pHTFBcxzeh10+G0sqrbQ+O9jK7pBKU4JuVdFlq/4px9Y+32FWOGaac9v+Tm3aDSV4dDjCIcFZMO5bUus+T1SUR57xbwnddUopegJmxhLBHL5pfg0s50czM9hI+EWCCbUFvT7Ou9cPQSUCCd5x8q443AEoy/cQCQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from VI1EUR05FT038.eop-eur05.prod.protection.outlook.com (2a01:111:e400:fc12::3b) by VI1EUR05HT059.eop-eur05.prod.protection.outlook.com (2a01:111:e400:fc12::116) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.11; Thu, 5 Mar 2020 18:01:45 +0000 Received: from AM6PR03MB5170.eurprd03.prod.outlook.com (10.233.242.56) by VI1EUR05FT038.mail.protection.outlook.com (10.233.242.220) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2793.11 via Frontend Transport; Thu, 5 Mar 2020 18:01:45 +0000 Received: from AM6PR03MB5170.eurprd03.prod.outlook.com ([fe80::1956:d274:cab3:b4dd]) by AM6PR03MB5170.eurprd03.prod.outlook.com ([fe80::1956:d274:cab3:b4dd%6]) with mapi id 15.20.2772.019; Thu, 5 Mar 2020 18:01:45 +0000 Received: from [192.168.1.101] (92.77.140.102) by AM4PR0701CA0018.eurprd07.prod.outlook.com (2603:10a6:200:42::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2814.6 via Frontend Transport; Thu, 5 Mar 2020 18:01:44 +0000 From: Bernd Edlinger To: Andrew Burgess CC: "gdb-patches@sourceware.org" Subject: Re: [PATCH 2/2] gdb: Add support for tracking the DWARF line table is-stmt field Date: Thu, 5 Mar 2020 18:01:45 +0000 Message-ID: References: <94e33268f64060fc887670f4ee5ed524050cbcc7.1580902412.git.andrew.burgess@embecosm.com> <20200211135710.GR4020@embecosm.com> In-Reply-To: x-microsoft-original-message-id: <7c2dc85d-97f7-159d-e4f1-2ad63cc395bd@hotmail.de> x-ms-exchange-antispam-messagedata: 25CpXkgKldO4MVq3H5yI2060idc07vBUWDMrCaYQ60jcQEiB+Ke1IV73KIDeRpdnuglFm0kf7aBmdnVmx+VTZyLJXBvP8TVTK0n92Uf5Q+Ai6q+Ok+W4+dy/PCprgVBnFThgkfpmkG9MksQXqn4dnQ== x-ms-exchange-transport-forked: True MIME-Version: 1.0 On 2/14/20 9:05 PM, Bernd Edlinger wrote: > On 2/11/20 2:57 PM, Andrew Burgess wrote: >> * Bernd Edlinger [2020-02-05 17:55:37 +0000]: >> >>> On 2/5/20 12:37 PM, Andrew Burgess wrote: >>> >>> did you mean, when we don't land in the middle of a line ? >> >> No, in this case I did write the correct thing. >> >> So GDB had/has some code designed to "improve" the handling of looping >> constructs. I think the idea is to handle cases like this: >> >> 0x100 LN=5 >> 0x104 <-\ >> 0x108 | >> 0x10c | LN=6 >> 0x110 | >> 0x114 --/ >> >> So when line 6 branches back to the middle of line 5, we don't stop >> (because we are in the middle of line 5), but we do want to stop at >> the start of line 6, so we "reset" the starting point back to line 5. >> >> This is done by calling set_step_info in process_event_stop_test, in >> infrun.c. >> >> What we actually did though was whenever we were at an address that >> didn't exactly match a SAL (in the example above, marked LN=5, LN=6) >> we called set_step_info. This worked fine, at address 0x104 we reset >> back to LN=5, same at address 0x108. >> >> However, in the new world things can look different, for example, like >> this: >> >> 0x100 LN=5,STMT=1 >> 0x104 >> 0x108 LN=6,STMT=0 >> 0x10c LN=6,STMT=1 >> 0x110 >> 0x114 >> >> In this world, when we move forward to 0x100 we don't have a matching >> SAL, so we get the SAL for address 0x100. We can then safely call >> set_step_info like we did before, that's fine. But when we step to >> 0x108 we do now have a matching SAL, but we don't want to stop yet as >> this address is 'STMT=0'. However, if we call set_step_info then GDB >> is going to think we are stepping away from LN=6, and so will refuse >> to stop at address 0x10c. >> >> The proposal in this commit is that if we land at an address which >> doesn't specifically match a SAL, 0x104, 0x110, 0x114 in the above >> example, then we do call set_step_info, but otherwise we don't. >> >> There are going to be situations where the behaviour of GDB changes >> after this patch, but I don't think we can avoid that. What we had >> previously was just a heuristic. I think enabling this new >> information in GDB will allow us to do better overall, so I think we >> should make this change and fix any issues as they show up. >> > > I agree with that, thanks for this explanation. > > However, I think I found a small problem in this patch. > You can see it with the next-inline test case. > When you use just step the execution does not stop > between the inline functions, because not calling > > current behaviour is this: > > Breakpoint 1, main () at next-inline.cc:63 > 63 get_alias_set (&xx); > (gdb) s > get_alias_set (t=0x404040 ) at next-inline.cc:50 > 50 if (t != NULL > (gdb) s > 51 && TREE_TYPE (t).z != 1 > (gdb) s > 0x0000000000401169 in tree_check (i=0, t=0x404040 ) at next-inline.h:34 > 34 if (t->x != i) > (gdb) s > 0x000000000040117d in tree_check (i=0, t=0x404040 ) at next-inline.h:34 > 34 if (t->x != i) > (gdb) s > 0x000000000040118f in tree_check (i=0, t=0x404040 ) at next-inline.h:34 > 34 if (t->x != i) > (gdb) s > main () at next-inline.cc:64 > 64 return 0; > (gdb) > > I debugged a bit because I was not sure why that happens, and it looks > like set_step_info does also set the current inline frame, but you > only want to suppress the line number not the currently executing > inline function. > With this small patch the step works as expected. > Hmm, I think this got stuck, so I just worked a follow-up patch out for you. I also noticed that the test case cannot work with gcc before gcc-8, since the is_stmt feature is not implemented earlier, at least with gcc-4.8 and gcc-5.4 the test case fails. I thought it would be good to detect that by adding the -gstatement-frontiers option, so that the gcc driver complains when it is not able to generate sufficient debug info for that test. Thoughts? Bernd. From 3f39d068e3386ba204a598fa3d2946d357b1d7b2 Mon Sep 17 00:00:00 2001 From: Bernd Edlinger Date: Fri, 14 Feb 2020 20:41:51 +0100 Subject: [PATCH] Fix next-inline test case with step Should stop between inline function invocations. Add -gstatement-frontiers compile option to avoid running test with gcc version that don't support this feature, which would fail the test unnecessarily. Instead make the compile step fail which is less noisy. Add a prefix use_header / no_header to all test cases. gdb: 2020-03-05 Bernd Edlinger * infrun.c (process_event_stop_test): Set step_frame_id. gdb/testsuite: 2020-03-05 Bernd Edlinger * gdb.cp/next-inline.exp: Extend test case. --- gdb/infrun.c | 2 ++ gdb/testsuite/gdb.cp/next-inline.exp | 45 +++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/gdb/infrun.c b/gdb/infrun.c index 6c35805..e8221ba 100644 --- a/gdb/infrun.c +++ b/gdb/infrun.c @@ -7219,6 +7219,8 @@ process_event_stop_test (struct execution_control_state *ecs) ecs->event_thread->control.step_range_start = stop_pc_sal.pc; ecs->event_thread->control.step_range_end = stop_pc_sal.end; ecs->event_thread->control.may_range_step = 1; + ecs->event_thread->control.step_frame_id = get_frame_id (frame); + ecs->event_thread->control.step_stack_frame_id = get_stack_frame_id (frame); if (refresh_step_info) set_step_info (frame, stop_pc_sal); diff --git a/gdb/testsuite/gdb.cp/next-inline.exp b/gdb/testsuite/gdb.cp/next-inline.exp index 0b2b22d..ec04694 100644 --- a/gdb/testsuite/gdb.cp/next-inline.exp +++ b/gdb/testsuite/gdb.cp/next-inline.exp @@ -20,12 +20,16 @@ standard_testfile .cc proc do_test { use_header } { global srcfile testfile - set options {c++ debug nowarnings optimize=-O2} + set options {c++ debug nowarnings optimize=-O2\ -gstatement-frontiers} if { $use_header } { lappend options additional_flags=-DUSE_NEXT_INLINE_H set executable "$testfile-with-header" + set hdrfile "next-inline.h" + set prefix "use_header" } else { set executable "$testfile-no-header" + set hdrfile "$srcfile" + set prefix "no_header" } if { [prepare_for_testing "failed to prepare" $executable \ @@ -33,6 +37,8 @@ proc do_test { use_header } { return -1 } + with_test_prefix $prefix { + if ![runto_main] { fail "can't run to main" return @@ -64,6 +70,43 @@ proc do_test { use_header } { gdb_test "bt" \ "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ "not in inline 5" + + if {!$use_header} { + return #until that is fixed + } + + if ![runto_main] { + fail "can't run to main pass 2" + return + } + + gdb_test "bt" "\\s*\\#0\\s+main.*" "in main pass 2" + gdb_test "step" ".*" "step into get_alias_set pass 2" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "in get_alias_set pass 2" + gdb_test "step" ".*TREE_TYPE.*" "step 1" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 1 pass 2" + gdb_test "step" ".*if \\(t->x != i\\).*" "step 2" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "in inline 1 pass 2" + gdb_test "step" ".*TREE_TYPE.*" "step 3" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 2 pass 2" + gdb_test "step" ".*if \\(t->x != i\\).*" "step 4" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "in inline 2 pass 2" + gdb_test "step" ".*TREE_TYPE.*" "step 5" + gdb_test "bt" "\\s*\\#0\\s+get_alias_set\[^\r\]*${srcfile}:.*" \ + "not in inline 3 pass 2" + gdb_test "step" ".*if \\(t->x != i\\).*" "step 6" + gdb_test "bt" "\\s*\\#0\\s+\[^\r\]*tree_check\[^\r\]*${hdrfile}:.*" \ + "in inline 3 pass 2" + gdb_test "step" "return 0.*" "step 7" + gdb_test "bt" \ + "\\s*\\#0\\s+(main|get_alias_set)\[^\r\]*${srcfile}:.*" \ + "not in inline 4 pass 2" + } } do_test 0 -- 1.9.1