[rfa/symtab/c++] fix c++ rtti type lookup

[rfa/symtab/c++] fix c++ rtti type lookup

[Michael Elizabeth Chastain]

This patch is a partial fix for PR c++/1465, which is the bug where RTTI
lookup gets the "namespace" symbol instead of the class symbol.  It also
fixes PR c++/1377, which was another manifestation of the same bug.

  http://sources.redhat.com/gdb/bugs/1465
  http://sources.redhat.com/gdb/bugs/1377

These bugs are regressions versus gdb 6.0 so they are high priority.

First, lookup_rtti_type is a new function which takes a name and a block
and returns the class type of that name.  It's basically a block of code
extracted from gnuv3_rtti_type.  gnuv3_rtti_type calls lookup_rtti_type
now.

Then I changed gnuv2_rtti_type to call lookup_rtti_type.  This
changes the symbol lookup from VAR_DOMAIN to STRUCT_DOMAIN.

lookup_rtti_type is full of checks and warnings so that it doesn't
return crap to its caller.

This fixes the simple cases of the bug.

More work is needed:

. The calls to lookup_rtti_type need a proper "block" parameter.
  The old code needed this too; I haven't regressed anything.
  I put FIXME notes in for this.

. hpacc_value_rtti_type has the same buggy code.
  I can't change the code because I can't test it,
    but I can put it a big FIXME into it.
  (I wonder if anyone still uses HP aCC with gdb).

. Nested types give a warning and don't work.
  It would be nice to make them work.

. Types with virtual bases appear to work with v3, but give a warning
  and don't work with v2.  "don't work" probably means that they fall back
  to the static type rather than the dynamic type.

Testing: I tested with gcc v2 and v3, dwarf-2 and stabs+.   
Nothing got worse.  gdb.cp/class2.exp has a specific test for this,
which now passes.

Some tests in virtfunc.exp that broke after the 2003-09-11 namespace
commit started working again.  That was pr gdb/1377.

I think I need approval from a symtab maintainer to add the
new utility function "lookup_rtti_type", and then approval
from a C++ maintainer to change gnuv2_rtti_type and gnuv3_rtti_type
to call lookup_rtti_type.

Okay to commit?

Michael C

2003-11-26  Michael Chastain  <mec.gnu@mindspring.com>

	Partial fix for PR c++/1465.
	Fix for PR c++/1377.
	* symtab.h (lookup_rtti_type): New function.
	* gdbtypes.c (lookup_rtti_type): New function.
	* gnu-v2-abi.c: Update copyright years.
	(gnuv2_rtti_type): Call lookup_rtti_type.
	* gnu-v3-abi.c: Update copyright years.
	(gnuv3_rtti_type): Call lookup_rtti_type.

