public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [rfc] Work around invalid G++ DWARF for unnamed aggregates
@ 2010-03-25 10:22 Ulrich Weigand
  2010-03-25 20:03 ` Tom Tromey
  0 siblings, 1 reply; 4+ messages in thread
From: Ulrich Weigand @ 2010-03-25 10:22 UTC (permalink / raw)
  To: gdb-patches

Hello,

a couple of tests in gdb.cp/inherit.exp relating to unnamed aggregates have
been failing for quite some time for me.  One of these is caused by a GCC
problem that has recently been fixed (PR debug/41828), where GCC would
generate spurious DW_AT_name attributes of the form "._%d" or "<anonymous
struct>", instead of providing no DW_AT_name at all for unnamed entities.

It should be easy to work around this problem with older GCC versions by
simply ignoring DW_AT_name attributes with such names in dwarf2_name.
The patch below implements this.  It also fixes a logic bug in completer.c
exposed by this change, where in an unnamed struct *every* method was
considered to be a constructor (instead of none).

However, the test in test_print_anon_union would still fail for me because
it tests for "long int b", while I'd see just "long b".  Elsewhere, both
versions are accepted, but it proved difficult to implement this within
the cp_test_ptype_class framework, which doesn't support regular expressions.
As this particular test was somewhat of an abuse of that framework (as
already mentioned in a comment), I've replaced this by a direct test.

Also, an earlier test assumed that the "long" member of a union has the
same value as the "int" member.  This is not true in general, and in 
particular not true on big-endian 64-bit systems like ppc64.  I've fixed
the test to not make any assumptions on the value of the "long" member.

Finally, a different test (test_ptype_si) fails for me because G++ 4.1
does not provide *any* reference to the tagless_struct type name in its
generated DWARF info for the following source code:

typedef struct {
  int one;
  int two;
} tagless_struct;

(If compiled as C instead of C++, we see a DW_TAG_typedef.  If compiled
with a recent G++, we see "tagless_struct" used as DW_AT_name of the
DW_TAG_structure_type.)

As the information is just not there, there's no way for GDB to work
around the problem.  I've XFAILed the test case if that situation is
detected.

Tested on powerpc64-linux with no regression, fixes:
FAIL: gdb.cp/inherit.exp: ptype tagless struct
FAIL: gdb.cp/inherit.exp: print variable of type anonymous union
FAIL: gdb.cp/inherit.exp: print type of anonymous union // unrecognized line type 1: class_with_anon_union::._0;

Does this look reasonable?

Bye,
Ulrich



ChangeLog:

	* dwarf2read.c (dwarf2_name): Work around GCC bugzilla debug/41828 by
	ignoring spurious DW_AT_name attributes for unnamed structs or unions.
	* completer.c (add_struct_fields): Fix inverted logic.

testsuite/ChangeLog:

	* gdb.cp/inherit.exp (test_ptype_si): XFAIL test for GCC versions
	that do not provide the tagless_struct type name at all.
	(test_print_anon_union): Do not check value of uninitialized
	union member.  Do not use cp_test_ptype_class, so we can accept
	"long" as well as "long int".


Index: gdb/completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.36
diff -u -p -r1.36 completer.c
--- gdb/completer.c	1 Jan 2010 07:31:30 -0000	1.36
+++ gdb/completer.c	25 Mar 2010 09:37:44 -0000
@@ -401,7 +401,7 @@ add_struct_fields (struct type *type, in
 	      computed_type_name = 1;
 	    }
 	  /* Omit constructors from the completion list.  */
-	  if (type_name && strcmp (type_name, name))
+	  if (!type_name || strcmp (type_name, name))
 	    {
 	      output[*nextp] = xstrdup (name);
 	      ++*nextp;
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.372
diff -u -p -r1.372 dwarf2read.c
--- gdb/dwarf2read.c	22 Mar 2010 13:21:39 -0000	1.372
+++ gdb/dwarf2read.c	25 Mar 2010 09:37:48 -0000
@@ -9159,6 +9159,18 @@ dwarf2_name (struct die_info *die, struc
       /* These tags always have simple identifiers already; no need
 	 to canonicalize them.  */
       return DW_STRING (attr);
+    case DW_TAG_class_type:
+    case DW_TAG_interface_type:
+    case DW_TAG_structure_type:
+    case DW_TAG_union_type:
+      /* Some GCC versions emit spurious DW_AT_name attributes for unnamed
+	 structures or unions.  These were of the form "._%d" in GCC 4.1,
+	 or simply "<anonymous struct>" or "<anonymous union>" in GCC 4.3
+	 and GCC 4.4.  We work around this problem by ignoring these.  */
+      if (strncmp (DW_STRING (attr), "._", 2) == 0
+	  || strncmp (DW_STRING (attr), "<anonymous", 10) == 0)
+	return NULL;
+      /* Fall through.  */
     default:
       if (!DW_STRING_IS_CANONICAL (attr))
 	{
Index: gdb/testsuite/gdb.cp/inherit.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/inherit.exp,v
retrieving revision 1.15
diff -u -p -r1.15 inherit.exp
--- gdb/testsuite/gdb.cp/inherit.exp	1 Jan 2010 07:32:01 -0000	1.15
+++ gdb/testsuite/gdb.cp/inherit.exp	25 Mar 2010 09:37:49 -0000
@@ -114,6 +114,11 @@ proc test_ptype_si { } {
 	    # gcc 3.4.1 -gstabs+
 	    pass "$name"
 	}
+	-re "No symbol \"tagless_struct\" in current context.$nl$gdb_prompt $" {
+	    # Several GCC 4.x versions provide neither a DW_TAG_typedef DIE
+	    # nor use the typedef name as struct tag name.
+	    xfail "$name"
+	}
     }
 
     set name "ptype variable of type tagless struct"
@@ -490,25 +495,20 @@ proc test_print_anon_union {} {
 
     set name "print variable of type anonymous union"
     gdb_test_multiple "print g_anon_union" $name {
-	-re "$vhn = \{one = 1, \{a = 2, b = 2\}\}$nl$gdb_prompt $" {
+	-re "$vhn = \{one = 1, \{a = 2, b = \[0-9\]+\}\}$nl$gdb_prompt $" {
 	    pass $name
 	}
     }
 
-    # The nested union prints as a multi-line field, but the class body
-    # scanner is inherently line-oriented.  This is ugly but it works.
-
-    cp_test_ptype_class \
-	"ptype g_anon_union" "print type of anonymous union" \
-	"class" "class_with_anon_union" \
-	{
-	    { field public "int one;" }
-	    { field public "union \{" }
-	    { field public "int a;" }
-	    { field public "long int b;" }
-	    { field public "\};" }
+    set name "print type of anonymous union"
+    set re_tag "class_with_anon_union"
+    set re_class "(class $re_tag \{${ws}public:|struct $re_tag\{)"
+    set re_fields "int one;${ws}union \{${ws}int a;${ws}long( int)? b;${ws}\};"
+    gdb_test_multiple "ptype g_anon_union" $name {
+	-re "type = $re_class${ws}$re_fields$nl\}$nl$gdb_prompt $" {
+	    pass $name
 	}
-
+    }
 }
 
 
-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [rfc] Work around invalid G++ DWARF for unnamed aggregates
  2010-03-25 10:22 [rfc] Work around invalid G++ DWARF for unnamed aggregates Ulrich Weigand
@ 2010-03-25 20:03 ` Tom Tromey
  2010-03-26 18:07   ` Ulrich Weigand
  0 siblings, 1 reply; 4+ messages in thread
From: Tom Tromey @ 2010-03-25 20:03 UTC (permalink / raw)
  To: Ulrich Weigand; +Cc: gdb-patches

>>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:

Ulrich> It should be easy to work around this problem with older GCC
Ulrich> versions by simply ignoring DW_AT_name attributes with such
Ulrich> names in dwarf2_name.  The patch below implements this.  It also
Ulrich> fixes a logic bug in completer.c exposed by this change, where
Ulrich> in an unnamed struct *every* method was considered to be a
Ulrich> constructor (instead of none).
[...]
Ulrich> Does this look reasonable?

FWIW, yes, it looks reasonable to me.

Tom

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [rfc] Work around invalid G++ DWARF for unnamed aggregates
  2010-03-25 20:03 ` Tom Tromey
@ 2010-03-26 18:07   ` Ulrich Weigand
  2010-04-06 12:50     ` [commit] Fix typo (Re: [rfc] Work around invalid G++ DWARF for unnamed aggregates) Ulrich Weigand
  0 siblings, 1 reply; 4+ messages in thread
From: Ulrich Weigand @ 2010-03-26 18:07 UTC (permalink / raw)
  To: tromey; +Cc: gdb-patches

Tom Tromey wrote:
> >>>>> "Ulrich" == Ulrich Weigand <uweigand@de.ibm.com> writes:
> 
> Ulrich> It should be easy to work around this problem with older GCC
> Ulrich> versions by simply ignoring DW_AT_name attributes with such
> Ulrich> names in dwarf2_name.  The patch below implements this.  It also
> Ulrich> fixes a logic bug in completer.c exposed by this change, where
> Ulrich> in an unnamed struct *every* method was considered to be a
> Ulrich> constructor (instead of none).
> [...]
> Ulrich> Does this look reasonable?
> 
> FWIW, yes, it looks reasonable to me.

Thanks for having a look!

Due to a conflict with Keith's recent patch, I've had to rework the
dwarf2_name change a bit.  Updated patch below.

Retested on powerpc64-linux.  Committed to mainline.

Bye,
Ulrich


ChangeLog:

	* dwarf2read.c (dwarf2_name): Work around GCC bugzilla debug/41828 by
	ignoring spurious DW_AT_name attributes for unnamed structs or unions.
	* completer.c (add_struct_fields): Fix inverted logic.

testsuite/ChangeLog:

	* gdb.cp/inherit.exp (test_ptype_si): XFAIL test for GCC versions
	that do not provide the tagless_struct type name at all.
	(test_print_anon_union): Do not check value of uninitialized
	union member.  Do not use cp_test_ptype_class, so we can accept
	"long" as well as "long int".


Index: gdb/completer.c
===================================================================
RCS file: /cvs/src/src/gdb/completer.c,v
retrieving revision 1.36
diff -u -r1.36 completer.c
--- gdb/completer.c	1 Jan 2010 07:31:30 -0000	1.36
+++ gdb/completer.c	26 Mar 2010 13:57:55 -0000
@@ -401,7 +401,7 @@
 	      computed_type_name = 1;
 	    }
 	  /* Omit constructors from the completion list.  */
-	  if (type_name && strcmp (type_name, name))
+	  if (!type_name || strcmp (type_name, name))
 	    {
 	      output[*nextp] = xstrdup (name);
 	      ++*nextp;
Index: gdb/dwarf2read.c
===================================================================
RCS file: /cvs/src/src/gdb/dwarf2read.c,v
retrieving revision 1.374
diff -u -r1.374 dwarf2read.c
--- gdb/dwarf2read.c	25 Mar 2010 22:13:15 -0000	1.374
+++ gdb/dwarf2read.c	26 Mar 2010 13:57:57 -0000
@@ -9187,6 +9187,7 @@
       /* These tags always have simple identifiers already; no need
 	 to canonicalize them.  */
       return DW_STRING (attr);
+
     case DW_TAG_subprogram:
       /* Java constructors will all be named "<init>", so return
 	 the class name when we see this special case.  */
@@ -9214,17 +9215,33 @@
 	    }
 	  while (die->tag != DW_TAG_compile_unit);
 	}
-      /* fall through */
+      break;
+
+    case DW_TAG_class_type:
+    case DW_TAG_interface_type:
+    case DW_TAG_structure_type:
+    case DW_TAG_union_type:
+      /* Some GCC versions emit spurious DW_AT_name attributes for unnamed
+	 structures or unions.  These were of the form "._%d" in GCC 4.1,
+	 or simply "<anonymous struct>" or "<anonymous union>" in GCC 4.3
+	 and GCC 4.4.  We work around this problem by ignoring these.  */
+      if (strncmp (DW_STRING (attr), "._", 2) == 0
+	  || strncmp (DW_STRING (attr), "<anonymous", 10) == 0)
+	return NULL;
+      break;
+
     default:
-      if (!DW_STRING_IS_CANONICAL (attr))
-	{
-	  DW_STRING (attr)
-	    = dwarf2_canonicalize_name (DW_STRING (attr), cu,
-					&cu->objfile->objfile_obstack);
-	  DW_STRING_IS_CANONICAL (attr) = 1;
-	}
-      return DW_STRING (attr);
+      break;
+    }
+
+  if (!DW_STRING_IS_CANONICAL (attr))
+    {
+      DW_STRING (attr)
+	= dwarf2_canonicalize_name (DW_STRING (attr), cu,
+				    &cu->objfile->objfile_obstack);
+      DW_STRING_IS_CANONICAL (attr) = 1;
     }
+  return DW_STRING (attr);
 }
 
 /* Return the die that this die in an extension of, or NULL if there
Index: gdb/testsuite/gdb.cp/inherit.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/inherit.exp,v
retrieving revision 1.15
diff -u -r1.15 inherit.exp
--- gdb/testsuite/gdb.cp/inherit.exp	1 Jan 2010 07:32:01 -0000	1.15
+++ gdb/testsuite/gdb.cp/inherit.exp	26 Mar 2010 13:57:57 -0000
@@ -114,6 +114,11 @@
 	    # gcc 3.4.1 -gstabs+
 	    pass "$name"
 	}
+	-re "No symbol \"tagless_struct\" in current context.$nl$gdb_prompt $" {
+	    # Several GCC 4.x versions provide neither a DW_TAG_typedef DIE
+	    # nor use the typedef name as struct tag name.
+	    xfail "$name"
+	}
     }
 
     set name "ptype variable of type tagless struct"
@@ -490,25 +495,20 @@
 
     set name "print variable of type anonymous union"
     gdb_test_multiple "print g_anon_union" $name {
-	-re "$vhn = \{one = 1, \{a = 2, b = 2\}\}$nl$gdb_prompt $" {
+	-re "$vhn = \{one = 1, \{a = 2, b = \[0-9\]+\}\}$nl$gdb_prompt $" {
 	    pass $name
 	}
     }
 
-    # The nested union prints as a multi-line field, but the class body
-    # scanner is inherently line-oriented.  This is ugly but it works.
-
-    cp_test_ptype_class \
-	"ptype g_anon_union" "print type of anonymous union" \
-	"class" "class_with_anon_union" \
-	{
-	    { field public "int one;" }
-	    { field public "union \{" }
-	    { field public "int a;" }
-	    { field public "long int b;" }
-	    { field public "\};" }
+    set name "print type of anonymous union"
+    set re_tag "class_with_anon_union"
+    set re_class "(class $re_tag \{${ws}public:|struct $re_tag\{)"
+    set re_fields "int one;${ws}union \{${ws}int a;${ws}long( int)? b;${ws}\};"
+    gdb_test_multiple "ptype g_anon_union" $name {
+	-re "type = $re_class${ws}$re_fields$nl\}$nl$gdb_prompt $" {
+	    pass $name
 	}
-
+    }
 }
 
 


-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [commit] Fix typo (Re: [rfc] Work around invalid G++ DWARF for unnamed aggregates)
  2010-03-26 18:07   ` Ulrich Weigand
@ 2010-04-06 12:50     ` Ulrich Weigand
  0 siblings, 0 replies; 4+ messages in thread
From: Ulrich Weigand @ 2010-04-06 12:50 UTC (permalink / raw)
  To: gdb-patches


> 	* gdb.cp/inherit.exp (test_ptype_si): XFAIL test for GCC versions
> 	that do not provide the tagless_struct type name at all.
> 	(test_print_anon_union): Do not check value of uninitialized
> 	union member.  Do not use cp_test_ptype_class, so we can accept
> 	"long" as well as "long int".

> +    set re_class "(class $re_tag \{${ws}public:|struct $re_tag\{)"

The re_class string is missing a space in the "struct" case, which I
just noticed when testing against an old GCC version.

Fixed by the following patch.  Tested on powerpc64-linux, committed
to mainline.

Bye,
Ulrich

ChangeLog:

	* gdb.cp/inherit.exp (test_print_anon_union): Fix re_class pattern.

Index: gdb/testsuite/gdb.cp/inherit.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.cp/inherit.exp,v
retrieving revision 1.16
diff -u -p -r1.16 inherit.exp
--- gdb/testsuite/gdb.cp/inherit.exp	26 Mar 2010 18:05:46 -0000	1.16
+++ gdb/testsuite/gdb.cp/inherit.exp	6 Apr 2010 12:40:45 -0000
@@ -502,7 +502,7 @@ proc test_print_anon_union {} {
 
     set name "print type of anonymous union"
     set re_tag "class_with_anon_union"
-    set re_class "(class $re_tag \{${ws}public:|struct $re_tag\{)"
+    set re_class "(class $re_tag \{${ws}public:|struct $re_tag \{)"
     set re_fields "int one;${ws}union \{${ws}int a;${ws}long( int)? b;${ws}\};"
     gdb_test_multiple "ptype g_anon_union" $name {
 	-re "type = $re_class${ws}$re_fields$nl\}$nl$gdb_prompt $" {

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-04-06 12:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-25 10:22 [rfc] Work around invalid G++ DWARF for unnamed aggregates Ulrich Weigand
2010-03-25 20:03 ` Tom Tromey
2010-03-26 18:07   ` Ulrich Weigand
2010-04-06 12:50     ` [commit] Fix typo (Re: [rfc] Work around invalid G++ DWARF for unnamed aggregates) Ulrich Weigand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).