From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id AVNcLQ4qp2b/MDUAWB0awg (envelope-from ) for ; Mon, 29 Jul 2024 01:35:10 -0400 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=HOTMAIL.DE header.i=@HOTMAIL.DE header.a=rsa-sha256 header.s=selector1 header.b=Ul/OW35O; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 89E511E0D0; Mon, 29 Jul 2024 01:35:10 -0400 (EDT) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id B39381E030 for ; Mon, 29 Jul 2024 01:35:06 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 56F7A385840B for ; Mon, 29 Jul 2024 05:35:06 +0000 (GMT) Received: from EUR05-VI1-obe.outbound.protection.outlook.com (mail-vi1eur05olkn2054.outbound.protection.outlook.com [40.92.90.54]) by sourceware.org (Postfix) with ESMTPS id 105F13858CDB for ; Mon, 29 Jul 2024 05:34:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 105F13858CDB Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=hotmail.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=hotmail.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 105F13858CDB Authentication-Results: server2.sourceware.org; arc=pass smtp.remote-ip=40.92.90.54 ARC-Seal: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1722231285; cv=pass; b=dGHKDgpx4w89E85cptZ05YOl9j5py5+fyHFLCrsOZ8PfZiCPqKxVeonZanTh4BR5799G1j9YI+l1f3IWj5QVujTRvjmhw4Q4hTmCTC8pUv6H1CeYAIlXQIvp+1XqYKTuibxjDKFcoIyATdXhGNvykYqVp5QMlrZw6OU7BCY6U8c= ARC-Message-Signature: i=2; a=rsa-sha256; d=sourceware.org; s=key; t=1722231285; c=relaxed/simple; bh=zrTx3rRWcVluiOza60yMlQOqIAuRNi5jElosdEefLc0=; h=DKIM-Signature:Message-ID:Date:Subject:To:From:MIME-Version; b=H7ruHxDbQfC26Jr3Utzjw9fS9NBwJqSSqUhcVArMXdLBP3eZLJ9vdAYmsLvJI7uUSStzFAkvFGUOXf4NLzRZgqRkete8cQphScUSeDx2b6AB7SMktTY05Zv0Xoxw3n1FMC5KQMbPt+Z/S3OsSt5AmGy9624VgqVLX5swy4EV9io= ARC-Authentication-Results: i=2; server2.sourceware.org ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=uTPM/x3KwYqFju8Lyqnv1XARfOMRh+15hjoSxnFhK0GbBonOoeef349j2iSq0aK5TM7AgOAqdpoHfDNLmNUfmrSkvUXBRcZU3ZCC77c10uvrN+r43CNL//WAEnlsY8nba6xjIx2oV5SA8JXJ3U6z5wGHF5KdPX73RcKvRwapoygMtWZLxfKnNs61QrYuvK3BnzANdlcEtohum8sSq6KNeMq5U9RpfrzkTt+kzFu2XFUkG0RzBxds8SCVsPg/t1RP7LuOQUTG1Fk90jjQkai4xqfmU0k4QrKBR8oXRnJtyEaIeIRAhOx9Qwz9WHuhaOz9Ekz+ZsXMe8PwBrAIOlvESw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; 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=UL5b1MAXP9U5jKSbE7w3XKUH1IMMnL8Hhh+MX3txbzc=; b=D4M5GYSfFpbx/3oXiHbCT/13ZpoZa+1Ykfaqz4h4nch379engzWLmh+ZHZjZOqB058eg6rK/blmm/ZVTOsGRRL5P7LodgUdaP1WAMCwA9Ns1KwBHNHZDnv5lDS4LLlGE/cUznFvtBB03bfSikwInA2UBOZAWbRZyQFk0BN7Z8mC1xEIl9m4+n7zJzslYFBRBgDlGoTkdBkZ4PHfP/BcFu9Fy3eXn58A2W11UDme1kwbx7Gw+Hv3l1Mrz4rwkf6kcXEn4wU+7qyUZj2ufzQhMetBZz4ApoTJ+HubVCxzZx3p5Y6JK4d+eCr7Fwja8ffh/Zb7T7UKVtq9a0Q9NKEL84w== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=HOTMAIL.DE; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=UL5b1MAXP9U5jKSbE7w3XKUH1IMMnL8Hhh+MX3txbzc=; b=Ul/OW35Oo0hf28W10xiCwxaUfnttKXPugQmSXnN4V7CRmo55y/XvB+rYVsXxcZkpuHl0lcW+F2dYXvNwkhobgZlNPUkq8y5o+ZdRJYUQI4y2ZEgBZhCmF2Wqbcze1EBLk4BYLzUQgCsjqnC+bGoERtYrbdk8nzJKpA7AbyRe7q7wy0q8VAhW6Z2LOYrId5hfAGLlsHQZe01Qfw6P1Ku6Wot2J0eqK3FT0IpFbP+HIWibfQ2fv3pMKbN1pCXrZOZZhnxi7Jlu2g7yGNbSEpWArKbKeK/FUxyrqPURDuS0QgdeX1nbWTQuQ128G/m/WAzL7x9c5dG6ekKPiZE1F2ANHA== Received: from AS8P193MB1285.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:333::21) by AS8P193MB2350.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:44a::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7807.27; Mon, 29 Jul 2024 05:34:37 +0000 Received: from AS8P193MB1285.EURP193.PROD.OUTLOOK.COM ([fe80::e65d:5dd2:5662:c61f]) by AS8P193MB1285.EURP193.PROD.OUTLOOK.COM ([fe80::e65d:5dd2:5662:c61f%6]) with mapi id 15.20.7807.026; Mon, 29 Jul 2024 05:34:37 +0000 Message-ID: Date: Mon, 29 Jul 2024 07:34:25 +0200 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 3/3] Fix range end handling of inlined subroutines To: Guinevere Larsen , "gdb-patches@sourceware.org" References: <7ee0cb7d-c142-4a16-9c00-a95d66f5c12f@redhat.com> Content-Language: en-US From: Bernd Edlinger In-Reply-To: <7ee0cb7d-c142-4a16-9c00-a95d66f5c12f@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-TMN: [aMeIJqQ59zNnlPYAohwZg3fGXqo8Yb/46kH9m5TszDalQ3nRhoXp3EZ96Ee6Ojar] X-ClientProxiedBy: FR3P281CA0109.DEUP281.PROD.OUTLOOK.COM (2603:10a6:d10:a3::9) To AS8P193MB1285.EURP193.PROD.OUTLOOK.COM (2603:10a6:20b:333::21) X-Microsoft-Original-Message-ID: <782d70c0-fecb-4851-b7e8-4e92334784c6@hotmail.de> MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: AS8P193MB1285:EE_|AS8P193MB2350:EE_ X-MS-Office365-Filtering-Correlation-Id: 2eb55d1c-13f7-47e3-2dd4-08dcaf9022d6 X-Microsoft-Antispam: BCL:0; ARA:14566002|5072599006|461199028|8060799006|19110799003|440099028|3412199025|4302099013|1602099012; X-Microsoft-Antispam-Message-Info: uDXYXg8n9P57+9Lv+Fw0eDxtnhecnu/vlYFuEqisBtU9gb0vqwRJ/OHHNVMqFsHQpzcrZ5BmDtaOm3Ca8iPORiPXLt4MHgNzZ41kitTBhbYQtrYWE35LcCEznIK/SPUgeVIq1kkCFHG6oE4tFVmbXk/HWPZZNlvDCQCFN3LoB0zC8dX+ccZ+LbLjgGb6oqxXwCevl1RaCzFM12BufaRH23NSHP3BGgFqc22O3S+WfebKJXlaxIbkYr8wdam6RViK5tf+wLC3R8eEbUoIFtezFFio19hXn6KcBDM9t57NqBaNa3+y5QXNEi3RS2pBWB3EUbu1912d7svLqt3KnWxEjQUAAZsVbeedtSftBIsXDxRrW90dGPRdzXYCRdgo7tjdlV7XPFgtNa/IdrpRiZmT7N6+HMhG3D8MtINvWEVtVkyQ5nqMcreN42qDfq0KD7zCMtbpIqJBFVl2i/9k1h9gsFr8JCmtAkj8WM6BTwjoCxaYezXSzMSSAH9mPB5RiqQmISZBN+O/CaU9Dj8YXBbs2AGyfWOyup0hXYLb6Wc3ELTSaEB6kweFDq9/uan/XXJlM9pC+gY3AS4w/s+DonF+nzaEAXvy0NDLJGHh7pUAOlPGrTP3S91HrS4HfjwqJJXcb+UXKp4auF9M9814lEMyvtAUf3qtuVogrTHeooo7NeDX+Glqpz/sGxwAnMiT4EI+F5OpPrCFyjMT0XsaaWKbHAjlFGH+0SoSoTHw9lEGP9+SZQw50dQGrwO2B9CbPXg5ReIOsFBzujWyEkXcuV0NCg== X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?LzRHMHQ0bWlkTG5PcGwvSmN2UG1VSmVJVDZlZEhETHBOOEFhYmpMR1dLS1d6?= =?utf-8?B?aTBNemhlaEcyTkt4d3NuYmtLcWZMK0dWM2JYQzNOUjZGck01ZlQxNTZKZVVT?= =?utf-8?B?S0FVeTl0enBjcmIwOUtrMC85bXNFR1hvRWxFSzkrR1FlV0c1RSs0cTZjRzkx?= =?utf-8?B?Z1hibGluWmxvdThmY2RNWlplMmpYc3pXaHBRRWlNUUVrVFlTOVlmNDlXSHN6?= =?utf-8?B?R0M4cmhDcVJPdlZVNUpKcHhnWHgwcmNRNnJFS3dPUmxHWGlsWjh2YmlvL1kz?= =?utf-8?B?enlDbVJ1cTNtRExYbWQ4cWlvMjZ3YldYVjByTlpkT2xBbTNzUFJKQmMzcEtm?= =?utf-8?B?dURlWXlHMkxJalFDR1MzVTg0K2t3QnJIcVdsSkFLVitlVnBFem5hemZ1TXd2?= =?utf-8?B?NHlUbHczYUt3dWxia3ZXS3lzbWZDeU91K1Y2RzRuVDY0dEhnSDJRd0tLSXdJ?= =?utf-8?B?NXhIK1FUdFBIellkeERJd241R09nc1FEVDQrU0w1WWxUNzJ3alRpTmtMVWRC?= =?utf-8?B?TE5qWkVIQTdZZGY5Y3Zua2lMYUVTdDlkMzdkZ0pranhadUUweE81U2x2NVlF?= =?utf-8?B?TXI1SEtiZ2dzMmxWTE03OG8vbE5QeW5qVnhOVkhVTlhCNFN2bm1rV3ZrZk5h?= =?utf-8?B?Rm0xWmVMdEgyZ3MzMlErZGtyMzdEanFWZ3Z0Y0lldGdaRmxqYUwvUGZyVlpz?= =?utf-8?B?Qi9tMEdmakl3M05HbHlUOEdjbkI0U1B2QzlIQmxIWVBNRVdDdlZhWXJUQTh4?= =?utf-8?B?TE5JYzBHUUxINVN2L3poQ1N1cThXK0p2N0NaUnNMNHYyenA1UmZ4clc2R1pt?= =?utf-8?B?QzZBWlRNb3VrOWNLMmFOS2NPMGdCak9CZVZZZmhsdGI3Z0I1SnVpZVRDZGJx?= =?utf-8?B?QURDcUVkcWkrRlRXSDN1L1QySnBFK0tVZ1R0ZSsxZ3E4eHhLTzZCL2VYR1Jv?= =?utf-8?B?UTFrUUwwZnJsVkZvV0oxbjh2QWxqaWF5dWdXbi9QNHdJeEE4WmRsNWV0ck9E?= =?utf-8?B?VUNRVjlkTnhoMTJLbzlsYTQ2SXphMGFkenhndUFVVWR6bGdXek1uRXpDOE1t?= =?utf-8?B?anNWN1V0bmduZ2l1NkFOM1Q3SnllbFBDK1ViRmxhSkhXTzF3RlhOT1dFMVBV?= =?utf-8?B?bjQ1aHh1d0RZSXZJN2R0OFFtOVpWWStxdE8xeVg0MDNZZm1YMkR2WmJvVmN1?= =?utf-8?B?UHh6WDRzRnJmcS9MMEVoc24zaHJVUmNmZ0pwTjNhZnpVb2tWR0JYejVXQ2Va?= =?utf-8?B?MGhLT1F5Z2hyd2ptMVVFeEZwb2owMVVnSGpiYit6dXpxTjYwM0RtelBVcHBQ?= =?utf-8?B?MjQ5bmpXVG5aTm9sSGthaFFHUURuZTFUalRIeXBYeWlrcTN1dlRZU0FLbnln?= =?utf-8?B?dytNaGpJQjErSVorWVlvbWlrb1NXQzJEYTAyUzJsaU0zWUdWeEk0VllzWm84?= =?utf-8?B?aVowNDF4dVV5MnczSG05aEttVlh5WC9BOXp2Yi90YitoTFJjWkJsRHJmY2pP?= =?utf-8?B?OVp6b1Y5Z0pFSkNGWEthc2ZpdmtPMUNvMFJEaTM0aW9tU3U5RTk1UW9xSm1X?= =?utf-8?B?V3ErdG52WURENUpTbXdRVloxelpOVzBBajhZY0o5VWY5ZjQ4MS8yMUpXT0pt?= =?utf-8?B?Ump1ZHJiMkRKUmhCYjU5aElsTjNOWURpZzJXeXI4ZUZNaHQ4RzdRRWx4c1RF?= =?utf-8?B?YlpYNXhTM29RWkpZQWNZM1M1NExVSDkrZmc5YnRwdVUrODJ3Y0RVWUJXemk0?= =?utf-8?Q?/JCH3xUJLs8qHjG/ji35a67q9vNN8QTDSVLQtyW?= X-OriginatorOrg: sct-15-20-7741-18-msonline-outlook-01732.templateTenant X-MS-Exchange-CrossTenant-Network-Message-Id: 2eb55d1c-13f7-47e3-2dd4-08dcaf9022d6 X-MS-Exchange-CrossTenant-AuthSource: AS8P193MB1285.EURP193.PROD.OUTLOOK.COM X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2024 05:34:37.7931 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8P193MB2350 X-Spam-Status: No, score=-11.0 required=5.0 tests=BAYES_00, BODY_8BITS, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_PASS, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org On 7/19/24 20:48, Guinevere Larsen wrote: > On 7/5/24 6:18 AM, Bernd Edlinger wrote: >> Currently there is a problem when debugging >> optimized code when the inferior stops at an inline >> sub-range end PC.  It is unclear if that location >> is from the inline function or from the calling >> function.  Therefore the call stack is often >> wrong. >> >> This patch detects the "weak" line table entries >> which are likely part of the previous inline block, >> and if we have such a location, it assumes the >> location belongs to the previous block. >> >> Additionally it may happen that the infrun machinery >> steps from one inline range to another inline range >> of the same inline function.  That can look like >> jumping back and forth from the calling program >> to the inline function, while really the inline >> function just jumps from a hot to a cold section >> of the code, i.e. error handling. >> >> Additionally it may happen that one inline sub-range >> is empty or the inline is completely empty.  But >> filtering that information away is not the right >> solution, since although there is no actual code >> from the inline, it is still possible that variables >> from an inline function can be inspected here. > > Hi! Thanks for working on this. > > As I said on my reply to v2, I'd like to see the commits split differently. Some of my questions here may be redundant or obvious when I have less concurrent changes to keep in my head. > Not sure If I can do that, but I will of course give it a try, it will certainly take a while so please be patient. >> >> The issue with the empty ranges is also discussed here: >> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94474 >> >> Conceptually this patch uses a heuristic to work around >> a deficiency in the dwarf-4 and dwarf-5 rnglists structure. >> There should be a location view number for each inline >> sub-range begin PC and end PC, similar to the DW_AT_GNU_entry_view >> which is the location view for the inline entry point. >> --- >>   gdb/block.c                                   |  15 +- >>   gdb/dwarf2/read.c                             |  77 ++----- >>   gdb/infcmd.c                                  |   3 +- >>   gdb/infrun.c                                  |  30 ++- >>   gdb/symtab.c                                  |  17 +- >>   gdb/testsuite/gdb.base/empty-inline.c         |  39 ++++ >>   gdb/testsuite/gdb.base/empty-inline.exp       |  56 +++++ >>   gdb/testsuite/gdb.cp/empty-inline.cc          |  33 +++ >>   gdb/testsuite/gdb.cp/empty-inline.exp         |  50 +++++ >>   gdb/testsuite/gdb.cp/step-and-next-inline.cc  |   6 +- >>   gdb/testsuite/gdb.cp/step-and-next-inline.exp | 194 +++++++----------- >>   11 files changed, 322 insertions(+), 198 deletions(-) >>   create mode 100644 gdb/testsuite/gdb.base/empty-inline.c >>   create mode 100644 gdb/testsuite/gdb.base/empty-inline.exp >>   create mode 100644 gdb/testsuite/gdb.cp/empty-inline.cc >>   create mode 100644 gdb/testsuite/gdb.cp/empty-inline.exp >> >> diff --git a/gdb/block.c b/gdb/block.c >> index 511689c9738..d9c4435c9e2 100644 >> --- a/gdb/block.c >> +++ b/gdb/block.c >> @@ -197,7 +197,20 @@ blockvector_for_pc_sect (CORE_ADDR pc, struct obj_section *section, >>       return NULL; >>       if (pblock) >> -    *pblock = b; >> +    { >> +      struct symtab_and_line sal = find_pc_sect_line (pc, section, 0); >> +      if (sal.line != 0 && sal.pc == pc && sal.is_weak) >> +    { >> +      const struct block *b2 = find_block_in_blockvector (bl, pc - 1); >> +      const struct block *b0 = b; >> +      while (b0->superblock () && !b0->function ()) >> +        b0 = b0->superblock (); >> +      if (b0->contains (b2)) >> +        b = b2; >> +    } >> +      *pblock = b; >> +    } >> + >>     return bl; >>   } >>   diff --git a/gdb/dwarf2/read.c b/gdb/dwarf2/read.c >> index 60fd8b45eb5..e40679611fe 100644 >> --- a/gdb/dwarf2/read.c >> +++ b/gdb/dwarf2/read.c >> @@ -10733,10 +10733,6 @@ dwarf2_rnglists_process (unsigned offset, struct dwarf2_cu *cu, >>         return false; >>       } >>   -      /* Empty range entries have no effect.  */ >> -      if (range_beginning == range_end) >> -    continue; >> - >>         /* Only DW_RLE_offset_pair needs the base address added.  */ >>         if (rlet == DW_RLE_offset_pair) >>       { >> @@ -10855,10 +10851,6 @@ dwarf2_ranges_process (unsigned offset, struct dwarf2_cu *cu, dwarf_tag tag, >>         return 0; >>       } >>   -      /* Empty range entries have no effect.  */ >> -      if (range_beginning == range_end) >> -    continue; >> - >>         range_beginning = (unrelocated_addr) ((CORE_ADDR) range_beginning >>                           + (CORE_ADDR) *base); >>         range_end = (unrelocated_addr) ((CORE_ADDR) range_end >> @@ -11080,8 +11072,8 @@ dwarf2_get_pc_bounds (struct die_info *die, unrelocated_addr *lowpc, >>     if (ret == PC_BOUNDS_NOT_PRESENT || ret == PC_BOUNDS_INVALID) >>       return ret; >>   -  /* partial_die_info::read has also the strict LOW < HIGH requirement.  */ >> -  if (high <= low) >> +  /* partial_die_info::read has also the same low < high requirement.  */ > We use scream case to indicate variable names, so there's no need for this change. >> +  if (low > high || (low == high && die->tag != DW_TAG_inlined_subroutine)) >>       return PC_BOUNDS_INVALID; >>       /* When using the GNU linker, .gnu.linkonce. sections are used to >> @@ -18027,21 +18019,9 @@ class lnp_state_machine >>       /* Additional bits of state we need to track.  */ >>   -  /* The last file that we called dwarf2_start_subfile for. >> -     This is only used for TLLs.  */ >> -  unsigned int m_last_file = 0; >>     /* The last file a line number was recorded for.  */ >>     struct subfile *m_last_subfile = NULL; >>   -  /* The address of the last line entry.  */ >> -  unrelocated_addr m_last_address; >> - >> -  /* Set to true when a previous line at the same address (using >> -     m_last_address) had LEF_IS_STMT set in m_flags.  This is reset to false >> -     when a line entry at a new address (m_address different to >> -     m_last_address) is processed.  */ >> -  bool m_stmt_at_address = false; >> - >>     /* When true, record the lines we decode.  */ >>     bool m_currently_recording_lines = true; >>   @@ -18199,7 +18179,8 @@ dwarf_record_line_1 (struct gdbarch *gdbarch, struct subfile *subfile, >>     static void >>   dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile, >> -           unrelocated_addr address, struct dwarf2_cu *cu) >> +           unrelocated_addr address, struct dwarf2_cu *cu, >> +           bool end_sequence) >>   { >>     if (subfile == NULL) >>       return; >> @@ -18212,7 +18193,8 @@ dwarf_finish_line (struct gdbarch *gdbarch, struct subfile *subfile, >>             paddress (gdbarch, (CORE_ADDR) address)); >>       } >>   -  dwarf_record_line_1 (gdbarch, subfile, 0, address, LEF_IS_STMT, cu); >> +  dwarf_record_line_1 (gdbarch, subfile, end_sequence ? 0 : -1, address, >> +               LEF_IS_STMT, cu); >>   } >>     void >> @@ -18240,38 +18222,17 @@ lnp_state_machine::record_line (bool end_sequence) >>     /* For now we ignore lines not starting on an instruction boundary. >>        But not when processing end_sequence for compatibility with the >>        previous version of the code.  */ >> -  else if (m_op_index == 0 || end_sequence) >> -    { >> -      /* When we switch files we insert an end maker in the first file, >> -     switch to the second file and add a new line entry.  The >> -     problem is that the end marker inserted in the first file will >> -     discard any previous line entries at the same address.  If the >> -     line entries in the first file are marked as is-stmt, while >> -     the new line in the second file is non-stmt, then this means >> -     the end marker will discard is-stmt lines so we can have a >> -     non-stmt line.  This means that there are less addresses at >> -     which the user can insert a breakpoint. >> - >> -     To improve this we track the last address in m_last_address, >> -     and whether we have seen an is-stmt at this address.  Then >> -     when switching files, if we have seen a stmt at the current >> -     address, and we are switching to create a non-stmt line, then >> -     discard the new line.  */ >> -      bool file_changed >> -    = m_last_subfile != m_cu->get_builder ()->get_current_subfile (); >> -      bool ignore_this_line >> -    = ((file_changed && !end_sequence && m_last_address == m_address >> -        && ((m_flags & LEF_IS_STMT) == 0) >> -        && m_stmt_at_address) >> -       || (!end_sequence && m_line == 0)); > > Could you explain why you think its safe to remove this logic of adding fake entries to the line table to avoid missing out on some valid breakpoint locations? > > I can imagine this could cause issues with the is_stmt logic, but I would think the way forward is to have some more check on ignore_this_line, otherwise we're improving the situation on one side but making it somewhat worse on another. Have you tried tweaking this logic to accommodate for the is_weak flag? > I actually do not want to ignore any line, instead they may become weak line table entries, if necessary. So instead of simply skipping the call to dwarf_finish_line and the following even more important dwarf_record_line_1, the logic in dwarf_finish_line is changed to be non-destructive if the added last parameter is false, thus the ignore_this_line is morphed into "end_sequence || (m_flags & LEF_IS_STMT) != 0" at the call to dwarf_finish_line below. >> - >> -      if ((file_changed && !ignore_this_line) || end_sequence) >> +  else if ((m_op_index == 0 && m_line != 0) || end_sequence) >> +    { >> +      if (m_last_subfile != m_cu->get_builder ()->get_current_subfile () >> +      || end_sequence) >>       { >>         dwarf_finish_line (m_gdbarch, m_last_subfile, m_address, >> -                 m_currently_recording_lines ? m_cu : nullptr); >> +                 m_currently_recording_lines ? m_cu : nullptr, >> +                 end_sequence || (m_flags & LEF_IS_STMT) != 0); >>       } >>   -      if (!end_sequence && !ignore_this_line) >> +      if (!end_sequence) >>       { >>         linetable_entry_flags lte_flags = m_flags; >>         if (producer_is_codewarrior (m_cu)) >> @@ -18291,15 +18252,6 @@ lnp_state_machine::record_line (bool end_sequence) >>         m_last_line = m_line; >>       } >>       } >> - >> -  /* Track whether we have seen any IS_STMT true at m_address in case we >> -     have multiple line table entries all at m_address.  */ >> -  if (m_last_address != m_address) >> -    { >> -      m_stmt_at_address = false; >> -      m_last_address = m_address; >> -    } >> -  m_stmt_at_address |= (m_flags & LEF_IS_STMT) != 0; >>   } >>     lnp_state_machine::lnp_state_machine (struct dwarf2_cu *cu, gdbarch *arch, >> @@ -18313,8 +18265,7 @@ lnp_state_machine::lnp_state_machine (struct dwarf2_cu *cu, gdbarch *arch, >>          This is currently used by MIPS code, >>          cf. `mips_adjust_dwarf2_line'.  */ >>       m_address ((unrelocated_addr) gdbarch_adjust_dwarf2_line (arch, 0, 0)), >> -    m_flags (lh->default_is_stmt ? LEF_IS_STMT : (linetable_entry_flags) 0), >> -    m_last_address (m_address) >> +    m_flags (lh->default_is_stmt ? LEF_IS_STMT : (linetable_entry_flags) 0) >>   { >>   } >>   diff --git a/gdb/infcmd.c b/gdb/infcmd.c >> index 71514d5ba66..7598c9e3032 100644 >> --- a/gdb/infcmd.c >> +++ b/gdb/infcmd.c >> @@ -996,7 +996,8 @@ prepare_one_step (thread_info *tp, struct step_command_fsm *sm) >>             if (sym->aclass () == LOC_BLOCK) >>           { >>             const block *block = sym->value_block (); >> -          if (block->end () < tp->control.step_range_end) >> +          if (block->end () < tp->control.step_range_end >> +              && block->end () > tp->control.step_range_start) >>               tp->control.step_range_end = block->end (); >>           } >>           } >> diff --git a/gdb/infrun.c b/gdb/infrun.c >> index 1f32a63ad54..097b42c4e92 100644 >> --- a/gdb/infrun.c >> +++ b/gdb/infrun.c >> @@ -8114,6 +8114,31 @@ process_event_stop_test (struct execution_control_state *ecs) >>         return; >>       } >>   +  /* Handle the case when subroutines have multiple ranges. >> +     When we step from one part to the next part of the same subroutine, >> +     all subroutine levels are skipped again which begin here. >> +     Compensate for this by removing all skipped subroutines, >> +     which were already executing from the user's perspective.  */ >> + >> +  if (get_stack_frame_id (frame) >> +      == ecs->event_thread->control.step_stack_frame_id >> +      && inline_skipped_frames (ecs->event_thread) >> +      && ecs->event_thread->control.step_frame_id.artificial_depth > 0 >> +      && ecs->event_thread->control.step_frame_id.code_addr_p) >> +    { >> +      const struct block *prev, *curr; >> +      int depth = 0; >> +      prev = block_for_pc (ecs->event_thread->control.step_frame_id.code_addr); >> +      curr = block_for_pc (ecs->event_thread->stop_pc ()); >> +      while (curr && curr->inlined_p () && !curr->contains (prev)) >> +    { >> +      depth ++; >> +      curr = curr->superblock (); >> +    } >> +      while (inline_skipped_frames (ecs->event_thread) > depth) >> +    step_into_inline_frame (ecs->event_thread); >> +    } >> + >>     /* Look for "calls" to inlined functions, part one.  If the inline >>        frame machinery detected some skipped call sites, we have entered >>        a new inline function.  */ >> @@ -8172,6 +8197,8 @@ process_event_stop_test (struct execution_control_state *ecs) >>         infrun_debug_printf ("stepping through inlined function"); >>           if (ecs->event_thread->control.step_over_calls == STEP_OVER_ALL >> +      || ecs->event_thread->stop_pc () != stop_pc_sal.pc >> +      || !stop_pc_sal.is_stmt >>         || inline_frame_is_marked_for_skip (false, ecs->event_thread)) >>       keep_going (ecs); >>         else >> @@ -8220,7 +8247,8 @@ process_event_stop_test (struct execution_control_state *ecs) >>         end_stepping_range (ecs); >>         return; >>       } >> -      else if (*curr_frame_id == original_frame_id) >> +      else if (get_stack_frame_id (frame) >> +           == ecs->event_thread->control.step_stack_frame_id) >>       { >>         /* We are not at the start of a statement, and we have not changed >>            frame. >> diff --git a/gdb/symtab.c b/gdb/symtab.c >> index 41d71beec2a..957fe971d3c 100644 >> --- a/gdb/symtab.c >> +++ b/gdb/symtab.c >> @@ -3290,7 +3290,10 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) >>        0) instead of a real line.  */ >>           if (prev && prev->line >> -      && (!best || prev->unrelocated_pc () > best->unrelocated_pc ())) >> +      && (!best || prev->unrelocated_pc () > best->unrelocated_pc () >> +            || (prev->unrelocated_pc () == best->unrelocated_pc () >> +            && (best->pc (objfile) == pc >> +                ? !best->is_stmt : best->is_weak)))) >>       { >>         best = prev; >>         best_symtab = iter_s; >> @@ -3309,7 +3312,7 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) >>                && (tmp - 1)->unrelocated_pc () == tmp->unrelocated_pc () >>                && (tmp - 1)->line != 0 && !tmp->is_stmt) >>           --tmp; >> -          if (tmp->is_stmt) >> +          if (tmp->is_stmt && (tmp->pc (objfile) == pc || !tmp->is_weak)) >>           best = tmp; >>           } >>   @@ -3333,18 +3336,14 @@ find_pc_sect_line (CORE_ADDR pc, struct obj_section *section, int notcurrent) >>        We used to return alt->line - 1 here, but that could be >>        anywhere; if we don't have line number info for this PC, >>        don't make some up.  */ >> -      val.pc = pc; >> -    } >> -  else if (best->line == 0) >> -    { >> -      /* If our best fit is in a range of PC's for which no line >> -     number info is available (line number is zero) then we didn't >> -     find any valid line information.  */ > Why did you remove the logic handling not finding any line information? Surely it is still possible that we can't find debug information for other reasons. >> +      if (notcurrent) >> +    pc++; >>         val.pc = pc; >>       } >>     else >>       { >>         val.is_stmt = best->is_stmt; >> +      val.is_weak = best->is_weak; >>         val.symtab = best_symtab; >>         val.line = best->line; >>         val.pc = best->pc (objfile); >> diff --git a/gdb/testsuite/gdb.base/empty-inline.c b/gdb/testsuite/gdb.base/empty-inline.c >> new file mode 100644 >> index 00000000000..4ecb3ff14a3 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/empty-inline.c >> @@ -0,0 +1,39 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> +   Copyright 2024 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 .  */ >> + >> +static int test0 (void) >> +{ >> +  asm (""); /* line 20 */ >> +  return 1; /* line 21 */ > > In general, I'd like if these "tag" comments were more informative than a line. In here, you could use things like: > >  "empty asm inlined function" >  "return statement inlined function" > > And similar informative comments in this .c file and in the other proposed test. > I just have them here, because the line-numbers are checked in the empty-inline.exp file, so I do not forget that when the line numbers change that the empty-inline.exp file needs to be adjusted. >> +} >> + >> +int __attribute__((noinline, noclone)) >> +#ifdef __CET__ >> +  __attribute__((nocf_check)) >> +#endif >> +test1 (int x) >> +{ >> +  asm (""); >> +  return x+1; /* line 31 */ >> +} >> + >> +int >> +main() >> +{ test1 (test0 ()); /* line 36 */ >> +  test1 (test0 ()); /* line 37 */ >> +  return 0;         /* line 38 */ >> +} >> diff --git a/gdb/testsuite/gdb.base/empty-inline.exp b/gdb/testsuite/gdb.base/empty-inline.exp >> new file mode 100644 >> index 00000000000..0d9cfe922cf >> --- /dev/null >> +++ b/gdb/testsuite/gdb.base/empty-inline.exp >> @@ -0,0 +1,56 @@ >> +# Copyright 2024 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 . >> + >> +standard_testfile .c >> + >> +if [get_compiler_info] { >> +    return -1 >> +} >> + >> +if { [test_compiler_info gcc*] && ![supports_statement_frontiers] } { > It would be nice to have an "untested" statement here. I was testing with clang and (the next test) just silently didn't run and took me a moment to understand why. >> +    return -1 >> +} >> + >> +global srcfile testfile >> + >> +set options {debug nowarnings optimize=-O2} >> +if { [supports_statement_frontiers] } { >> +    lappend options additional_flags=-gstatement-frontiers >> +} >> + >> +if { [prepare_for_testing "failed to prepare" $binfile \ >> +      $srcfile $options] } { >> +    return -1 >> +} >> + >> +if ![runto_main] { >> +    return >> +} >> + >> +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:36.*" "in main" >> +gdb_test_multiple "step" "step into test0" { >> +    -re ".*test0.*${srcfile}:20.*$::gdb_prompt $" { >> +    gdb_test "step" ".*line 21.*" $gdb_test_name >> +    } >> +    -re ".*test0.*${srcfile}:21.*$::gdb_prompt $" { >> +    pass $gdb_test_name >> +    } > > If I use clang to run this test, I still get a failure here. Clang's line table is completely different to gcc's, and so isn't affected by your patch at all, printing line 36 on the main function instead of line 20, and there is no entry for line 21. Considering this, I'd suggest checking for clang at the start and skipping the test (with an UNTESTED). > Ah, okay, thanks for testing this. >> +} >> +gdb_test "frame 1" "\\s*\\#1\\s+main.*${srcfile}:36.*" "frame1" >> +gdb_test "step" ".*test1.*${srcfile}:31.*" "step into test1" >> +gdb_test "frame 1" "\\s*\\#1.*in main.*${srcfile}:36.*" "frame2" >> +gdb_test "step" ".*main.*${srcfile}:37.*" "step back to main" >> +gdb_test "next" ".*return 0;.*" "step over test0+1" >> +gdb_test "frame 0" "\\s*\\#0\\s+main.*${srcfile}:38.*" "in main again" >> diff --git a/gdb/testsuite/gdb.cp/empty-inline.cc b/gdb/testsuite/gdb.cp/empty-inline.cc >> new file mode 100644 >> index 00000000000..a960d5f7ec0 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.cp/empty-inline.cc >> @@ -0,0 +1,33 @@ >> +/* This testcase is part of GDB, the GNU debugger. >> + >> +   Copyright 2024 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 .  */ >> + >> +/* PR 25987 */ >> +struct MyClass; >> +struct ptr { >> +    MyClass* get() { return t; }     /* line 21 */ >> +    MyClass* t; >> +}; >> +struct MyClass { void call(); }; >> +void MyClass::call() { >> +    *(volatile char*)-1 = 1;         /* line 26 */ >> +} >> +static void intermediate(ptr p) { >> +    p.get()->call();                 /* line 29 */ >> +} >> +int main() { >> +    intermediate(ptr{new MyClass}); >> +} >> diff --git a/gdb/testsuite/gdb.cp/empty-inline.exp b/gdb/testsuite/gdb.cp/empty-inline.exp >> new file mode 100644 >> index 00000000000..7848c963f59 >> --- /dev/null >> +++ b/gdb/testsuite/gdb.cp/empty-inline.exp >> @@ -0,0 +1,50 @@ >> +# Copyright 2024 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 . >> + >> +# PR 25987 >> +standard_testfile .cc >> + >> +if {![supports_statement_frontiers]} { > Similarly here, it would be nice to, one, emit an untested statement, and two, filter clang out even if it eventually supports statement frontiers. > Okay, sure. Thanks Bernd.