[Patch] PR python/15464 and python/16113

[Patch] PR python/15464 and python/16113

[Siva Chandra]

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

I was not aware of "unnamed fields" until I saw Tom's update to 15464
today. Playing with them and my previous patch for 16113, I realized
that my patch does not handle unnamed fields thoroughly. It does
however work for the example posted with the 15464 report.

[I probably would have looked up unnamed fields when working on my
earlier patch, but the documentation for gdb.Field.name only says
this: "The name of the field, or None for anonymous fields." I know
anonymous fields exist in Go, and they show up with a name behind the
scenes. Hence, I considered the documentation to be incorrect. I had
made a note to ask about this or fix this, but missed getting back to
it.]

Unnamed fields have a name equal to an empty string (""). Hence, if
there were only one unnamed field, my previous patch works as it looks
up a field by name. It fails when there are more than one unnamed
fields. The attached patch fixes this by looking up unnamed fields
using 'bitpos' instead of the field name.

2013-12-27  Siva Chandra Reddy  <sivachandra@google.com>

        PR python/15464
        PR python/16133
        * valops.c (value_struct_elt_bitpos): New function
        * python/py-value.c (valpy_getitem): Use 'bitpos' attribute to
        look for a field when 'name' is 'None' or empty.

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

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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 87701b4..5c9c003 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2013-12-27  Siva Chandra Reddy  <sivachandra@google.com>
+
+	PR python/15464
+	PR python/16133
+	* valops.c (value_struct_elt_bitpos): New function
+	* python/py-value.c (valpy_getitem): Use 'bitpos' attribute to
+	look for a field when 'name' is 'None' or empty.
+
 2013-12-23  Sterling Augustine  <saugustine@google.com>
 
 	* linespec.c (add_sal_to_sals): Use "<unknown>" when a symbol
diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index df25179..60a916d 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -573,6 +573,7 @@ valpy_getitem (PyObject *self, PyObject *key)
   value_object *self_value = (value_object *) self;
   char *field = NULL;
   PyObject *base_class_type_object = NULL;
+  long bitpos = -1;
   volatile struct gdb_exception except;
   PyObject *result = NULL;
 
@@ -614,10 +615,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);
+	      name_obj = NULL;
+	      if (field == NULL)
+		return NULL;
+	    }
+
+	  if (name_obj == Py_None || field[0] == '\0')
+	    {
+	      PyObject *bitpos_obj;
+	      int valid;
+
+	      Py_XDECREF (name_obj); /* We do not need NAME_OBJ anymore.  */
+
+	      if (!PyObject_HasAttrString (key, "bitpos"))
+		{
+		  xfree (field);
+		  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;
+	    }
 	}
     }
 
@@ -627,7 +658,9 @@ valpy_getitem (PyObject *self, PyObject *key)
       struct cleanup *cleanup = make_cleanup_value_free_to_mark (value_mark ());
       struct value *res_val = NULL;
 
