From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 59306 invoked by alias); 21 Aug 2018 16:12:52 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 59296 invoked by uid 89); 21 Aug 2018 16:12:51 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00 autolearn=ham version=3.3.2 spammy=replied, adjustments, gdbpatches, gdb-patches X-HELO: sesbmg22.ericsson.net Received: from sesbmg22.ericsson.net (HELO sesbmg22.ericsson.net) (193.180.251.48) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 21 Aug 2018 16:12:49 +0000 DKIM-Signature: v=1; a=rsa-sha256; d=ericsson.com; s=mailgw201801; c=relaxed/simple; q=dns/txt; i=@ericsson.com; t=1534867966; h=From:Sender:Reply-To:Subject:Date:Message-ID:To:Cc:MIME-Version:Content-Type: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=aoH62VerrGF/QB+Rglwxjv5BUovefHEDHf4C3FyHnuE=; b=Z78s4j6ovzEGCwyB63lRC/BUr8EJwe9kYFFLc5uG66XTk2NpuBmuGCuvEZYynEkU OdxzO1w6vMx8vQETveY97bKfg3UddQqwVVT9l5Li/h7173a2KMIwPj66OOZKjNNQ mreX9iW0jJlPiDJqY2jn+p2FQI+uZD9tweixxIx/UX0=; Received: from ESESSMB501.ericsson.se (Unknown_Domain [153.88.183.119]) by sesbmg22.ericsson.net (Symantec Mail Security) with SMTP id B8.C7.21978.EF93C7B5; Tue, 21 Aug 2018 18:12:46 +0200 (CEST) Received: from ESESBMB504.ericsson.se (153.88.183.171) by ESESSMB501.ericsson.se (153.88.183.162) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3; Tue, 21 Aug 2018 18:12:46 +0200 Received: from NAM03-DM3-obe.outbound.protection.outlook.com (153.88.183.157) by ESESBMB504.ericsson.se (153.88.183.171) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.1466.3 via Frontend Transport; Tue, 21 Aug 2018 18:12:46 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ericsson.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Eq1kFBPeWujQ+1iEiP9UHuYjKes+Jgpld+4jGErPlhs=; b=NYcARWAgUr//7Gg6+ZmhWynleoY+zvXUoFuXo/S3STMmwrTEFv9Pyo4bDdM0BQpAlJ4ML8+bHZ0RjHUlD0eZk9hZ2Sjtlie03Q7Mfl7o/2/voC6rZw9pHWeSVBWblTU8xtivbKZSicU4SDqNB/e4zyOFMazsAazBgPok8KlgQ00= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=simon.marchi@ericsson.com; Received: from [142.133.61.8] (192.75.88.130) by SN6PR15MB2398.namprd15.prod.outlook.com (2603:10b6:805:24::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1059.24; Tue, 21 Aug 2018 16:12:43 +0000 Subject: Re: [PATCH v3 3/8] Add support for non-contiguous blocks to find_pc_partial_function To: Kevin Buettner , References: <20180820152512.671a7dc7@pinnacle.lan> <20180820153922.025b0188@pinnacle.lan> From: Simon Marchi Message-ID: <99e3e28e-f449-8772-1d5c-fb73db2e87c1@ericsson.com> Date: Tue, 21 Aug 2018 16:12:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180820153922.025b0188@pinnacle.lan> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-Path: simon.marchi@ericsson.com Received-SPF: None (protection.outlook.com: ericsson.com does not designate permitted sender hosts) X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00492.txt.bz2 On 2018-08-20 06:39 PM, Kevin Buettner wrote: > This change adds an optional output parameter BLOCK to > find_pc_partial_function. If BLOCK is non-null, then *BLOCK will be > set to the address of the block corresponding to the function symbol > if such a symbol was found during lookup. Otherwise it's set to a > NULL value. Callers may wish to use the block information to > determine whether the block contains any non-contiguous ranges. The > caller may also iterate over or examine those ranges. > > When I first started looking at the broken stepping behavior associated > with functions w/ non-contiguous ranges, I found that I could "fix" > the problem by disabling the find_pc_partial_function cache. It would > sometimes happen that the PC passed in would be between the low and > high cache values, but would be in some other function that happens to > be placed in between the ranges for the cached function. This caused > incorrect values to be returned. > > So dealing with this cache turns out to be very important for fixing > this problem. I explored three different ways of dealing with the cache. > > My first approach was to clear the cache when a block was encountered > with more than one range. This would cause the non-cache pathway to > be executed on the next call to find_pc_partial_function. > > Another approach, which I suspect is slightly faster, checks to see > whether the PC is within one of the ranges associated with the cached > block. If so, then the cached values can be used. It falls back to > the original behavior if there is no cached block. > > The current approach, suggested by Simon Marchi, is to restrict the > low/high pc values recorded for the cache to the beginning and end of > the range containing the PC value under consideration. This allows us > to retain the simple (and fast) test for determining whether the > memoized (cached) values apply to the PC passed to > find_pc_partial_function. > > Another choice that had to be made regards setting *ADDRESS and > *ENDADDR. There are three possibilities which might make sense: > > 1) *ADDRESS and *ENDADDR represent the lowest and highest address > of the function. > 2) *ADDRESS and *ENDADDR are set to the start and end address of > the range containing the entry pc. > 3) *ADDRESS and *ENDADDR are set to the start and end address of > the range in which PC is found. > > An earlier version of this patch implemented option #1. I found out > that it's not very useful though and, in fact, returns results that > are incorrect when used in the context of determining the start and > end of the function for doing prologue analysis. While debugging a > function in which the entry pc was in the second range (of a function > containing two non-contiguous ranges), I noticed that > amd64_skip_prologue called find_pc_partial_function - the returned > start address was set to the beginning of the first range. This is > incorrect for this function. What was also interesting was that this > first invocation of find_pc_partial_function correctly set the cache > for the PC on which it had been invoked, but a slightly later call > from skip_prologue_using_sal could not use this cached value because > it was now being used to lookup the very lowest address of the > function - which is in a range not containing the entry pc. > > Option #2 is attractive as it would provide a desirable result > when used in the context of prologue analysis. However, many callers, > including some which do prologue analysis want the condition > *ADDRESS <= PC < *ENDADDR to hold. This will not be the case when > find_pc_partial_function is called on a PC that's in a non-entry-pc > range. A later patch to this series adds find_pc_partial_entry_range > as a wrapper of find_pc_partial_function. > > Option #3 causes the *ADDRESS <= PC < *ENDADDR property to hold. If > find_pc_partial_function is called with a PC that's within entry pc's > range, then it will correctly return the limits of that range. So, if > the result of a minsym search is passed to find_pc_partial_function > to find the limits, then correct results will be achieved. Returned > limits (for prologue analysis) won't be correct when PC is within some > other (non-entry-pc) range. I don't yet know how big of a problem > this might be; I'm guessing that it won't be a serious problem - if a > compiler generates functions which have non-contiguous ranges, then it > also probably generates DWARF2 CFI which makes a lot of the old > prologue analysis moot. > > I've implemented option #3 for this version of the patch. I don't see > any regressions for x86-64. Moreover, I don't expect to see > regressions for other targets either simply because > find_pc_partial_function behaves the same as it did before for the > contiguous address range case. That said, there may be some > adjustments needed if GDB encounters a function with requires prologue > analysis which also occupies non-contiguous ranges. > > gdb/ChangeLog: > > * symtab.h (find_pc_partial_function): Add new parameter `block'. > * blockframe.c (cache_pc_function_block): New static global. > (clear_pc_function_cache): Clear cache_pc_function_block. > (find_pc_partial_function): Move comment to symtab.h. Add > support for non-contiguous blocks. Woops, I replied to the v2 by mistake. See here: https://sourceware.org/ml/gdb-patches/2018-08/msg00491.html Simon