Re: [Patch] PR python/15464 and python/16113

[Siva Chandra <sivachandra@google.com>]

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

Attached is a new version of the patch which addresses Tom's comments.

Siva> +   if (name_obj != Py_None)
Siva> +     {
Siva> +       field = python_string_to_host_string (name_obj);
Siva> +       Py_DECREF (name_obj);
Siva> +       name_obj = NULL;
Siva> +       if (field == NULL)
Siva> +         return NULL;
Siva> +     }
Siva> +
Siva> +   if (name_obj == Py_None || field[0] == '\0')
Siva> +     {
Siva> +       PyObject *bitpos_obj;
Siva> +       int valid;
Siva> +
Siva> +       Py_XDECREF (name_obj);

Tom> I think that if name_obj is the Python string "", then it will be
Tom> decref'd twice.

I had "name_obj = NULL" in the first if block with the second decref
an x-decref to avoid decref-ing twice.  This is irrelevant now as I
have modified to make the name 'None' if the name is "".

Siva> + res_val = value_struct_elt_bitpos (&tmp, bitpos, "struct/class/union");

Tom> I think this approach will fail in the situation where multiple
Tom> anonymous sub-objects appear at the same bitpos.  I think this happens
Tom> with inheritance, typically at bitpos 0 but perhaps elsewhere with
Tom> multiple inheritance.

I apologize for not being thorough here.  Even a simple union with two
un-named fields can show the problem.

Tom> It may be sufficient to also pass in an expected type, which could be
Tom> extracted from the Field object.

The attached patch takes this route.

2014-12-07  Siva Chandra Reddy  <sivachandra@google.com>

        PR python/15464
        PR python/16133
        * valops.c (value_struct_elt_bitpos): New function
        * py-type.c (convert_field): Set 'name' attribute of a gdb.Field
        object to 'None' if the field name is an empty string ("").
        * python/py-value.c (valpy_getitem): Use 'bitpos' and 'type'
        attribute to look for a field when 'name' is 'None'.
        (get_field_type): New function

        testsuite/
        * gdb.python/py-type.c: Enhance test case.
        * gdb.python/py-value-cc.cc: Likewise
        * gdb.python/py-type.exp: Add new tests.
        * gdb.python/py-value-cc.exp: Likewise

[-- Attachment #2: value_field_subscript_patch_v3.txt --]
[-- Type: text/plain, Size: 11129 bytes --]

diff --git a/gdb/python/py-type.c b/gdb/python/py-type.c
index 16bb442..c904d3a 100644
--- a/gdb/python/py-type.c
+++ b/gdb/python/py-type.c
@@ -206,15 +206,22 @@ convert_field (struct type *type, int field)
       Py_DECREF (arg);
     }
 
+  arg = NULL;
   if (TYPE_FIELD_NAME (type, field))
-    arg = PyString_FromString (TYPE_FIELD_NAME (type, field));
-  else
+    {
+      const char *field_name = TYPE_FIELD_NAME (type, field);
+      if (field_name[0] != '\0')
+	{
+	  arg = PyString_FromString (TYPE_FIELD_NAME (type, field));
+	  if (arg == NULL)
+	    goto fail;
+	}
+    }
+  if (arg == NULL)
     {
       arg = Py_None;
       Py_INCREF (arg);
     }
-  if (!arg)
-    goto fail;
   if (PyObject_SetAttrString (result, "name", arg) < 0)
     goto failarg;
   Py_DECREF (arg);
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index 4ab782d..c23e4f9 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -563,6 +563,29 @@ get_field_flag (PyObject *field, const char *flag_name)
   return flag_value;
 }
 
+/* Return the "type" attribute of a gdb.Field object.  
+   Returns NULL on error, with a Python exception set.  */
+
+static struct type *
+get_field_type (PyObject *field)
+{
+  PyObject *ftype_obj = PyObject_GetAttrString (field, "type");
+  struct type *ftype;
+
+  if (ftype_obj == NULL)
+    return NULL;
+  ftype = type_object_to_type (ftype_obj);
+  Py_DECREF (ftype_obj);
+  if (ftype == NULL)
+    {
+      PyErr_SetString (PyExc_TypeError,
+		       _("'type' attribute of gdb.Field object is not a "
+			 "gdb.Type object."));
+    }
+
+  return ftype;
+}
+
 /* Given string name or a gdb.Field object corresponding to an element inside
    a structure, return its value object.  Returns NULL on error, with a python
    exception set.  */
@@ -572,7 +595,8 @@ valpy_getitem (PyObject *self, PyObject *key)
 {
   value_object *self_value = (value_object *) self;
   char *field = NULL;
-  PyObject *base_class_type_object = NULL;
+  struct type *base_class_type = NULL, *field_type = NULL;
+  long bitpos = -1;
   volatile struct gdb_exception except;
   PyObject *result = NULL;
 
@@ -603,8 +627,8 @@ valpy_getitem (PyObject *self, PyObject *key)
 	return NULL;
       else if (is_base_class > 0)
 	{
-	  base_class_type_object = PyObject_GetAttrString (key, "type");
-	  if (base_class_type_object == NULL)
+	  base_class_type = get_field_type (key);
+	  if (base_class_type == NULL)
 	    return NULL;
 	}
       else
@@ -614,10 +638,40 @@ valpy_getitem (PyObject *self, PyObject *key)
 	  if (name_obj == NULL)
 	    return NULL;
 
-	  field = python_string_to_host_string (name_obj);
-	  Py_DECREF (name_obj);
-	  if (field == NULL)
-	    return NULL;
+	  if (name_obj != Py_None)
+	    {
+	      field = python_string_to_host_string (name_obj);
+	      Py_DECREF (name_obj);
+	      if (field == NULL)
+		return NULL;
+	    }
+	  else
+	    {
+	      PyObject *bitpos_obj;
+	      int valid;
+
+	      Py_DECREF (name_obj);
+
+	      if (!PyObject_HasAttrString (key, "bitpos"))
+		{
+		  PyErr_SetString (PyExc_AttributeError,
+				   _("gdb.Field object has no name and no "
+                                     "'bitpos' attribute."));
+
+		  return NULL;
+		}
+	      bitpos_obj = PyObject_GetAttrString (key, "bitpos");
+	      if (bitpos_obj == NULL)
+		return NULL;
+	      valid = gdb_py_int_as_long (bitpos_obj, &bitpos);
+	      Py_DECREF (bitpos_obj);
+	      if (!valid)
+		return NULL;
+
+	      field_type = get_field_type (key);
+	      if (field_type == NULL)
+		return NULL;
+	    }
 	}
     }
 
@@ -629,14 +683,12 @@ valpy_getitem (PyObject *self, PyObject *key)
 
       if (field)
 	res_val = value_struct_elt (&tmp, NULL, field, 0, NULL);
-      else if (base_class_type_object != NULL)
+      else if (bitpos >= 0)
+	res_val = value_struct_elt_bitpos (&tmp, bitpos, field_type,
+					   "struct/class/union");
+      else if (base_class_type != NULL)
 	{
-	  struct type *base_class_type, *val_type;
-
-	  base_class_type = type_object_to_type (base_class_type_object);
-	  Py_DECREF (base_class_type_object);
-	  if (base_class_type == NULL)
-	    error (_("Field type not an instance of gdb.Type."));
+	  struct type *val_type;
 
 	  val_type = check_typedef (value_type (tmp));
 	  if (TYPE_CODE (val_type) == TYPE_CODE_PTR)
diff --git a/gdb/testsuite/gdb.python/py-type.c b/gdb/testsuite/gdb.python/py-type.c
index 7cee383..697e29c 100644
--- a/gdb/testsuite/gdb.python/py-type.c
+++ b/gdb/testsuite/gdb.python/py-type.c
@@ -21,6 +21,12 @@ struct s
   int b;
 };
 
+struct SS
+{
+  union { int x; char y; };
+  union { int a; char b; };
+};
+
 typedef struct s TS;
 TS ts;
 
