Re: [Linux-ia64] Re: gdb null ptr

[Daniel Berlin <dberlin@redhat.com>]

Kevin Buettner <kevinb@cygnus.com> writes:

> [CC to linux-ia64@linuxia64.org trimmed since this is primarly a gdb
> matter.]
> 
> On Nov 3, 10:20pm, Daniel Berlin wrote:
> 
> > > I will leave it to the dwarf2 maintainers to decide whether this
> > > patch is acceptable or if it would be better to implement one of
> > > Jim's other suggestions.
> > > 
> > 
> > Unfortuantely , this is actually still wrong for languages other than
> > C++, because we don't have the same guarantees about uniqueness in the name.
> > 
> > I was actually in the process of readying patches that add the same
> > type of name based caching (based on mangled name) to partial and
> > normal symbol reading, which gives us an amazing win for C++.
> > 
> > These patches also moved all of the name caching into "if (cu_language
> > == language_cplus)" blocks, doing what we used to do in the old case
> > (IE no caching).
> > 
> > Rather than let this stay broken until i finish cleaning up those
> > patches, here is a patch that moves the type caching so it only
> > happens for C++ CU's.
> 
> I hereby withdraw my patch from consideration in favor of Daniel's
> patch.  In my opinion, Daniel's patch should go in ASAP since the code
> in question has been broken since June 5 according to cvs annotate. 
> For some reason, the corresponding ChangeLog entry is May 30.
> 
> The other alternative is to revert the May 30/June 5 patch.

This may be a better idea, it depends on how people want the patches
broken up, I guess.

It might make more sense to revert the old one, then submit the
patch to re-add caching of types, symbols, and psymbols (each seperately).
As opposed to submitting a patch to fix the caching of types, then add
symbols and psymbols caching.
Makes no difference to me, might seem more logical to someone else.

It was something I had intended to do a long while ago, as it's been
pretty far along for a long while (formatting and changelog entries have
to be finished, and some testcases worked up, but the code itself,
seems to work fine), but i've been sidetracked.


> 
> > Unless other languages make the same guarantees, we can't do the same
> > optimization.
> 
> Agreed.  I've constructed a small example and have convinced myself that
> the code presently in dwarf2read.c is broken for C, but not for C++.

Yup. I've got some comments in the patch that adds similar caching for
symbols and psymbols on why it works, especially since the fact that
it works for statics is dependent on the way the dwarf2 reader works
right now (In particular it resets the cached hash tables at the
beginning of each CU, so you end up with at most one of each
type/symbol/psymbol of a given mangled name per file. ). I also added
warnings and notes in the approriate places saying "If you change the
dwarf2 reader in such a way that it no longer resets the hash tables
at the beginning of each symtab, you'll need to change this code".

> 
> > I have added the same type of code kevin has to the patches i am
> > readying.
> 
> Good.
> 
> You may wish to prepare a new interim patch which has a proper
> ChangeLog entry, fixes the formatting of the long ``nameoftype = ...''
> line, and which incorporates my proposed fix of not caching the type
> unless it has a name or tag name.

I plan on it. It's on my todo list for tomorrow. 

>  This latter check is still
> necessary to account for error conditions in reading the type die. 

Yup.

> you feel that you do not have time to do this, let me know, and I'll
> put it together for you.

I should, my todo list for tomorrow includes this patch, and the partial
symbol lookup fix.

The only reason i haven't submitted it, the changelogs, with an RFA in
the subject, is because i wanted to make sure there was nothing
egregiously wrong with it that anyone else could see before I polish
it and submit.
--Dan

> 
> Kevin