[PATCH] gdb: fix path expression for anonymous union members

[PATCH] gdb: fix path expression for anonymous union members

[Stephan Rohr]

From: "Bobko, Sergey" <sergey.bobko@intel.com>

Consider the following structure:

  struct SBase {};
  class S : SBase {
    private:
      int a;
    public:
      int b;
      union {
	int c;
      };
  };

  S s;

When we create a variable object for variable "s", path expression for the
field "s.c" is not computed correctly:

  interpret mi "-var-info-path-expression var1.public.1_anonymous.public.c"
  ^done,path_expr="(().c)"

To compute a path expression for a struct member, GDB looks up to the parent
var object.  If the parent has a valid expression, GDB uses it to build
the expression for the child.  Otherwise, it goes one level up to check the
parent->parent, until the valid parent expression is found.

The reason for "(().c)" is that the check c_is_path_expr_parent incorrectly
returns "true" for the anonymous union parent.  Then the path expression of
the anonymous union, which is "", is used to compute the result.

Inside the check, it takes the type field name by the index of the var-object.
The index of the var object is the child index within its parent
var-object.  However, there are artificial layers for access-levels, e.g.,
public, private or protected.  So the indexing of the child var object might
be done within an artificial layer, while the type fields are indexed as a
flat list of all fields, including the base types.

For the struct above, the var-object tree is as follows:

  var1		 -> index -1
    var1.SBase	 -> index 0
    var1.public	 -> index 1
      var1.public.b -> index 0
      var1.public.1_anonymous -> index 1
	var1.public.1_anonymous.public -> index 0
	  var1.public.1_anonymous.public.c -> index 0
    var1.private -> index 2
      var1.private.a -> index 0

Note, that the base type member is the direct child of var1, and the
other fields have an artificial access parent.

The fields of type S are indexed as:

  SBase -> 0
  a -> 1
  b -> 2
  "" -> 3

where "" is the anonymous union.

Extract the required logic from cplus_describe_child and use it to get
the correct index of the field within the parent type.  With this change
the path expression is computed correctly:

  interpret mi "-var-info-path-expression var1.public.1_anonymous.public.c"
  ^done,path_expr="((s).c)"

I modified the related test to include both base type and access layers.
However, to correctly parse the base types within the MI var tree I had to
introduce a heuristic that if the child's type is equal to child's name
then it is a base type, and the path expression is different in this case.
---
 gdb/c-varobj.c                     | 89 +++++++++++++++++++-----------
 gdb/testsuite/gdb.mi/mi-var-cp.cc  | 18 +++++-
 gdb/testsuite/gdb.mi/mi-var-cp.exp | 17 ++++++
 gdb/testsuite/lib/mi-support.exp   |  4 ++
 4 files changed, 96 insertions(+), 32 deletions(-)

diff --git a/gdb/c-varobj.c b/gdb/c-varobj.c
index 5312eccdaca..dc88ce6e29a 100644
--- a/gdb/c-varobj.c
+++ b/gdb/c-varobj.c
@@ -22,6 +22,10 @@
 
 static void cplus_class_num_children (struct type *type, int children[3]);
 
+static int
+type_field_num_from_child_index (int index, const varobj *parent,
+				 type *parent_type);
+
 /* The names of varobjs representing anonymous structs or unions.  */
 #define ANONYMOUS_STRUCT_NAME _("<anonymous struct>")
 #define ANONYMOUS_UNION_NAME _("<anonymous union>")
@@ -163,8 +167,12 @@ c_is_path_expr_parent (const struct varobj *var)
 	    {
 	      const char *field_name;
 
-	      gdb_assert (var->index < parent_type->num_fields ());
-	      field_name = parent_type->field (var->index).name ();
+	      int type_index = type_field_num_from_child_index (var->index,
+								var->parent,
+								parent_type);
+
+	      gdb_assert (type_index < parent_type->num_fields ());
+	      field_name = parent_type->field (type_index).name ();
 	      return !(field_name == NULL || *field_name == '\0');
 	    }
 	}