@@ -58,6 +64,7 @@ main ()
 {
   int ar[2] = {1,2};
   struct s st;
+  struct SS ss;
 #ifdef __cplusplus
   C c;
   c.c = 1;
@@ -72,6 +79,8 @@ main ()
   st.b = 5;
 
   e = v2;
+
+  ss.x = 100;
   
   return 0;      /* break to inspect struct and array.  */
 }
diff --git a/gdb/testsuite/gdb.python/py-type.exp b/gdb/testsuite/gdb.python/py-type.exp
index 93301d0..6b61f48 100644
--- a/gdb/testsuite/gdb.python/py-type.exp
+++ b/gdb/testsuite/gdb.python/py-type.exp
@@ -85,6 +85,15 @@ proc test_fields {lang} {
     gdb_test "python print (fields\[0\].name)" "a" "Check structure field a name"
     gdb_test "python print (fields\[1\].name)" "b" "Check structure field b name"
 
+    # Test that unamed fields have 'None' for name.
+    gdb_py_test_silent_cmd "python ss = gdb.parse_and_eval('ss')" "init ss" 1
+    gdb_py_test_silent_cmd "python ss_fields = ss.type.fields()" \
+      "get fields from ss.type" 1
+    gdb_test "python print len(ss_fields)" "2" "Check length of ss_fields"
+    gdb_test "python print ss_fields\[0\].name is None" "True" \
+      "Check ss_fields\[0\].name"
+    gdb_test "python print ss_fields\[1\].name is None" "True" \
+      "Check ss_fields\[1\].name"
     # Regression test for
     # http://sourceware.org/bugzilla/show_bug.cgi?id=12070.
     gdb_test "python print ('type' in dir(fields\[0\]))" "True" \
diff --git a/gdb/testsuite/gdb.python/py-value-cc.cc b/gdb/testsuite/gdb.python/py-value-cc.cc
index 59f1dec..ace957a 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.cc
+++ b/gdb/testsuite/gdb.python/py-value-cc.cc
@@ -30,8 +30,21 @@ class B : public A {
   char a;
 };
 
+struct X
+{
+  union { int x; char y; };
+  union { int a; char b; };
+};
+
+union UU
+{
+  union { int x; char y; };
+  union { int a; char b; };
+};
+
 typedef B Btd;
 typedef int *int_ptr;
+typedef X Xtd;
 
 int
 func (const A &a)
@@ -57,6 +70,16 @@ func (const A &a)
   U u;
   u.a = 99;
 
+  X x;
+  x.x = 101;
+  x.a = 102;
+
+  UU uu;
+  uu.x = 1000;
+
+  X *x_ptr = &x;
+  Xtd *xtd = &x;
+
   return 0; /* Break here.  */
 }
 
diff --git a/gdb/testsuite/gdb.python/py-value-cc.exp b/gdb/testsuite/gdb.python/py-value-cc.exp
index 1faec5f..e6351f6 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.exp
+++ b/gdb/testsuite/gdb.python/py-value-cc.exp
@@ -53,6 +53,14 @@ gdb_test_no_output "python b_ref = gdb.parse_and_eval('b_ref')" "init b_ref"
 gdb_test_no_output "python b_td = gdb.parse_and_eval('b_td')" "init b_td"
 gdb_test_no_output "python u = gdb.parse_and_eval('u')" "init u"
 gdb_test_no_output "python u_fields = u.type.fields()" "init u_fields"
+gdb_test_no_output "python x = gdb.parse_and_eval('x')" "init x"
+gdb_test_no_output "python x_fields = x.type.fields()" "init x_fields"
+gdb_test_no_output "python uu = gdb.parse_and_eval('uu')" "init uu"
+gdb_test_no_output "python uu_fields = uu.type.fields()" "init uu_fields"
+gdb_test_no_output "python x_ptr = gdb.parse_and_eval('x_ptr')" "init x_ptr"
+gdb_test_no_output "python xtd = gdb.parse_and_eval('xtd')" "init xtd"
+
+gdb_test "python print(b\[b_fields\[1\]\])" "97 'a'" "b.a via field"
 
 gdb_test "python print(b\[b_fields\[1\]\])" "97 'a'" "b.a via field"
 gdb_test "python print(b\[b_fields\[0\]\].type)" "A" \
