Re: [rfa] generate fully-qualified names for types

[Daniel Jacobowitz <drow at mvista dot com>]

On Wed, Nov 12, 2003 at 10:53:01AM -0800, David Carlton wrote:
> This patch tells GDB to generated fully-qualified names for types that
> are nested within other types or within namespaces.  It only does this
> in the DWARF-2 case; I leave the other debug formats (stabs, in
> particular) up to somebody else.

I can do stabs.  It was remarkably easy the last time I tried. 
However, to get any kind of usable result requires fixing GCC's stabs
emission to include a little more information and I ran into a
roadblock, so I put it down until I felt more ... whatever it is that I
need to feel to work on using stabs for C++.  It can wait.

> As with all of this namespace work, I try to support both the case
> where GCC generates DW_TAG_namespace and where GCC doesn't generate
> it.  (The latter being, unfortunately, the case for all released
> versions of GCC, and the last I heard it wasn't going to be fixed for
> GCC 3.4 either.)  If, when compiling a given compilation unit, GDB
> ever sees a DW_TAG_namespace, it uses the hierarchical structure of
> DWARF combined with DW_AT_name to generate the name attributes.  If
> GDB hasn't seen such a tag then, when it sees a class that isn't
> nested within another class, GDB looks for a member function with a
> mangled name and demangles that name to figure out if the class is
> defined within a namespace.
> 
> This will make a big difference; the main effect will be that GDB will
> stop constantly printing out RTTI warnings.
> 
> There are, however, some issues, even some regressions.  Some of these
> I have fixed on my branch, and will submit subsequent patches to deal
> with them.  (This patch, however, is big enough as is that I really
> didn't want to include those fixes right here.)  Some of them I know
> how to fix but haven't.  Some of them seem to be unavoidable
> side-effects of the fact that GCC isn't generating good enough debug
> info.  And there are doubltess some lurking gotchas that I'm not aware
> of.
> 
> The issues that come to mind are:
> 
> * I haven't modified all of the symbol reading functions to properly
>   use this information.  I'll need to modify linespec, the overload
>   handling functions, and lookup_transparent_type.  I've done this on
>   the branch; I'll move that over next.
> 
> * lookup_transparent_type poses particular problems.  That function
>   deals with the situation where a class has an abstract declaration
>   but not a full definition.  Unfortunately, in this situation we
>   don't have any member functions around to demangle, which means that
>   we generate symbols with the wrong name.  (If GCC doesn't generate
>   DW_TAG_namespace, that is.)  Oops.  I have a hack on my branch that
>   increases the number of cases where we get the right symbol back,
>   but it really is a hack.  I'm not sure that there's anything we can
>   do about this without more help from GCC.  (Hmm: why do we generate
>   symbols associated to those declarations in the first place?  Maybe
>   that's worth thinking about.)  You'll notice that gdb.cp/rtti.exp

We need them.  You can have this, and often do in GDB:

struct Foo;
extern void foofoo(struct Foo *);

We have to have a type associated with struct Foo in order to print the
type of this function, call it, et cetera.

>   has one KFAIL=>PASS transition and one KFAIL=>FAIL transition
>   (unless you use a hacked GCC); the latter is a manifestation of this
>   issue.
> 
> * The dependency on the demangler is unfortunate, and it leads to
>   regressions in gdb.cp/templates.exp.  There are various ways that
>   the demangler could be used.  If I'm in a situation like this:
> 
>   namespace N {
>     class C {
>       void foo();
>     };
>   }
> 
>   then we need to use the demangler to deduce the existence of N at
>   all.  Basically, we look at the mangled name for N::C::foo(),
>   demangle it, and notice that it starts with 'N::C'.
> 
>   But there are two ways of using this information: I could grab all
>   of 'N::C' from the demangler, or I could grab just 'N::' from the
>   demangler, and grab 'C' from DW_AT_name.  It doesn't make a
>   difference in this simple example, but if C is a templated class, it
>   can make a difference.
> 
>   I chose to do the former.  The reason for this is that the demangler
>   also gets used in another situation: when trying to figure out the
>   RTTI information, we look at this symbol associated to the vtable,
>   take its demangled name, and look at the part of it that should give
>   the name of the class in question.  If that name doesn't match the
>   name of the relevant symbol, we get this annoying RTTI warning.  So
>   I thought it was better to use the demangler in both places.
> 
>   But it would be better still if it didn't matter; I'm planning to
>   patch the demangler, though I haven't yet done that on my branch.
>   The failures in templates.exp stem from these differences:
> 
>   1) The demangler introduces whitespace.  This seems perfectly
>      acceptable (and will be handled just fine by our symbol lookup
>      code); here, we should fix templates.exp to accept it.
> 
>   2) The demangler uses 'char const *' instead of 'const char *'.  In
>      an ideal world, our symbol lookup code would handle this
>      difference (PR gdb/931); that ideal world is currently nothing
>      more than a hallucination, so for the time being we have to pick
>      the one or the other.  The compiler uses const char *, and most
>      programmers do as well, so we should see if we can fix the
>      demangler to use it too.
> 
>   3) In some situations, the demangler is introducing unnecessary
>      parentheses; as with case #2, we should see if we can fix the
>      demangler to not do that.  (But this really is very much a corner
>      case.)