@@ -669,6 +677,51 @@ cplus_name_of_variable (const struct varobj *parent)
   return c_name_of_variable (parent);
 }
 
+/* Return the type-field index within the PARENT_TYPE for the child with
+   the var-index INDEX in the PARENT var object.  */
+
+static int
+type_field_num_from_child_index (int index, const varobj *parent,
+				 type *parent_type)
+{
+  gdb_assert (parent_type->code () == TYPE_CODE_STRUCT
+	      || parent_type->code () == TYPE_CODE_UNION);
+
+  /* If the parent is not an artificial layer, just return the index.  */
+  if (!CPLUS_FAKE_CHILD (parent))
+    return index;
+
+  /* The fields of the class type are ordered as they
+     appear in the class.  We are given an index for a
+     particular access control type ("public","protected",
+     or "private").  We must skip over fields that don't
+     have the access control we are looking for to properly
+     find the indexed field.  */
+  int type_index = TYPE_N_BASECLASSES (parent_type);
+  int vptr_fieldno;
+  type *basetype = nullptr;
+  enum accessibility acc = accessibility::PUBLIC;
+
+  if (parent->name == "private")
+    acc = accessibility::PRIVATE;
+  else if (parent->name == "protected")
+    acc = accessibility::PROTECTED;
+
+  vptr_fieldno = get_vptr_fieldno (parent_type, &basetype);
+  while (index >= 0)
+    {
+      if ((parent_type == basetype && type_index == vptr_fieldno)
+	  || parent_type->field (type_index).is_artificial ())
+	; /* ignore vptr */
+      else if (parent_type->field (type_index).accessibility () == acc)
+	--index;
+      ++type_index;
+    }
+  --type_index;
+
+  return type_index;
+}
+
 static void
 cplus_describe_child (const struct varobj *parent, int index,
 		      std::string *cname, struct value **cvalue, struct type **ctype,
@@ -710,38 +763,12 @@ cplus_describe_child (const struct varobj *parent, int index,
 
       if (CPLUS_FAKE_CHILD (parent))
 	{
-	  /* The fields of the class type are ordered as they
-	     appear in the class.  We are given an index for a
-	     particular access control type ("public","protected",
-	     or "private").  We must skip over fields that don't
-	     have the access control we are looking for to properly
-	     find the indexed field.  */
-	  int type_index = TYPE_N_BASECLASSES (type);
-	  enum accessibility acc = accessibility::PUBLIC;
-	  int vptr_fieldno;
-	  struct type *basetype = NULL;
-	  const char *field_name;
-
-	  vptr_fieldno = get_vptr_fieldno (type, &basetype);
-	  if (parent->name == "private")
-	    acc = accessibility::PRIVATE;
-	  else if (parent->name == "protected")
-	    acc = accessibility::PROTECTED;
-
-	  while (index >= 0)
-	    {
-	      if ((type == basetype && type_index == vptr_fieldno)
-		  || type->field (type_index).is_artificial ())
-		; /* ignore vptr */
-	      else if (type->field (type_index).accessibility () == acc)
-		--index;
-	      ++type_index;
-	    }
-	  --type_index;
+	  int type_index = type_field_num_from_child_index (index, parent,
+							    type);
 
 	  /* If the type is anonymous and the field has no name,
 	     set an appropriate name.  */
-	  field_name = type->field (type_index).name ();
+	  const char *field_name = type->field (type_index).name ();
 	  if (field_name == NULL || *field_name == '\0')
 	    {
 	      if (cname)
diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc
index 624ddce325c..39b1294b4c1 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.cc
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc
@@ -205,7 +205,8 @@ int path_expression ()
   /*: END: path_expression :*/
 }
 
-class Anonymous
+class AnonymousBase {};
+class Anonymous : AnonymousBase
 {
 public:
   struct { /* index: 0 */
@@ -229,8 +230,23 @@ public:
 	int i;
 	char j;
       };
+      struct {
+	int k;
+      private:
+	union {
+	  int l;
+	  char m;
+	};
+      } with_name;
     };
   };
+
+  /* The vptr is an artificial private field and must not be listed
+     among the private children.  */
+  virtual ~Anonymous () {}
+
+private:
+  int priv = 9;
 };
 
 /* Test anonymous structs and unions.  */
diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp
index fb850ad51db..ce2fd368000 100644
--- a/gdb/testsuite/gdb.mi/mi-var-cp.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp
@@ -56,6 +56,7 @@ mi_execute_to "exec-continue" "breakpoint-hit" \
 
 set tree {
   Anonymous a {
+    AnonymousBase AnonymousBase {}
     {} public {
       anonymous struct {
 	{} public {
@@ -90,11 +91,27 @@ set tree {
 		  char j {}
 		}
 	      }
+	      "struct {...}" with_name {
+		{} public {
+		  int k {}
+		}
+		{} private {
+		  anonymous union {
+		    {} public {
+		      int l {}
+		      char m {}
+		    }
+		  }
+		}
+	      }
 	    }
 	  }
 	}
       }
     }
