Re: [patch] Do not add partial_symbol again and again to the list

[Aleksandar Ristovski <aristovski@qnx.com>]

Daniel Jacobowitz wrote:
> On Mon, Feb 11, 2008 at 05:43:22PM -0500, Aleksandar Ristovski wrote:
>> If one compilation unit has a list of symbols and they appear more than 
>> once... do we really need to have all duplicate records in partial symbols 
>> list? The partial symbol lookup (by symbol name) will find only the first 
>> one matching and probably cause loading the full symbols at which point 
>> all works as before.
> 
> Sure.  But your patches aren't checking one compilation unit, they're
> checking every compilation unit in the objfile at once.  That's why
> they found duplicates.  I don't think any one compilation unit will
> have a duplicate.
Then I am definitely missing something big. Please correct me where I'm wrong.

struct dwarf2_cu has a pointer to struct objfile, which (if I'm not mistaken) 
get's allocated per compilation unit.

Further, struct objfile contains pointer to struct bcache: psymbol_cache which 
gets initialized in objfile initialization (allocate_objfile function). 
Therefore, there should be exactly one psymbol_cache per objfile.

bcache_data hashes objects in psymbol_cache, that is, it will hash only partial 
symbols of that particular compilation unit.

There are two pointers to struct psymbol_allocation_list, both declared in 
struct objfile: global_psymbols and static_psymbols. Therefore, there should be 
exactly one of each per each compilation unit.

In dwarf2read.c we determine whether a symbol belongs to the global or static 
psymbol list and we pass this list along with the objfile pointer to 
add_psymbol_to_list function.

Now we get to the point where I made the change:
The first patch I submitted allows for having a psymbol listed in each list, but 
only once (as opposed to many times before). The second patch I submitted will 
prevent adding a partial symbol to both lists.

What am I missing?

> 
>> Not sure if it is a valid indicator but I didn't have any regressions in 
>> 'make check'.
> 
> Yeah, I think I could write some testcases that were affected by this,
> but I'm not sure.  It's tricky because if something else causes the
> full symtab to load, the problem won't appear.

Yes, that is the tricky part. We want lookup by symbol name to occur in order to 
test this. Maybe adding a maintenance command that would explicitly lookup a 
partial symbol by name?

> 
> Maybe there's some way we can avoid needing psymbols for types at all.
> But I think we need to have a real design and some documentation for
> it instead of just accidentally omitting them.
> 

Not sure how would that work. But I think it would make sense to have only one 
global_psymbols list.