Are the failures in templates.exp real failures caused by
internal-to-GDB differences in output, or are they caused by tolerable
changes in the output?  I saw mostly the latter when working with
stabs.

I would appreciate it if you could prepare a matching patch to
templates.exp which either fixes the failures if they are output
issues, or adds KFAIL patterns otherwise.  I know it's a nuisance but I
do not want to leave failures that we aren't going to fix in the short
term.

A KFAIL for the rtti.exp test would be an added bonus.

Oh, and as I said elsewhere I don't think that "fixing" the demangler
is a worthwhile exercise.

> * DW_AT_specification leads to some charming issues: what that does is
>   allows DIEs to refer to other DIEs anywhere in the hierarchy.  This
>   is a major headache if, when looking at a DIE, you need information
>   that's only stored in its parents.  I've dealt with this in the
>   symbol case, though I'm sure I missed some spots; I've completely
>   ignored this in the partial symbol case.  Basically, as far as I can
>   tell, there's simply no way to deal with this in the partial symbol
>   case without jumping through hoops (or making the partial symbol
>   reader look an awful lot like the full symbol reader).  The real
>   solution is to get GCC to generate .debug_pubtypes; until that
>   happens, I'm optimistic that this isn't not going to be _too_ much
>   of a problem in practice.  (In concrete terms, I got bug reports
>   here at Kealia when I hadn't realized that this issue existed at
>   all; those bug reports went away when I implemented a fix for full
>   symbols.)

OK.  By the way, I don't intend for GCC to generate .debug_pubtypes,
but I do intend for it to generate something twice proposed on the
dwarf2 list: .debug_info_index.  Which, as you say, looks "an awful lot
like the full symbol reader" - it uses DIEs.

> * This doesn't deal with the interactions between nested types and
>   inheritance.  Given a situation like this:
> 
>   class C {
>     class N;
>   };
> 
>   class D : public C {
>   };
> 
>   then D::N should be the same as C::N, but GDB has no idea what
>   you're talking about if you refer to D::N.  (I think this is the
>   issue behind PR gdb/1417.)  I haven't even begun to think about what
>   to do with that; this patch doesn't make the situation any worse,
>   however.

Also OK.  It would be nice to fix this, but it requires even more
drastic surgery to the symbol tables.  I have some ideas but not enough
time :)

> 
> I think that's about it.  I've tested this on i686-pc-linux-gnu, with
> GCC 3.2.3.  The results:
> 
> * With DWARF-2, all the new tests pass except for the four KFAILed
>   ones.  One test in rtti.exp goes KFAIL=>PASS; another one goes
>   KFAIL=>FAIL.  (See the second bullet point above.)  12 tests in
>   templates.exp go (PASS,KFAIL)=>FAIL.  (See the third bullet point
>   above.)
> 
> * With a GCC version that produces DW_TAG_namespace, all the new tests
>   pass except for the four KFAILed ones.  Both the rtti tests go
>   KFAIL=>PASS.  Those 12 template.exp tests FAIL.
> 
> * With stabs, all the new tests {,K}FAIL; no other changes.  A month
>   or two ago, Michael Chastain noticed that a patch of mine introduced
>   two regressions with GCC 3.3, stabs; my hope is that this patch will
>   fix one of those regressions, but I haven't checked to see.
> 
> As I've explained above, I think that these regressions are
> acceptable.

Agreed with the caveat above.

> @@ -214,22 +225,15 @@ cp_set_block_scope (const struct symbol 
>  
>    if (SYMBOL_CPLUS_DEMANGLED_NAME (symbol) != NULL)
>      {
> -#if 0
> -      /* FIXME: carlton/2003-06-12: As mentioned above,
> -	 'processing_has_namespace_info' currently isn't entirely
> -	 reliable, so let's always use demangled names to get this
> -	 information for now.  */
> -
>        if (processing_has_namespace_info)
>  	{
>  	  block_set_scope
> -	    (block, obsavestring (processing_current_namespace,
> -				  strlen (processing_current_namespace),
> +	    (block, obsavestring (processing_current_prefix,
> +				  strlen (processing_current_prefix),
>  				  obstack),
>  	     obstack);
>  	}
>        else
> -#endif
>  	{
>  	  /* Try to figure out the appropriate namespace from the
>  	     demangled name.  */

How many of these prefixes end up being identical - is it usually close
to one, or do we get one copy for each method in a class, et cetera? 
We could use a bcache for this if we have duplicates.

Otherwise I have no complaints about this patch.  The non-symtab parts
are OK and the rest looks good to me.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer