From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6424 invoked by alias); 20 Feb 2003 20:57:55 -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 6415 invoked from network); 20 Feb 2003 20:57:54 -0000 Received: from unknown (HELO jackfruit.Stanford.EDU) (171.64.38.136) by 172.16.49.205 with SMTP; 20 Feb 2003 20:57:54 -0000 Received: (from carlton@localhost) by jackfruit.Stanford.EDU (8.11.6/8.11.6) id h1KKvcl32647; Thu, 20 Feb 2003 12:57:38 -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 Subject: Re: [rfa] more lookup_symbol_aux_minsyms futzing References: <15957.13747.748204.408563@localhost.redhat.com> From: David Carlton Date: Thu, 20 Feb 2003 20:57:00 -0000 In-Reply-To: <15957.13747.748204.408563@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: 2003-02/txt/msg00496.txt.bz2 On Thu, 20 Feb 2003 15:08:19 -0500, Elena Zannoni said: > A few comments Ack! I was considering this patch to be withdrawn, and I hadn't noticed that it got added to GNATS. Right now, all I'm proposing is the search_symbols part of this patch (with a comment added), as in PR symtab/1049. (Incidentally, I'm curious if Daniel considers the patch in symtab/1070 to still be active: if so, please review the e-mail discussion that Daniel and I had about this. I actually have more to say about that if you're considering approving that patch.) Having said that, I'm glad that you're thinking about this, because I am soon planning to propose a similar (or maybe identical) patch to modify lookup_symbol_aux_minsyms. So here's a discussion of facts relevant to a future version of this patch. At the time that I proposed this patch originally, I didn't understand the history of symbols and partial symbols very well. At the time I proposed this patch, I had assumed that partial symbol were supposed to be demangled, and that the fact that they weren't was a bug. But then Peter Schauer kindly explained more of the history of partial symbols and demangling; I have a _much_ clearer picture of the situation now, and I believe I understand pretty much all of the details of lookup_symbol_aux. For a long time, partial symbols were intentionally left mangled, because demangling partial symbols posed an unacceptable time cost (and a less important but nontrivial memory cost). This led to a problem in lookup_symbol: lookup_symbol has to get passed a demangled name (after all, it usually derives from something the user types), but the symbol that it's supposed to find might be in a psymtab that hasn't yet been converted to a symtab, in which case we need access to the mangled name to find it. So how can we find that symbol? The solution is this: if a symbol's mangled name and demangled name are different, then there must be a minimal symbol associated to it. (After all, name mangling is only done in order to make the linker happy: and that's exactly where minimal symbols come into play.) And, while we weren't demangling names of partial symbols, we were demangling names of minimal symbols. So the logic (once we get to the "look for global symbols part" of lookup_symbol_aux) is: 1) Look in symtabs. Demangled names are okay here. 2) Now we want to look in psymtabs, but we can't: we have a demangled name, and we need a mangled name to look in symtabs. The solution then is: 3) Use the demangled name to find a minimal symbol, if we can. If we do find a minimal symbol, then we might be able to find the symtab directly (if the symbol is associated to a function), or we might need to get the mangled name from the minimal symbol. 4) Once we have the mangled name from the minimal symbol, we can proceed with partial symbol lookup; we do this via a recursive call to lookup_symbol_aux. 5) If we didn't find a minimal symbol in step 3, then maybe we didn't need a mangled name; in that case, just proceed along to the partial symbol check, using the demangled name. In particular, I'm now confident that the role for minimal symbols is to do demangled->mangled name translation; any other use of minimal symbols is either an optimization or an unintended side effect. Of course, there weren't comments actually _explaining_ this, and the resulting control flow depended on some unclear and subtle assumptions. It broke code in the HP case; and everything broke when it was no longer possible to pass lookup_symbol_aux a mangled name instead of a demangled name. But now partial symbols store demangled names in a pleasantly efficient way, thanks to Daniel's recent patch. Which means that the whole raison d'etre of using minimal symbols in lookup_symbol_aux is no more. This has two consequences: 1) The second part of lookup_symbol_aux_minsyms should definitely go. It's caused problems in the past, it's causing problems right now, and it's entirely superfluous: whatever consequences it might have now are entirely negative ones. 2) The rest of lookup_symbol_aux_minsyms is unnecessary. It may be an optimization; it may be a pessimization. It's certainly a pessimization in some circumstances; given that it complicates the function in question, we should get rid of it unless we can find situations where it makes a real difference to the good. My guess is that there are no such situations, but that's only a guess; I have some ideas about how to test this, so I'll do some timings and come back with a recommendation. So if, for now, you could just give PR symtab/1049 a scan and approve it (it shouldn't be too controversial, I hope), I'd appreciate it. And think about the above, just to make sure that I'm not sounding crazy. Once it's approved (and once I have a spare moment!), I'll then submit two patches: 1) A patch to correct a slight bug that remains in lookup_partial_symbol. Basically, partial symbols are sorted via strcmp but we want to use strcmp_iw as our matching criterion; strcmp and strcmp_iw aren't _quite_ suitable to be used together in this way. 2) A patch to either get rid of the second half of lookup_symbol_aux_minsyms or to get rid of all of it, depending on whether or not I can find a situation where it helps to leave it in place. David Carlton carlton@math.stanford.edu