Re: [rfa] more lookup_symbol_aux_minsyms futzing

[David Carlton <carlton at math dot stanford dot edu>]

On Mon, 23 Dec 2002 22:59:39 -0800, Jason Molenda <jason-swarelist@molenda.com> said:

> lookup_symbol_aux is called with a C++ unmanged+mangled name,
> lookup_symbol_aux_symtabs doesn't get a match because we've only
> read in the psymtab at this point; lookup_symbol_aux_minsyms calls
> lookup_symbol_aux with the mangled+mangled names as a last resort,
> and something lame happened from there on (but I don't remember
> exactly...) -- and eventually we call lookup_symbol_aux_psymtabs
> with the mangled+mangled names -- lookup_partial_symbol gets passed
> a mangled name, says there is a match because psymtabs have only
> mangled names.  We parse in the symtab, then we do lookup_block_symbol
> with the mangled name, but the lookup_block_symbol hash of the mangled
> name doesn't match that of the unmangled name, so we issue the error

>               error ("Internal: %s symbol `%s' found in %s psymtab but not in symtab.\n%s may be an inlined function, or may be a template function\n(if a template, try specifying an instantiation: %s<type>).",
>                      block_index == GLOBAL_BLOCK ? "global" : "static",

That sounds like an entirely plausible scenario to me.  I didn't spend
the time coming up with a test case triggering that code path (it's
really tedious to write test cases for partial symbol tests,
at least if they interact with hashing like this), but that code path
is there, and it shouldn't be.

> It also seemed incorrect for lookup_symbol_aux to call
> lookup_symbol_aux_minsyms before lookup_symbol_aux_psymtabs, but 
> I'm guessing this is done on purpose based on this comment in your
> patch -

>> +	  /* We found a minimal symbol in spite of not finding a
>> +	     symbol.  This probably means that the symbol in question
>> +	     is in a partial symbol table that hasn't been loaded, but
>> +	     it may mean that it's from code compiled without -g.
>> +	     Partial symbol table searches are kind of expensive; if
>> +	     the symbol corresponds to a function, we can find the
>> +	     appropriate symtab more quickly by calling
>> +	     find_pc_sect_symtab, so let's try that.  */
>> +

Keep in mind while reading that comment that it's purely a guess
trying to justify existing code years after it was written.  (Or
really, after it's been accreting layers of not-entirely-compatible
changes for years.)  I think it's a fairly well-informed guess; on the
other hand, when writing the comment, my frame of mind was more
"assuming that there's a reason for the code to be done this way, what
is that reason" rather than "should the code _really_ be done this
way?"

I do think that it's reasonable for the code to have a minimal symbol
check before the partial symbol check - the partial symbol check
doesn't use the fastest algorithm in the world, so why not speed it up
if it's easy to do so?  On the other hand, if I were writing the code
from scratch, I'd probably omit the minsym check entirely; if
optimization proved desirable, I'd rather tackle that issue head-on.
I don't think I'd move the minsym check after the psymtab check: it
seems to me that if the psymtab check succeeds but the minsym check
fails then that's a bug in the partial symbols, and I'd rather know
about such bugs than have them papered over like that.  (I can't think
of any situations where the minsym check should succeed but the psymtab
check should fail, modulo issues like (lack of) mangling in the
psymtabs.)

> The test case I have from the customer is a nice little thing but it
> pulls in a big gop of Apple system libraries so I don't know how
> easy it'd be to make a portable version.  At the very least, I'll be
> looking at this tomorrow and should know better how we can fix it
> up.

With luck, my patch in the message you're replying to and my other
patch from yesterday demangling partial symbol names will be more or
less what you're looking for.  Let me know how it turns out!

David Carlton
carlton@math.stanford.edu