-      if (field)
+      if (bitpos >= 0)
+	res_val = value_struct_elt_bitpos (&tmp, bitpos, "struct/class/union");
+      else if (field)
 	res_val = value_struct_elt (&tmp, NULL, field, 0, NULL);
       else if (base_class_type_object != NULL)
 	{
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog
index 97ad49b..4502a3c 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,10 @@
+2013-12-27  Siva Chandra Reddy  <sivachandra@google.com>
+
+	PR python/15464
+	PR python/16133
+	* gdb.python/py-value-cc.cc: Enhance test case.
+	* gdb.python/py-value-cc.exp: Add new tests.
+
 2013-12-19  Sergio Durigan Junior  <sergiodj@redhat.com>
 
 	PR breakpoints/16297
diff --git a/gdb/testsuite/gdb.python/py-value-cc.cc b/gdb/testsuite/gdb.python/py-value-cc.cc
index 80094ec..8b9aa5a 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.cc
+++ b/gdb/testsuite/gdb.python/py-value-cc.cc
@@ -30,8 +30,15 @@ class B : public A {
   char a;
 };
 
+struct X
+{
+  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 +64,13 @@ func (const A &a)
   U u;
   u.a = 99;
 
+  X x;
+  x.x = 101;
+  x.a = 102;
+
+  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 eacaf2e..08dc462 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.exp
+++ b/gdb/testsuite/gdb.python/py-value-cc.exp
@@ -53,6 +53,12 @@ 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 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 +85,11 @@ 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"
diff --git a/gdb/valops.c b/gdb/valops.c
index d43c758..f147854 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2248,6 +2248,49 @@ 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) at the
+   specified BITPOS.
+   Returns NULL if BITPOS is invalid.  */
+
+struct value *
+value_struct_elt_bitpos (struct value **argp, int bitpos, 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))
+	return value_primitive_field (*argp, 0, i, t);
+    }
+
+  error (_("Attempt to extract a component with an invalid bitpos."));
+
+  /* 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 6b158df..a279803 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -670,6 +670,10 @@ 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,
+					      const char *err);
+
 extern struct value *value_aggregate_elt (struct type *curtype,
 					  char *name,
 					  struct type *expect_type,

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

[Siva Chandra]

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

Attached is a newer version of the patch which an 'xfree' moved to a
more appropriate place.

2013-12-30  Siva Chandra Reddy  <sivachandra@google.com>

        PR python/15464
        PR python/16133
        * valops.c (value_struct_elt_bitpos): New function
        * python/py-value.c (valpy_getitem): Use 'bitpos' attribute to
        look for a field when 'name' is 'None' or empty.

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

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

diff --git a/gdb/python/py-value.c b/gdb/python/py-value.c
index df25179..69194b0 100644
--- a/gdb/python/py-value.c
+++ b/gdb/python/py-value.c
@@ -573,6 +573,7 @@ valpy_getitem (PyObject *self, PyObject *key)
   value_object *self_value = (value_object *) self;
   char *field = NULL;
   PyObject *base_class_type_object = NULL;
+  long bitpos = -1;
   volatile struct gdb_exception except;
   PyObject *result = NULL;
 
@@ -614,10 +615,41 @@ 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);
+	      name_obj = NULL;
+	      if (field == NULL)
+		return NULL;
+	    }
+
+	  if (name_obj == Py_None || field[0] == '\0')
+	    {
+	      PyObject *bitpos_obj;
+	      int valid;
+
+	      Py_XDECREF (name_obj);
+	      xfree (field);
+	      field = NULL;
+
+	      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;
+	    }
 	}
     }
 
@@ -629,6 +661,8 @@ valpy_getitem (PyObject *self, PyObject *key)
 
       if (field)
 	res_val = value_struct_elt (&tmp, NULL, field, 0, NULL);
+      else if (bitpos >= 0)
+	res_val = value_struct_elt_bitpos (&tmp, bitpos, "struct/class/union");
       else if (base_class_type_object != NULL)
 	{
 	  struct type *base_class_type, *val_type;
diff --git a/gdb/testsuite/gdb.python/py-value-cc.cc b/gdb/testsuite/gdb.python/py-value-cc.cc
index 80094ec..8b9aa5a 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.cc
+++ b/gdb/testsuite/gdb.python/py-value-cc.cc
@@ -30,8 +30,15 @@ class B : public A {
   char a;
 };
 
+struct X
+{
+  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 +64,13 @@ func (const A &a)
   U u;
   u.a = 99;
 
+  X x;
+  x.x = 101;
+  x.a = 102;
+
+  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 eacaf2e..08dc462 100644
--- a/gdb/testsuite/gdb.python/py-value-cc.exp
+++ b/gdb/testsuite/gdb.python/py-value-cc.exp
@@ -53,6 +53,12 @@ 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 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 +85,11 @@ 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"
diff --git a/gdb/valops.c b/gdb/valops.c
index d43c758..f147854 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2248,6 +2248,49 @@ 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) at the
+   specified BITPOS.
+   Returns NULL if BITPOS is invalid.  */
+
+struct value *
+value_struct_elt_bitpos (struct value **argp, int bitpos, 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))
+	return value_primitive_field (*argp, 0, i, t);
+    }
+
+  error (_("Attempt to extract a component with an invalid bitpos."));
+
+  /* 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 6b158df..a279803 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -670,6 +670,10 @@ 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,
+					      const char *err);
+
 extern struct value *value_aggregate_elt (struct type *curtype,
 					  char *name,
 					  struct type *expect_type,

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

[Tom Tromey]

>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:

Siva> Unnamed fields have a name equal to an empty string ("").

That seems like an undiagnosed bug in py-type.c:convert_field.
I guess TYPE_FIELD_NAME is ""?
It seems like it may be better to check that case there and convert it
to None.

Tom

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

[Tom Tromey]

>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:

Siva> Attached is a newer version of the patch which an 'xfree' moved to a
Siva> more appropriate place.

Thanks.

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

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

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

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

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

Tom

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

[Siva Chandra]

[-- 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,

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

[Tom Tromey]

>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:

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.

My example here was totally wrong -- the Field iterator doesn't recurse
into superclasses, so conflicts of the sort I was imagining can't occur.
This seems like a reasonable spot to extend the API someday.

Luckily you found a way to interpret my comment that actually made sense :)

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

Pessimistic date :)

Siva>         PR python/15464
Siva>         PR python/16133

I think it is PR python/16113, not 16133.

This patch is ok with those two nits fixed.

thanks,
Tom

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

[Siva Chandra]

On Mon, Jan 13, 2014 at 1:12 PM, Tom Tromey <tromey@redhat.com> wrote:
> Siva> 2014-12-07  Siva Chandra Reddy  <sivachandra@google.com>
>
> Pessimistic date :)
>
> Siva>         PR python/15464
> Siva>         PR python/16133
>
> I think it is PR python/16113, not 16133.
>
> This patch is ok with those two nits fixed.

Thanks Tom.
Pushed after addressing the nits: b5b08fb4ffa53ec088f8ad865bee0fd6edb2906f

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

[Siva Chandra]

On Mon, Jan 13, 2014 at 1:12 PM, Tom Tromey <tromey@redhat.com> wrote:
> This patch is ok with those two nits fixed.

Forgot to ask, should this go into 7.7 branch as well? The first patch
is in 7.7.

Thanks,
Siva Chandra

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

[Tom Tromey]

>>>>> "Siva" == Siva Chandra <sivachandra@google.com> writes:

Siva> On Mon, Jan 13, 2014 at 1:12 PM, Tom Tromey <tromey@redhat.com> wrote:
>> This patch is ok with those two nits fixed.

Siva> Forgot to ask, should this go into 7.7 branch as well? The first patch
Siva> is in 7.7.

It seems reasonable to me, given that the feature is a bit incomplete
without this patch, but I'd rather defer to Joel for 7.7 decisions.

Tom

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

[Joel Brobecker]

> Siva> Forgot to ask, should this go into 7.7 branch as well? The first patch
> Siva> is in 7.7.
> 
> It seems reasonable to me, given that the feature is a bit incomplete
> without this patch, but I'd rather defer to Joel for 7.7 decisions.

I have no objection in this case, so I'll defer to Tom in return :).
The patch is fairly large, in areas where I don't necessarily understand
all the repercutions. Hence it is good that Tom's OK with it - so
go head, and push to 7.7.

By the way, I happened to notice a couple of style violations which
I just fixed:

+    {
+      const char *field_name = TYPE_FIELD_NAME (type, field);
+      if (field_name[0] != '\0')
+       {

Empty line after local declaration.

+  if (ftype == NULL)
+    {
+      PyErr_SetString (PyExc_TypeError,
+                      _("'type' attribute of gdb.Field object is not a "
+                        "gdb.Type object."));
+    }

No need for the curly braces.

They are obviously not important for the gdb-7.7 branch, unless you
think there is a chance we might touch this code again in the near
future, and would want to backport to 7.7. In that case, cherry-picking
those two commits on the gdb-7.7 branch would help avoiding patching
conflicts...

-- 
Joel

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

[Siva Chandra]

On Wed, Jan 15, 2014 at 4:46 AM, Joel Brobecker <brobecker@adacore.com> wrote:
> I have no objection in this case, so I'll defer to Tom in return :).
> The patch is fairly large, in areas where I don't necessarily understand
> all the repercutions. Hence it is good that Tom's OK with it - so
> go head, and push to 7.7.
>
> By the way, I happened to notice a couple of style violations which
> I just fixed:
>
> +    {
> +      const char *field_name = TYPE_FIELD_NAME (type, field);
> +      if (field_name[0] != '\0')
> +       {
>
> Empty line after local declaration.
>
> +  if (ftype == NULL)
> +    {
> +      PyErr_SetString (PyExc_TypeError,
> +                      _("'type' attribute of gdb.Field object is not a "
> +                        "gdb.Type object."));
> +    }
>
> No need for the curly braces.

Thanks for fixing these.

> They are obviously not important for the gdb-7.7 branch, unless you
> think there is a chance we might touch this code again in the near
> future, and would want to backport to 7.7. In that case, cherry-picking
> those two commits on the gdb-7.7 branch would help avoiding patching
> conflicts...

I have cherry-picked all three commits into gdb-7.7-branch.  I built
and tested of course, but I hope I did not screw up something else.

Thank you,
Siva Chandra