public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFA] mi/10586
@ 2011-11-11 21:30 Keith Seitz
  2011-11-14 15:45 ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Seitz @ 2011-11-11 21:30 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]

Hi,

This bug deals with anonymous structs/unions in varobj. As in: anonymous 
structs/unions are unaddressable by MI clients.

This bug has been sitting around for quite some time. Vladimir Prus 
first mentioned this in 2006 (yikes!), and this bug was filed in 2009. 
At this time, Nick Roberts responded to the bug with a patch that fixed 
the problem.

I don't know why Nick never submitted the patch here, so I've tweaked 
his original patch and written some tests for it, and I am now 
submitting this.

Tested on x86_64-linux.

Keith

ChangeLog
2011-11-11  Keith Seitz  <keiths@redhat.com>

	Based on work by Nick Roberts  <nickrob@snap.net.nz>:
	* varobj.c (c_describe_child): Synthesize a variable name for
	anonymous structs and unions.
	(cplus_describe_child): Likewise.

testsuite/ChangeLog
2011-11-11  Keith Seitz  <keiths@redhat.com>

	* gdb.mi/mi-var-cp.cc (anonymous_structs): New function.
	(class A): New class.
	(class B): New class.
	(main): Call anonymous_structs.
	* gdb.mi/mi-var-cp.exp: Add anonymous struct tests and
	adjust test results.
	* gdb.mi/mi2-var-cp.exp: Likewise.
	* gdb.mi/var-cmd.c (struct _simple_struct): Add two anonymous
	structs.
	* gdb.mi/mi-var-dislay.exp: Add anonymous struct tests and
	adjust test results.
	* gdb.mi/mi2-var-display.exp: Likewise.

[-- Attachment #2: 10586.patch --]
[-- Type: text/x-patch, Size: 9703 bytes --]

 gdb/testsuite/gdb.mi/mi-var-cp.cc        |   42 +++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-cp.exp       |   64 ++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/mi-var-display.exp  |   14 ++++++-
 gdb/testsuite/gdb.mi/mi2-var-display.exp |   14 ++++++-
 gdb/testsuite/gdb.mi/var-cmd.c           |    8 ++++
 gdb/varobj.c                             |   58 ++++++++++++++++++---------
 6 files changed, 179 insertions(+), 21 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc
index 54439e6..8cd189f 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.cc
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc
@@ -205,6 +205,47 @@ int path_expression ()
   /*: END: path_expression :*/
 }
 
+class A
+{
+public:
+  struct {
+    int a;
+    float b;
+  };
+  struct {
+    int c;
+    float d;
+  };
+};
+
+class B : public A
+{
+public:
+  struct {
+    int e;
+    float f;
+  };
+  struct {
+    int g;
+    float h;
+  };
+};
+
+static void
+anonymous_structs (void)
+{
+  B b;
+  b.a = 1;
+  b.b = 2.2;
+  b.c = 3;
+  b.d = 4.4;
+  b.e = 5;
+  b.f = 6.6;
+  b.g = 7;
+  b.h = 8.8;  /* anonymous_structs breakpoint */
+  return;
+}
+
 int main ()
 {
   reference_update_tests ();
@@ -212,5 +253,6 @@ int main ()
   reference_to_pointer ();
   reference_to_struct ();
   path_expression ();
+  anonymous_structs ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp
index 4ca3a68..e905f6b 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp
@@ -46,5 +46,69 @@ mi_run_inline_test reference_to_pointer
 mi_run_inline_test reference_to_struct
 mi_run_inline_test path_expression
 
+# Test anonymous structures
+set lineno [gdb_get_line_number "anonymous_structs breakpoint"]
+mi_create_breakpoint \
+    "$srcfile:$lineno" {[0-9]+} keep {anonymous_structs\(\)} \
+    ".*mi-var-cp.cc" $lineno $hex "break-insert anonymous_structs"
+mi_execute_to "exec-continue" "breakpoint-hit" "anonymous_structs" "" \
+    ".*" ".*" {"" "disp=\"keep\""} "continue to anonymous_structs breakpoint"
+
+mi_create_varobj "b" "b" "create varobj for b"
+mi_list_varobj_children "b" {
+  {b.A A 1 A}
+  {b.public public 2}
+} "list children of b"
+
+mi_list_varobj_children "b.A" {
+  {b.A.public public 2}
+} "list children of b.A"
+
+mi_list_varobj_children "b.A.public" {
+  {b.A.public.anonymous0 anonymous0 1 "struct {...}"}
+  {b.A.public.anonymous1 anonymous1 1 "struct {...}"}
+} "list children of b.A.public"
+
+mi_list_varobj_children "b.A.public.anonymous0" {
+  {b.A.public.anonymous0.public public 2}
+} "list children of b.A.public.anonymous0"
+
+mi_list_varobj_children "b.A.public.anonymous0.public" {
+  {b.A.public.anonymous0.public.a a 0 int}
+  {b.A.public.anonymous0.public.b b 0 float}
+} "list children of b.A.public.anonymous0.public"
+
+mi_list_varobj_children "b.A.public.anonymous1" {
+  {b.A.public.anonymous1.public public 2}
+} "list children of b.A.public.anonymous1"
+
+mi_list_varobj_children "b.A.public.anonymous1.public" {
+  {b.A.public.anonymous1.public.c c 0 int}
+  {b.A.public.anonymous1.public.d d 0 float}
+} "list children of b.A.public.anonymous1.public"
+
+mi_list_varobj_children "b.public" {
+  {b.public.anonymous1 anonymous1 1 "struct {...}"}
+  {b.public.anonymous2 anonymous2 1 "struct {...}"}
+} "list children of b.public"
+
+mi_list_varobj_children "b.public.anonymous1" {
+  {b.public.anonymous1.public public 2}
+} "list children of b.public.anonymous1"
+
+mi_list_varobj_children "b.public.anonymous1.public" {
+  {b.public.anonymous1.public.e e 0 int}
+  {b.public.anonymous1.public.f f 0 float}
+} "list children of b.public.anonymous1.public"
+
+mi_list_varobj_children "b.public.anonymous2" {
+  {b.public.anonymous2.public public 2}
+} "list children of b.public.anonymous2"
+
+mi_list_varobj_children "b.public.anonymous2.public" {
+  {b.public.anonymous2.public.g g 0 int}
+  {b.public.anonymous2.public.h h 0 float}
+} "list children of b.public.anonymous2.public"
+
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.mi/mi-var-display.exp b/gdb/testsuite/gdb.mi/mi-var-display.exp
index ff27407..a56b19b 100644
--- a/gdb/testsuite/gdb.mi/mi-var-display.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-display.exp
@@ -474,7 +474,7 @@ mi_gdb_test "-var-show-attributes s" \
 # Test: c_variable-7.34
 # Desc: number of children of s
 mi_gdb_test "-var-info-num-children s" \
-	"\\^done,numchild=\"6\"" \
+	"\\^done,numchild=\"8\"" \
 	"get number of children of s"
 
 # Test: c_variable-7.35
@@ -486,9 +486,21 @@ mi_list_varobj_children s {
         {s.signed_character signed_character 0 "signed char"}
         {s.char_ptr char_ptr 1 {char \*}}
         {s.array_of_10 array_of_10 10 {int \[10\]}}
+        {s.anonymous6 anonymous6 2 "struct {...}"}
+        {s.anonymous7 anonymous7 2 "struct {...}"}
 } "get children of s"
 #} {integer unsigned_integer character signed_character char_ptr array_of_10}
 
+mi_list_varobj_children s.anonymous6 {
+  {s.anonymous6.a a 0 int}
+  {s.anonymous6.b b 0 float}
+} "get childern of s.anonymous6"
+
+mi_list_varobj_children s.anonymous7 {
+  {s.anonymous7.c c 0 int}
+  {s.anonymous7.d d 0 float}
+} "get children of s.anonymous7"
+
 # Test: c_variable-7.40
 # Desc: create anons
 mi_create_varobj anons anons "create local variable anons"
diff --git a/gdb/testsuite/gdb.mi/mi2-var-display.exp b/gdb/testsuite/gdb.mi/mi2-var-display.exp
index 828614b..dde1830 100644
--- a/gdb/testsuite/gdb.mi/mi2-var-display.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-display.exp
@@ -473,7 +473,7 @@ mi_gdb_test "-var-show-attributes s" \
 # Test: c_variable-7.34
 # Desc: number of children of s
 mi_gdb_test "-var-info-num-children s" \
-	"\\^done,numchild=\"6\"" \
+	"\\^done,numchild=\"8\"" \
 	"get number of children of s"
 
 # Test: c_variable-7.35
@@ -485,9 +485,21 @@ mi_list_varobj_children s {
         {s.signed_character signed_character 0 "signed char"}
         {s.char_ptr char_ptr 1 {char \*}}
         {s.array_of_10 array_of_10 10 {int \[10\]}}
+        {s.anonymous6 anonymous6 2 "struct {...}"}
+        {s.anonymous7 anonymous7 2 "struct {...}"}
 } "get children of s"
 #} {integer unsigned_integer character signed_character char_ptr array_of_10}
 
+mi_list_varobj_children s.anonymous6 {
+  {s.anonymous6.a a 0 int}
+  {s.anonymous6.b b 0 float}
+} "get childern of s.anonymous6"
+
+mi_list_varobj_children s.anonymous7 {
+  {s.anonymous7.c c 0 int}
+  {s.anonymous7.d d 0 float}
+} "get children of s.anonymous7"
+
 # Test: c_variable-7.40
 # Desc: create anons
 mi_create_varobj anons anons "create local variable anons"
diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
index 71c5b9d..5b6af0e 100644
--- a/gdb/testsuite/gdb.mi/var-cmd.c
+++ b/gdb/testsuite/gdb.mi/var-cmd.c
@@ -26,6 +26,14 @@ struct _simple_struct {
   signed char signed_character;
   char *char_ptr;
   int array_of_10[10];
+  struct {
+    int a;
+    float b;
+  };
+  struct {
+    int c;
+    float d;
+  };
 };
 
 typedef struct _simple_struct simpleton;
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f17ff1a..17efd1d 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -3027,26 +3027,38 @@ c_describe_child (struct varobj *parent, int index,
 
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
-      if (cname)
-	*cname = xstrdup (TYPE_FIELD_NAME (type, index));
+      {
+	char *type_name, *name;
 
-      if (cvalue && value)
-	{
-	  /* For C, varobj index is the same as type index.  */
-	  *cvalue = value_struct_element_index (value, index);
-	}
+	type_name = TYPE_FIELD_NAME (type, index);
+	if (*type_name == '\0')
+	  name = xstrprintf ("anonymous%d", index);
+	else
+	  name = type_name;
 
-      if (ctype)
-	*ctype = TYPE_FIELD_TYPE (type, index);
+	if (cname)
+	  *cname = xstrdup (name);
 
-      if (cfull_expression)
-	{
-	  char *join = was_ptr ? "->" : ".";
+	if (cvalue && value)
+	  {
+	    /* For C, varobj index is the same as type index.  */
+	    *cvalue = value_struct_element_index (value, index);
+	  }
 
-	  *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join,
-					  TYPE_FIELD_NAME (type, index));
-	}
+	if (ctype)
+	  *ctype = TYPE_FIELD_TYPE (type, index);
+
+	if (cfull_expression)
+	  {
+	    char *join = was_ptr ? "->" : ".";
+
+	    *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join,
+					    name);
+	  }
 
+	if (*type_name == '\0')
+	  xfree (name);
+      }
       break;
 
     case TYPE_CODE_PTR:
