From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id MJrsLqbL7F/UDwAAWB0awg (envelope-from ) for ; Wed, 30 Dec 2020 13:49:10 -0500 Received: by simark.ca (Postfix, from userid 112) id B2F911F0AA; Wed, 30 Dec 2020 13:49:10 -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.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from 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 RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id CD0E61E590 for ; Wed, 30 Dec 2020 13:49:08 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id B51DF3858038; Wed, 30 Dec 2020 18:49:07 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B51DF3858038 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1609354147; bh=sHjHpaDmdg6GzmOToWPivWX2Gbh9GgmM8SUoMJQm8Fw=; h=Subject:To:References:Date:In-Reply-To:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=Fy+I9F/3iJiSaK32jQcUJ+5gY60vqBubEPqER3cW+SboHmLhu5HZqMRyG+fE6eURw iPLUptqfsJj3hi/Z4q/eiVA5fUoJUvnfOLmRjDxmKN/do+RekU8/AmZxCy9wbUVJR8 v3Aa5HBSBcpghmAfbxBopwF8DWN8S76UT9/KGrw8= Received: from smtp.polymtl.ca (smtp.polymtl.ca [132.207.4.11]) by sourceware.org (Postfix) with ESMTPS id 5B1283858038 for ; Wed, 30 Dec 2020 18:49:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5B1283858038 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id 0BUImjeE028633 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Wed, 30 Dec 2020 13:48:50 -0500 DKIM-Filter: OpenDKIM Filter v2.11.0 smtp.polymtl.ca 0BUImjeE028633 Received: from [10.0.0.213] (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 444421E590; Wed, 30 Dec 2020 13:48:45 -0500 (EST) Subject: Re: [PATCH v5] Fix range end handling of inlined subroutines To: Bernd Edlinger , "gdb-patches@sourceware.org" , Andrew Burgess , Joel Brobecker , Tom Tromey , Pedro Alves , Eli Zaretskii References: Message-ID: <9dccd1ed-3a8a-e9ef-abee-056ce4642482@polymtl.ca> Date: Wed, 30 Dec 2020 13:48:44 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Wed, 30 Dec 2020 18:48:45 +0000 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" > 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. 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. 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. Simon