From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4136 invoked by alias); 24 Feb 2003 18:17:54 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 4129 invoked from network); 24 Feb 2003 18:17:54 -0000 Received: from unknown (HELO mx1.redhat.com) (172.16.49.200) by 172.16.49.205 with SMTP; 24 Feb 2003 18:17:54 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.11.6/8.11.6) with ESMTP id h1OIHse15395 for ; Mon, 24 Feb 2003 13:17:54 -0500 Received: from pobox.corp.redhat.com (pobox.corp.redhat.com [172.16.52.156]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h1OIHsq20581 for ; Mon, 24 Feb 2003 13:17:54 -0500 Received: from localhost.redhat.com (romulus-int.sfbay.redhat.com [172.16.27.46]) by pobox.corp.redhat.com (8.11.6/8.11.6) with ESMTP id h1OIHqI23299; Mon, 24 Feb 2003 13:17:52 -0500 Received: by localhost.redhat.com (Postfix, from userid 469) id AB3E8FF79; Mon, 24 Feb 2003 13:21:56 -0500 (EST) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15962.25283.781474.281523@localhost.redhat.com> Date: Mon, 24 Feb 2003 18:17:00 -0000 To: David Carlton Cc: Daniel Jacobowitz , gdb-patches@sources.redhat.com, Elena Zannoni , Jim Blandy Subject: Re: [rfa] delete lookup_symbol_aux_minsyms In-Reply-To: References: <20030222205011.GA25494@nevyn.them.org> X-SW-Source: 2003-02/txt/msg00592.txt.bz2 David Carlton writes: > On Sat, 22 Feb 2003 15:50:11 -0500, Daniel Jacobowitz said: > > On Sat, Feb 22, 2003 at 11:54:40AM -0800, David Carlton wrote: > > >> So what's the conclusion? Performance considerations don't seem to > >> give a clear answer. So we should go with whatever's cleanest. My > >> recommendation: > >> > >> * Delete the 'else' clause: it might cause correctness problems. > >> > >> * Comment out the remaining part of lookup_symbol_aux_minsyms: if > >> somebody comes up with a situation where we spend lots of time > >> searching for functions that aren't in a loaded symtab, we can > >> consider uncommenting it and adding it back in. > > > I'm pretty sure the answer is "none at all" based on skimming the > > code, but what affect does removing lookup_symbol_aux_minsyms have > > when looking for something which turns out not to have debugging > > info? lookup_symbol would fail, so it doesn't matter - is that > > right? > > That's right: lookup_symbol tries to find symbols, so if a function > doesn't have debugging info, then lookup_symbol will return NULL > (unless we're in some sort of bizarre situation like having a static > function elsewhere with the same name), at which point it's the > caller's responsibility to call lookup_minimal_symbol if the caller is > in a position to deal with minimal symbols instead of symbols. And > lookup_symbol would have returned NULL either before or after my > patch: the call to 'find_pc_sect_symtab' would have failed. > > > I don't know if I like this comment-out-part-delete-part business; > > if we don't want the function, let's kill it. > > It's not my first choice, either, but Elena has shown a preference in > the past for commenting out code like this instead of deleting it. > (C.f. the already commented-out bit in lookup_symbol_aux.) This seems > to me a situation where it's not worth arguing about it. However, if > Elena would rather delete it, I'd be happy to go along with that, too. > > David Carlton > carlton@math.stanford.edu Actually, I did some gcov on that function last night, running make check, here is what I got. About deleting the function. I think I would prefer if we delete the 'else' part first. Then comment out the function. But I am still not convinced that we should delete it yet. elena static struct symbol * lookup_symbol_aux_minsyms (const char *name, const char *mangled_name, const namespace_enum namespace, int *is_a_field_of_this, struct symtab **symtab) { 1442 struct symbol *sym; struct blockvector *bv; const struct block *block; struct minimal_symbol *msymbol; struct symtab *s; 1442 if (namespace == VAR_NAMESPACE) { 1332 msymbol = lookup_minimal_symbol (name, NULL, NULL); 1332 if (msymbol != NULL) { /* OK, we found a minimal symbol in spite of not finding any symbol. There are various possible explanations for this. One possibility is the symbol exists in code not compiled -g. Another possibility is that the 'psymtab' isn't doing its job. A third possibility, related to #2, is that we were confused by name-mangling. For instance, maybe the psymtab isn't doing its job because it only know about demangled names, but we were given a mangled name... */ /* We first use the address in the msymbol to try to locate the appropriate symtab. Note that find_pc_sect_symtab() has a side-effect of doing psymtab-to-symtab expansion, for the found symtab. */ 329 s = find_pc_sect_symtab (SYMBOL_VALUE_ADDRESS (msymbol), SYMBOL_BFD_SECTION (msymbol)); 329 if (s != NULL) { /* This is a function which has a symtab for its address. */ 201 bv = BLOCKVECTOR (s); 201 block = BLOCKVECTOR_BLOCK (bv, GLOBAL_BLOCK); /* This call used to pass `SYMBOL_NAME (msymbol)' as the `name' argument to lookup_block_symbol. But the name of a minimal symbol is always mangled, so that seems to be clearly the wrong thing to pass as the unmangled name. */ 201 sym = lookup_block_symbol (block, name, mangled_name, namespace); /* We kept static functions in minimal symbol table as well as in static scope. We want to find them in the symbol table. */ 201 if (!sym) { 39 block = BLOCKVECTOR_BLOCK (bv, STATIC_BLOCK); 39 sym = lookup_block_symbol (block, name, mangled_name, namespace); 201 } /* NOTE: carlton/2002-12-04: The following comment was taken from a time when two versions of this function were part of the body of lookup_symbol_aux: this comment was taken from the version of the function that was #ifdef HPUXHPPA, and the comment was right before the 'return NULL' part of lookup_symbol_aux. (Hence the "Fall through and return 0" comment.) Elena did some digging into the situation for Fortran, and she reports: "I asked around (thanks to Jeff Knaggs), and I think the story for Fortran goes like this: "Apparently, in older Fortrans, '_' was not part of the user namespace. g77 attached a final '_' to procedure names as the exported symbols for linkage (foo_) , but the symbols went in the debug info just like 'foo'. The rationale behind this is not completely clear, and maybe it was done to other symbols as well, not just procedures." */ /* If we get here with sym == 0, the symbol was found in the minimal symbol table but not in the symtab. Fall through and return 0 to use the msymbol definition of "foo_". (Note that outer code generally follows up a call to this routine with a call to lookup_minimal_symbol(), so a 0 return means we'll just flow into that other routine). This happens for Fortran "foo_" symbols, which are "foo" in the symtab. This can also happen if "asm" is used to make a regular symbol but not a debugging symbol, e.g. asm(".globl _main"); asm("_main:"); */ 201 if (symtab != NULL && sym != NULL) 155 *symtab = s; 201 return fixup_symbol_section (sym, s->objfile); 128 } 128 else if (MSYMBOL_TYPE (msymbol) != mst_text && MSYMBOL_TYPE (msymbol) != mst_file_text && !STREQ (name, SYMBOL_NAME (msymbol))) { /* This is a mangled variable, look it up by its mangled name. */ 2 return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, NULL, namespace, is_a_field_of_this, 2 symtab); 126 } 1129 } 1239 } 1239 return NULL; 1442 }