@@ -3432,6 +3444,7 @@ cplus_describe_child (struct varobj *parent, int index,
 	  enum accessibility acc = public_field;
 	  int vptr_fieldno;
 	  struct type *basetype = NULL;
+	  char *name, *type_name;
 
 	  vptr_fieldno = get_vptr_fieldno (type, &basetype);
 	  if (strcmp (parent->name, "private") == 0)
@@ -3450,8 +3463,14 @@ cplus_describe_child (struct varobj *parent, int index,
 	    }
 	  --type_index;
 
+	  type_name = TYPE_FIELD_NAME (type, type_index);
+	  if (*type_name == '\0')
+	    name = xstrprintf ("anonymous%d", type_index);
+	  else
+	    name = type_name;
+
 	  if (cname)
-	    *cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
+	    *cname = xstrdup (name);
 
 	  if (cvalue && value)
 	    *cvalue = value_struct_element_index (value, type_index);
@@ -3461,9 +3480,10 @@ cplus_describe_child (struct varobj *parent, int index,
 
 	  if (cfull_expression)
 	    *cfull_expression
-	      = xstrprintf ("((%s)%s%s)", parent_expression,
-			    join, 
-			    TYPE_FIELD_NAME (type, type_index));
+	      = xstrprintf ("((%s)%s%s)", parent_expression, join, name);
+
+	  if (*type_name == '\0')
+	    xfree (name);
 	}
       else if (index < TYPE_N_BASECLASSES (type))
 	{
-- 
1.7.6.4


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

* Re: [RFA] mi/10586
  2011-11-11 21:30 [RFA] mi/10586 Keith Seitz
@ 2011-11-14 15:45 ` Tom Tromey
  2011-11-14 18:04   ` Keith Seitz
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2011-11-14 15:45 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> This bug deals with anonymous structs/unions in varobj. As in:
Keith> anonymous structs/unions are unaddressable by MI clients.

Keith> This bug has been sitting around for quite some time. Vladimir Prus
Keith> first mentioned this in 2006 (yikes!), and this bug was filed in
Keith> 2009. At this time, Nick Roberts responded to the bug with a patch
Keith> that fixed the problem.

Keith> I don't know why Nick never submitted the patch here, so I've tweaked
Keith> his original patch and written some tests for it, and I am now
Keith> submitting this.

Thanks.

Keith> 2011-11-11  Keith Seitz  <keiths@redhat.com>
Keith> 	Based on work by Nick Roberts  <nickrob@snap.net.nz>:
Keith> 	* varobj.c (c_describe_child): Synthesize a variable name for
Keith> 	anonymous structs and unions.
Keith> 	(cplus_describe_child): Likewise.

ChangeLog should mention the PR.

Keith> +	type_name = TYPE_FIELD_NAME (type, index);
Keith> +	if (*type_name == '\0')
Keith> +	  name = xstrprintf ("anonymous%d", index);
Keith> +	else
Keith> +	  name = type_name;

What happens if there is a clash between a named field's name and the
generated name for an anonymous field?

Tom

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

* Re: [RFA] mi/10586
  2011-11-14 15:45 ` Tom Tromey
@ 2011-11-14 18:04   ` Keith Seitz
  2011-11-14 18:18     ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Seitz @ 2011-11-14 18:04 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches@sourceware.org ml

On 11/14/2011 07:45 AM, Tom Tromey wrote:

> ChangeLog should mention the PR.

That would be useful, no? I don't know why I sometimes forget. I've 
added that.

> Keith>  +	type_name = TYPE_FIELD_NAME (type, index);
> Keith>  +	if (*type_name == '\0')
> Keith>  +	  name = xstrprintf ("anonymous%d", index);
> Keith>  +	else
> Keith>  +	  name = type_name;
>
> What happens if there is a clash between a named field's name and the
> generated name for an anonymous field?

Ha. My guess: bad things (TM).

Given that variables cannot start with numbers, I've changed the name 
string to "%d_anonymous".

Other comments?
Keith

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

* Re: [RFA] mi/10586
  2011-11-14 18:04   ` Keith Seitz
@ 2011-11-14 18:18     ` Tom Tromey
  2011-11-14 19:28       ` Keith Seitz
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2011-11-14 18:18 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> Given that variables cannot start with numbers, I've changed the name
Keith> string to "%d_anonymous".

Keith> Other comments?

It also seems like it will do the wrong thing for -var-info-expression and
-var-info-path-expression.

Tom

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

* Re: [RFA] mi/10586
  2011-11-14 18:18     ` Tom Tromey
@ 2011-11-14 19:28       ` Keith Seitz
  2011-11-14 20:28         ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Seitz @ 2011-11-14 19:28 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches@sourceware.org ml

On 11/14/2011 10:17 AM, Tom Tromey wrote:
> It also seems like it will do the wrong thing for -var-info-expression and
> -var-info-path-expression.

Hmm. Yeah, those will need special handling.

Consider:

struct a
{
   struct {
     int b;
   };
   struct {
     int c;
   };
};

-var-create a * a
-var-list-children a = "public" (if c++)
-var-list-children a.public = "0_anonymous", "1_anonymous"
-var-list-children a.public.0_anonymous = "b" (via "public" if c++)
-var-list-children a.public.1_anonymous = "c" (via "public" if c++)

So far, so good, I think.

Now:

-var-info-expression a = "a"
-var-info-expression a.public = "public"
-var-info-expression a.public.0_anonymous = "0_anonymous"
-var-info-expression a.public.0_anonymous.b = "b"

According to the documentation, -var-info-expression is supposed to 
return a name of the variable/child which is to be presented to the 
user. I don't think we want to present "0_anonymous". GCC uses 
"<anonymous struct>", and that seems like a reasonable convention to follow.

-var-info-path-expression a = "a"
-var-info-path-expression a.public = ""
-var-info-path-expression a.public.0_anonymous = "((a).2_anonymous)"
-var-info-path-expression a.public.0_anonymous.b = "((((a).0_anonymous)).b)"

The documentation on this command says that it should return a valid 
expression that may be used, e.g., to create another varobj or to set a 
watchpoint.

Clearly the two last elements dealing with 0_anonymous are incorrect. I 
believe these should be:
-var-info-path-expression a.public.0_anonymous = ""
-var-info-path-expression a.public.0_anonymous.b = "((a).b)"

If we can agree what to do, I shall set about implementing the desired 
change.

Keith

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

* Re: [RFA] mi/10586
  2011-11-14 19:28       ` Keith Seitz
@ 2011-11-14 20:28         ` Tom Tromey
  2011-11-15 17:10           ` Keith Seitz
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2011-11-14 20:28 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> According to the documentation, -var-info-expression is supposed to
Keith> return a name of the variable/child which is to be presented to the
Keith> user. I don't think we want to present "0_anonymous". GCC uses
Keith> "<anonymous struct>", and that seems like a reasonable convention to
Keith> follow.

Actually, thinking about it more, it seems to me that it would be ok for
these cases to just be errors.  There's no really good way to refer to
the anonymous field as its own entity, and I don't think we should hack
up the parser and whatever else to support this.

Keith> Clearly the two last elements dealing with 0_anonymous are
Keith> incorrect. I believe these should be:
Keith> -var-info-path-expression a.public.0_anonymous = ""

This one, I think should be an error.

Keith> -var-info-path-expression a.public.0_anonymous.b = "((a).b)"

But I agree about this one.

What do you think?

Tom

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

* Re: [RFA] mi/10586
  2011-11-14 20:28         ` Tom Tromey
@ 2011-11-15 17:10           ` Keith Seitz
  2011-11-15 17:30             ` Tom Tromey
  2011-12-02 22:28             ` Keith Seitz
  0 siblings, 2 replies; 15+ messages in thread
From: Keith Seitz @ 2011-11-15 17:10 UTC (permalink / raw)
  To: Tom Tromey; +Cc: gdb-patches@sourceware.org ml

On 11/14/2011 12:28 PM, Tom Tromey wrote:

> Actually, thinking about it more, it seems to me that it would be ok for
> these cases to just be errors.  There's no really good way to refer to
> the anonymous field as its own entity, and I don't think we should hack
> up the parser and whatever else to support this.

An error... I'm not so sure that I like that, but to be honest, I'm not 
sure I like/dislike it sufficiently to argue about it.

> Keith>  Clearly the two last elements dealing with 0_anonymous are
> Keith>  incorrect. I believe these should be:
> Keith>  -var-info-path-expression a.public.0_anonymous = ""
>
> This one, I think should be an error.
>

Yes, that one could be an error. I was just mirroring what the "fake" 
children currently do. (-var-info-path-expression a.public = "").

> Keith>  -var-info-path-expression a.public.0_anonymous.b = "((a).b)"
>
> But I agree about this one.

Ok, so we're close. This is probably the "trickier" bit to get correct, 
so I can start on writing some more elaborate tests for this.

Keith

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

* Re: [RFA] mi/10586
  2011-11-15 17:10           ` Keith Seitz
@ 2011-11-15 17:30             ` Tom Tromey
  2011-12-02 22:28             ` Keith Seitz
  1 sibling, 0 replies; 15+ messages in thread
From: Tom Tromey @ 2011-11-15 17:30 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> On 11/14/2011 12:28 PM, Tom Tromey wrote:
>> Actually, thinking about it more, it seems to me that it would be ok for
>> these cases to just be errors.  There's no really good way to refer to
>> the anonymous field as its own entity, and I don't think we should hack
>> up the parser and whatever else to support this.

Keith> An error... I'm not so sure that I like that, but to be honest, I'm
Keith> not sure I like/dislike it sufficiently to argue about it.

I guess it could be done by returning something like:

   *(struct whatever*) ((char *) original_expr + offset)

That is only mildly horrible.

Keith> Yes, that one could be an error. I was just mirroring what the "fake"
Keith> children currently do. (-var-info-path-expression a.public = "").

Oh, ok.  That is fine then.

Tom

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

* Re: [RFA] mi/10586
  2011-11-15 17:10           ` Keith Seitz
  2011-11-15 17:30             ` Tom Tromey
@ 2011-12-02 22:28             ` Keith Seitz
  2011-12-13  1:28               ` Keith Seitz
                                 ` (2 more replies)
  1 sibling, 3 replies; 15+ messages in thread
From: Keith Seitz @ 2011-12-02 22:28 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 2124 bytes --]

On 11/15/2011 09:10 AM, Keith Seitz wrote:
> On 11/14/2011 12:28 PM, Tom Tromey wrote:
>
>> Actually, thinking about it more, it seems to me that it would be ok for
>> these cases to just be errors. There's no really good way to refer to
>> the anonymous field as its own entity, and I don't think we should hack
>> up the parser and whatever else to support this.
>
> An error... I'm not so sure that I like that, but to be honest, I'm not
> sure I like/dislike it sufficiently to argue about it.

Ok, so in the end, I've decided against making this an error. Instead, 
what I've implemented is analogous to how varobj/MI currently deal with 
the CPLUS_FAKE_CHILD cases: path expressions and the like are "".

If you would like me to do otherwise, please let me know.

I am attaching the latest incarnation of this patch, which covers all 
these cases (AFAIK) and utilizes the new varobj tree testing that I 
committed last week.

What do you think?

Keith

ChangeLog
2011-12-02  Keith Seitz  <keiths@redhat.com>

	PR mi/10586
	Based on work by Nick Roberts  <nickrob@snap.net.nz>:
	* varobj.c [ANONYMOUS_STRUCT_NAME]: New constant.
	[ANONYMOUS_UNION_NAME]: Likewise.
	(is_anonymous_child): New function.
	(create_child_with_value): If the child is nameless,
	synthesize a name for it.
	(c_describe_child): If the child has no field or type names
	for a union or sturct, use ANONYMOUS_STRUCT_NAME or
	ANONYMOUS_UNION_NAME.
	Set path expression for these fields to the empty string.
	(cplus_describe_child): Likewise.
  	(is_path_expr_parent): New function.
	(get_path_expr_parent): New function.

testsuite/ChangeLog
2011-12-02  Keith Seitz  <keiths@redhat.com>

	PR mi/10586
	* gdb.mi/mi-var-cp.cc (anonymous_structs): New function.
	(class A): New class.
	(class B): New class.
	(main): Call anonymous_structs.
	* gdb.mi/mi-var-cp.exp: Add anonymous struct tests and
	adjust test results.
	* gdb.mi/mi2-var-cp.exp: Likewise.
	* gdb.mi/var-cmd.c (struct _simple_struct): Add two anonymous
	structs.
	* gdb.mi/mi-var-dislay.exp: Add anonymous struct tests and
	adjust test results.
	* gdb.mi/mi2-var-display.exp: Likewise.

[-- Attachment #2: 10586-2.patch --]
[-- Type: text/x-patch, Size: 13916 bytes --]

diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc
index 54439e6..51d30cf 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.cc
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc
@@ -205,6 +205,51 @@ int path_expression ()
   /*: END: path_expression :*/
 }
 
+class Anonymous
+{
+public:
+  struct { /* index: 0 */
+    int b;
+  };
+  struct { /* index: 1 */
+    int c;
+  };
+  struct { /* index: 2 */
+    int d;
+    struct { /* index: 1 */
+      int e;
+      struct { /* index: 0 */
+        int f;
+        union { /* index: 0 */
+          int g;
+          char h;
+        };
+      };
+      union { /* index: 0 */
+        int i;
+        char j;
+      };
+    };
+  };
+};
+
+/* Test anonymous structs and unions.  */
+int
+anonymous_structs_and_unions (void)
+{
+  Anonymous a;
+  a.b = 1;
+  a.c = 2;
+  a.d = 3;
+  a.e = 4;
+  a.f = 5;
+  a.g = 6;
+  a.h = '7';
+  a.i = 8;
+  a.j = '8';
+  return 0;  /* anonymous_structs_and_unions */
+}
+
 int main ()
 {
   reference_update_tests ();
@@ -212,5 +257,6 @@ int main ()
   reference_to_pointer ();
   reference_to_struct ();
   path_expression ();
+  anonymous_structs_and_unions ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp
index 4ca3a68..258a3af 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp
@@ -46,5 +46,78 @@ mi_run_inline_test reference_to_pointer
 mi_run_inline_test reference_to_struct
 mi_run_inline_test path_expression
 
+set lineno [gdb_get_line_number "/* anonymous_structs_and_unions */"]
+mi_create_breakpoint \
+    "$srcfile:$lineno" {[0-9]+} keep {anonymous_structs_and_unions\(\)} \
+    ".*mi-var-cp.cc" $lineno $hex "break in anonymous_structs_and_unions"
+mi_execute_to "exec-continue" "breakpoint-hit" \
+    "anonymous_structs_and_unions" "" ".*" ".*" {"" "disp=\"keep\""} \
+    "continue to anonymous_structs breakpoint"
+
+set tree {
+  Anonymous a {
+    {} public {
+      anonymous struct {
+        {} public {
+          int b {}
+        }
+      }
+      anonymous struct {
+        {} public {
+          int c {}
+        }
+      }
+      anonymous struct {
+        {} public {
+          int d {}
+          anonymous struct {
+            {} public {
+              int e {}
+              anonymous struct {
+                {} public {
+                  int f {}
+                  anonymous union {
+                    {} public {
+                      int g {}
+                      char h {}
+                    }
+                  }
+                }
+              }
+	      anonymous union {
+		{} public {
+		  int i {}
+		  char j {}
+		}
+	      }
+	    }
+	  }
+	}
+      }
+    }
+  }
+}
+
+proc verify_everything {variable_name} {
+  # Test -var-list-children
+  mi_varobj_tree_test_children_callback $variable_name
+
+  # Bring the variable named by VARIABLE_NAME into the current scope
+  # in VAROBJ.
+  upvar #0 $variable_name varobj
+
+  # Test -var-info-path-expression
+  mi_gdb_test "-var-info-path-expression $varobj(obj_name)" \
+      "\\^done,path_expr=\"[string_to_regexp $varobj(path_expr)]\"" \
+      "path expression for $varobj(obj_name)"
+
+  # Test -var-info-expression
+  mi_gdb_test "-var-info-expression $varobj(obj_name)" \
+      "\\^done,lang=\"C\\+\\+\",exp=\"[string_to_regexp $varobj(display_name)]\"" \
+      "expression for $varobj(obj_name)"
+}
+
+mi_walk_varobj_tree $tree verify_everything
+
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp
index 0f9b4d4..a940a00 100644
--- a/gdb/testsuite/gdb.mi/mi2-var-child.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp
@@ -1,4 +1,5 @@
-# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009 Free Software Foundation
+# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009, 2011
+# Free Software Foundation
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -1139,7 +1140,69 @@ mi_gdb_test "-var-update *" \
 	"update all vars psnp->next->next->long_ptr (and 2.long_ptr) changed"
 
 
+# Anonymous type tests
+set lineno [gdb_get_line_number "anonymous type tests breakpoint"]
+mi_create_breakpoint \
+    "$srcfile:$lineno" {[0-9]+} keep {do_anonymous_type_tests} \
+    ".*var-cmd.c" $lineno $hex "break in do_anonymous_type_tests"
+mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\
+    ".*" ".*" {"" "disp=\"keep\""} \
+    "continue to do_anonymous_type_tests breakpoint"
+
+# Run the varobj tree on variable "ptr".
+set tree {
+  {struct anonymous **} ptr {
+    {struct anonymous *} {*ptr} {
+      int a {}
+      anonymous struct {
+	int b {}
+	{char *} c {
+	  char {*c} {}
+	}
+	anonymous union {
+	  int d {}
+	  {void *} e {}
+	  char f {}
+	  anonymous struct {
+	    char g {}
+	    {const char **} h {
+	      {const char *} {*h} {
+		{const char} {**h} {}
+	      }
+	    }
+	    {simpleton ***} simple {
+	      {simpleton **} {*simple} {
+		{simpleton *} {**simple} {
+		  int integer {}
+		  {unsigned int} unsigned_integer {}
+		  char character {}
+		  {signed char} signed_character {}
+		  {char *} char_ptr {
+		    char {*char_ptr} {}
+		  }
+		  {int [10]} array_of_10 {
+		    int 0 {}
+		    int 1 {}
+		    int 2 {}
+		    int 3 {}
+		    int 4 {}
+		    int 5 {}
+		    int 6 {}
+		    int 7 {}
+		    int 8 {}
+		    int 9 {}
+		  }
+		}
+	      }
+	    }
+	  }
+	}
+      }
+    }
+  }
+}
 
+mi_walk_varobj_tree $tree
 
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
index 71c5b9d..b048f45 100644
--- a/gdb/testsuite/gdb.mi/var-cmd.c
+++ b/gdb/testsuite/gdb.mi/var-cmd.c
@@ -92,6 +92,24 @@ struct _struct_n_pointer {
   struct _struct_n_pointer *next;
 };
 
+struct anonymous {
+  int a;
+  struct {
+    int b;
+    char *c;
+    union {
+      int d;
+      void *e;
+      char f;
+      struct {
+	char g;
+	const char **h;
+	simpleton ***simple;
+      };
+    };
+  };
+};
+
 void do_locals_tests (void);
 void do_block_tests (void);
 void subroutine1 (int, long *);
@@ -503,6 +521,26 @@ void do_bitfield_tests ()
   /*: END: bitfield :*/  
 }
 
+void
+do_anonymous_type_tests (void)
+{
+  struct anonymous *anon;
+  struct anonymous **ptr;
+
+  anon = malloc (sizeof (struct anonymous));
+  anon->a = 1;
+  anon->b = 2;
+  anon->c = (char *) 3;
+  anon->d = 4;
+  anon->g = '5';
+  anon->h = (const char **) 6;
+  anon->simple = (simpleton ***) 7;
+
+  ptr = &anon;
+  free (anon);
+  return; /* anonymous type tests breakpoint */
+}
+
 int
 main (int argc, char *argv [])
 {
@@ -513,6 +551,7 @@ main (int argc, char *argv [])
   do_frozen_tests ();
   do_at_tests ();
   do_bitfield_tests ();
+  do_anonymous_type_tests ();
   exit (0);
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7c68a93..8da744b 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -43,6 +43,12 @@
 typedef int PyObject;
 #endif
 
+/* The names of varobjs representing anonymous structs or unions.
+   Note that is_name_anonymous_type makes assumptions about these two
+   constants.  */
+#define ANONYMOUS_STRUCT_NAME _("<anonymous struct>")
+#define ANONYMOUS_UNION_NAME _("<anonymous union>")
+
 /* Non-zero if we want to see trace of varobj level stuff.  */
 
 int varobjdebug = 0;
@@ -1289,6 +1295,39 @@ varobj_get_gdb_type (struct varobj *var)
   return var->type;
 }
 
+/* Is VAR a path expression parent, i.e., can it be used to construct
+   a valid path expression?  */
+
+static int
+is_path_expr_parent (struct varobj *var)
+{
+  struct type *type;
+
+  /* "Fake" children are not path_expr parents.  */
+  if (CPLUS_FAKE_CHILD (var))
+    return 0;
+
+  type  = get_value_type (var);
+
+  /* Anonymous unions and structs are also not path_expr parents.  */
+  return !((TYPE_CODE (type) == TYPE_CODE_STRUCT
+	    || TYPE_CODE (type) == TYPE_CODE_UNION)
+	   && TYPE_NAME (type) == NULL);
+}
+
+/* Return the path expression parent for VAR.  */
+
+static struct varobj *
+get_path_expr_parent (struct varobj *var)
+{
+  struct varobj *parent = var;
+
+  while (!is_path_expr_parent (parent))
+    parent = parent->parent;
+
+  return parent;
+}
+
 /* Return a pointer to the full rooted expression of varobj VAR.
    If it has not been computed yet, compute it.  */
 char *
@@ -2170,6 +2209,19 @@ create_child (struct varobj *parent, int index, char *name)
 				  value_of_child (parent, index));
 }
 