@@ -79,3 +87,15 @@ gdb_test "python print(b_td\[b_fields\[0\]\]\['a'\])" "100" \
 
 gdb_test "python print(u\[u_fields\[0\]\])" "99.*" "u's first field via field"
 gdb_test "python print(u\[u_fields\[1\]\])" "99.*" "u's second field via field"
+
+gdb_test "python print len(x_fields)" "2" "number for fields in u"
+gdb_test "python print x\[x_fields\[0\]\]\['x'\]" "101" "x.x via field"
+gdb_test "python print x\[x_fields\[1\]\]\['a'\]" "102" "x.a via field"
+gdb_test "python print x_ptr\[x_fields\[0\]\]\['x'\]" "101" "x_ptr->x via field"
+gdb_test "python print x_ptr\[x_fields\[1\]\]\['a'\]" "102" "x_ptr->a via field"
+gdb_test "python print xtd\[x_fields\[0\]\]\['x'\]" "101" "xtd->x via field"
+gdb_test "python print xtd\[x_fields\[1\]\]\['a'\]" "102" "xtd->a via field"
+
+gdb_test "python print len(uu_fields)" "2" "number of fields in uu"
+gdb_test "python print uu\[uu_fields\[0\]\]\['x'\]" "1000" "uu.x via field"
+gdb_test "python print uu\[uu_fields\[1\]\]\['a'\]" "1000" "uu.a via field"
diff --git a/gdb/valops.c b/gdb/valops.c
index fdae3ad..2b604b9 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2248,6 +2248,51 @@ value_struct_elt (struct value **argp, struct value **args,
   return v;
 }
 
+/* Given *ARGP, a value of type structure or union, or a pointer/reference
+   to a structure or union, extract and return its component (field) of
+   type FTYPE at the specified BITPOS.
+   Throw an exception on error.  */
+
+struct value *
+value_struct_elt_bitpos (struct value **argp, int bitpos, struct type *ftype,
+			 const char *err)
+{
+  struct type *t;
+  struct value *v;
+  int i;
+  int nbases;
+
+  *argp = coerce_array (*argp);
+
+  t = check_typedef (value_type (*argp));
+
+  while (TYPE_CODE (t) == TYPE_CODE_PTR || TYPE_CODE (t) == TYPE_CODE_REF)
+    {
+      *argp = value_ind (*argp);
+      if (TYPE_CODE (check_typedef (value_type (*argp))) != TYPE_CODE_FUNC)
+	*argp = coerce_array (*argp);
+      t = check_typedef (value_type (*argp));
+    }
+
+  if (TYPE_CODE (t) != TYPE_CODE_STRUCT
+      && TYPE_CODE (t) != TYPE_CODE_UNION)
+    error (_("Attempt to extract a component of a value that is not a %s."),
+	   err);
+
+  for (i = TYPE_N_BASECLASSES (t); i < TYPE_NFIELDS (t); i++)
+    {
+      if (!field_is_static (&TYPE_FIELD (t, i))
+	  && bitpos == TYPE_FIELD_BITPOS (t, i)
+	  && types_equal (ftype, TYPE_FIELD_TYPE (t, i)))
+	return value_primitive_field (*argp, 0, i, t);
+    }
+
+  error (_("No field with matching bitpos and type."));
+
+  /* Never hit.  */
+  return NULL;
+}
+
 /* Search through the methods of an object (and its bases) to find a
    specified method.  Return the pointer to the fn_field list of
    overloaded instances.
diff --git a/gdb/value.h b/gdb/value.h
index 5924b2f..f846669 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -670,6 +670,11 @@ extern struct value *value_struct_elt (struct value **argp,
 				       const char *name, int *static_memfuncp,
 				       const char *err);
 
+extern struct value *value_struct_elt_bitpos (struct value **argp,
+					      int bitpos,
+					      struct type *field_type,
+					      const char *err);
+
 extern struct value *value_aggregate_elt (struct type *curtype,
 					  char *name,
 					  struct type *expect_type,