+    {} private {
+      int priv {}
+    }
   }
 }
 
diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
index cd7e0a61189..465cf3fd69c 100644
--- a/gdb/testsuite/lib/mi-support.exp
+++ b/gdb/testsuite/lib/mi-support.exp
@@ -2401,6 +2401,10 @@ namespace eval ::varobj_tree {
       set is_compound 0
     }
 
+    if {$type eq $name} {
+	# This is the base type.
+	return "((class $type) $path_parent(path_expr))"
+    }
     if {[string index $parent(type) end] == "\]"} {
       # Parent is an array.
       return "($path_parent(path_expr))\[$name\]"
-- 
2.43.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH] gdb: fix path expression for anonymous union members

[Keith Seitz]

Hi,

Thanks for the patch.

On 10/7/26 1:27 AM, Stephan Rohr wrote:
> 
> Extract the required logic from cplus_describe_child and use it to get
> the correct index of the field within the parent type.  With this change
> the path expression is computed correctly:
> 
>    interpret mi "-var-info-path-expression var1.public.1_anonymous.public.c"
>    ^done,path_expr="((s).c)"
> 

I think this is the correct approach.

> I modified the related test to include both base type and access layers.
> However, to correctly parse the base types within the MI var tree I had to
> introduce a heuristic that if the child's type is equal to child's name
> then it is a base type, and the path expression is different in this case.

Let's look at this a bit in the actual change:

> diff --git a/gdb/testsuite/lib/mi-support.exp b/gdb/testsuite/lib/mi-support.exp
> index cd7e0a61189..465cf3fd69c 100644
> --- a/gdb/testsuite/lib/mi-support.exp
> +++ b/gdb/testsuite/lib/mi-support.exp
> @@ -2401,6 +2401,10 @@ namespace eval ::varobj_tree {
>         set is_compound 0
>       }
>   
> +    if {$type eq $name} {
> +	# This is the base type.
> +	return "((class $type) $path_parent(path_expr))"
> +    }
>       if {[string index $parent(type) end] == "\]"} {
>         # Parent is an array.
>         return "($path_parent(path_expr))\[$name\]"

IIUC, if we have a construct like:

class Outer {
public:
   struct Inner { int x; };
   Inner Inner;
};

Outer o;

Then the proposed heuristic change would return "((class Inner) o)".
However, the real path should be "((o).Inner)".

While this is probably unlikely to actually affect anything,
perhaps we can tighten this a little bit by applying the heuristic
only if its parent is not one of these artificial access parents?

That is, something like:

proc is_cplus_fake_child {variable} {
   upvar #0 $variable varobj
   return [string equal $varobj(type) ""]
}

# Note that proc get_path_expr walks parent_variable -- we need to save
# it before that.
if {$type eq $name && is_cplus_fake_child $saved_parent} {
    return #...
}

That might be a simple change to mitigate some future frustration.

Keith