RFA: fix for GDB C++ STABS problem

RFA: fix for GDB C++ STABS problem

[Jim Blandy]

Here's a fix for the GDB C++ STABS problem I just posted a test case
for.  This prevents `read_struct_type' from scribbling over an
existing definition, but it doesn't prevent `cleanup_undefined_types'
from producing bogus cv chains.  That's the next patch coming.

The real fix, of course, is to switch to Dwarf 2.

2002-05-01  Jim Blandy  <jimb@redhat.com>

	* stabsread.c (multiply_defined_struct): New complaint.
	(read_struct_type): If the type we were passed isn't empty, or
	incomplete, don't read the new struct type into it; complain,
	and return the original type unchanged.  Take a new `type_code'
	argument, which is the type code for the new type.
	(read_type): Rather than storing the type's type code here, pass
	it as an argument to read_struct_type, and let that take care of
	storing it.  That way, we don't overwrite the original type code,
	so read_struct_type can use it to decide whether we're overwriting
	something we shouldn't.
	(complain_about_struct_wipeout): New function.

Index: gdb/stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.30
diff -c -r1.30 stabsread.c
*** gdb/stabsread.c	21 Mar 2002 00:53:44 -0000	1.30
--- gdb/stabsread.c	1 May 2002 22:52:37 -0000
***************
*** 136,141 ****
--- 136,142 ----
  attach_fields_to_type (struct field_info *, struct type *, struct objfile *);
  
  static struct type *read_struct_type (char **, struct type *,
+                                       enum type_code,
  				      struct objfile *);
  
  static struct type *read_array_type (char **, struct type *,
***************
*** 235,240 ****
--- 236,244 ----
  static struct complaint unresolved_sym_chain_complaint =
  {"%s: common block `%s' from global_sym_chain unresolved", 0, 0};
  
+ static struct complaint multiply_defined_struct =
+ {"struct/union type gets multiply defined: %s%s", 0, 0};
+ 
  static struct complaint stabs_general_complaint =
  {"%s", 0, 0};
  
***************
*** 2801,2818 ****
  
      case 's':			/* Struct type */
      case 'u':			/* Union type */
!       type = dbx_alloc_type (typenums, objfile);
!       switch (type_descriptor)
! 	{
! 	case 's':
! 	  TYPE_CODE (type) = TYPE_CODE_STRUCT;
! 	  break;
! 	case 'u':
! 	  TYPE_CODE (type) = TYPE_CODE_UNION;
! 	  break;
! 	}
!       type = read_struct_type (pp, type, objfile);
!       break;
  
      case 'a':			/* Array type */
        if (**pp != 'r')
--- 2805,2825 ----
  
      case 's':			/* Struct type */
      case 'u':			/* Union type */
!       {
!         enum type_code type_code = TYPE_CODE_UNDEF;
!         type = dbx_alloc_type (typenums, objfile);
!         switch (type_descriptor)
!           {
!           case 's':
!             type_code = TYPE_CODE_STRUCT;
!             break;
!           case 'u':
!             type_code = TYPE_CODE_UNION;
!             break;
!           }
!         type = read_struct_type (pp, type, type_code, objfile);
!         break;
!       }
  
      case 'a':			/* Array type */
        if (**pp != 'r')
***************
*** 4156,4161 ****
--- 4163,4208 ----
    return 1;
  }
  
+ 
+ /* Sometimes GCC emits struct type forward references without the
+    necessary qualification.  If there's more than one definition for
+    different types that share the same unqualified name, GDB will
+    think they're both definitions of the same type, and try to wipe
+    out each earlier definition with it successor.
+ 
+    Complain that this is happening to TYPE.  */
+ static void 
+ complain_about_struct_wipeout (struct type *type)
+ {
+   char *name = "";
+   char *kind = "";
+ 
+   if (TYPE_TAG_NAME (type))
+     {
+       name = TYPE_TAG_NAME (type);
+       switch (TYPE_CODE (type))
+         {
+         case TYPE_CODE_STRUCT: kind = "struct "; break;
+         case TYPE_CODE_UNION:  kind = "union ";  break;
+         case TYPE_CODE_ENUM:   kind = "enum ";   break;
+         default: kind = "";
+         }
+     }
+   else if (TYPE_NAME (type))
+     {
+       name = TYPE_NAME (type);
+       kind = "";
+     }
+   else
+     {
+       name = "<unknown>";
+       kind = "";
+     }
+ 
+   complain (&multiply_defined_struct, kind, name);
+ }
+ 
+ 
  /* Read the description of a structure (or union type) and return an object
     describing the type.
  
***************
*** 4171,4177 ****
   */
  
  static struct type *
! read_struct_type (char **pp, struct type *type, struct objfile *objfile)
  {
    struct cleanup *back_to;
    struct field_info fi;
--- 4218,4225 ----
   */
  
  static struct type *
! read_struct_type (char **pp, struct type *type, enum type_code type_code,
!                   struct objfile *objfile)
  {
    struct cleanup *back_to;
    struct field_info fi;
***************
*** 4179,4187 ****
--- 4227,4250 ----
    fi.list = NULL;
    fi.fnlist = NULL;
  
+   /* Sometimes GCC emits struct type forward references without the
+      necessary qualification.  If there's more than one definition for
+      different types that share the same unqualified name, GDB will
+      think they're both definitions of the same type, and try to wipe
+      out each earlier definition with it successor.  */
+   if (! (TYPE_CODE (type) == TYPE_CODE_UNDEF
+          || TYPE_STUB (type)))
+     {
+       complain_about_struct_wipeout (type);
+ 
+       /* It's probably best to return the type unchanged.  */
+       return type;
+     }
+ 
    back_to = make_cleanup (null_cleanup, 0);
  
    INIT_CPLUS_SPECIFIC (type);
+   TYPE_CODE (type) = type_code;
    TYPE_FLAGS (type) &= ~TYPE_FLAG_STUB;
  
    /* First comes the total size in bytes.  */

Re: RFA: fix for GDB C++ STABS problem

[Daniel Jacobowitz]

On Wed, May 01, 2002 at 08:22:50PM -0500, Jim Blandy wrote:
> 
> Here's a fix for the GDB C++ STABS problem I just posted a test case
> for.  This prevents `read_struct_type' from scribbling over an
> existing definition, but it doesn't prevent `cleanup_undefined_types'
> from producing bogus cv chains.  That's the next patch coming.
> 
> The real fix, of course, is to switch to Dwarf 2.

Amen!  Thanks for tracking this down.

We could also start supporting the current Sun STABS spec, and then
make GCC emit it - it supports things like namespaces quite well.  But
at that point, well, most ELF targets already default to DWARF-2 in GCC
3.1.

I'd rather just go forward with the namespace improvements we were
discussing and leave STABS to rot.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer