From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 18318 invoked by alias); 19 Dec 2002 22:33:21 -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 18308 invoked from network); 19 Dec 2002 22:33:20 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by 209.249.29.67 with SMTP; 19 Dec 2002 22:33:20 -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 gBJM6mg25221 for ; Thu, 19 Dec 2002 17:06:48 -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 gBJMX8231999 for ; Thu, 19 Dec 2002 17:33:08 -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 gBJMX6L01555; Thu, 19 Dec 2002 17:33:06 -0500 Received: by localhost.redhat.com (Postfix, from userid 469) id 25849FF79; Thu, 19 Dec 2002 17:37:33 -0500 (EST) From: Elena Zannoni MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <15874.18989.470549.778392@localhost.redhat.com> Date: Thu, 19 Dec 2002 15:39:00 -0000 To: David Carlton Cc: Elena Zannoni , gdb-patches@sources.redhat.com, Jim Blandy Subject: Re: [rfa] delete 'force_return' from lookup_symbol_aux_minsyms In-Reply-To: References: <15861.1750.232869.77936@localhost.redhat.com> <15873.63708.366837.407436@localhost.redhat.com> X-SW-Source: 2002-12/txt/msg00560.txt.bz2 David Carlton writes: > On Thu, 19 Dec 2002 11:50:36 -0500, Elena Zannoni said: > > David Carlton writes: > > >> 'found_misc' also seems to have gone away; > > > Actually it is still there, the code I posted is for > > list/search_symbols. > > Oh, I apologize, I completely misunderstood what you were saying. I > thought that the code you dug up was an earlier version of > lookup_symbol. > > Now I understand your point: I was claiming that no callers of > lookup_symbol depended on this NULL return that I'm trying to get rid > of, but you suspect that search_symbols might be such a caller. And, > indeed, I hadn't considered that particular caller. > > Looking into it further, I think you might have reason to worry. > Here's the relevant section of search_symbols: > > if (0 == find_pc_symtab (SYMBOL_VALUE_ADDRESS (msymbol))) > { > if (kind == FUNCTIONS_NAMESPACE > || lookup_symbol (SYMBOL_NAME (msymbol), > (struct block *) NULL, > VAR_NAMESPACE, > 0, (struct symtab **) NULL) == NULL) > found_misc = 1; > } > > And here's an (abbreviated) version of all of the uses of force_return > in lookup_symbol_aux_minsyms: > > s = find_pc_sect_symtab (SYMBOL_VALUE_ADDRESS (msymbol), > SYMBOL_BFD_SECTION (msymbol)); > if (s != NULL) > { > > } > 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. */ > *force_return = 1; > return lookup_symbol_aux (SYMBOL_NAME (msymbol), mangled_name, > NULL, namespace, is_a_field_of_this, > symtab); > } > > These two sections of code are remarkably parallel, for reasons that I > don't understand. And they're clearly trying to investigate the same > minimal symbol: the point of that code in search_symbols it to > determine whether or not a particular minimal symbol has debugging > info, so the question at hand is whether or not lookup_symbol is > supposed to terminate when it hits the minimal symbol that > search_symbols is interested in. So I think it's safe to assume that > we're only interested in comparing the two pieces of code when > 'msymbol' has the same value in both. > Yes, I agree with this interpretation. > In that case, the find_pc_symtab in search_symbols corresponds to the > find_pc_sect_symtab in lookup_symbol_aux_minsyms (and should probably > be changed to find_pc_sect_symtab, but never mind that for now). > We're assuming that that symtab is 0; that means that we're in the > else clause of the lookup_symbol_aux_minsyms call. > yes > So when does that else clause happens? It's guarded by a conditional: > that tests that the MSYMBOL_TYPE of the msymbol isn't text, and that > the name we're searching under isn't the SYMBOL_NAME of the msymbol. > > The former condition is true: we're assuming that kind isn't > FUNCTIONS_NAMESPACE, so the minimal symbol shouldn't be text. But > we've called lookup_symbol with the 'name' argument equal to > SYMBOL_NAME (msymbol). And, in that case, the test for > > !STREQ (name, SYMBOL_NAME (msymbol) > > should fail. > ok... > Except that isn't right, either! Because SYMBOL_NAME (msymbol) would > be the mangled name of 'msymbol', yes, > and lookup_symbol demangled it, so ah right, it would become modified_name > the 'name' argument to lookup_symbol_aux_minsyms would actually be > demangled. ok, I think I follow up to here. > So, indeed, we might well be in a situation where > force_return comes into play. > I am lost now. > Phew. Assuming that analysis is correct, I have two comments and a > suggestion. > > Comment #1: This whole logic is hopelessly unclear. I think I'd > rather turn the code into something possibly broken but clearer, and > then try to fix any possible breakage, than try to leave it as is. > tempting > Comment #2: Just what is up with the call to lookup_symbol_aux from > within lookup_symbol_aux_minsyms, anyways? It's passing in a mangled > name as the first argument; but lookup_symbol_aux normally expects its > first argument to be demangled. I'm not sure what's going on here: > that call might be broken, or there might be some part of > lookup_symbol_aux that does something with a mangled first argument. > If the latter is the case, then there should be comments making this > explicit, and the call to lookup_symbol_aux should be replaced by a > call to lookup_symbol_aux_. > I think it's just broken. The call in search_symbols predates the introduction of lookup_symbol_aux and the demangling logic. So I think it just wasn't updated to reflect the new behavior. > Suggestion: The whole purpose of the call to lookup_symbol from within > search_symbols is to try to track down information about one > particular minimal symbol: does it have debugging information, or > doesn't it? Given that that's the case, doing that via lookup_symbol > is at best overkill and at worst wrong. This suggests that we might > be able to get around the issue by replacing that call to > lookup_symbol by a call to lookup_symbol_aux_minsyms. The only > question that I have is whether the first argument should then be the > mangled name of 'msymbol' or the demangled name; I'd be happier if we > understood the situation with respect to my "Comment #2". > Yeah, I think it's just a coincidence that lookup_symbol is still called. At the time that call was introduced, lookup_symbol was maybe the only function available for this sort of things. We could try to call the lookup_symbol_aux_minsyms function. > A datum which may or may not be relevant: currently, partial symbols > never have their names demangled. I'd assumed that this was a bug; > but I just noticed the following comment in search_symbols: > > This is in particular necessary for demangled variable names, > which are no longer put into the partial symbol tables. > > Sigh. So I have another suggestion: > > Suggestion #2: Maybe we should put this particular patch on hold and > come to some sort of consensus as to how to deal with > mangled/demangled names. I'll post an RFC for that later today. > Ok, whatever seems easier for you. Although I think we can try to fix the parameter problem, at least, and see what breaks. Elena > David Carlton > carlton@math.stanford.edu