From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2080.outbound.protection.outlook.com [40.107.21.80]) by sourceware.org (Postfix) with ESMTPS id BE0743855599 for ; Fri, 5 May 2023 15:00:27 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BE0743855599 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=arm.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oJqOXsl9rOdCMgIh9fAabHZPsT64IO95vJe3qjEBqi0=; b=JPTmCM3kGxKrlUnD4qEzjYefrnWnFEGStho+OR9dat8xyjoWd8b+WegynrG5xMR0hY/jGSTwE4aN+x+k4ylHJvOpvQx90tSgyY0d5Z9owrsIajz4HF7vi2zvT7QTjJoRuMwZrODH3uwn4rDHDXy3xNh1xrWzT6PMiEQuwMMYbtY= Received: from AS9PR06CA0673.eurprd06.prod.outlook.com (2603:10a6:20b:49c::17) by PA4PR08MB7484.eurprd08.prod.outlook.com (2603:10a6:102:2a8::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6340.30; Fri, 5 May 2023 15:00:17 +0000 Received: from AM7EUR03FT025.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:49c:cafe::b6) by AS9PR06CA0673.outlook.office365.com (2603:10a6:20b:49c::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.27 via Frontend Transport; Fri, 5 May 2023 15:00:17 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; pr=C Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM7EUR03FT025.mail.protection.outlook.com (100.127.140.199) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6387.12 via Frontend Transport; Fri, 5 May 2023 15:00:17 +0000 Received: ("Tessian outbound 5154e9d36775:v136"); Fri, 05 May 2023 15:00:17 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 306b93c600a6b21c X-CR-MTA-TID: 64aa7808 Received: from 715b78abba7e.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 1B9A0EB0-9602-4229-97E6-C6C7DDC885E3.1; Fri, 05 May 2023 15:00:09 +0000 Received: from EUR01-VE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 715b78abba7e.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Fri, 05 May 2023 15:00:09 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=d04Y1LO5XEPbNfvGE6IDLXpZ9XlZjPwjbCjlyxRKZ1aRxnATITuYr94LXWreh74gpnFGCg02qAWvMwdfgchOMQ2xRP44ztU020sNfzM1Bytnqf0SdaNEpbaBnHCLgvgjLH9glS0/Oo+5BeMGXkq44VACek4lKDpEV3BgupbvYk4zaW4ysioYbftS9za1FALoDpPq2lRvGNCWmoHf63nflGYB8RHgTXedIsgTwtlYkisakYMr1Ht8KB6jFd+yWBd0UcZdB9D1XW1qluZ917iWGkilshW8cAlgppoyMGd6xxE8bRAhPh512lfQqjBeI32gVfa2wWuOxc5Xi61hdM8+pA== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=oJqOXsl9rOdCMgIh9fAabHZPsT64IO95vJe3qjEBqi0=; b=bkp5N4DZX4UZKIrTJN7CLkuek1N1suNFZTXCxe37wgSO3Q22sOC4vFvE8cYZp3mzbZxArEi6e4xcqzBHTZwgGTjleTL00YYrQsBPCAE4rJP1ZEcJWQZat53mFAtbbZ3WXgvLUmroENsbfMqw9AixW+Jr6QVb423zLEiak3MpmjW6VzpWVCOPZD+C6jpDPb41UNfbbAeP1Ir0+zyYry9S7u14IggKapQt1H/Ufo3k9K93VFUesvitEkQENYHIsx7iLe7Vh4W68Ca4FkFxxp7Nxkbd27iNlzvmdxQIAkNXUGsNcwsO8rFhR1npqCdE4O3/iBXhjvJ779VQ2gahPWjNgA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oJqOXsl9rOdCMgIh9fAabHZPsT64IO95vJe3qjEBqi0=; b=JPTmCM3kGxKrlUnD4qEzjYefrnWnFEGStho+OR9dat8xyjoWd8b+WegynrG5xMR0hY/jGSTwE4aN+x+k4ylHJvOpvQx90tSgyY0d5Z9owrsIajz4HF7vi2zvT7QTjJoRuMwZrODH3uwn4rDHDXy3xNh1xrWzT6PMiEQuwMMYbtY= Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) by AS8PR08MB5926.eurprd08.prod.outlook.com (2603:10a6:20b:29d::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6363.27; Fri, 5 May 2023 15:00:06 +0000 Received: from VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::f85f:ba2d:763d:fed6]) by VI1PR08MB3919.eurprd08.prod.outlook.com ([fe80::f85f:ba2d:763d:fed6%6]) with mapi id 15.20.6363.025; Fri, 5 May 2023 15:00:04 +0000 Message-ID: <23c8e441-f6bc-37b2-8101-e9a4030c7c8b@arm.com> Date: Fri, 5 May 2023 15:59:54 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 Subject: Re: [PATCH v3] Fix reverse stepping multiple contiguous PC ranges over the line table. Content-Language: en-US To: Carl Love , Bruno Larsen , gdb-patches@sourceware.org, UlrichWeigand , pedro@palves.net References: <74630f1ccb6e9258ae60682105ee5490726fb255.camel@us.ibm.com> <46d73c69-9168-44c6-b515-23dd893fc0eb@redhat.com> <86c65f2ad74caffc162f100e4e9c5be9062a7f59.camel@us.ibm.com> From: Luis Machado In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: SN1PR12CA0064.namprd12.prod.outlook.com (2603:10b6:802:20::35) To VI1PR08MB3919.eurprd08.prod.outlook.com (2603:10a6:803:c4::31) MIME-Version: 1.0 X-MS-TrafficTypeDiagnostic: VI1PR08MB3919:EE_|AS8PR08MB5926:EE_|AM7EUR03FT025:EE_|PA4PR08MB7484:EE_ X-MS-Office365-Filtering-Correlation-Id: 54a5998d-838e-4e8f-5635-08db4d797056 x-checkrecipientrouted: true NoDisclaimer: true X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: QC4H2AQnNxtpibBs0gwiV37GAy+rbfyP15ELd8r3fj/cv740MCbB21baeChYhTvFmQrmwsBAkKY9HoLrycZKK2jZJs/dCMc2DHy5rh8W/OlWhFHVCS3htpRGZ6SVySR3VT+x97z5bIEPwkkVVIcmBqqZLmqXe6/KVSW4Ucg+85E8XmQ2qE1PzBod6t6TXZYSCrMCz0Av3ncKGLnnXhsu6fgQD+Qq+zGFkbzzjwLSTxlU79DefESwlWEjvPF6q7txLFLiQgpJ24PNlZUGVNcXaaAfak+cRc1M76EnXVvxeFZHhlvfhrvRa/qCniYlD39ENcsEZaU1uu9o/vsiGXUbIUKtN+0bluZibvBshvwxhfK9glUlFpVJ42OpSEEOm8SMIjvHKiafkRDUkf1m7NTWJqfwCnK4OSnWzeVTZOvDq5swYblBx5YOqbnvuZjxGM4W2Me3i+EZ9VqkwVo4LDqfzDfM/x2OKiTP0qKIpaUtLnnD26MFiJJCRLfNiZodjU8SBFiJtK1TS0hFFEwfjH/TS3AC905fwebCk7p7lh+vIdqwKGdv8Dim4Q3FOuvdZpn+wfH3qJ92TcM8irTjI0xLWf99u0rpuB+w/S3fJKSSMGjUcOCN4+31sLIcDjEFrtfNwOI9xmu02JBOR3LbomTEYDYiTo00ZvI57sGxNkquFt1npC7DNKBfLrKXRwkjvswr X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:VI1PR08MB3919.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230028)(4636009)(136003)(376002)(366004)(39860400002)(346002)(396003)(451199021)(36756003)(86362001)(31696002)(478600001)(41300700001)(316002)(110136005)(66946007)(66556008)(66476007)(966005)(6486002)(6666004)(8676002)(5660300002)(8936002)(30864003)(2906002)(44832011)(2616005)(38100700002)(186003)(6512007)(26005)(6506007)(53546011)(83380400001)(66899021)(84970400001)(31686004)(2004002)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR08MB5926 Original-Authentication-Results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM7EUR03FT025.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: b409fc11-73c0-4bfb-4a69-08db4d796711 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: IEvxSAsNTd2hiAF7S21P/BBl4YGoKhyrNzmQ0FX3HdkxVOlSUr0KlZE1cESGngYceu9YseNHbvDGqSzW8UlNdTq6gbobRNKphgv7R0LTTEdndZSnSzBGOOhE9lZvtTBFJNWvFJsL3gvHy+kesf6oKTWTqRdvKdJC+16MB5PEdCj2ef0hJLJ6XexBp03MwuJwBx5OeLe0JxvdTX0OM19CcqFkfyKhWfpqYEu3dXmuKQDoWQbh+hFqx+ceWuxwOQ1SmnuQsmSwjjUqGz8vrUlWpH6lSxMFuPE+S5fqT1bIo05lngICyPr/SZsZ9ED235nXx8tdTCQEJGFtmO+VPdrATlfsvR9QRRVt9mUzUKyOQre7cpXXT+Yc+TuxIKXiXskwiyFTVdT3x6Kq2Ba6gGZwev0VF72eeqwFilXdpZxzSnmP2VTJohMrD9BDEPytehDBQa2B4Z1067e10oaAKZXgD70M0+wO6yJdVbaHbA/agLZIVipYPTxTpvEP7uTSWjKuPZtbcLTlSuZvSCaKAnq8FfS4pb76EFCTWvlinsV24jJKeH0F0d38hpmk/BGoYRaMuBc6ydY9x4J5btIMywNmsxkg0r+R+oC6T2b88lX4skB1vm0iW/Tt0nzha9tK3Wzw665ZJ+ccrDwRa8akSROMGcTIAz5AgpaDk6eEML6R32J9r4gOwPBmpFE/pmJLzDjxEjq1QPeEOjX8WwoUZ+pwZL0+pJ5AEAmGgdPMf+7qwnc/NUh4IWaCFF0XR5pqsn84y1gBSCYkPqJEFz7/ggmdpkGWLTRi8qa6g+kxgAe2VPo= X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(13230028)(4636009)(136003)(39860400002)(346002)(376002)(396003)(451199021)(40470700004)(46966006)(36840700001)(2616005)(70206006)(478600001)(36860700001)(31686004)(186003)(83380400001)(47076005)(110136005)(966005)(53546011)(84970400001)(6486002)(6666004)(6506007)(6512007)(336012)(26005)(40460700003)(316002)(66899021)(8676002)(8936002)(41300700001)(82740400003)(44832011)(5660300002)(30864003)(81166007)(40480700001)(2906002)(36756003)(70586007)(31696002)(86362001)(82310400005)(356005)(2004002)(43740500002);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 May 2023 15:00:17.3363 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 54a5998d-838e-4e8f-5635-08db4d797056 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM7EUR03FT025.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: PA4PR08MB7484 X-Spam-Status: No, score=-13.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,FORGED_SPF_HELO,GIT_PATCH_0,KAM_ASCII_DIVIDERS,KAM_DMARC_NONE,KAM_SHORT,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 5/4/23 16:59, Carl Love wrote: > > Bruno, GDB maintainers: f> > Version 3, added the gcc version check as discussed further from > version 2 of the patch. Also updated the tests to check for supporting > reverse execution rather than requiring recording. I also noticed > there were a couple more instances of a requirement check, i.e. if [] > which I changed to "require" per the current style for checking on the > test requirements. > > > The following patch fixes issues on PowerPC with the reverse-step and > reverse-next instructions when there are multiple assignment statements > on the same line and when there are multiple function calls on the same > line. The commit log below discusses these issues in further depth. > The discussion included what the correct operation should be for these > commands based on the GDB documentation. The proposed patch at that > time changed how the commands worked on other platforms such as X86 in > a way they no longer matched the documentation. > > The issue is the line table contains multiple entries for the same > source line. The patch adds a function to search the line table to > find the address of the first instruction of a line. When setup up the > reverse stepping range, the function is called to make sure the start > of the range corresponds to the address of the first instruction for > the line. This approach was used. When Luis initially developed the > patch, he considered merging the contiguous ranges in the line table > when reading the line tables. He decided it was better to work with the > data directly in the line table rather than creating and using a > modified version of the line table. > > The following patch fixes the execution of the reveres-step and > reverse-next commands for both senarios of multiple statements on the > same line for PowerPC and aarch64-linux. Unlike the previous patch, it > does not change the operation of the commands on other platforms, i.e. > X86. The patch adds new test cases for both scenarios to verify they > work correctly. > > The patch has been tested on PowerPC, Intel X86 and aarch64-linux with > no new regression failures. > > Please let me know if the patch is acceptable for mainline. Thanks. > > Carl > > --------------------------------------------------------------- > Fix reverse stepping multiple contiguous PC ranges over the line table. > > There are a couple of scenarios where the GDB reverse-step and reverse-next > commands do not work correctly. > > Scenario 1 issue description by Luis Machado: > > When running GDB's testsuite on aarch64-linux/Ubuntu 20.04 (also spotted on > the ppc backend), I noticed some failures in gdb.reverse/solib-precsave.exp > and gdb.reverse/solib-reverse.exp. > > The failure happens around the following code: > > 38 b[1] = shr2(17); /* middle part two */ > 40 b[0] = 6; b[1] = 9; /* generic statement, end part two */ > 42 shr1 ("message 1\n"); /* shr1 one */ > > Normal execution: > > - step from line 38 will land on line 40. > - step from line 40 will land on line 42. > > Reverse execution: > - step from line 42 will land on line 40. > - step from line 40 will land on line 40. > - step from line 40 will land on line 38. > > The problem here is that line 40 contains two contiguous but distinct > PC ranges in the line table, like so: > > Line 40 - [0x7ec ~ 0x7f4] > Line 40 - [0x7f4 ~ 0x7fc] > > The two distinct ranges are generated because GCC started outputting source > column information, which GDB doesn't take into account at the moment. > > When stepping forward from line 40, we skip both of these ranges and land on > line 42. When stepping backward from line 42, we stop at the start PC of the > second (or first, going backwards) range of line 40. > > Since we've reached ecs->event_thread->control.step_range_start, we stop > stepping backwards. > > --------------------------------------------------------- > > Scenario 2 issue described by Pedro Alves: > > The following explanation of the issue was taken from the gdb mailing list > discussion of the withdrawn patch to change the behavior of the reverse-step > and reverse-next commands. Specifically, message from Pedro Alves > where he demonstrates the issue where you have multiple > function calls on the same source code line: > > https://sourceware.org/pipermail/gdb-patches/2023-January/196110.html > > The source line looks like: > > func1 (); func2 (); > > so stepping backwards over that line should always stop at the first > instruction of the line, not in the middle. Let's simplify this. > > Here's the full source code of my example: > > (gdb) list 1 > 1 void func1 () > 2 { > 3 } > 4 > 5 void func2 () > 6 { > 7 } > 8 > 9 int main () > 10 { > 11 func1 (); func2 (); > 12 } > > Compiled with: > > $ gcc reverse.c -o reverse -g3 -O0 > $ gcc -v > ... > gcc version 11.3.0 (Ubuntu 11.3.0-1ubuntu1~22.04) > > Now let's debug it with target record, using current gdb git master (f3d8ae90b236), > without your patch: > > $ gdb ~/reverse > GNU gdb (GDB) 14.0.50.20230124-git > ... > Reading symbols from /home/pedro/reverse... > (gdb) start > Temporary breakpoint 1 at 0x1147: file reverse.c, line 11. > Starting program: /home/pedro/reverse > [Thread debugging using libthread_db enabled] > Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". > > Temporary breakpoint 1, main () at reverse.c:11 > 11 func1 (); func2 (); > (gdb) record > > (gdb) disassemble /s > Dump of assembler code for function main: > reverse.c: > 10 { > 0x000055555555513f <+0>: endbr64 > 0x0000555555555143 <+4>: push %rbp > 0x0000555555555144 <+5>: mov %rsp,%rbp > > 11 func1 (); func2 (); > => 0x0000555555555147 <+8>: mov $0x0,%eax > 0x000055555555514c <+13>: call 0x555555555129 > 0x0000555555555151 <+18>: mov $0x0,%eax > 0x0000555555555156 <+23>: call 0x555555555134 > 0x000055555555515b <+28>: mov $0x0,%eax > > 12 } > 0x0000555555555160 <+33>: pop %rbp > 0x0000555555555161 <+34>: ret > End of assembler dump. > > (gdb) n > 12 } > > So far so good, a "next" stepped over the whole of line 11 and stopped at line 12. > > Let's confirm where we are now: > > (gdb) disassemble /s > Dump of assembler code for function main: > reverse.c: > 10 { > 0x000055555555513f <+0>: endbr64 > 0x0000555555555143 <+4>: push %rbp > 0x0000555555555144 <+5>: mov %rsp,%rbp > > 11 func1 (); func2 (); > 0x0000555555555147 <+8>: mov $0x0,%eax > 0x000055555555514c <+13>: call 0x555555555129 > 0x0000555555555151 <+18>: mov $0x0,%eax > 0x0000555555555156 <+23>: call 0x555555555134 > 0x000055555555515b <+28>: mov $0x0,%eax > > 12 } > => 0x0000555555555160 <+33>: pop %rbp > 0x0000555555555161 <+34>: ret > End of assembler dump. > > Good, we're at the first instruction of line 12. > > Now let's undo the "next", with "reverse-next": > > (gdb) reverse-next > 11 func1 (); func2 (); > > Seemingly stopped at line 11. Let's see exactly where: > > (gdb) disassemble /s > Dump of assembler code for function main: > reverse.c: > 10 { > 0x000055555555513f <+0>: endbr64 > 0x0000555555555143 <+4>: push %rbp > 0x0000555555555144 <+5>: mov %rsp,%rbp > > 11 func1 (); func2 (); > 0x0000555555555147 <+8>: mov $0x0,%eax > 0x000055555555514c <+13>: call 0x555555555129 > => 0x0000555555555151 <+18>: mov $0x0,%eax > 0x0000555555555156 <+23>: call 0x555555555134 > 0x000055555555515b <+28>: mov $0x0,%eax > > 12 } > 0x0000555555555160 <+33>: pop %rbp > 0x0000555555555161 <+34>: ret > End of assembler dump. > (gdb) > > And lo, we stopped in the middle of line 11! That is a bug, we should have > stepped back all the way to the beginning of the line. The "reverse-next" > should have fully undone the prior "next" command. > > The above issues were fixed by introducing a new function that looks for > adjacent PC ranges for the same line, until we notice a line change. Then > we take that as the start PC of the range. The new start PC for the range > is used for the control.step_range_start when setting up a step range. > > The test case gdb.reverse/map-to-same-line.exp is added to test the fix > for the issues in scenario 1. > > The test case gdb.reverse/func-map-to-same-line.exp was added to test the > fix for scenario 2 when the binary was compiled with and without line > table information. > > bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28426 > > Co-authored-by: Luis Machado > Co-authored-by: Carl Love > --- > gdb/infrun.c | 57 +++++++ > gdb/symtab.c | 49 ++++++ > gdb/symtab.h | 16 ++ > .../gdb.reverse/func-map-to-same-line.c | 36 ++++ > .../gdb.reverse/func-map-to-same-line.exp | 146 ++++++++++++++++ > gdb/testsuite/gdb.reverse/map-to-same-line.c | 58 +++++++ > .../gdb.reverse/map-to-same-line.exp | 156 ++++++++++++++++++ > 7 files changed, 518 insertions(+) > create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line.c > create mode 100644 gdb/testsuite/gdb.reverse/func-map-to-same-line.exp > create mode 100644 gdb/testsuite/gdb.reverse/map-to-same-line.c > create mode 100644 gdb/testsuite/gdb.reverse/map-to-same-line.exp > > diff --git a/gdb/infrun.c b/gdb/infrun.c > index efe2c00c489..8555e3c979f 100644 > --- a/gdb/infrun.c > +++ b/gdb/infrun.c > @@ -114,6 +114,9 @@ static struct async_event_handler *infrun_async_inferior_event_token; > Starts off as -1, indicating "never enabled/disabled". */ > static int infrun_is_async = -1; > > +static CORE_ADDR update_line_range_start (CORE_ADDR pc, > + struct execution_control_state *ecs); > + > /* See infrun.h. */ > > void > @@ -6769,6 +6772,25 @@ handle_signal_stop (struct execution_control_state *ecs) > process_event_stop_test (ecs); > } > > +CORE_ADDR > +update_line_range_start (CORE_ADDR pc, struct execution_control_state *ecs) > +{ > + /* The line table may have multiple entries for the same source code line. > + Given the PC, check the line table and return the PC that corresponds > + to the line table entry for the source line that PC is in. */ > + CORE_ADDR start_line_pc = ecs->event_thread->control.step_range_start; > + gdb::optional real_range_start; > + > + /* Call find_line_range_start to get smallest address in the s/smallest/the smallest > + linetable for multiple Line X entries in the line table. */ > + real_range_start = find_line_range_start (pc); > + > + if (real_range_start.has_value ()) > + start_line_pc = *real_range_start; > + > + return start_line_pc; > +} > + > /* Come here when we've got some debug event / signal we can explain > (IOW, not a random signal), and test whether it should cause a > stop, or whether we should resume the inferior (transparently). > @@ -7570,6 +7592,28 @@ process_event_stop_test (struct execution_control_state *ecs) > > if (stop_pc_sal.is_stmt) > { > + if (execution_direction == EXEC_REVERSE) > + { > + /* We are stepping backwards make sure we have reached the > + beginning of the line. */ > + CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); > + CORE_ADDR start_line_pc > + = update_line_range_start (stop_pc, ecs); > + > + if (stop_pc != start_line_pc) > + { > + /* Have not reached the beginning of the source code line. > + Set a step range. Execution should stop in any function > + calls we execute back into before reaching the beginning > + of the line. */ > + ecs->event_thread->control.step_range_start = start_line_pc; > + ecs->event_thread->control.step_range_end = stop_pc; > + set_step_info (ecs->event_thread, frame, stop_pc_sal); > + keep_going (ecs); > + return; > + } > + } > + > /* We are at the start of a statement. > > So stop. Note that we don't stop if we step into the middle of a > @@ -7632,6 +7676,19 @@ process_event_stop_test (struct execution_control_state *ecs) > set_step_info (ecs->event_thread, frame, stop_pc_sal); > > infrun_debug_printf ("keep going"); > + > + if (execution_direction == EXEC_REVERSE) > + { > + CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); > + > + /* Make sure the stop_pc is set to the beginning of the line. */ > + if (stop_pc != ecs->event_thread->control.step_range_start) > + { > + stop_pc = update_line_range_start (stop_pc, ecs); > + ecs->event_thread->control.step_range_start = stop_pc; > + } > + } > + > keep_going (ecs); > } > > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 27611a34ec4..91d35616eb9 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -3282,6 +3282,55 @@ find_pc_line (CORE_ADDR pc, int notcurrent) > return sal; > } > > +/* Compare two symtab_and_line entries. Return true if both have > + the same line number and the same symtab pointer. That means we > + are dealing with two entries from the same line and from the same > + source file. > + > + Return false otherwise. */ > + > +static bool > +sal_line_symtab_matches_p (const symtab_and_line &sal1, > + const symtab_and_line &sal2) > +{ > + return (sal1.line == sal2.line && sal1.symtab == sal2.symtab); > +} > + > +/* See symtah.h. */ > + > +gdb::optional > +find_line_range_start (CORE_ADDR pc) > +{ > + struct symtab_and_line current_sal = find_pc_line (pc, 0); > + > + if (current_sal.line == 0) > + return {}; > + > + struct symtab_and_line prev_sal = find_pc_line (current_sal.pc - 1, 0); > + > + /* If the previous entry is for a different line, that means we are already > + at the entry with the start PC for this line. */ > + if (!sal_line_symtab_matches_p (prev_sal, current_sal)) > + return current_sal.pc; > + > + /* Otherwise, keep looking for entries for the same line but with > + smaller PC's. */ > + bool done = false; > + CORE_ADDR prev_pc; > + while (!done) > + { > + prev_pc = prev_sal.pc; > + > + prev_sal = find_pc_line (prev_pc - 1, 0); > + > + /* Did we notice a line change? If so, we are done with the search. */ > + if (!sal_line_symtab_matches_p (prev_sal, current_sal)) > + done = true; > + } > + > + return prev_pc; > +} > + > /* See symtab.h. */ > > struct symtab * > diff --git a/gdb/symtab.h b/gdb/symtab.h > index 404d0ab30a8..f54305636da 100644 > --- a/gdb/symtab.h > +++ b/gdb/symtab.h > @@ -2346,6 +2346,22 @@ extern struct symtab_and_line find_pc_line (CORE_ADDR, int); > extern struct symtab_and_line find_pc_sect_line (CORE_ADDR, > struct obj_section *, int); > > +/* Given PC, and assuming it is part of a range of addresses that is part of a > + line, go back through the linetable and find the starting PC of that > + line. > + > + For example, suppose we have 3 PC ranges for line X: > + > + Line X - [0x0 - 0x8] > + Line X - [0x8 - 0x10] > + Line X - [0x10 - 0x18] > + > + If we call the function with PC == 0x14, we want to return 0x0, as that is > + the starting PC of line X, and the ranges are contiguous. > +*/ > + > +extern gdb::optional find_line_range_start (CORE_ADDR pc); > + > /* Wrapper around find_pc_line to just return the symtab. */ > > extern struct symtab *find_pc_line_symtab (CORE_ADDR); > diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.c b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c > new file mode 100644 > index 00000000000..412ab180943 > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.c > @@ -0,0 +1,36 @@ > +/* Copyright 2008-2023 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . > + > + This test is used to test the reverse-step and reverse-next instruction > + execution for a source line that contains multiple function calls. */ > + > +void > +func1 () > +{ > +} // END FUNC1 > + > +void > +func2 () > +{ > +} // END FUNC2 > + > +int main () > +{ > + int a, b; > + a = 1; > + b = 2; > + func1 (); func2 (); > + a = a + b; // START REVERSE TEST > +} > diff --git a/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp > new file mode 100644 > index 00000000000..4eae042a6bf > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/func-map-to-same-line.exp > @@ -0,0 +1,146 @@ > +# Copyright 2008-2023 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . */ > + > +# This file is part of the GDB testsuite. It tests reverse stepping. > +# Lots of code borrowed from "step-test.exp". > + > +# This test checks to make sure there is no regression failures for > +# the reverse-next command when stepping back over two functions in > +# the same line. > + > +require supports_reverse > + > +# This test uses the gcc no-column-info command which was added in gcc 7.1. > +require get_compiler_info "gcc-7-*" > + > +proc run_tests {msg} { > + global srcfile > + global executable > + > + runto_main > + set target_remote [gdb_is_target_remote] > + > + gdb_test_no_output "record" "turn on process record" > + > + # This regression test verifies the reverse-step and reverse-next commands > + # work properly when executing backwards thru a source line containing > + # two function calls on the same source line, i.e. func1 (); func2 (); > + # This test is compiled so the dwarf info not contain the line table > + # information. > + > + # Test 1, reverse-next command > + # Set breakpoint at the line after the function calls. > + set bp_start_reverse_test [gdb_get_line_number "START REVERSE TEST" \ > + $srcfile] > + gdb_breakpoint $srcfile:$bp_start_reverse_test temporary > + > + # Continue to break point for reverse-next test. > + # Command definition: reverse-next [count] > + # Run backward to the beginning of the previous line executed in the > + # current (innermost) stack frame. If the line contains function calls, > + # they will be “un-executed” without stopping. Starting from the first > + # line of a function, reverse-next will take you back to the caller of > + # that function, before the function was called, just as the normal next > + # command would take you from the last line of a function back to its > + # return to its caller 2 . > + gdb_continue_to_breakpoint \ > + "$msg: test1: stopped at command reverse-next test start location" \ > + ".*$srcfile:$bp_start_reverse_test\r\n.*" > + > + # The reverse-next should step all the way back to the beginning of the > + # line, i.e. at the beginning of the func1 call. > + gdb_test "reverse-next" ".*func1 \\(\\); func2 \\(\\);.*" \ > + "$msg: test1: reverse-next to line with two functions" > + > + # We should be stopped at the first instruction of the line. A reverse-step > + # should step back and stop at the beginning of the previous line b = 2, > + # i.e. not in func1 (). > + gdb_test "reverse-stepi" ".*b = 2;.*" \ > + "$msg: test1: reverse-stepi to previous line b = 2" > + > + > + # Setup for test 2 > + clean_restart $executable > + runto_main > + > + gdb_test_no_output "record" "turn on process record" > + > + # Test 2, reverse-step command > + # Set breakpoint at the line after the function calls. > + gdb_breakpoint $srcfile:$bp_start_reverse_test temporary > + > + # Continue to the start of the reverse-step test. > + # Command definition: reverse-step [count] > + # Run the program backward until control reaches the start of a > + # different source line; then stop it, and return control to gdb. > + # Like the step command, reverse-step will only stop at the beginning > + # of a source line. It “un-executes” the previously executed source > + # line. If the previous source line included calls to debuggable > + # functions, reverse-step will step (backward) into the called function, > + # stopping at the beginning of the last statement in the called > + # function (typically a return statement). Also, as with the step > + # command, if non-debuggable functions are called, reverse-step will > + # run thru them backward without stopping. > + > + gdb_continue_to_breakpoint \ > + "$msg: test2: stopped at command reverse-step test start location" \ > + ".*$srcfile:$bp_start_reverse_test\r\n.*" > + > + # The first reverse step should take us call of func2 (). > + gdb_test "reverse-step" ".*END FUNC2.*" \ > + "$msg: test2: reverse-step into func2 " > + > + # The second reverse step should take us into func1 (). > + gdb_test "reverse-step" ".*END FUNC1.*" \ > + "$msg: test2: reverse-step into func1 " > + > + # The third reverse step should take us call of func1 (). > + gdb_test "reverse-step" ".*func1 \\(\\); func2 \\(\\);.*" \ > + "$msg: test2: reverse-step to line func1(); func2(), at call for func1 " > + > + # We should be stopped at the first instruction of the line. A reverse > + # stepi should take us to b = 2 (). > + gdb_test "reverse-stepi" ".*b = 2;.*" \ > + "$msg: test2: reverse-stepi to line b = 2 " > +} > + > +set srcfile func-map-to-same-line.c > +set executable func-map-to-same-line > + > +# test with gcc column info enabled > +set options [list debug additional_flags=] > + > +if {[build_executable "failed to prepare" $executable $srcfile $options] == -1}\ > + { > + return -1 > +} > + > +clean_restart $executable > + > +run_tests {"with-column-info"} > + > + > +#test with gcc column info disabled > +set options [list debug additional_flags=-gno-column-info] > + > +if {[build_executable "failed to prepare" $executable $srcfile $options] == -1}\ > + { > + return -1 > +} > + > +set $executable executable_without_column_info > +clean_restart $executable > + > +run_tests {"no-column-info"} > diff --git a/gdb/testsuite/gdb.reverse/map-to-same-line.c b/gdb/testsuite/gdb.reverse/map-to-same-line.c > new file mode 100644 > index 00000000000..f20d778f40e > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.c > @@ -0,0 +1,58 @@ > +/* Copyright 2008-2023 Free Software Foundation, Inc. > + > + This program is free software; you can redistribute it and/or modify > + it under the terms of the GNU General Public License as published by > + the Free Software Foundation; either version 3 of the License, or > + (at your option) any later version. > + > + This program is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + GNU General Public License for more details. > + > + You should have received a copy of the GNU General Public License > + along with this program. If not, see . */ > + > +/* The purpose of this test is to create a DWARF line table that contains two > + or more entries for the same line. When stepping (forwards or backwards), > + GDB should step over the entire line and not just a particular entry in the > + line table. */ > + > +int > +main () > +{ /* TAG: main prologue */ > + asm ("main_label: .globl main_label"); > + int i = 1, j = 2, k; > + float f1 = 2.0, f2 = 4.1, f3; > + const char *str_1 = "foo", *str_2 = "bar", *str_3; > + > + asm ("line1: .globl line1"); > + k = i; f3 = f1; str_3 = str_1; /* TAG: line 1 */ > + > + asm ("line2: .globl line2"); > + k = j; f3 = f2; str_3 = str_2; /* TAG: line 2 */ > + > + asm ("line3: .globl line3"); > + k = i; f3 = f1; str_3 = str_1; /* TAG: line 3 */ > + > + asm ("line4: .globl line4"); > + k = j; f3 = f2; str_3 = str_2; /* TAG: line 4 */ > + > + asm ("line5: .globl line5"); > + k = i; f3 = f1; str_3 = str_1; /* TAG: line 5 */ > + > + asm ("line6: .globl line6"); > + k = j; f3 = f2; str_3 = str_2; /* TAG: line 6 */ > + > + asm ("line7: .globl line7"); > + k = i; f3 = f1; str_3 = str_1; /* TAG: line 7 */ > + > + asm ("line8: .globl line8"); > + k = j; f3 = f2; str_3 = str_2; /* TAG: line 8 */ > + > + asm ("main_return: .globl main_return"); > + k = j; f3 = f2; str_3 = str_2; /* TAG: main return */ > + > + asm ("end_of_sequence: .globl end_of_sequence"); > + return 0; /* TAG: main return */ > +} > diff --git a/gdb/testsuite/gdb.reverse/map-to-same-line.exp b/gdb/testsuite/gdb.reverse/map-to-same-line.exp > new file mode 100644 > index 00000000000..02f3f4d8c9c > --- /dev/null > +++ b/gdb/testsuite/gdb.reverse/map-to-same-line.exp > @@ -0,0 +1,156 @@ > +# Copyright 2008-2023 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + > +# When stepping (forwards or backwards), GDB should step over the entire line > +# and not just a particular entry in the line table. This test was added to > +# verify the find_line_range_start function properly sets the step range for a > +# line that consists of multiple statements, i.e. multiple entries in the line > +# table. This test creates a DWARF line table that contains two entries for > +# the same line to do the needed testing. > + > +# This test can only be run on targets which support DWARF-2 and use gas. > +require dwarf2_support > +load_lib dwarf.exp > + > +# The DWARF assembler requires the gcc compiler. > +require is_c_compiler_gcc > + > +# This test suitable only for process that can do reverse execution > +requires supports_reverse > + > +standard_testfile .c .S > + > +if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile}] } { > + return -1 > +} > + > +set asm_file [standard_output_file $srcfile2] > +Dwarf::assemble $asm_file { > + global srcdir subdir srcfile > + declare_labels integer_label L > + > + # Find start address and length of program > + lassign [function_range main [list ${srcdir}/${subdir}/$srcfile]] \ > + main_start main_len > + set main_end "$main_start + $main_len" > + > + cu {} { > + compile_unit { > + {language @DW_LANG_C} > + {name map-to-same-line.c} > + {stmt_list $L DW_FORM_sec_offset} > + {low_pc 0 addr} > + } { > + subprogram { > + {external 1 flag} > + {name main} > + {low_pc $main_start addr} > + {high_pc $main_len DW_FORM_data4} > + } > + } > + } > + > + lines {version 2 default_is_stmt 1} L { > + include_dir "${srcdir}/${subdir}" > + file_name "$srcfile" 1 > + > + # Generate the line table program with distinct source lines being > + # mapped to the same line entry. Line 1, 5 and 8 contain 1 statement > + # each. Line 2 contains 2 statements. Line 3 contains 3 statements. > + program { > + DW_LNE_set_address $main_start > + line [gdb_get_line_number "TAG: main prologue"] > + DW_LNS_copy > + DW_LNE_set_address line1 > + line [gdb_get_line_number "TAG: line 1" ] > + DW_LNS_copy > + DW_LNE_set_address line2 > + line [gdb_get_line_number "TAG: line 2" ] > + DW_LNS_copy > + DW_LNE_set_address line3 > + line [gdb_get_line_number "TAG: line 2" ] > + DW_LNS_copy > + DW_LNE_set_address line4 > + line [gdb_get_line_number "TAG: line 3" ] > + DW_LNS_copy > + DW_LNE_set_address line5 > + line [gdb_get_line_number "TAG: line 3" ] > + DW_LNS_copy > + DW_LNE_set_address line6 > + line [gdb_get_line_number "TAG: line 3" ] > + DW_LNS_copy > + DW_LNE_set_address line7 > + line [gdb_get_line_number "TAG: line 5" ] > + DW_LNS_copy > + DW_LNE_set_address line8 > + line [gdb_get_line_number "TAG: line 8" ] > + DW_LNS_copy > + DW_LNE_set_address main_return > + line [gdb_get_line_number "TAG: main return"] > + DW_LNS_copy > + DW_LNE_set_address end_of_sequence > + DW_LNE_end_sequence > + } > + } > +} > + > +if { [prepare_for_testing "failed to prepare" ${testfile} \ > + [list $srcfile $asm_file] {nodebug} ] } { > + return -1 > +} > + > +runto_main > + > +# Print the line table > +gdb_test_multiple "maint info line-table ${testfile}" "" { > + -re "\r\n$decimal\[ \t\]+$decimal\[ \t\]+($hex)\[ \t\]+Y\[^\r\n\]*" { > + lappend is_stmt $expect_out(1,string) > + exp_continue > + } > + -re -wrap "" { > + } > +} > + > +# Do the reverse-step test > +gdb_test_no_output "record" "turn on process record" > + > +set bp_main_return [gdb_get_line_number "TAG: main return" $srcfile] > +gdb_breakpoint $srcfile:$bp_main_return > +gdb_continue_to_breakpoint "run to end of main, reverse-step test" ".*$srcfile:$bp_main_return.*" > +gdb_test "display \$pc" ".*pc =.*" "display pc, reverse-step test" > + > +# At this point, GDB has already recorded the execution up until the return > +# statement. Reverse-step and test if GDB transitions between lines in the > +# expected order. It should reverse-step across lines 8, 5, 3, 2 and 1. > +foreach line {8 5 3 2 1} { > + gdb_test "reverse-step" ".*TAG: line $line.*" "reverse step to line $line" > +} > + > +## Clean restart, test reverse-next command > +clean_restart ${testfile} > +runto_main > +gdb_test_no_output "record" "turn on process record, reverst-next test" > + > +set bp_main_return [gdb_get_line_number "TAG: main return" $srcfile] > +gdb_breakpoint $srcfile:$bp_main_return > +gdb_continue_to_breakpoint "run to end of main, reverse-next test" ".*$srcfile:$bp_main_return.*" > +gdb_test "display \$pc" ".*pc =.*" "display pc, reverse-next test" > + > +# At this point, GDB has already recorded the execution up until the return > +# statement. Reverse-next and test if GDB transitions between lines in the > +# expected order. It should reverse-next across lines 8, 5, 3, 2 and 1. > +foreach line {8 5 3 2 1} { > + gdb_test "reverse-next" ".*TAG: line $line.*" "reverse next to line $line" > +} Other than the nit, this LGTM. Thanks for picking this one up and improving it to fix other issues. Reviewed-by: Luis Machado