From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10080.outbound.protection.outlook.com [40.107.1.80]) by sourceware.org (Postfix) with ESMTPS id B8BD23858404 for ; Thu, 10 Mar 2022 13:34:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B8BD23858404 Received: from AM0PR02CA0126.eurprd02.prod.outlook.com (2603:10a6:20b:28c::23) by VI1PR0802MB2237.eurprd08.prod.outlook.com (2603:10a6:800:9c::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5038.15; Thu, 10 Mar 2022 13:33:50 +0000 Received: from VE1EUR03FT017.eop-EUR03.prod.protection.outlook.com (2603:10a6:20b:28c:cafe::e9) by AM0PR02CA0126.outlook.office365.com (2603:10a6:20b:28c::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5038.14 via Frontend Transport; Thu, 10 Mar 2022 13:33:50 +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 VE1EUR03FT017.mail.protection.outlook.com (10.152.18.90) 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 13:33:49 +0000 Received: ("Tessian outbound 2877e54fe176:v113"); Thu, 10 Mar 2022 13:33:49 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 6b7a5831c9c3d423 X-CR-MTA-TID: 64aa7808 Received: from 964cd83a21df.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 8CE78ED6-78CD-414E-87E3-9282DF32E59C.1; Thu, 10 Mar 2022 13:33:43 +0000 Received: from EUR05-AM6-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 964cd83a21df.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 10 Mar 2022 13:33:43 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aS6toqt86S30soJIhIYLrFDiH6XYmuMCnbhtjRZ9mRDGOHh65r8AaDELRiDTgQFWdFghvy4Q7tYcrDfGzZ6SRr/KME47jMwWjc4mgdq1pNDq9sTejfH44vr+q19w92apl60etKNJKLp48/reKneVFA9mkrIMV+92bvuy9Rv0L/PlN5xP5ys1kCcKHaOVkQuCD33GNyBWDXLYK4ayWoO5p7gteCvJAqUCzTcctTg7XI3C0f5DXc/NQlxoTb5oBzGgMdY0A590X6uOAUpe9TNPZty7HYdKEYYYvgzEmcTsFr58YQh0olzrnmuEdpp/QEKgv1506PImsI+WIbem8QlyTQ== 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=EWwzw79JPuXK+sdiVpsy+E+XnJNQXytjxO6ZgUvAIpU=; b=DUqxByfeK93CHCajyyf39kPpjCPnbe7cpLKCJo+dQkANiE0WHUr3/DoMevGaQfqULx/6dDD5Ql/D3BzgW9JsW2do6hrQcBsv37djv7cavZjDEs5Tnp6aE7DZ5GMPz/qxJNvC0sUo640ZidduNye2X9kUaJtdgw8eweDjqlVEAITZZj7vASTL3ZShzPu4RsO3EpLaKdCG+4kinNsy1wb7oc3Ppng2h/kAsvamBbxY21RjaYCFkwD6c0+8z3mp5NbQcwSZrEoOMa9pgrgOi9QDmtGhsqlihm9gRTWNkLmNIRJdpLO6ZmdQ6p3+38q9RoAylqqF4uYNhLUIMYQCjdfv5A== 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 AM6PR08MB4916.eurprd08.prod.outlook.com (2603:10a6:20b:ca::17) 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 13:33:37 +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 13:33:36 +0000 Message-ID: <4fb3d2f7-944e-996a-1b6a-2bf7ee3e04f4@arm.com> Date: Thu, 10 Mar 2022 13:33:33 +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, simon.marchi@polymtl.ca References: <442684e3f81aa1df073960bd45918106acefa2b9.camel@us.ibm.com> <81c5bece-012b-cad6-8349-71bb255ff242@arm.com> <49264d6f-d66c-774e-f57b-9eb2efd767d5@redhat.com> From: Luis Machado In-Reply-To: <49264d6f-d66c-774e-f57b-9eb2efd767d5@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-ClientProxiedBy: LO2P265CA0104.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:c::20) To AM6PR08MB3911.eurprd08.prod.outlook.com (2603:10a6:20b:80::27) MIME-Version: 1.0 X-MS-Office365-Filtering-Correlation-Id: 29fe81db-26ac-4111-cf8a-08da029a9c75 X-MS-TrafficTypeDiagnostic: AM6PR08MB4916:EE_|VE1EUR03FT017:EE_|VI1PR0802MB2237: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: zceWoY5nlC47MZzradzDUJUm465paOwy70WJc6aDjRs7KUolt0ll8tBDcspYgjRPuWMbTm9McO0JYiAKIOERNdKyP/e/KmybFt7Ku5LTUsRTKfD8j7IUOViW5llJ0741HFhSfkrkyCfnqDantLqRhps86gyKfyhrIgTqEYA5+FxNaYlkV9Q+KUV51uWkdOPzwTrBISF1/4rCo+eFnpgK1rmqNKzfvZXEjjKbkHRe9mX0NeXzr7lFHFEyKFoPZbgevaAzsojbXJA+2z1JL+O/vhFZSvHYtQoQs3p3XcPvCPtcTuS+ftZBL0wmyFeB1WqJLuct0yf6TccuurWQXP3eeEqDIa1gJWZyT1y7qOKH6Li+vrPWLQLarppB8lqZ78Ip/CbZEBjSJkZoOnJJcqDvT4HvdI5LKijYNlZhiQiP7T3bQ8MlDnutdBQSlBpdvSNUeLd/C296bdHnCMOoBbQ9O7xHAyBYt017iJChvltu4ZC8sbqODZDhgKggivgOH4DycZ2JVsYYteAPr5TYzuBXCCpQFx1WNCF8aamlB5Wgc7q6KKDr81AKMIjYIEZyNMoboP+71E9U6wqgtNjU3fHmf4uhxM52CafJez3oAlH6buGHIax9IPRsAoHJPaBvtU+HdrwYlOJxTxQm+9Rrm71ctOas/+TxlzwV/6pEI7Rw4Z5ACSUQjRp6ly7RjaJV/WGD88cwy4AmESmTmnNJJuY+3wScDE7QkRfboQ1YU7nbGC/UaHCjlYVytocQ00QNykIn97ouLAUEmHAaskrUMKrMtD0nfmbsuRr2biMRO9Q67ukVFxBzGDXk8Z2McLBjdcoxrToykPGltmeILdJcmR4ukg== 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)(31696002)(53546011)(2906002)(31686004)(4326008)(508600001)(8676002)(66476007)(316002)(66556008)(66946007)(83380400001)(6486002)(15650500001)(44832011)(110136005)(6506007)(86362001)(38100700002)(8936002)(36756003)(186003)(6512007)(84970400001)(6666004)(2616005)(26005)(5660300002)(45980500001)(43740500002); DIR:OUT; SFP:1101; X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB4916 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: VE1EUR03FT017.eop-EUR03.prod.protection.outlook.com X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id-Prvs: dcd8c63d-724e-4432-4d48-08da029a93a1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 8ABNDshq9f8Jd7oRmSEdKYxABRLpEZkILEQz8xLjpxushKKECZkoT/bKMm1/BuVdzU5qJZVGQmyT9O3/78NbgGrOcWnET4rGzaT86gb4jZ3vNkUBUeUPuRXPwclcG8Ye4Um/D9hlV5/B5DXyT299WqyOJDtUnYovGHKuM5GimPIAwSbJ/swbV4fP92dHu8HPzOF6JkvJLkLyMxskJtM7JMR5sMTgW52OmV1Y58D+6f8Numn0UXTh/EMQXZO5EviF98DPe7HkUG4p0d1QOtjcvB2NWOSp+hl/QytKtY9xksAbCYoQ7NR+PR0RJom0COkloKkpTbSE5NetJyVKWTxXAkDxHCL+cxcK1wuJilrYliotzuAcRZj082l8as7YZGorBoaDeO48o3e9q5/aqxxaVw67KveZNYmk8ego6BS3rF+tIwz+25ngD4Em5XHCLmjEuthIcehwLG1vZW3U5Qn6HDjg7fPbfpIcf1Y0Fgfnv+xxjEXJIS5+o4MQ6PJR0YdwOD99QTLwl0jBKN+2w5E4I26ZuPLXhEJO9f4+NHqiGztsQGQpPfkbeO01wWKo2AD1NpVwP0uAmCILNYBk4wa4P4eFlI/z3kq0ZkwnRSyicNZxtGCIHG44YfIHFxcYqVusH19zVdgNmirP65nSfHKWem8HgPCHhWGtifR0MsRXc9ToK5cBTdZnU8EqkMVUfG/m4xk5PK/UOsg4vGJ9yUhZ/1+hB7qToaisOqwXMVKMbv+OzD4wKzHUaHTFnnfF1gQsLXIj3iGi4bGzWTOAATp/EM6b16ld1oiZ7j7ylVmXwqQ= 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)(46966006)(36840700001)(8936002)(316002)(8676002)(4326008)(70586007)(70206006)(47076005)(186003)(26005)(81166007)(356005)(107886003)(31686004)(508600001)(5660300002)(44832011)(6486002)(6666004)(6506007)(53546011)(36756003)(15650500001)(6512007)(82310400004)(84970400001)(2906002)(110136005)(336012)(83380400001)(2616005)(36860700001)(86362001)(31696002)(40460700003)(43740500002); DIR:OUT; SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Mar 2022 13:33:49.8126 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 29fe81db-26ac-4111-cf8a-08da029a9c75 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: VE1EUR03FT017.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0802MB2237 X-Spam-Status: No, score=-11.9 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 13:34:04 -0000 On 3/10/22 13:19, Bruno Larsen wrote: > On 3/10/22 08:13, Luis Machado wrote: >> 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? > > I meant that they can be in the same line: >     if (stop_pc == range_start && stop_pc != ecs->stop_func_start > Got it. I can do that. >> >>>>         && 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? >> > It does.  My main point was asking if finding a 0 was cause for leaving > with an error, instead of leaving with the answer that could be wrong. > But the base answer is already wrong, so this would probably be a less > wrong answer, so this is fine > Finding a line 0 could be either valid or invalid. If the line table is sane and we find 0 because there is no line number for a particular PC, it is valid, and we've reached the end of the lookup. If we find 0 because the line table is missing an entry for the PC we're looking for, then the answer is incorrect. But that would be a problem with the line table. Not much GDB can do other than warn. I'll refresh/re-test this patch and will address some previous comments Simon had.