From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 66766 invoked by alias); 21 Aug 2019 10:32:31 -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 66756 invoked by uid 89); 21 Aug 2019 10:32:31 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-24.5 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=FULL, showed, life X-HELO: mail-wr1-f65.google.com Received: from mail-wr1-f65.google.com (HELO mail-wr1-f65.google.com) (209.85.221.65) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 21 Aug 2019 10:32:29 +0000 Received: by mail-wr1-f65.google.com with SMTP id s18so1542232wrn.1 for ; Wed, 21 Aug 2019 03:32:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=xcPWrSOHLTSH8UINrHD01uI/EPEGePA3icLUu59br+Q=; b=Lhy+lu69sD8jlgaoRJSlrWOs6snxN8ZH4lKZernvpTS0JwUnX5hbcT2ZQ2gi3yX2bf 4uKJKQ6ahJZ8A/kY5JYSt+8GClDCyAIHsHaYjBEPEg6XMoYhrsOVDLm+jGfBG5r+HtSk 2k4T0mdnbqndA7ayrcj0QKh2554lgw4UJAQz4zTmcHQtlIdSD7DQbBppDJZOIoMzQMps aWhzMzgjBRvXsqFmWX3FrfqGJR7mat+NKtUOu07Q0j1UOof5TcHWJBRaiO30pUWCOF7M YAE+7hK0jJ/Xxv3ziq7yNbdxnl6nUu8FjwivdJ/JWCxHN+cwAo9689GI5KiC5RFdSIy4 uDWA== Return-Path: Received: from localhost (host86-128-12-79.range86-128.btcentralplus.com. [86.128.12.79]) by smtp.gmail.com with ESMTPSA id c8sm19781939wrn.50.2019.08.21.03.32.26 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 21 Aug 2019 03:32:26 -0700 (PDT) Date: Wed, 21 Aug 2019 10:32:00 -0000 From: Andrew Burgess To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v2 4/8] Search global block from basic_lookup_symbol_nonlocal Message-ID: <20190821103225.GF6076@embecosm.com> References: <20190801170412.5553-1-tromey@adacore.com> <20190801170412.5553-5-tromey@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190801170412.5553-5-tromey@adacore.com> X-Fortune: I once decorated my apartment entirely in ten foot salad forks!! X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes X-SW-Source: 2019-08/txt/msg00464.txt.bz2 * Tom Tromey [2019-08-01 11:04:08 -0600]: > This changes basic_lookup_symbol_nonlocal to look in the global block > of the passed-in block. If no block was passed in, it reverts to the > previous behavior. > > This change is needed to ensure that 'FILENAME'::NAME lookups work > properly. As debugging Pedro's test case showed, this was not working > properly in the case where multiple identical names could be found > (the one situation where this feature is truly needed :-). > > This also removes some old comments from basic_lookup_symbol_nonlocal > that no longer apply once this patch goes in. So I guess the tests for this are going to be in the gdb.base/print-file-var.exp changes that are part of patch #8. It would be great if the commit message could mention this - it just makes life easier later on. I wonder if we need to update other *_lookup_symbol_nonlocal functions in a similar way? For example can the C tests be compiled as C++, which should cause GDB to use cp_lookup_symbol_nonlocal. Looking at both basic_lookup_symbol_nonlocal and cp_lookup_symbol_nonlocal, I wonder if your fix could be moved into lookup_global_symbol? And just have 'block_global_block (block)' checked before the search of all global blocks? Some other languages provide their own *_lookup_symbol_nonlocal, I don't know if these would also need fixing. Thanks, Andrew > > gdb/ChangeLog > 2019-08-01 Tom Tromey > > * symtab.c (basic_lookup_symbol_nonlocal): Search global block. > Remove old comments. > --- > gdb/ChangeLog | 5 +++++ > gdb/symtab.c | 44 ++++++++++++++++---------------------------- > 2 files changed, 21 insertions(+), 28 deletions(-) > > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 0ff212e0d97..b8f33509c09 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -2417,34 +2417,6 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef, > { > struct block_symbol result; > > - /* NOTE: carlton/2003-05-19: The comments below were written when > - this (or what turned into this) was part of lookup_symbol_aux; > - I'm much less worried about these questions now, since these > - decisions have turned out well, but I leave these comments here > - for posterity. */ > - > - /* NOTE: carlton/2002-12-05: There is a question as to whether or > - not it would be appropriate to search the current global block > - here as well. (That's what this code used to do before the > - is_a_field_of_this check was moved up.) On the one hand, it's > - redundant with the lookup in all objfiles search that happens > - next. On the other hand, if decode_line_1 is passed an argument > - like filename:var, then the user presumably wants 'var' to be > - searched for in filename. On the third hand, there shouldn't be > - multiple global variables all of which are named 'var', and it's > - not like decode_line_1 has ever restricted its search to only > - global variables in a single filename. All in all, only > - searching the static block here seems best: it's correct and it's > - cleanest. */ > - > - /* NOTE: carlton/2002-12-05: There's also a possible performance > - issue here: if you usually search for global symbols in the > - current file, then it would be slightly better to search the > - current global block before searching all the symtabs. But there > - are other factors that have a much greater effect on performance > - than that one, so I don't think we should worry about that for > - now. */ > - > /* NOTE: dje/2014-10-26: The lookup in all objfiles search could skip > the current objfile. Searching the current objfile first is useful > for both matching user expectations as well as performance. */ > @@ -2453,6 +2425,22 @@ basic_lookup_symbol_nonlocal (const struct language_defn *langdef, > if (result.symbol != NULL) > return result; > > + /* If a block was passed in, we want to search the corresponding > + global block now. This yields "more expected" behavior, and is > + needed to support 'FILENAME'::VARIABLE lookups. */ > + const struct block *global_block = block_global_block (block); > + if (global_block != nullptr) > + { > + result.symbol = lookup_symbol_in_block (name, > + symbol_name_match_type::FULL, > + global_block, domain); > + if (result.symbol != nullptr) > + { > + result.block = global_block; > + return result; > + } > + } > + > /* If we didn't find a definition for a builtin type in the static block, > search for it now. This is actually the right thing to do and can be > a massive performance win. E.g., when debugging a program with lots of > -- > 2.20.1 >