From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8285 invoked by alias); 23 Jul 2019 00:54:38 -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 8100 invoked by uid 89); 23 Jul 2019 00:54:37 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-18.7 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=H*r:sk:TLS_AES, H*r:TLSv1.3 X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 23 Jul 2019 00:54:36 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (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 9BC3E1EC28; Mon, 22 Jul 2019 20:54:34 -0400 (EDT) Subject: Re: [PATCH] Allow passing a block to lookup_global_symbol_from_objfile To: Christian Biesinger , gdb-patches@sourceware.org References: <20190722205854.GE23204@embecosm.com> <20190722210025.92079-1-cbiesinger@google.com> From: Simon Marchi Message-ID: Date: Tue, 23 Jul 2019 00:54:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.7.2 MIME-Version: 1.0 In-Reply-To: <20190722210025.92079-1-cbiesinger@google.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-07/txt/msg00527.txt.bz2 Hi Christian, Code-wise, this LGTM, but I think it would need a bit of justification. On its own it doesn't change the behavior of GDB and does not appear to be a cleanup, so why is this change useful? I suppose it will be necessary for a further patch. If so, it's common to post it as a preparatory patch, in the same series as that other patch. Then in the commit message you can say that "a further patch will need to call lookup_global_symbol_from_objfile with STATIC_BLOCK", for example, that this preparatory patch is just to isolate that change, and that on its own does not bring any intended functional changes. On 2019-07-22 5:00 p.m., Christian Biesinger via gdb-patches wrote: > gdb/ChangeLog: > > 2019-07-22 Christian Biesinger > > * compile/compile-object-load.c (compile_object_load): Pass GLOBAL_SCOPE. > * solib-spu.c (spu_lookup_lib_symbol): Pass GLOBAL_SCOPE. > * solib-svr4.c (elf_lookup_lib_symbol): Pass GLOBAL_SCOPE. > * symtab.c (lookup_global_symbol_from_objfile): Add a scope parameter. > * symtab.h: Add a scope parameter to lookup_global_symbol_from_objfile. The last line should also be * symtab.h (lookup_global_symbol_from_objfile): Add a scope parameter. or * symtab.c (lookup_global_symbol_from_objfile): Likewise. > --- > gdb/compile/compile-object-load.c | 1 + > gdb/solib-spu.c | 3 ++- > gdb/solib-svr4.c | 3 ++- > gdb/symtab.c | 7 +++++-- > gdb/symtab.h | 2 ++ > 5 files changed, 12 insertions(+), 4 deletions(-) > > diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c > index 4e70205195..3a765a345b 100644 > --- a/gdb/compile/compile-object-load.c > +++ b/gdb/compile/compile-object-load.c > @@ -639,6 +639,7 @@ compile_object_load (const compile_file_names &file_names, > objfile = objfile_holder.get (); > > func_sym = lookup_global_symbol_from_objfile (objfile, > + GLOBAL_BLOCK, > GCC_FE_WRAPPER_FUNCTION, > VAR_DOMAIN).symbol; > if (func_sym == NULL) > diff --git a/gdb/solib-spu.c b/gdb/solib-spu.c > index 448e1a64f4..5b97b9bcf6 100644 > --- a/gdb/solib-spu.c > +++ b/gdb/solib-spu.c > @@ -392,7 +392,8 @@ spu_lookup_lib_symbol (struct objfile *objfile, > const domain_enum domain) > { > if (bfd_get_arch (objfile->obfd) == bfd_arch_spu) > - return lookup_global_symbol_from_objfile (objfile, name, domain); > + return lookup_global_symbol_from_objfile (objfile, GLOBAL_BLOCK, name, > + domain); > > if (svr4_so_ops.lookup_lib_global_symbol != NULL) > return svr4_so_ops.lookup_lib_global_symbol (objfile, name, domain); > diff --git a/gdb/solib-svr4.c b/gdb/solib-svr4.c > index 8cd5b7d8e7..c0c505acaa 100644 > --- a/gdb/solib-svr4.c > +++ b/gdb/solib-svr4.c > @@ -3226,7 +3226,8 @@ elf_lookup_lib_symbol (struct objfile *objfile, > if (abfd == NULL || scan_dyntag (DT_SYMBOLIC, abfd, NULL, NULL) != 1) > return {}; > > - return lookup_global_symbol_from_objfile (objfile, name, domain); > + return lookup_global_symbol_from_objfile (objfile, GLOBAL_BLOCK, name, > + domain); > } > > void > diff --git a/gdb/symtab.c b/gdb/symtab.c > index 5b8bfc1df7..87a0c8e4da 100644 > --- a/gdb/symtab.c > +++ b/gdb/symtab.c > @@ -2224,15 +2224,18 @@ lookup_symbol_in_block (const char *name, symbol_name_match_type match_type, > > struct block_symbol > lookup_global_symbol_from_objfile (struct objfile *main_objfile, > + enum block_enum block_index, > const char *name, > const domain_enum domain) > { > + gdb_assert (block_index == GLOBAL_BLOCK || block_index == STATIC_BLOCK); > + > for (objfile *objfile : main_objfile->separate_debug_objfiles ()) > { > struct block_symbol result > - = lookup_symbol_in_objfile (objfile, GLOBAL_BLOCK, name, domain); > + = lookup_symbol_in_objfile (objfile, block_index, name, domain); > > - if (result.symbol != NULL) > + if (result.symbol != nullptr) > return result; > } > > diff --git a/gdb/symtab.h b/gdb/symtab.h > index b91454c85c..35f3c6be71 100644 > --- a/gdb/symtab.h > +++ b/gdb/symtab.h > @@ -2048,10 +2048,12 @@ extern enum language main_language (void); > /* Lookup symbol NAME from DOMAIN in MAIN_OBJFILE's global blocks. > This searches MAIN_OBJFILE as well as any associated separate debug info > objfiles of MAIN_OBJFILE. > + BLOCK_INDEX can be GLOBAL_BLOCK or STATIC_BLOCK. > Upon success fixes up the symbol's section if necessary. */ On top of saying which values it can take, it would be useful to say (very brefly) how it affects the behavior of the function. Simon