Re: [patch] Workaround buggy GCC PR 47510 produced code

[Jan Kratochvil <jan dot kratochvil at redhat dot com>]

On Thu, 14 Apr 2011 22:54:36 +0200, Tom Tromey wrote:
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Jan> The proposed patch could make false warnings if any code out there
> Jan> has DW_TAG_typedef with children, it does not check for specific
> Jan> GCC versions.  Still such DWARF is invalid, although in such case
> Jan> complaint() would be more appropriate than warning().  Does it make
> Jan> sense to differentiate it?
> 
> If there is no specific reason to use warning, I think you should use
> complaint.

It was intentional as normal users do not see `complaint' and they may be
confused.  But GDB could then crash when trying to use such objfile.

I hope all the use case were caught and GDB will never print invalid data and
GDB will never crash as `complaint' is normally not seen by the user.

No regressions on {x86_64,x86_64-m32,i686}-fedora15-linux-gnu.

-=recent GCC
+=buggy GCC

 (gdb) ptype t::t
-type = void (t * const)
-(gdb) PASS: gdb.cp/anon-struct.exp: print type of t::t
+Invalid anonymous type t [in module .../gdb.cp/anon-struct], GCC PR debug/47510 bug?
+(gdb) FAIL: gdb.cp/anon-struct.exp: print type of t::t

(OK, there could be XFAIL.)


Thanks,
Jan


gdb/
2011-04-16  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* c-exp.y (qualified_name): Call destructor_name_p with $1.type.
	(classify_inner_name): Call cp_lookup_nested_type with
	yylval.tsym.type.
	* cp-namespace.c (cp_lookup_nested_type): New variable
	saved_parent_type.  Call CHECK_TYPEDEF for parent_type.  Call
	type_name_no_tag_or_error with saved_parent_type.
	* dwarf2read.c (load_partial_dies): Read in any children of
	DW_TAG_typedef with complaint in such case.
	* gdbtypes.c (type_name_no_tag_or_error): New function.
	* gdbtypes.h (type_name_no_tag_or_error): New prototype.
	* valops.c (destructor_name_p): New comment for parameter type.  Remove
	type const.  Make dname and cp const.  Call type_name_no_tag_or_error.
	* value.h (destructor_name_p): Remove type const.

--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -804,7 +804,7 @@ qualified_name:	TYPENAME COLONCOLON name
 			  tmp_token.ptr[tmp_token.length] = 0;
 
 			  /* Check for valid destructor name.  */
-			  destructor_name_p (tmp_token.ptr, type);
+			  destructor_name_p (tmp_token.ptr, $1.type);
 			  write_exp_elt_opcode (OP_SCOPE);
 			  write_exp_elt_type (type);
 			  write_exp_string (tmp_token);
@@ -2486,7 +2486,7 @@ classify_inner_name (struct block *block, int first_name)
     return NAME;
 
   copy = copy_name (yylval.tsym.stoken);
-  new_type = cp_lookup_nested_type (type, copy, block);
+  new_type = cp_lookup_nested_type (yylval.tsym.type, copy, block);
 
   if (new_type == NULL)
     /* We know the caller won't expect us to update yylval.  */
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -630,6 +630,12 @@ cp_lookup_nested_type (struct type *parent_type,
 		       const char *nested_name,
 		       const struct block *block)
 {
+  /* type_name_no_tag_required provides better error reporting using the
+     original type.  */
+  struct type *saved_parent_type = parent_type;
+
+  CHECK_TYPEDEF (parent_type);
+
   switch (TYPE_CODE (parent_type))
     {
     case TYPE_CODE_STRUCT:
@@ -643,7 +649,7 @@ cp_lookup_nested_type (struct type *parent_type,
 	   just like members of namespaces; in particular,
 	   lookup_symbol_namespace works when looking them up.  */
 
-	const char *parent_name = TYPE_TAG_NAME (parent_type);
+	const char *parent_name = type_name_no_tag_or_error (saved_parent_type);
 	struct symbol *sym
 	  = cp_lookup_symbol_in_namespace (parent_name, nested_name,
 					   block, VAR_DOMAIN);
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8951,7 +8951,7 @@ load_partial_dies (bfd *abfd, gdb_byte *buffer, gdb_byte *info_ptr,
       if (parent_die == NULL
 	  && part_die->has_specification == 0
 	  && part_die->is_declaration == 0
-	  && (part_die->tag == DW_TAG_typedef
+	  && ((part_die->tag == DW_TAG_typedef && !part_die->has_children)
 	      || part_die->tag == DW_TAG_base_type
 	      || part_die->tag == DW_TAG_subrange_type))
 	{
@@ -8964,6 +8964,20 @@ load_partial_dies (bfd *abfd, gdb_byte *buffer, gdb_byte *info_ptr,
 	  continue;
 	}
 
+      /* The exception for DW_TAG_typedef with has_children above is
+	 a workaround of GCC PR debug/47510.  In the case of this complaint
+	 type_name_no_tag_or_error will error on such types later.
+
+	 GDB skipped children of DW_TAG_typedef by the shortcut above and then
+	 it could not find the child DIEs referenced later, this is checked
+	 above.  In correct DWARF DW_TAG_typedef should have no children.  */
+
+      if (part_die->tag == DW_TAG_typedef && part_die->has_children)
+	complaint (&symfile_complaints,
+		   _("DW_TAG_typedef has childen - GCC PR debug/47510 bug "
+		     "- DIE at 0x%x [in module %s]"),
+		   part_die->offset, cu->objfile->name);
+
       /* If we're at the second level, and we're an enumerator, and
 	 our parent has no specification (meaning possibly lives in a
 	 namespace elsewhere), then we can add the partial symbol now
--- a/gdb/gdbtypes.c
+++ b/gdb/gdbtypes.c
@@ -1105,6 +1105,32 @@ type_name_no_tag (const struct type *type)
   return TYPE_NAME (type);
 }
 
+/* A wrapper of type_name_no_tag which calls error if the type is anonymous.
+   Since GCC PR debug/47510 DWARF provides associated information to detect the
+   anonymous class linkage name from its typedef.
+
+   Parameter TYPE should not yet have CHECK_TYPEDEF applied, this function will
+   apply it itself.  */
+
+const char *
+type_name_no_tag_or_error (struct type *type)
+{
+  struct type *saved_type = type;
+  const char *name;
+  struct objfile *objfile;
+
+  CHECK_TYPEDEF (type);
+
+  name = type_name_no_tag (type);
+  if (name != NULL)
+    return name;
+
+  name = type_name_no_tag (saved_type);
+  objfile = TYPE_OBJFILE (saved_type);
+  error (_("Invalid anonymous type %s [in module %s], GCC PR debug/47510 bug?"),
+	 name ? name : "<anonymous>", objfile ? objfile->name : "<arch>");
+}
+
 /* Lookup a typedef or primitive type named NAME, visible in lexical
    block BLOCK.  If NOERR is nonzero, return zero if NAME is not
    suitably defined.  */
--- a/gdb/gdbtypes.h
+++ b/gdb/gdbtypes.h
@@ -1362,6 +1362,8 @@ extern struct type *allocate_stub_method (struct type *);
 
 extern char *type_name_no_tag (const struct type *);
 
+extern const char *type_name_no_tag_or_error (struct type *type);
+
 extern struct type *lookup_struct_elt_type (struct type *, char *, int);
 
 extern struct type *make_pointer_type (struct type *, struct type **);
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -3098,14 +3098,16 @@ classify_oload_match (struct badness_vector *oload_champ_bv,
 
 /* C++: return 1 is NAME is a legitimate name for the destructor of
    type TYPE.  If TYPE does not have a destructor, or if NAME is
-   inappropriate for TYPE, an error is signaled.  */
+   inappropriate for TYPE, an error is signaled.  Parameter TYPE should not yet
+   have CHECK_TYPEDEF applied, this function will apply it itself.  */
+
 int
-destructor_name_p (const char *name, const struct type *type)
+destructor_name_p (const char *name, struct type *type)
 {
   if (name[0] == '~')
     {
-      char *dname = type_name_no_tag (type);
-      char *cp = strchr (dname, '<');
+      const char *dname = type_name_no_tag_or_error (type);
+      const char *cp = strchr (dname, '<');
       unsigned int len;
 
       /* Do not compare the template part for template classes.  */
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -736,7 +736,7 @@ extern int binop_user_defined_p (enum exp_opcode op, struct value *arg1,
 
 extern int unop_user_defined_p (enum exp_opcode op, struct value *arg1);
 
-extern int destructor_name_p (const char *name, const struct type *type);
+extern int destructor_name_p (const char *name, struct type *type);
 
 extern void value_incref (struct value *val);