+/* Does CHILD represent a child with no name?  This happens when
+   the child is an anonmous struct or union and it has no field name
+   in its parent variable.
+
+   This has already been determined by *_describe_child. The easiest
+   thing to do is see if the child's name begins with "<anonymous ".  */
+
+static int
+is_anonymous_child (struct varobj *child)
+{
+  return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0;
+}
+
 static struct varobj *
 create_child_with_value (struct varobj *parent, int index, const char *name,
 			 struct value *value)
@@ -2185,8 +2237,13 @@ create_child_with_value (struct varobj *parent, int index, const char *name,
   child->index = index;
   child->parent = parent;
   child->root = parent->root;
-  childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
+
+  if (is_anonymous_child (child))
+    childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index);
+  else
+    childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
   child->obj_name = childs_name;
+
   install_variable (child);
 
   /* Compute the type of the child.  Must do this before
@@ -2955,7 +3012,7 @@ c_describe_child (struct varobj *parent, int index,
   if (cfull_expression)
     {
       *cfull_expression = NULL;
-      parent_expression = varobj_get_path_expr (parent);
+      parent_expression = varobj_get_path_expr (get_path_expr_parent (parent));
     }
   adjust_value_for_child_access (&value, &type, &was_ptr);
       
@@ -2990,26 +3047,49 @@ c_describe_child (struct varobj *parent, int index,
 
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
-      if (cname)
-	*cname = xstrdup (TYPE_FIELD_NAME (type, index));
+      {
+	char *field_name;
 
-      if (cvalue && value)
-	{
-	  /* For C, varobj index is the same as type index.  */
-	  *cvalue = value_struct_element_index (value, index);
-	}
+	/* If the type is anonymous and the field has no name,
+	   set an appropriate name.  */
+	field_name = TYPE_FIELD_NAME (type, index);
+	if (*field_name == '\0')
+	  {
+	    if (cname)
+	      {
+		if (TYPE_CODE (TYPE_FIELD_TYPE (type, index))
+		    == TYPE_CODE_STRUCT)
+		  *cname = xstrdup (ANONYMOUS_STRUCT_NAME);
+		else
+		  *cname = xstrdup (ANONYMOUS_UNION_NAME);
+	      }
 
-      if (ctype)
-	*ctype = TYPE_FIELD_TYPE (type, index);
+	    if (cfull_expression)
+	      *cfull_expression = xstrdup ("");
+	  }
+	else
+	  {
+	    if (cname)
+	      *cname = xstrdup (field_name);
 
-      if (cfull_expression)
-	{
-	  char *join = was_ptr ? "->" : ".";
+	    if (cfull_expression)
+	      {
+		char *join = was_ptr ? "->" : ".";
 
-	  *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join,
-					  TYPE_FIELD_NAME (type, index));
-	}
+		*cfull_expression = xstrprintf ("(%s)%s%s", parent_expression,
+						join, field_name);
+	      }
+	  }
 
