From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28784 invoked by alias); 19 Dec 2002 19:44:52 -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 28769 invoked from network); 19 Dec 2002 19:44:50 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by 209.249.29.67 with SMTP; 19 Dec 2002 19:44:50 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id gBJJiWZ09860; Thu, 19 Dec 2002 11:44:32 -0800 X-Authentication-Warning: jackfruit.Stanford.EDU: carlton set sender to carlton@math.stanford.edu using -f To: Elena Zannoni Cc: gdb-patches@sources.redhat.com, Jim Blandy Subject: Re: [rfa] delete 'force_return' from lookup_symbol_aux_minsyms References: <15861.1750.232869.77936@localhost.redhat.com> <15873.63708.366837.407436@localhost.redhat.com> From: David Carlton Date: Thu, 19 Dec 2002 11:47:00 -0000 In-Reply-To: <15873.63708.366837.407436@localhost.redhat.com> Message-ID: User-Agent: Gnus/5.0808 (Gnus v5.8.8) XEmacs/21.4 (Common Lisp) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2002-12/txt/msg00556.txt.bz2 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. 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. 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. Except that isn't right, either! Because SYMBOL_NAME (msymbol) would be the mangled name of 'msymbol', and lookup_symbol demangled it, so the 'name' argument to lookup_symbol_aux_minsyms would actually be demangled. So, indeed, we might well be in a situation where force_return comes into play. 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. 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_. 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". 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. David Carlton carlton@math.stanford.edu