Index: symtab.h
===================================================================
RCS file: /cvs/src/src/gdb/symtab.h,v
retrieving revision 1.83
diff -c -3 -p -r1.83 symtab.h
*** symtab.h	11 Nov 2003 20:04:52 -0000	1.83
--- symtab.h	27 Nov 2003 01:20:19 -0000
*************** extern struct type *lookup_union (char *
*** 1050,1055 ****
--- 1050,1059 ----
  
  extern struct type *lookup_enum (char *, struct block *);
  
+ /* Lookup an rtti type by name, within a specified block */
+ 
+ extern struct type *lookup_rtti_type (const char *, struct block *);
+ 
  /* from blockframe.c: */
  
  /* lookup the function symbol corresponding to the address */
Index: gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.78
diff -c -3 -p -r1.78 gdbtypes.c
*** gdbtypes.c	6 Oct 2003 19:27:12 -0000	1.78
--- gdbtypes.c	27 Nov 2003 01:20:22 -0000
*************** lookup_struct_elt_type (struct type *typ
*** 1287,1292 ****
--- 1287,1335 ----
    return (struct type *) -1;	/* For lint */
  }
  
+ /* Lookup the rtti type for a class name. */
+ 
+ struct type *
+ lookup_rtti_type (const char *name, struct block *block)
+ {
+   struct symbol * rtti_sym;
+   struct type * rtti_type;
+ 
+   rtti_sym = lookup_symbol (name, block, STRUCT_DOMAIN, NULL, NULL);
+ 
+   if (rtti_sym == NULL)
+     {
+       warning ("RTTI symbol not found for class '%s'", name);
+       return NULL;
+     }
+ 
+   if (SYMBOL_CLASS (rtti_sym) != LOC_TYPEDEF)
+     {
+       warning ("RTTI symbol for class '%s' is not a typedef", name);
+       return NULL;
+     }
+ 
+   rtti_type = SYMBOL_TYPE (rtti_sym);
+ 
+   switch (TYPE_CODE (rtti_type))
+     {
+     case TYPE_CODE_CLASS:
+       break;
+     case TYPE_CODE_NAMESPACE:
+       /* chastain/2003-11-26: the symbol tables often contain fake
+          symbols for namespaces with the same name as the struct.
+ 	 This warning is an indication of a bug in the lookup order
+ 	 or a bug in the way that the symbol tables are populated.  */
+       warning ("RTTI symbol for class '%s' is a namespace", name);
+       return NULL;
+     default:
+       warning ("RTTI symbol for class '%s' has bad type", name);
+       return NULL;
+     }
+ 
+   return rtti_type;
+ }
+ 
  /* If possible, make the vptr_fieldno and vptr_basetype fields of TYPE
     valid.  Callers should be aware that in some cases (for example,
     the type or one of its baseclasses is a stub type and we are
Index: gnu-v2-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v2-abi.c,v
retrieving revision 1.13
diff -c -3 -p -r1.13 gnu-v2-abi.c
*** gnu-v2-abi.c	16 Sep 2003 18:56:35 -0000	1.13
--- gnu-v2-abi.c	27 Nov 2003 01:20:23 -0000
***************
*** 1,6 ****
  /* Abstraction of GNU v2 abi.
  
!    Copyright 2001, 2003 Free Software Foundation, Inc.
  
     Contributed by Daniel Berlin <dberlin@redhat.com>
  
--- 1,6 ----
  /* Abstraction of GNU v2 abi.
  
!    Copyright 2001, 2002, 2003 Free Software Foundation, Inc.
  
     Contributed by Daniel Berlin <dberlin@redhat.com>
  
*************** gnuv2_value_rtti_type (struct value *v, 
*** 259,267 ****
    *(strchr(demangled_name,' '))=0;
  
    /* Lookup the type for the name */
!   rtti_type=lookup_typename(demangled_name, (struct block *)0,1);
! 
!   if (rtti_type==NULL)
      return NULL;
  
    if (TYPE_N_BASECLASSES(rtti_type) > 1 &&  full && (*full) != 1)
--- 259,267 ----
    *(strchr(demangled_name,' '))=0;
  
    /* Lookup the type for the name */
!   /* FIXME: chastain/2003-11-26: block=NULL is bogus.  See pr gdb/1465. */
!   rtti_type = lookup_rtti_type (demangled_name, NULL);
!   if (rtti_type == NULL)
      return NULL;
  
    if (TYPE_N_BASECLASSES(rtti_type) > 1 &&  full && (*full) != 1)
Index: gnu-v3-abi.c
===================================================================
RCS file: /cvs/src/src/gdb/gnu-v3-abi.c,v
retrieving revision 1.19
diff -c -3 -p -r1.19 gnu-v3-abi.c
*** gnu-v3-abi.c	22 Aug 2003 20:45:55 -0000	1.19
--- gnu-v3-abi.c	27 Nov 2003 01:20:23 -0000
***************
*** 1,7 ****
  /* Abstraction of GNU v3 abi.
     Contributed by Jim Blandy <jimb@redhat.com>
  
!    Copyright 2001, 2002 Free Software Foundation, Inc.
  
     This file is part of GDB.
  
--- 1,7 ----
  /* Abstraction of GNU v3 abi.
     Contributed by Jim Blandy <jimb@redhat.com>
  
!    Copyright 2001, 2002, 2003 Free Software Foundation, Inc.
  
     This file is part of GDB.
  
*************** gnuv3_rtti_type (struct value *value,
*** 196,202 ****
    struct minimal_symbol *vtable_symbol;
    const char *vtable_symbol_name;
    const char *class_name;
-   struct symbol *class_symbol;
    struct type *run_time_type;
    struct type *base_type;
    LONGEST offset_to_top;
--- 196,201 ----
*************** gnuv3_rtti_type (struct value *value,
*** 255,280 ****
    class_name = vtable_symbol_name + 11;
  
    /* Try to look up the class name as a type name.  */
!   class_symbol = lookup_symbol (class_name, 0, STRUCT_DOMAIN, 0, 0);
!   if (! class_symbol)
!     {
!       warning ("can't find class named `%s', as given by C++ RTTI", class_name);
!       return NULL;
!     }
! 
!   /* Make sure the type symbol is sane.  (An earlier version of this
!      code would find constructor functions, who have the same name as
!      the class.)  */
!   if (SYMBOL_CLASS (class_symbol) != LOC_TYPEDEF
!       || TYPE_CODE (SYMBOL_TYPE (class_symbol)) != TYPE_CODE_CLASS)
!     {
!       warning ("C++ RTTI gives a class name of `%s', but that isn't a type name",
! 	       class_name);
!       return NULL;
!     }
! 
!   /* This is the object's run-time type!  */
!   run_time_type = SYMBOL_TYPE (class_symbol);
  
    /* Get the offset from VALUE to the top of the complete object.
       NOTE: this is the reverse of the meaning of *TOP_P.  */
--- 254,263 ----
    class_name = vtable_symbol_name + 11;
  
    /* Try to look up the class name as a type name.  */
!   /* FIXME: chastain/2003-11-26: block=NULL is bogus.  See pr gdb/1465. */
!   run_time_type = lookup_rtti_type (class_name, NULL);
!   if (run_time_type == NULL)
!     return NULL;
  
    /* Get the offset from VALUE to the top of the complete object.
       NOTE: this is the reverse of the meaning of *TOP_P.  */

Re: [rfa/symtab/c++] fix c++ rtti type lookup

[David Carlton]

On Thu, 27 Nov 2003 02:09:03 -0500 (EST), mec.gnu@mindspring.com (Michael Elizabeth Chastain) said:

> This patch is a partial fix for PR c++/1465, which is the bug where
> RTTI lookup gets the "namespace" symbol instead of the class symbol.

Thanks; I can't recommend this for approval, but it basically looks
good to me.  A few quibbles:

> I think I need approval from a symtab maintainer to add the new
> utility function "lookup_rtti_type", and then approval from a C++
> maintainer to change gnuv2_rtti_type and gnuv3_rtti_type to call
> lookup_rtti_type.

Just C++ approval is fine - it's not in symtab.c, and we have other
C++-specific symbol-lookup code that was moved out of symtab.c exactly
so that it wouldn't require symtab approval.  Though I would put the
new function in cp-namespace.c/cp-support.h, until we decide to create
a cp-symtab.c, because that's where the other C++-specific symtab
stuff currently is.

> +   if (SYMBOL_CLASS (rtti_sym) != LOC_TYPEDEF)
> +     {
> +       warning ("RTTI symbol for class '%s' is not a typedef", name);

I would change this to 'is not a type' - we put them in something
called LOC_TYPEDEF, but users would expect 'typedef' to refer to the
C/C++ meaning of the term.

David Carlton
carlton@kealia.com

Re: [rfa/symtab/c++] fix c++ rtti type lookup

[Elena Zannoni]

Michael Elizabeth Chastain writes:
 > This patch is a partial fix for PR c++/1465, which is the bug where RTTI
 > lookup gets the "namespace" symbol instead of the class symbol.  It also
 > fixes PR c++/1377, which was another manifestation of the same bug.
 > 
 >   http://sources.redhat.com/gdb/bugs/1465
 >   http://sources.redhat.com/gdb/bugs/1377
 > 
 > These bugs are regressions versus gdb 6.0 so they are high priority.
 > 
 > First, lookup_rtti_type is a new function which takes a name and a block
 > and returns the class type of that name.  It's basically a block of code
 > extracted from gnuv3_rtti_type.  gnuv3_rtti_type calls lookup_rtti_type
 > now.
 > 
 > Then I changed gnuv2_rtti_type to call lookup_rtti_type.  This
 > changes the symbol lookup from VAR_DOMAIN to STRUCT_DOMAIN.
 > 
 > lookup_rtti_type is full of checks and warnings so that it doesn't
 > return crap to its caller.
 > 
 > This fixes the simple cases of the bug.
 > 
 > More work is needed:
 > 
 > . The calls to lookup_rtti_type need a proper "block" parameter.
 >   The old code needed this too; I haven't regressed anything.
 >   I put FIXME notes in for this.
 > 
 > . hpacc_value_rtti_type has the same buggy code.
 >   I can't change the code because I can't test it,
 >     but I can put it a big FIXME into it.
 >   (I wonder if anyone still uses HP aCC with gdb).
 > 

here is a quandary. Should we carry around such code if nobody uses
it. If we cannot test it we might as well explicitly admit it.

 > . Nested types give a warning and don't work.
 >   It would be nice to make them work.
 > 

is there a bug/testcase?

 > . Types with virtual bases appear to work with v3, but give a warning
 >   and don't work with v2.  "don't work" probably means that they fall back
 >   to the static type rather than the dynamic type.
 > 
 > Testing: I tested with gcc v2 and v3, dwarf-2 and stabs+.   
 > Nothing got worse.  gdb.cp/class2.exp has a specific test for this,
 > which now passes.
 > 
 > Some tests in virtfunc.exp that broke after the 2003-09-11 namespace
 > commit started working again.  That was pr gdb/1377.
 > 
 > I think I need approval from a symtab maintainer to add the

If you move the function to cp-support.c and cp-support.h I think you
can avoid touching symtab.h altogether.

 > new utility function "lookup_rtti_type", and then approval
 > from a C++ maintainer to change gnuv2_rtti_type and gnuv3_rtti_type
 > to call lookup_rtti_type.
 > 
 > Okay to commit?
 > 

I think David ok'd it already.

elena


 

Re: [rfa/symtab/c++] fix c++ rtti type lookup

[David Carlton]

On Thu, 4 Dec 2003 16:16:37 -0500, Elena Zannoni <ezannoni@redhat.com> said:

>> Okay to commit?

> I think David ok'd it already.

I can't approve it; it needs Daniel's approval.

David Carlton
carlton@kealia.com

Re: [rfa/symtab/c++] fix c++ rtti type lookup

[Michael Elizabeth Chastain]

eza> here is a quandary. Should we carry around such code if nobody uses
eza> it. If we cannot test it we might as well explicitly admit it.

We have a big issue here.

There are a lot of multi-user Unix systems that aren't getting tested
enough, or tested at all: aix, hpux, irix, solaris.  There are very few
posts to gdb-testers for those architectures.

When I clean up my mailbox and my back-log, I'm going to work on this.
I'm going to ask around for people who can give me ssh accounts on such
machines, or are willing to become volunteer testers.

mec> . Nested types give a warning and don't work.
mec>   It would be nice to make them work.
eza> is there a bug/testcase?

Yes, gdb.cp/local.exp.

eza> If you move the function to cp-support.c and cp-support.h I think you
eza> can avoid touching symtab.h altogether.

Right.  I put out a second version of the patch organized like that.
So you can discard the first version.

Michael C