+	if (cvalue && value)
+	  {
+	    /* For C, varobj index is the same as type index.  */
+	    *cvalue = value_struct_element_index (value, index);
+	  }
+
+	if (ctype)
+	  *ctype = TYPE_FIELD_TYPE (type, index);
+      }
       break;
 
     case TYPE_CODE_PTR:
@@ -3357,14 +3437,16 @@ cplus_describe_child (struct varobj *parent, int index,
       value = parent->parent->value;
       type = get_value_type (parent->parent);
       if (cfull_expression)
-	parent_expression = varobj_get_path_expr (parent->parent);
+	parent_expression
+	  = varobj_get_path_expr (get_path_expr_parent (parent->parent));
     }
   else
     {
       value = parent->value;
       type = get_value_type (parent);
       if (cfull_expression)
-	parent_expression = varobj_get_path_expr (parent);
+	parent_expression
+	  = varobj_get_path_expr (get_path_expr_parent (parent));
     }
 
   adjust_value_for_child_access (&value, &type, &was_ptr);
@@ -3386,6 +3468,7 @@ cplus_describe_child (struct varobj *parent, int index,
 	  enum accessibility acc = public_field;
 	  int vptr_fieldno;
 	  struct type *basetype = NULL;
+	  char *field_name;
 
 	  vptr_fieldno = get_vptr_fieldno (type, &basetype);
 	  if (strcmp (parent->name, "private") == 0)
@@ -3404,20 +3487,40 @@ cplus_describe_child (struct varobj *parent, int index,
 	    }
 	  --type_index;
 
-	  if (cname)
-	    *cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
+	  /* If the type is anonymous and the field has no name,
+	     set an appopriate name.  */
+	  field_name = TYPE_FIELD_NAME (type, type_index);
+	  if (*field_name == '\0')
+	    {
+	      if (cname)
+		{
+		  if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index))
+		      == TYPE_CODE_STRUCT)
+		    *cname = xstrdup (ANONYMOUS_STRUCT_NAME);
+		  else if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index))
+			   == TYPE_CODE_UNION)
+		    *cname = xstrdup (ANONYMOUS_UNION_NAME);
+		}
+
+	      if (cfull_expression)
+		*cfull_expression = xstrdup ("");
+	    }
+	  else
+	    {
+	      if (cname)
+		*cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
+
+	      if (cfull_expression)
+		*cfull_expression
+		  = xstrprintf ("((%s)%s%s)", parent_expression, join,
+				field_name);
+	    }
 
 	  if (cvalue && value)
 	    *cvalue = value_struct_element_index (value, type_index);
 
 	  if (ctype)
 	    *ctype = TYPE_FIELD_TYPE (type, type_index);
-
-	  if (cfull_expression)
-	    *cfull_expression
-	      = xstrprintf ("((%s)%s%s)", parent_expression,
-			    join, 
-			    TYPE_FIELD_NAME (type, type_index));
 	}
       else if (index < TYPE_N_BASECLASSES (type))
 	{

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

* Re: [RFA] mi/10586
  2011-12-02 22:28             ` Keith Seitz
@ 2011-12-13  1:28               ` Keith Seitz
  2011-12-13 19:58               ` Tom Tromey
  2011-12-13 20:34               ` Crash regression with Eclipse [Re: [RFA] mi/10586] Jan Kratochvil
  2 siblings, 0 replies; 15+ messages in thread
From: Keith Seitz @ 2011-12-13  1:28 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

Ping

On 12/02/2011 02:28 PM, Keith Seitz wrote:
> On 11/15/2011 09:10 AM, Keith Seitz wrote:
>> On 11/14/2011 12:28 PM, Tom Tromey wrote:
>>
>>> Actually, thinking about it more, it seems to me that it would be ok for
>>> these cases to just be errors. There's no really good way to refer to
>>> the anonymous field as its own entity, and I don't think we should hack
>>> up the parser and whatever else to support this.
>>
>> An error... I'm not so sure that I like that, but to be honest, I'm not
>> sure I like/dislike it sufficiently to argue about it.
>
> Ok, so in the end, I've decided against making this an error. Instead,
> what I've implemented is analogous to how varobj/MI currently deal with
> the CPLUS_FAKE_CHILD cases: path expressions and the like are "".
>
> If you would like me to do otherwise, please let me know.
>
> I am attaching the latest incarnation of this patch, which covers all
> these cases (AFAIK) and utilizes the new varobj tree testing that I
> committed last week.
>
> What do you think?
>
> Keith
>
> ChangeLog
> 2011-12-02 Keith Seitz <keiths@redhat.com>
>
> PR mi/10586
> Based on work by Nick Roberts <nickrob@snap.net.nz>:
> * varobj.c [ANONYMOUS_STRUCT_NAME]: New constant.
> [ANONYMOUS_UNION_NAME]: Likewise.
> (is_anonymous_child): New function.
> (create_child_with_value): If the child is nameless,
> synthesize a name for it.
> (c_describe_child): If the child has no field or type names
> for a union or sturct, use ANONYMOUS_STRUCT_NAME or
> ANONYMOUS_UNION_NAME.
> Set path expression for these fields to the empty string.
> (cplus_describe_child): Likewise.
> (is_path_expr_parent): New function.
> (get_path_expr_parent): New function.
>
> testsuite/ChangeLog
> 2011-12-02 Keith Seitz <keiths@redhat.com>
>
> PR mi/10586
> * gdb.mi/mi-var-cp.cc (anonymous_structs): New function.
> (class A): New class.
> (class B): New class.
> (main): Call anonymous_structs.
> * gdb.mi/mi-var-cp.exp: Add anonymous struct tests and
> adjust test results.
> * gdb.mi/mi2-var-cp.exp: Likewise.
> * gdb.mi/var-cmd.c (struct _simple_struct): Add two anonymous
> structs.
> * gdb.mi/mi-var-dislay.exp: Add anonymous struct tests and
> adjust test results.
> * gdb.mi/mi2-var-display.exp: Likewise.

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

* Re: [RFA] mi/10586
  2011-12-02 22:28             ` Keith Seitz
  2011-12-13  1:28               ` Keith Seitz
@ 2011-12-13 19:58               ` Tom Tromey
  2011-12-17  1:55                 ` Keith Seitz
  2011-12-13 20:34               ` Crash regression with Eclipse [Re: [RFA] mi/10586] Jan Kratochvil
  2 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2011-12-13 19:58 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> +/* The names of varobjs representing anonymous structs or unions.
Keith> +   Note that is_name_anonymous_type makes assumptions about these two
Keith> +   constants.  */

I didn't see 'is_name_anonymous_type' anywhere in the tree or in the patch.
I guess it needs an update.

Keith> +static int
Keith> +is_path_expr_parent (struct varobj *var)
[...]
Keith> +  type  = get_value_type (var);

Extra space.

Keith> +  return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0;

I think this is wrong since the macros have _() in their expansion.
I think you have to use strlen.

Keith> +	field_name = TYPE_FIELD_NAME (type, index);
Keith> +	if (*field_name == '\0')

Can field_name == NULL?
It is not clear to me.  There is some code in gdb that checks this, but
I don't know whether that is defensive programming or checking a
condition that is truly possible.

Otherwise this looks good to me.

Tom

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

* Crash regression with Eclipse  [Re: [RFA] mi/10586]
  2011-12-02 22:28             ` Keith Seitz
  2011-12-13  1:28               ` Keith Seitz
  2011-12-13 19:58               ` Tom Tromey
@ 2011-12-13 20:34               ` Jan Kratochvil
  2 siblings, 0 replies; 15+ messages in thread
From: Jan Kratochvil @ 2011-12-13 20:34 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

On Fri, 02 Dec 2011 23:28:19 +0100, Keith Seitz wrote:
> I am attaching the latest incarnation of this patch, which covers
> all these cases (AFAIK) and utilizes the new varobj tree testing
> that I committed last week.

Is it tested with Eclipse?  Eclipse from Fedora 16 will crash GDB.
It does not crash with FSF GDB HEAD, it just does not expand "v" in Variables
window.  This is a regression.

int
main (void)
{
	struct
	{
	  int x;
	  struct
	    {
	      int a;
	    };
	  struct
	    {
	      int b;
	    };
	}
	v = {1, {2}, {3}};

	struct s
	{
	  int x, y;
	}
	n = {10, 20};

  return 0;	/* Step in Eclipse here, then expand "v" in Variables.  */
}

Program received signal SIGSEGV, Segmentation fault.
0x00000000007ab58b in get_value_type (var=0x0) at varobj.c:2397
2397	  if (var->value)
(gdb) bt
#0  in get_value_type (var=0x0) at varobj.c:2397
#1  in is_path_expr_parent (var=0x0) at varobj.c:1310
#2  in get_path_expr_parent (var=0x2cbbd80) at varobj.c:1325
#3  in c_describe_child (parent=0x2cbbd80, index=0, cname=0x0, cvalue=0x0, ctype=0x0, cfull_expression=0x2cbf5f8) at varobj.c:3015
#4  in c_path_expr_of_child (child=0x2cbf5f0) at varobj.c:3140
#5  in varobj_get_path_expr (var=0x2cbf5f0) at varobj.c:1344
#6  in mi_cmd_var_info_path_expression (command=0x256a200 "var-info-path-expression", argv=0x2cdd570, argc=1) at ./mi/mi-cmd-var.c:501
#7  in mi_cmd_execute (parse=0x2d0bee0) at ./mi/mi-main.c:2110
#8  in captured_mi_execute_command (uiout=0x28aca70, context=0x2d0bee0) at ./mi/mi-main.c:1854
#9  in mi_execute_command (cmd=0x2cbf6d0 "41-var-info-path-expression var1.x", from_tty=1) at ./mi/mi-main.c:1976
#10 in mi_execute_command_wrapper (cmd=0x2cbf6d0 "41-var-info-path-expression var1.x") at ./mi/mi-interp.c:291
#11 in gdb_readline2 (client_data=0x0) at event-top.c:717
#12 in stdin_event_handler (error=0, client_data=0x0) at event-top.c:375
#13 in handle_file_event (data=...) at event-loop.c:828
#14 in process_event () at event-loop.c:402
#15 in gdb_do_one_event () at event-loop.c:466
#16 in start_event_loop () at event-loop.c:491
#17 in mi_command_loop (mi_version=2) at ./mi/mi-interp.c:321
#18 in mi2_command_loop () at ./mi/mi-interp.c:303
#19 in current_interp_command_loop () at interps.c:304
#20 in captured_command_loop (data=0x0) at ./main.c:234
#21 in catch_errors (func=0x487987 <captured_command_loop>, func_args=0x0, errstring=0xe75107 "", mask=6) at exceptions.c:504
#22 in captured_main (data=0x7fff42a37b90) at ./main.c:944
#23 in catch_errors (func=0x4879d6 <captured_main>, func_args=0x7fff42a37b90, errstring=0xe75107 "", mask=6) at exceptions.c:504
#24 in gdb_main (args=0x7fff42a37b90) at ./main.c:953
#25 in main (argc=4, argv=0x7fff42a37c98) at gdb.c:35


Thanks,
Jan

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

* Re: [RFA] mi/10586
  2011-12-13 19:58               ` Tom Tromey
@ 2011-12-17  1:55                 ` Keith Seitz
  2011-12-20 16:03                   ` Tom Tromey
  0 siblings, 1 reply; 15+ messages in thread
From: Keith Seitz @ 2011-12-17  1:55 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

[-- Attachment #1: Type: text/plain, Size: 2930 bytes --]

On 12/13/2011 11:39 AM, Tom Tromey wrote:
> I didn't see 'is_name_anonymous_type' anywhere in the tree or in the patch.
> I guess it needs an update.

I hate it when that happens! Comment updated.

> Keith>  +static int
> Keith>  +is_path_expr_parent (struct varobj *var)
> [...]
> Keith>  +  type  = get_value_type (var);
>
> Extra space.

Fixed.

> Keith>  +  return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0;
>
> I think this is wrong since the macros have _() in their expansion.
> I think you have to use strlen.

Yeah, very wrong indeed! I've fixed that test.

> Keith>  +	field_name = TYPE_FIELD_NAME (type, index);
> Keith>  +	if (*field_name == '\0')
>
> Can field_name == NULL?
> It is not clear to me.  There is some code in gdb that checks this, but
> I don't know whether that is defensive programming or checking a
> condition that is truly possible.

Comments in gdbtypes.h seem to indicate that name could be NULL, but 
I've not seen that. To be defensive, I've treated name == NULL and name 
== "" the same.

On 12/13/2011 11:58 AM, Jan Kratochvil wrote:
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000007ab58b in get_value_type (var=0x0) at varobj.c:2397
> 2397	  if (var->value)
> (gdb) bt
> #0  in get_value_type (var=0x0) at varobj.c:2397
> #1  in is_path_expr_parent (var=0x0) at varobj.c:1310
> #2  in get_path_expr_parent (var=0x2cbbd80) at varobj.c:1325
> #3  in c_describe_child (parent=0x2cbbd80, index=0, cname=0x0,
cvalue=0x0, ctype=0x0, cfull_expression=0x2cbf5f8) at varobj.c:3015
> #4  in c_path_expr_of_child (child=0x2cbf5f0) at varobj.c:3140
[snip]

I've fixed this, too, and added a test for that. A very simple omission: 
get_path_expr_parent was attempting to walk the parent chain past the 
root variable. Whoops.

Keith

ChangeLog
2011-12-16  Keith Seitz  <keiths@redhat.com>

	PR mi/10586
	* varobj.c (ANONYMOUS_STRUCT_NAME): Define.
	(ANONYMOUS_UNION_NAME): Define.
	(is_path_expr_parent): New function.
	(get_path_expr_parent): New function.
	(is_anonymous_child): New function.
	(create_child_with_value): If the child is anonymous and without
	a name, assign an object name to it.
	(c_describe_child): Use get_path_expr_parent to determine
	the parent expression.
	If there field represents an anonymous struct or union and
	has no name, set an appropriate display name and expression.
	(cplus_describe_child): Likewise.

testsuite/ChangeLog
2011-12-16  Keith Seitz  <keiths@redhat.com>

	PR mi/10586
	* gdb.mi/var-cmd.c (struct anonymous): New structure.
	(do_anonymous_type_tests): New function.
	(main): Call do_anonymous_type_tests.
	* gdb.mi/mi2-var-child.exp: Add anonymous type tests.
	(verify_everything): New procedure.
	* gdb.mi/mi-var-cp.cc (class A): New class.
	(anonymous_structs_and_unions): New function.
	(main): Call anonymous_structs_and_unions.
	* gdb.mi/mi-var-cp.exp: Add anonymous type tests.
	(verify_everything): New procedure.

[-- Attachment #2: 10586-3.patch --]
[-- Type: text/x-patch, Size: 14992 bytes --]

diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc
index 54439e6..51d30cf 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.cc
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc
@@ -205,6 +205,51 @@ int path_expression ()
   /*: END: path_expression :*/
 }
 
+class Anonymous
+{
+public:
+  struct { /* index: 0 */
+    int b;
+  };
+  struct { /* index: 1 */
+    int c;
+  };
+  struct { /* index: 2 */
+    int d;
+    struct { /* index: 1 */
+      int e;
+      struct { /* index: 0 */
+        int f;
+        union { /* index: 0 */
+          int g;
+          char h;
+        };
+      };
+      union { /* index: 0 */
+        int i;
+        char j;
+      };
+    };
+  };
+};
+
+/* Test anonymous structs and unions.  */
+int
+anonymous_structs_and_unions (void)
+{
+  Anonymous a;
+  a.b = 1;
+  a.c = 2;
+  a.d = 3;
+  a.e = 4;
+  a.f = 5;
+  a.g = 6;
+  a.h = '7';
+  a.i = 8;
+  a.j = '8';
+  return 0;  /* anonymous_structs_and_unions */
+}
+
 int main ()
 {
   reference_update_tests ();
@@ -212,5 +257,6 @@ int main ()
   reference_to_pointer ();
   reference_to_struct ();
   path_expression ();
+  anonymous_structs_and_unions ();
   return 0;
 }
diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp
index 4ca3a68..4fea332 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp
@@ -46,5 +46,78 @@ mi_run_inline_test reference_to_pointer
 mi_run_inline_test reference_to_struct
 mi_run_inline_test path_expression
 
+set lineno [gdb_get_line_number "/* anonymous_structs_and_unions */"]
+mi_create_breakpoint \
+    "$srcfile:$lineno" {[0-9]+} keep {anonymous_structs_and_unions\(\)} \
+    ".*mi-var-cp.cc" $lineno $hex "break in anonymous_structs_and_unions"
+mi_execute_to "exec-continue" "breakpoint-hit" \
+    "anonymous_structs_and_unions" "" ".*" ".*" {"" "disp=\"keep\""} \
+    "continue to anonymous_structs breakpoint"
+
+set tree {
+  Anonymous a {
+    {} public {
+      anonymous struct {
+        {} public {
+          int b {}
+        }
+      }
+      anonymous struct {
+        {} public {
+          int c {}
+        }
+      }
+      anonymous struct {
+        {} public {
+          int d {}
+          anonymous struct {
+            {} public {
+              int e {}
+              anonymous struct {
+                {} public {
+                  int f {}
+                  anonymous union {
+                    {} public {
+                      int g {}
+                      char h {}
+                    }
+                  }
+                }
+              }
+	      anonymous union {
+		{} public {
+		  int i {}
+		  char j {}
+		}
+	      }
+	    }
+	  }
+	}
+      }
+    }
+  }
+}
+
+proc verify_everything {variable_name} {
+  # Test -var-list-children
+  mi_varobj_tree_test_children_callback $variable_name
+
+  # Bring the variable named by VARIABLE_NAME into the current scope
+  # in VAROBJ.
+  upvar #0 $variable_name varobj
+
+  # Test -var-info-path-expression
+  mi_gdb_test "-var-info-path-expression $varobj(obj_name)" \
+      "\\^done,path_expr=\"[string_to_regexp $varobj(path_expr)]\"" \
+      "path expression for $varobj(obj_name)"
+
+  # Test -var-info-expression
+  mi_gdb_test "-var-info-expression $varobj(obj_name)" \
+      "\\^done,lang=\"C\\+\\+\",exp=\"[string_to_regexp $varobj(display_name)]\"" \
+      "expression for $varobj(obj_name)"
+}
+
+mi_walk_varobj_tree c++ $tree verify_everything
+
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp
index 0f9b4d4..740a79f 100644
--- a/gdb/testsuite/gdb.mi/mi2-var-child.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp
@@ -1,4 +1,5 @@
-# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009 Free Software Foundation
+# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009, 2011
+# Free Software Foundation
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -1139,7 +1140,102 @@ mi_gdb_test "-var-update *" \
 	"update all vars psnp->next->next->long_ptr (and 2.long_ptr) changed"
 
 
+# Anonymous type tests
+proc verify_everything {variable_name} {
+  # Test -var-list-children
+  mi_varobj_tree_test_children_callback $variable_name
 
+  # Bring the variable named by VARIABLE_NAME into the current scope
+  # in VAROBJ.
+  upvar #0 $variable_name varobj
+
+  # Test -var-info-path-expression
+  mi_gdb_test "-var-info-path-expression $varobj(obj_name)" \
+      "\\^done,path_expr=\"[string_to_regexp $varobj(path_expr)]\"" \
+      "path expression for $varobj(obj_name)"
+
+  # Test -var-info-expression
+  mi_gdb_test "-var-info-expression $varobj(obj_name)" \
+      "\\^done,lang=\"C\",exp=\"[string_to_regexp $varobj(display_name)]\"" \
+      "expression for $varobj(obj_name)"
+}
+
+set lineno [gdb_get_line_number "anonymous type tests breakpoint"]
+mi_create_breakpoint \
+    "$srcfile:$lineno" {[0-9]+} keep {do_anonymous_type_tests} \
+    ".*var-cmd.c" $lineno $hex "break in do_anonymous_type_tests"
+mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\
+    ".*" ".*" {"" "disp=\"keep\""} \
+    "continue to do_anonymous_type_tests breakpoint"
+
+# Run the varobj tree on variable "ptr".
+set tree {
+  {struct anonymous **} ptr {
+    {struct anonymous *} {*ptr} {
+      int a {}
+      anonymous struct {
+	int b {}
+	{char *} c {
+	  char {*c} {}
+	}
+	anonymous union {
+	  int d {}
+	  {void *} e {}
+	  char f {}
+	  anonymous struct {
+	    char g {}
+	    {const char **} h {
+	      {const char *} {*h} {
+		{const char} {**h} {}
+	      }
+	    }
+	    {simpleton ***} simple {
+	      {simpleton **} {*simple} {
+		{simpleton *} {**simple} {
+		  int integer {}
+		  {unsigned int} unsigned_integer {}
+		  char character {}
+		  {signed char} signed_character {}
+		  {char *} char_ptr {
+		    char {*char_ptr} {}
+		  }
+		  {int [10]} array_of_10 {
+		    int 0 {}
+		    int 1 {}
+		    int 2 {}
+		    int 3 {}
+		    int 4 {}
+		    int 5 {}
+		    int 6 {}
+		    int 7 {}
+		    int 8 {}
+		    int 9 {}
+		  }
+		}
+	      }
+	    }
+	  }
+	}
+      }
+    }
+  }
+}
+
+mi_walk_varobj_tree c $tree verify_everything
+
+set tree {
+  {struct {...}} v {
+    int x {}
+    anonymous struct {
+      int a {}
+    }
+    anonymous struct {
+      int b {}
+    }
+  }
+}
+
+mi_walk_varobj_tree c $tree verify_everything
 
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
index 71c5b9d..f724eb0 100644
--- a/gdb/testsuite/gdb.mi/var-cmd.c
+++ b/gdb/testsuite/gdb.mi/var-cmd.c
@@ -92,6 +92,24 @@ struct _struct_n_pointer {
   struct _struct_n_pointer *next;
 };
 
+struct anonymous {
+  int a;
+  struct {
+    int b;
+    char *c;
+    union {
+      int d;
+      void *e;
+      char f;
+      struct {
+	char g;
+	const char **h;
+	simpleton ***simple;
+      };
+    };
+  };
+};
+
 void do_locals_tests (void);
 void do_block_tests (void);
 void subroutine1 (int, long *);
@@ -503,6 +521,38 @@ void do_bitfield_tests ()
   /*: END: bitfield :*/  
 }
 
