From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05on2048.outbound.protection.outlook.com [40.107.21.48]) by sourceware.org (Postfix) with ESMTPS id 4A6AF385781C for ; Thu, 10 Mar 2022 11:14:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4A6AF385781C Received: from DB6PR1001CA0039.EURPRD10.PROD.OUTLOOK.COM (2603:10a6:4:55::25) by AM6PR08MB3960.eurprd08.prod.outlook.com (2603:10a6:20b:ad::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5038.16; Thu, 10 Mar 2022 11:14:07 +0000 Received: from DB5EUR03FT048.eop-EUR03.prod.protection.outlook.com (2603:10a6:4:55:cafe::80) by DB6PR1001CA0039.outlook.office365.com (2603:10a6:4:55::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5061.22 via Frontend Transport; Thu, 10 Mar 2022 11:14:07 +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; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by DB5EUR03FT048.mail.protection.outlook.com (10.152.21.28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5061.22 via Frontend Transport; Thu, 10 Mar 2022 11:14:07 +0000 Received: ("Tessian outbound 341d209a0e52:v113"); Thu, 10 Mar 2022 11:14:07 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 11e85603e480f2cf X-CR-MTA-TID: 64aa7808 Received: from cadb01cecdeb.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id B95C407A-5A25-4215-9F06-B64E3E396124.1; Thu, 10 Mar 2022 11:14:01 +0000 Received: from EUR04-HE1-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id cadb01cecdeb.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 10 Mar 2022 11:14:01 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WtnazeFPyuPvQkjP+IDi5g0XGFeAm6IVxrTOjriRoHFCVpnFE4fVGuhIPTXpZ67iFBZP/wm5D/wsPuwLHh2Q7bb+OYyNdjvvFz7iGXTUJxUOezGwPCIfcC/hrC07RF0NvswGX3T1jq+vdUksB9e2n5yMB0gWslPrkObVgufPrS1HExAkRNqtTYLJcwK3bv3u06tZx9elxlXgnbZXe7cF9G8WYzv+uPvBmUEzHGQBsmTMpb8myZY8G3b0y9kG5PH+/Nolscn4b9LaGBsRE4rsrfVM4VOsju8uMq7kVAvfLRlxLz6Qo0pnTiU1XJ8tgmI07XOoy676bJS+qUAuZWo7YQ== 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=gAPzTFhy4SbwtfeD5qwX+crFKV9nYn9L/rgDhs+zDSA=; b=CDXLMUle3nl9sR7wGS1D7D94qBkpbysRk8h2dto8aJfBnf9Z3M21kLQl/z8XjSCer0bhg1wrxAnCLn3l45DvUxv0yfG+3a+7CQRzpqqNcv2h7Too/BzHsPrtzAO5N+Vy1RboH0nG69sAtpi3v7BbLXkQKrI3Nc/PQ4JT+5B9Xo0UKzXBW4jnZ7fia7/iuFv6uP8YePMAjdi8SMiMvDltNiFAI5X1duT0VWLLHbok2SMpYxRF87RsEDNyntsn2bz6OuitDGNqn5zhpc4O9MKFMQIr3cI/ddprOAJTcyfMFzJ7nN01mFWytLxR8ZeSpL2XZ9YNQwuG0gUpD+hG2fVpKg== 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 Authentication-Results-Original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com; Received: from AM6PR08MB3911.eurprd08.prod.outlook.com (2603:10a6:20b:80::27) by AM8PR08MB6370.eurprd08.prod.outlook.com (2603:10a6:20b:361::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5038.14; Thu, 10 Mar 2022 11:13:59 +0000 Received: from AM6PR08MB3911.eurprd08.prod.outlook.com ([fe80::fc12:d996:cca4:e61]) by AM6PR08MB3911.eurprd08.prod.outlook.com ([fe80::fc12:d996:cca4:e61%5]) with mapi id 15.20.5038.027; Thu, 10 Mar 2022 11:13:59 +0000 Message-ID: <81c5bece-012b-cad6-8349-71bb255ff242@arm.com> Date: Thu, 10 Mar 2022 11:13:57 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [PATCH] Updated, fix reverse stepping multiple contiguous PC ranges Content-Language: en-US To: Bruno Larsen , Carl Love , gdb-patches@sourceware.org Cc: Rogerio Alves , nd@arm.com References: <442684e3f81aa1df073960bd45918106acefa2b9.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: LO2P265CA0471.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a2::27) To AM6PR08MB3911.eurprd08.prod.outlook.com (2603:10a6:20b:80::27) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: f401a928-b67f-421e-2a22-08da02871824 X-MS-TrafficTypeDiagnostic: AM8PR08MB6370:EE_|DB5EUR03FT048:EE_|AM6PR08MB3960:EE_ X-Microsoft-Antispam-PRVS: 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: tWMk/wZI1z9lMefCCuxm7bq7AB/1Bkghj84ugqtqar9yTPwuCSFM3REJWOUAHncO8HdiD65nQ2hrnBwXNsk9U6aNl3gFF0r41Va4AsAYtePnV+FI1HKd0kWni6RiAneJ+Gb0hcdBKwELfrbf8cbxWHa3i9N+QxmLZI3324fhTWV7XAduv23FVhKkx0yF0/YPuTScP7LEGwWzekU88Mn0Kj1VX0Tu0yBC7KMDT6Bt04d1KmeVCA6R/4k3gEQbrAL9ngoXP0Ea9m6kA1ihDBkoFIzm6wDvhi274fW+WQRiFPLI02wUI4ptRUQBNqVXbDjP1GeFgUKRwFkNRBFpwf6rUjUMar4Y8r0EbxrI8XyMU0uwRsdjVo6vmsYKioaBjxZrswNRtQXqJq6LSZSvycKQZSVXc4ZGKl2F/Vbi+7TB9h31SrmlBuKQyZ11ARHN55Zw/J409+qIOBB/RISp6l2npZoocM2S2ptEYXmT22JBwOoogWnvm6+a0J3iV+4CZgxcEor00urTVwD96ooR7HCxz/EjnstJ6REauTxlgQbwm9t9hFan7lslgEUnWXKfgYiJGR9dV28gDlR6NDdlDJqcfBpuhiZK3Nwd6Y31En8kYdMPvTWRloh7Pr4J7rCZwpKdZVJYEBRK8bzI+J1phcdOtkmYM/fpPbma21wOfjIempw7Y1SJiRihhEtArlov4RgI8TK8vJdCdHTSRn4PRYpRc1FEZ8QYyP0OWkJTsIivvNvwbU6Efi5AvUIaK9xRuy7LUGX5FofikQkUDCMCz/JpOVhVI5n+9kIHGHWQ4hEJxHG4FM1ekkyJPMBSbF4MY/31 X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM6PR08MB3911.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230001)(4636009)(366004)(66946007)(66476007)(316002)(8676002)(66556008)(4326008)(84970400001)(6506007)(44832011)(2616005)(5660300002)(15650500001)(8936002)(186003)(26005)(6512007)(53546011)(31696002)(83380400001)(2906002)(38100700002)(86362001)(508600001)(31686004)(36756003)(6486002)(110136005)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8PR08MB6370 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: DB5EUR03FT048.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: 3c31a9da-4df3-480e-32be-08da028712ff X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: BYFSXZZEzDszoUxvQReiGRp21+LBh7KTa2J8oYTuQ/LaedLJ8CE372o52QczkYEQ2qk9WhaGcuMvmQ4pHWONP51SgZeVzToj083/2UDvWOXD5hP6hAWsBzgCutGAm/IYCsdzesnXuV/K7j+ibNqnm6kmpvXvWQcU+jkNVXsi2VKHJJkL+lw97+Ou1ltiYSGGfVqPwJ3FSpx5IR0ThwJkNDnczB0LFiV3/kxX8kWL0DjkkuHoXR1IZ1r0GbmvhgNf1amB2qLcl7ArBhkMhJ1CZY5HDtZgqcxblbdhS7KbAXWrejyoa7vWAiiijtH8H1OEA+KJWNO3/Fua4bLDq0+4b0StBOOo8BP3C9ENBuKAVPx4PwZoEMee8fVpeVJ6Z5Df63Wj3P1rm3CjddqfjfiLRakHhkp1/FZK9tVywiqDy1Xc8CGDqYRtvKkQGrsnEoFxfJtRKWKaoWu//aXjBlA3HZY8P1eTD7RRxqJaJjYT+Z2gw95yfS4dnBnkyCzL6twFKENuQuB2uASITSxiVw6Yv6osuJmd8CBVWph/qp6k+RRDp1qLUDgiAr0Mc7m9jk9lyca++efeyELRXYmanPkdhhY+Rm67Hrm88Zpm967IlHhsrhHydo9Kd9i2MarBpTFL7VzDXzb5HY4oTFp0PPHYA9IscrVOmz7aAMLkllVhJ0G3PcrVIyZfS3nCJumqb7wbqyzBNuRbaEHZtPNoEQSRvDmwjIJ8ies5yYtWJs8U4ut21v4waAU5quRA2OhihM4ywsQzwt389/sY/3dcs28xHrF0rZpVr7qwuHrW6A9s2TU= 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:(13230001)(4636009)(40470700004)(36840700001)(46966006)(8936002)(84970400001)(5660300002)(31686004)(4326008)(36756003)(40460700003)(508600001)(8676002)(82310400004)(70586007)(70206006)(44832011)(15650500001)(83380400001)(6486002)(86362001)(336012)(316002)(2906002)(110136005)(31696002)(6506007)(186003)(53546011)(26005)(2616005)(356005)(81166007)(6512007)(47076005)(36860700001)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2022 11:14:07.5090 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: f401a928-b67f-421e-2a22-08da02871824 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: DB5EUR03FT048.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB3960 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE, UNPARSEABLE_RELAY autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Mar 2022 11:14:16 -0000 Hi! On 3/8/22 20:21, Bruno Larsen via Gdb-patches wrote: > On 2/23/22 13:39, Carl Love via Gdb-patches wrote: >> >> GCC maintainers: >> >> The following patch was posted by Luis Machado on 2/1/2021.  There was >> a little discussion on the patch but it was never fully reviewed and >> approved.  The email for Luis no longer >> works. >> >> As of 2/21/2022, the patch does not compile.  I made a small fix to get >> it to compile.  I commented out the original line in gdb/infrun.c and >> added a new line with the fix and the comment //carll fix to indicate >> what I changed.  Clearly, the comment needs to be removed if the patch >> is accepted but I wanted to show what I changed. >> >> That said, I tested the patch on Powerpc and it fixed the 5 test >> failures in gdb.reverse/solib-precsave.exp and 5 test failures in >> gdb.reverse/solib-reverse.exp.  I tested on Intel 64-bit.  The two >> tests solib-precsave.exp and solib-reverse.exp both initially passed on >> Intel.  No additional regression failures were seen with the patch. >> >> Please let me know if you have comments on the patch or if it is >> acceptable as is.  Thank you. >> >>                       Carl Love > > Hello Carl! > > Thanks for looking at this. Since I don't test on aarch64 often, I am > not sure if I see regressions or racy testcases, but it does fix the > issue you mentioned, and there doesn't seem to be regressions on x86_64 > hardware. I have a few nits, but the main feedback is: could you add a > testcase for this, using the dwarf assembler and manually creating > contiguous PC ranges, so we can confirm that this is not regressed in > the future on any hardware? > > Also, I can't approve a patch, but with the testcase this patch is > mostly ok by me > >> ----------------------------------------------------------- >> From: Luis Machado >> Date: Mon, 21 Feb 2022 23:11:23 +0000 >> Subject: [PATCH] Fix reverse stepping multiple contiguous PC ranges >> >> When running GDB's testsuite on aarch64-linux/Ubuntu 20.04, 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 1 will land on line 2. >> - step from line 2 will land on line 3. >> >> Reverse execution: >> >> - step from line 3 will land on line 2. >> - step from line 2 will land on line 2. >> - step from line 2 will land on line 1. >> >> The problem here is that line 40 contains two contiguous but distinct >> PC ranges, like so: >> >> Line 40 - [0x7ec ~ 0x7f4] >> Line 40 - [0x7f4 ~ 0x7fc] >> >> When stepping forward from line 2, we skip both of these ranges and >> land on >> line 42. When stepping backward from line 3, we stop at the start PC >> of the >> second (or first, going backwards) range of line 40. >> >> This happens because we have this check in >> infrun.c:process_event_stop_test: >> >>        /* When stepping backward, stop at beginning of line range >>           (unless it's the function entry point, in which case >>           keep going back to the call point).  */ >>        CORE_ADDR stop_pc = ecs->event_thread->suspend.stop_pc; >>        if (stop_pc == ecs->event_thread->control.step_range_start >>            && stop_pc != ecs->stop_func_start >>            && execution_direction == EXEC_REVERSE) >>          end_stepping_range (ecs); >>        else >>          keep_going (ecs); >> >> Since we've reached ecs->event_thread->control.step_range_start, we stop >> stepping backwards. >> >> The right thing to do is to look 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. >> >> Another solution I thought about is to merge the contiguous ranges when >> we are reading the line tables. Though I'm not sure if we really want >> to process >> that data as opposed to keeping it as the compiler created, and then >> working >> around that. >> >> In any case, the following patch addresses this problem. >> >> I'm not particularly happy with how we go back in the ranges (using >> "pc - 1"). >> Feedback would be welcome. >> >> Validated on aarch64-linux/Ubuntu 20.04/18.04. >> >> Ubuntu 18.04 doesn't actually run into these failures because the >> compiler >> doesn't generate distinct PC ranges for the same line. >> >> gdb/ChangeLog: >> >> YYYY-MM-DD  Luis Machado >> >>          * infrun.c (process_event_stop_test): Handle backward stepping >>          across multiple ranges for the same line. >>          * symtab.c (find_line_range_start): New function. >>          * symtab.h (find_line_range_start): New prototype. >> >> >> Co-authored-by: Carl Love >> --- >>   gdb/infrun.c | 24 +++++++++++++++++++++++- >>   gdb/symtab.c | 35 +++++++++++++++++++++++++++++++++++ >>   gdb/symtab.h | 16 ++++++++++++++++ >>   3 files changed, 74 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/infrun.c b/gdb/infrun.c >> index 376a541faf6..997042d3e45 100644 >> --- a/gdb/infrun.c >> +++ b/gdb/infrun.c >> @@ -6782,11 +6782,33 @@ if (ecs->event_thread->control.proceed_to_finish >>        have software watchpoints).  */ >>         ecs->event_thread->control.may_range_step = 1; >> +      /* When we are stepping inside a particular line range, in >> reverse, >> +     and we are sitting at the first address of that range, we need to >> +     check if this address also shows up in another line range as the >> +     end address. >> + >> +     If so, we need to check what line such a step range points to. >> +     If it points to the same line as the current step range, that >> +     means we need to keep going in order to reach the first address >> +     of the line range.  We repeat this until we eventually get to the >> +     first address of a particular line we're stepping through.  */ >> +      CORE_ADDR range_start = >> ecs->event_thread->control.step_range_start; >> +      if (execution_direction == EXEC_REVERSE) >> +    { >> +      gdb::optional real_range_start >> +        //     = find_line_range_start >> (ecs->event_thread->suspend.stop_pc); >> +        = find_line_range_start (ecs->event_thread->stop_pc()); >> //carll fi> + >> + >> +      if (real_range_start.has_value ()) >> +        range_start = *real_range_start; >> +    } >> + >>         /* When stepping backward, stop at beginning of line range >>        (unless it's the function entry point, in which case >>        keep going back to the call point).  */ >>         CORE_ADDR stop_pc = ecs->event_thread->stop_pc (); >> -      if (stop_pc == ecs->event_thread->control.step_range_start >> +      if (stop_pc == range_start >>         && stop_pc != ecs->stop_func_start > > I think this could be moved to the line above. > Do you mean moving the range_start check downwards below the ecs->stop_func_start check? >>         && execution_direction == EXEC_REVERSE) >>       end_stepping_range (ecs); >> diff --git a/gdb/symtab.c b/gdb/symtab.c >> index 1a39372aad0..c40739919d1 100644 >> --- a/gdb/symtab.c >> +++ b/gdb/symtab.c >> @@ -3425,6 +3425,41 @@ find_pc_line (CORE_ADDR pc, int notcurrent) >>     return sal; >>   } >> +/* 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 (prev_sal.line != current_sal.line) >> +    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 (prev_sal.line != current_sal.line) >> +    done = true; > > Shouldn't prev_sal.line also be checked here and return an empty > optional? I am not sure when that happens, so please enlighten me if > there is no need to check. > I went through this again, and I don't think prev-sal.line needs to be checked. At this point we know current_sal.line is sane, so anything that differs from the current line, we bail out and return the address we currently have. Does that make sense?