From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id VXzeCPDn7F+ZEwAAWB0awg (envelope-from ) for ; Wed, 30 Dec 2020 15:49:52 -0500 Received: by simark.ca (Postfix, from userid 112) id 15DC71F0AA; Wed, 30 Dec 2020 15:49:52 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: * X-Spam-Status: No, score=1.9 required=5.0 tests=FORGED_MUA_MOZILLA, FREEMAIL_FROM,MAILING_LIST_MULTI,MSGID_FROM_MTA_HEADER,RDNS_NONE, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.2 Received: from sourceware.org (unknown [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 950FF1E965 for ; Wed, 30 Dec 2020 15:49:51 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 36B93385040F; Wed, 30 Dec 2020 20:49:51 +0000 (GMT) Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-oln040092065019.outbound.protection.outlook.com [40.92.65.19]) by sourceware.org (Postfix) with ESMTPS id A7323385040F for ; Wed, 30 Dec 2020 20:49:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org A7323385040F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=hotmail.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=bernd.edlinger@hotmail.de ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=AinWEODU03gLTKhpgJjCZ4mk6F3MtHM9rpW66TxaviYJhZiewPPfWbUSLCZRmczBmUQ/4+tY9M5phoLwykJX7nCqhNO9SjtPdfCYJfyv5oX+UAYgdfn1UXysgyaHWJqksbXsj8sWdIf8QqTrLFyddcnL21QoJoXrX5VjPQoLOLlwZHtf09lCDsXgGnWbaCJ+C+CCBPiOi64b9nZfuV1brp4TkIbEg4FIqpqclh7rzf6m0dyNXtE69GqFdC6+Ca7n4iOrNqXynzthlDv4XWO/D4zhftxaf6UWMKXMHvcNTUtl08BHjIwO4zEnnkNYcw7hakpS6N9V3OUcMug2hlhPhg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Zvrsmvlc+KVoWT0435cadc7NvwG2EA0k9yL3vtA2qjo=; b=FRPB3rsUHth6PQmuC2AkjrvpzZnN8cyCaVQgU5xY2xHcwnLRZstU37atLdHXdC1HC2NzoUdiBcIE7eKkMLmLgG9DrhXZ0sw2HH0iXMHmA5MAzOhmqjGRU2M1reV3vk2ydXsTgNq0t6Ke0NYtPyhFSQ3UPMk+cCjrbIRMwFftjYLe9wwy/v9v5MA1XZgHC2uDBKafS3XG1uhOO6B7nApGglHhih1tGSvLWCuVMUX3V0bPHHwfl4kfSOnYzIrdBWOAziZ3Jlt7/XIAqY1qiwNwcDCYV28joABBQPBs2xphKgiwiEJibqLlu9qqFWbov32xqMSfeFYcPYNl2ZDO2wWybQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from VE1EUR01FT056.eop-EUR01.prod.protection.outlook.com (2a01:111:e400:7e19::47) by VE1EUR01HT022.eop-EUR01.prod.protection.outlook.com (2a01:111:e400:7e19::394) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3700.27; Wed, 30 Dec 2020 20:49:45 +0000 Received: from AM0PR0602MB3410.eurprd06.prod.outlook.com (2a01:111:e400:7e19::53) by VE1EUR01FT056.mail.protection.outlook.com (2a01:111:e400:7e19::371) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3721.20 via Frontend Transport; Wed, 30 Dec 2020 20:49:45 +0000 X-IncomingTopHeaderMarker: OriginalChecksum:ED672005EDCF44EB971B288ADD12EFCFDC000FE118C3795197D722070165E69D; UpperCasedChecksum:DA891E8FD90A9F5B9B731383D321F6C4E637EC5066F806C65216AB7145A8B453; SizeAsReceived:8119; Count:47 Received: from AM0PR0602MB3410.eurprd06.prod.outlook.com ([fe80::60c8:86c2:bdaa:f0d2]) by AM0PR0602MB3410.eurprd06.prod.outlook.com ([fe80::60c8:86c2:bdaa:f0d2%3]) with mapi id 15.20.3721.019; Wed, 30 Dec 2020 20:49:45 +0000 Subject: Re: [PATCH v5] Fix range end handling of inlined subroutines To: Simon Marchi , "gdb-patches@sourceware.org" , Andrew Burgess , Joel Brobecker , Tom Tromey , Pedro Alves , Eli Zaretskii References: <9dccd1ed-3a8a-e9ef-abee-056ce4642482@polymtl.ca> From: Bernd Edlinger Message-ID: Date: Wed, 30 Dec 2020 21:49:42 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 In-Reply-To: <9dccd1ed-3a8a-e9ef-abee-056ce4642482@polymtl.ca> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit X-TMN: [6Opy9SaU/bofgkRZT/Jiaxe6U34Lo8Qd] X-ClientProxiedBy: AM4PR05CA0034.eurprd05.prod.outlook.com (2603:10a6:205::47) To AM0PR0602MB3410.eurprd06.prod.outlook.com (2603:10a6:208:21::24) X-Microsoft-Original-Message-ID: MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [192.168.1.101] (88.68.3.2) by AM4PR05CA0034.eurprd05.prod.outlook.com (2603:10a6:205::47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3721.20 via Frontend Transport; Wed, 30 Dec 2020 20:49:44 +0000 X-MS-PublicTrafficType: Email X-IncomingHeaderCount: 47 X-EOPAttributedMessage: 0 X-MS-Office365-Filtering-Correlation-Id: 45b9d47f-fbea-4264-5468-08d8ad0470ef X-MS-TrafficTypeDiagnostic: VE1EUR01HT022: X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: z2vdNDGA0Ax6r0H2QW9BPGUa1J6jzHyvNsTPw8X+mzQPk//B60KUjhS+MMfnR5TvG7HJkfvhbPH9VyL0giu/ZYSVHJbEyePQKbQmgjuDDV2CnObQJDXwSc9/DHACWcJoqhxPpzDPQb3tr/P9PbjC2sRDnQmXvqq2W3XnnV1RWdFddFZMU+82tkLXpUOvBunvf17fyAobzzszQCB8XRX6XQRPluwM3aj/p9dTucM2JaztDq/wzfOUMxyjE3SQAdC5 X-MS-Exchange-AntiSpam-MessageData: XIqD52HbGO1w1qbc+4WXjoESGwZoFLNY34TvDHx8jsSc/MA1HcKVJP4sf/gv+ONH3wpMmNlHDWVPcwMAimiJWHQhB/iBBtLV7ZsSqlgH4lvbEOm9f75DB9G1YRzPRbUKGRvRTuhZJWMO0DAZlZdfoA== X-OriginatorOrg: outlook.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Dec 2020 20:49:45.7383 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa X-MS-Exchange-CrossTenant-Network-Message-Id: 45b9d47f-fbea-4264-5468-08d8ad0470ef X-MS-Exchange-CrossTenant-AuthSource: VE1EUR01FT056.eop-EUR01.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: Internet X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000 X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1EUR01HT022 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: , Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" On 12/30/20 7:48 PM, Simon Marchi wrote: >> From 3c345ab5f7c497151d21945cba8668e91db00e7b Mon Sep 17 00:00:00 2001 >> From: Bernd Edlinger >> Date: Tue, 3 Nov 2020 18:41:43 +0100 >> Subject: [PATCH] Fix range end handling of inlined subroutines >> >> This introduces a new line table flag is_weak. >> The line entries at the end of a subroutine range, >> use this to indicate that they may possibly >> be part of the previous subroutine. >> >> When there is a sequence of line entries at the >> same address where an inline range ends, and the >> last item has is_stmt = 0, we force all previous >> items to have is_weak = 1. >> >> This is about the best we can do at the moment, >> unless location view information are added to the >> block ranges debug info structure, and location >> views are implemented in gdb in general. > > Ok, I'm not sure I can reasonably review this. > > First, it would make the patch easier and more inviting to review if > the commit message went a bit more in depth. If I know nothing about > the problem, it's some time-consuming patchwork to go read all the > pieces of informations in the bug reports, maybe the test cases, to > try to just understand what is the problem this patch is tackling. > > When fixing anything remotely complicated, the commit message should be > of the general form: > > - When we do ..., this happens > - This is wrong, we would expect ... > - This is caused by ... > - To avoid this, I propose ... > > After reading the commit message, I should at least be convinced that > there is indeed a problem to be fixed and that this patch, in this form > or another, is desirable. > Okay, I will try to improve the commit message. > Anyhow, I understand somewhat the problem now (at least the one described > in bug 25987). But the patch is changing bits of twisted code here and > there without much explanation, and I can't make a mental model of what > each bit does. > Yes, the whole patch accumulated some more or less related bits over time. Currently debugging of optimized code is much less convenient than it should actually be. The situation got actually worse with gcc-8 (+/- 1) although the debug information actually was supposed to be improved by brand new features as -gstatement-frontiers and -gvariable-location-views. Initially I became aware that the n command often steps into the last line of an inline function, and that the skip function is often not able to skip an inline function, because the call stack is not correct, i.e. misses the inline frame. This is always due to a line breakpoint at the end of an inline function, or the end of a subrange of an inline. When this happens we always have a !is-stmt line table entry from the calling program. Having breakpoints at these places looked odd initially, but now I think that it can be a point where result values from the inline function can be inspected, but that works only when the debugger knows the executing inline function from the program block structure. 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, which could theoretically wrong, but that happens virtually never, as far as I can tell. While currently the assumption is that this line belongs to the calling function, which causes the confusing call stacks, which do not match with the displayed visual location. 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. 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. Additionally it may happen that an inline function is completely empty, but as the test cases demonstrate, there is a value from the empty inline which can be inspected here. Additionally it may happen that the inline entry point is not the lowest address of an inline function, but currently all non-zero inline subranges are sorted by PC and the lowest PC is used when the user asks for a breakpiont on the function, I found that by ananlyzing differences on which breakpoint PCs are set with and without the patch. It turns out, that the entry point is often but not always the lowest PC. Conceptually my patch uses a heuristic to work around a deficiency in the dwarf-4 and dwarf-5 dwarf2_rnglists structure. That is in my opinion there should be a location view number for each inline begin and end range, as well as the DW_TAG_inlined_subroutine low_pc and high_pc, similar to the entry_point_view_number DW_AT_GNU_entry_view. But adding that information to dwarf-5 (or 6 ?) is out of reach for me. > Is it possible to split the patch into smaller pieces? For example, a first > patch could just the DWARF reader, and perhaps other pieces, with the goal > of having no functional changes (just augment GDB's internal line table), > with an explanation of what's added. With this kind of code, the smaller > the increment the better. > Yes, that should be possible. I will try to split the patch as you suggested. Thanks Bernd. > Simon >