+void
+do_anonymous_type_tests (void)
+{
+  struct anonymous *anon;
+  struct anonymous **ptr;
+  struct
+  {
+    int x;
+    struct
+    {
+      int a;
+    };
+    struct
+    {
+      int b;
+    };
+  } v = {1, {2}, {3}};
+
+  anon = malloc (sizeof (struct anonymous));
+  anon->a = 1;
+  anon->b = 2;
+  anon->c = (char *) 3;
+  anon->d = 4;
+  anon->g = '5';
+  anon->h = (const char **) 6;
+  anon->simple = (simpleton ***) 7;
+
+  ptr = &anon;
+  free (anon);
+  return; /* anonymous type tests breakpoint */
+}
+
 int
 main (int argc, char *argv [])
 {
@@ -513,6 +563,7 @@ main (int argc, char *argv [])
   do_frozen_tests ();
   do_at_tests ();
   do_bitfield_tests ();
+  do_anonymous_type_tests ();
   exit (0);
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 7c68a93..ab9dd4a 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -43,6 +43,10 @@
 typedef int PyObject;
 #endif
 
+/* The names of varobjs representing anonymous structs or unions.  */
+#define ANONYMOUS_STRUCT_NAME _("<anonymous struct>")
+#define ANONYMOUS_UNION_NAME _("<anonymous union>")
+
 /* Non-zero if we want to see trace of varobj level stuff.  */
 
 int varobjdebug = 0;
@@ -1289,6 +1293,39 @@ varobj_get_gdb_type (struct varobj *var)
   return var->type;
 }
 
