Re: [RFA] mi/10586

[Keith Seitz <keiths@redhat.com>]

[-- 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))
 	{