+/* Is VAR a path expression parent, i.e., can it be used to construct
+   a valid path expression?  */
+
+static int
+is_path_expr_parent (struct varobj *var)
+{
+  struct type *type;
+
+  /* "Fake" children are not path_expr parents.  */
+  if (CPLUS_FAKE_CHILD (var))
+    return 0;
+
+  type = get_value_type (var);
+
+  /* Anonymous unions and structs are also not path_expr parents.  */
+  return !((TYPE_CODE (type) == TYPE_CODE_STRUCT
+	    || TYPE_CODE (type) == TYPE_CODE_UNION)
+	   && TYPE_NAME (type) == NULL);
+}
+
+/* Return the path expression parent for VAR.  */
+
+static struct varobj *
+get_path_expr_parent (struct varobj *var)
+{
+  struct varobj *parent = var;
+
+  while (!is_root_p (parent) && !is_path_expr_parent (parent))
+    parent = parent->parent;
+
+  return parent;
+}
+
 /* Return a pointer to the full rooted expression of varobj VAR.
    If it has not been computed yet, compute it.  */
 char *
@@ -2170,6 +2207,20 @@ create_child (struct varobj *parent, int index, char *name)
 				  value_of_child (parent, index));
 }
 
+/* Does CHILD represent a child with no name?  This happens when
+   the child is an anonmous struct or union and it has no field name
+   in its parent variable.
+
+   This has already been determined by *_describe_child. The easiest
+   thing to do is to compare the child's name with ANONYMOUS_*_NAME.  */
+
+static int
+is_anonymous_child (struct varobj *child)
+{
+  return (strcmp (child->name, ANONYMOUS_STRUCT_NAME) == 0
+	  || strcmp (child->name, ANONYMOUS_UNION_NAME) == 0);
+}
+
 static struct varobj *
 create_child_with_value (struct varobj *parent, int index, const char *name,
 			 struct value *value)
@@ -2185,8 +2236,13 @@ create_child_with_value (struct varobj *parent, int index, const char *name,
   child->index = index;
   child->parent = parent;
   child->root = parent->root;
-  childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
+
+  if (is_anonymous_child (child))
+    childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index);
+  else
+    childs_name = xstrprintf ("%s.%s", parent->obj_name, name);
   child->obj_name = childs_name;
+
   install_variable (child);
 
   /* Compute the type of the child.  Must do this before
@@ -2955,7 +3011,7 @@ c_describe_child (struct varobj *parent, int index,
   if (cfull_expression)
     {
       *cfull_expression = NULL;
-      parent_expression = varobj_get_path_expr (parent);
+      parent_expression = varobj_get_path_expr (get_path_expr_parent (parent));
     }
   adjust_value_for_child_access (&value, &type, &was_ptr);
       
@@ -2990,26 +3046,49 @@ c_describe_child (struct varobj *parent, int index,
 
     case TYPE_CODE_STRUCT:
     case TYPE_CODE_UNION:
-      if (cname)
-	*cname = xstrdup (TYPE_FIELD_NAME (type, index));
+      {
+	char *field_name;
 
-      if (cvalue && value)
-	{
-	  /* For C, varobj index is the same as type index.  */
-	  *cvalue = value_struct_element_index (value, index);
-	}
+	/* If the type is anonymous and the field has no name,
+	   set an appropriate name.  */
+	field_name = TYPE_FIELD_NAME (type, index);
+	if (field_name == NULL || *field_name == '\0')
+	  {
+	    if (cname)
+	      {
+		if (TYPE_CODE (TYPE_FIELD_TYPE (type, index))
+		    == TYPE_CODE_STRUCT)
+		  *cname = xstrdup (ANONYMOUS_STRUCT_NAME);
+		else
+		  *cname = xstrdup (ANONYMOUS_UNION_NAME);
+	      }
 
-      if (ctype)
-	*ctype = TYPE_FIELD_TYPE (type, index);
+	    if (cfull_expression)
+	      *cfull_expression = xstrdup ("");
+	  }
+	else
+	  {
+	    if (cname)
+	      *cname = xstrdup (field_name);
 
-      if (cfull_expression)
-	{
-	  char *join = was_ptr ? "->" : ".";
+	    if (cfull_expression)
+	      {
+		char *join = was_ptr ? "->" : ".";
 
-	  *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join,
-					  TYPE_FIELD_NAME (type, index));
-	}
+		*cfull_expression = xstrprintf ("(%s)%s%s", parent_expression,
+						join, field_name);
+	      }
+	  }
 
+	if (cvalue && value)
+	  {
+	    /* For C, varobj index is the same as type index.  */
+	    *cvalue = value_struct_element_index (value, index);
+	  }
+
+	if (ctype)
+	  *ctype = TYPE_FIELD_TYPE (type, index);
+      }
       break;
 
     case TYPE_CODE_PTR:
@@ -3357,14 +3436,16 @@ cplus_describe_child (struct varobj *parent, int index,
       value = parent->parent->value;
       type = get_value_type (parent->parent);
       if (cfull_expression)
-	parent_expression = varobj_get_path_expr (parent->parent);
+	parent_expression
+	  = varobj_get_path_expr (get_path_expr_parent (parent->parent));
     }
   else
     {
       value = parent->value;
       type = get_value_type (parent);
       if (cfull_expression)
-	parent_expression = varobj_get_path_expr (parent);
+	parent_expression
+	  = varobj_get_path_expr (get_path_expr_parent (parent));
     }
 
   adjust_value_for_child_access (&value, &type, &was_ptr);
@@ -3386,6 +3467,7 @@ cplus_describe_child (struct varobj *parent, int index,
 	  enum accessibility acc = public_field;
 	  int vptr_fieldno;
 	  struct type *basetype = NULL;
+	  char *field_name;
 
 	  vptr_fieldno = get_vptr_fieldno (type, &basetype);
 	  if (strcmp (parent->name, "private") == 0)
@@ -3404,20 +3486,40 @@ cplus_describe_child (struct varobj *parent, int index,
 	    }
 	  --type_index;
 
-	  if (cname)
-	    *cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
+	  /* If the type is anonymous and the field has no name,
+	     set an appopriate name.  */
+	  field_name = TYPE_FIELD_NAME (type, type_index);
+	  if (field_name == NULL || *field_name == '\0')
+	    {
+	      if (cname)
+		{
+		  if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index))
+		      == TYPE_CODE_STRUCT)
+		    *cname = xstrdup (ANONYMOUS_STRUCT_NAME);
+		  else if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index))
+			   == TYPE_CODE_UNION)
+		    *cname = xstrdup (ANONYMOUS_UNION_NAME);
+		}
+
+	      if (cfull_expression)
+		*cfull_expression = xstrdup ("");
+	    }
+	  else
+	    {
+	      if (cname)
+		*cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
+
+	      if (cfull_expression)
+		*cfull_expression
+		  = xstrprintf ("((%s)%s%s)", parent_expression, join,
+				field_name);
+	    }
 
 	  if (cvalue && value)
 	    *cvalue = value_struct_element_index (value, type_index);
 
 	  if (ctype)
 	    *ctype = TYPE_FIELD_TYPE (type, type_index);
-
-	  if (cfull_expression)
-	    *cfull_expression
-	      = xstrprintf ("((%s)%s%s)", parent_expression,
-			    join, 
-			    TYPE_FIELD_NAME (type, type_index));
 	}
       else if (index < TYPE_N_BASECLASSES (type))
 	{

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

* Re: [RFA] mi/10586
  2011-12-17  1:55                 ` Keith Seitz
@ 2011-12-20 16:03                   ` Tom Tromey
  2012-01-12 23:05                     ` Keith Seitz
  0 siblings, 1 reply; 15+ messages in thread
From: Tom Tromey @ 2011-12-20 16:03 UTC (permalink / raw)
  To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

Keith> ChangeLog
Keith> 2011-12-16  Keith Seitz  <keiths@redhat.com>

Keith> 	PR mi/10586
Keith> 	* varobj.c (ANONYMOUS_STRUCT_NAME): Define.
Keith> 	(ANONYMOUS_UNION_NAME): Define.
Keith> 	(is_path_expr_parent): New function.
Keith> 	(get_path_expr_parent): New function.
Keith> 	(is_anonymous_child): New function.
Keith> 	(create_child_with_value): If the child is anonymous and without
Keith> 	a name, assign an object name to it.
Keith> 	(c_describe_child): Use get_path_expr_parent to determine
Keith> 	the parent expression.
Keith> 	If there field represents an anonymous struct or union and
Keith> 	has no name, set an appropriate display name and expression.
Keith> 	(cplus_describe_child): Likewise.

Keith> testsuite/ChangeLog
Keith> 2011-12-16  Keith Seitz  <keiths@redhat.com>

Keith> 	PR mi/10586
Keith> 	* gdb.mi/var-cmd.c (struct anonymous): New structure.
Keith> 	(do_anonymous_type_tests): New function.
Keith> 	(main): Call do_anonymous_type_tests.
Keith> 	* gdb.mi/mi2-var-child.exp: Add anonymous type tests.
Keith> 	(verify_everything): New procedure.
Keith> 	* gdb.mi/mi-var-cp.cc (class A): New class.
Keith> 	(anonymous_structs_and_unions): New function.
Keith> 	(main): Call anonymous_structs_and_unions.
Keith> 	* gdb.mi/mi-var-cp.exp: Add anonymous type tests.
Keith> 	(verify_everything): New procedure.

Ok.
Thanks.

Tom

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

* Re: [RFA] mi/10586
  2011-12-20 16:03                   ` Tom Tromey
@ 2012-01-12 23:05                     ` Keith Seitz
  0 siblings, 0 replies; 15+ messages in thread
From: Keith Seitz @ 2012-01-12 23:05 UTC (permalink / raw)
  To: gdb-patches@sourceware.org ml

On 12/20/2011 08:00 AM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz<keiths@redhat.com>  writes:
>
> Keith>  ChangeLog
> Keith>  2011-12-16  Keith Seitz<keiths@redhat.com>
>
> Keith>  	PR mi/10586
> Keith>  	* varobj.c (ANONYMOUS_STRUCT_NAME): Define.
> Keith>  	(ANONYMOUS_UNION_NAME): Define.
> Keith>  	(is_path_expr_parent): New function.
> Keith>  	(get_path_expr_parent): New function.
> Keith>  	(is_anonymous_child): New function.
> Keith>  	(create_child_with_value): If the child is anonymous and without
> Keith>  	a name, assign an object name to it.
> Keith>  	(c_describe_child): Use get_path_expr_parent to determine
> Keith>  	the parent expression.
> Keith>  	If there field represents an anonymous struct or union and
> Keith>  	has no name, set an appropriate display name and expression.
> Keith>  	(cplus_describe_child): Likewise.
>
> Keith>  testsuite/ChangeLog
> Keith>  2011-12-16  Keith Seitz<keiths@redhat.com>
>
> Keith>  	PR mi/10586
> Keith>  	* gdb.mi/var-cmd.c (struct anonymous): New structure.
> Keith>  	(do_anonymous_type_tests): New function.
> Keith>  	(main): Call do_anonymous_type_tests.
> Keith>  	* gdb.mi/mi2-var-child.exp: Add anonymous type tests.
> Keith>  	(verify_everything): New procedure.
> Keith>  	* gdb.mi/mi-var-cp.cc (class A): New class.
> Keith>  	(anonymous_structs_and_unions): New function.
> Keith>  	(main): Call anonymous_structs_and_unions.
> Keith>  	* gdb.mi/mi-var-cp.exp: Add anonymous type tests.
> Keith>  	(verify_everything): New procedure.
>
> Ok.
> Thanks.

Now that the testsuite updates are checked in, I've double-checked this 
patch and committed it.

Keith

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

end of thread, other threads:[~2012-01-12 22:52 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-11-11 21:30 [RFA] mi/10586 Keith Seitz
2011-11-14 15:45 ` Tom Tromey
2011-11-14 18:04   ` Keith Seitz
2011-11-14 18:18     ` Tom Tromey
2011-11-14 19:28       ` Keith Seitz
2011-11-14 20:28         ` Tom Tromey
2011-11-15 17:10           ` Keith Seitz
2011-11-15 17:30             ` Tom Tromey
2011-12-02 22:28             ` Keith Seitz
2011-12-13  1:28               ` Keith Seitz
2011-12-13 19:58               ` Tom Tromey
2011-12-17  1:55                 ` Keith Seitz
2011-12-20 16:03                   ` Tom Tromey
2012-01-12 23:05                     ` Keith Seitz
2011-12-13 20:34               ` Crash regression with Eclipse [Re: [RFA] mi/10586] Jan Kratochvil

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).