[RFC] [python] Fix field list of typedef regression

[RFC] [python] Fix field list of typedef regression

[Doug Evans]

Hi.
This patch fixes a regression in the current tree.
The field list of a typedef went from the underlying field list
to an empty list.

Question: What's the right way to handle ref counting here?

2011-10-29  Doug Evans  <dje@google.com>

	* python/py-type.c (typy_fields_items): Call check_typedef.

	testsuite/
	* gdb.python/py-type.c (TS): New typedef.
	(ts): New global.
	* gdb.python/py-type.exp: Test field list of typedef.

Index: python/py-type.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-type.c,v
retrieving revision 1.27
diff -u -p -r1.27 py-type.c
--- python/py-type.c	27 Oct 2011 09:14:27 -0000	1.27
+++ python/py-type.c	29 Oct 2011 08:04:06 -0000
@@ -295,11 +295,24 @@ static PyObject *
 typy_fields_items (PyObject *self, enum gdbpy_iter_kind kind)
 {
   PyObject *result = NULL, *iter = NULL;
-  
+  volatile struct gdb_exception except;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      struct type *type = ((type_object *) self)->type;
+      struct type *checked_type = type;
+
+      CHECK_TYPEDEF (checked_type);
+      if (checked_type != type)
+        self = type_to_type_object (checked_type);
+      /* FIXME: reference counting of self? */
+    }
+  GDB_PY_HANDLE_EXCEPTION (except);
+
   iter = typy_make_iter (self, kind);
   if (iter == NULL)
     return NULL;
-    
+
   result = PySequence_List (iter);
   Py_DECREF (iter);
   return result;
Index: testsuite/gdb.python/py-type.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-type.c,v
retrieving revision 1.5
diff -u -p -r1.5 py-type.c
--- testsuite/gdb.python/py-type.c	28 Sep 2011 20:06:01 -0000	1.5
+++ testsuite/gdb.python/py-type.c	29 Oct 2011 08:04:06 -0000
@@ -21,6 +21,9 @@ struct s
   int b;
 };
 
+typedef struct s TS;
+TS ts;
+
 #ifdef __cplusplus
 struct C
 {
Index: testsuite/gdb.python/py-type.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-type.exp,v
retrieving revision 1.14
diff -u -p -r1.14 py-type.exp
--- testsuite/gdb.python/py-type.exp	28 Sep 2011 20:06:01 -0000	1.14
+++ testsuite/gdb.python/py-type.exp	29 Oct 2011 08:04:06 -0000
@@ -64,6 +64,10 @@ proc runto_bp {bp} {
 proc test_fields {lang} {
   global gdb_prompt
 
+  # .fields() of a typedef should still return the underlying field list
+  gdb_test "python print len(gdb.parse_and_eval('ts').type.fields())" "2" \
+      "$lang typedef field list"
+
   if {$lang == "c++"} {
       # Test usage with a class
       gdb_py_test_silent_cmd "print c" "print value" 1

Re: [RFC] [python] Fix field list of typedef regression

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> Question: What's the right way to handle ref counting here?

Doug> +      CHECK_TYPEDEF (checked_type);
Doug> +      if (checked_type != type)
Doug> +        self = type_to_type_object (checked_type);
Doug> +      /* FIXME: reference counting of self? */

type_to_type_object returns a new reference.
So the caller is responsible for decref'ing it.

I think assigning to self is unexpected.

Doug> +    }
Doug> +  GDB_PY_HANDLE_EXCEPTION (except);

I'd pull the type_to_type_object call out of the TRY_CATCH.
Then you don't have to handle a decref on the exception return.

Tom

[RFA] [python] Fix field list of typedef regression

[Doug Evans]

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

On Sat, Oct 29, 2011 at 12:12 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>>>> "Doug" == Doug Evans <dje@google.com> writes:
>
> Doug> Question: What's the right way to handle ref counting here?
>
> Doug> +      CHECK_TYPEDEF (checked_type);
> Doug> +      if (checked_type != type)
> Doug> +        self = type_to_type_object (checked_type);
> Doug> +      /* FIXME: reference counting of self? */
>
> type_to_type_object returns a new reference.
> So the caller is responsible for decref'ing it.
>
> I think assigning to self is unexpected.
>
> Doug> +    }
> Doug> +  GDB_PY_HANDLE_EXCEPTION (except);
>
> I'd pull the type_to_type_object call out of the TRY_CATCH.
> Then you don't have to handle a decref on the exception return.
>
> Tom
>

Thanks.
I think this gets the ref-counting correct.
I singlestepped through it and it looked ok anyways.

Ok to check in?

2011-11-10  Doug Evans  <dje@google.com>

        * python/py-type.c (typy_fields_items): Call check_typedef.

        testsuite/
        * gdb.python/py-type.c (TS): New typedef.
        (ts): New global.
        * gdb.python/py-type.exp: Test field list of typedef.

[-- Attachment #2: gdb-111110-py-type-typedef-2.patch.txt --]
[-- Type: text/plain, Size: 2770 bytes --]

2011-11-10  Doug Evans  <dje@google.com>

	* python/py-type.c (typy_fields_items): Call check_typedef.

	testsuite/
	* gdb.python/py-type.c (TS): New typedef.
	(ts): New global.
	* gdb.python/py-type.exp: Test field list of typedef.

Index: python/py-type.c
===================================================================
RCS file: /cvs/src/src/gdb/python/py-type.c,v
retrieving revision 1.28
diff -u -p -r1.28 py-type.c
--- python/py-type.c	4 Nov 2011 11:57:04 -0000	1.28
+++ python/py-type.c	10 Nov 2011 18:43:42 -0000
@@ -294,14 +294,33 @@ make_fielditem (struct type *type, int i
 static PyObject *
 typy_fields_items (PyObject *self, enum gdbpy_iter_kind kind)
 {
+  PyObject *py_type = self;
   PyObject *result = NULL, *iter = NULL;
-  
-  iter = typy_make_iter (self, kind);
-  if (iter == NULL)
-    return NULL;
-    
-  result = PySequence_List (iter);
-  Py_DECREF (iter);
+  volatile struct gdb_exception except;
+  struct type *type = ((type_object *) py_type)->type;
+  struct type *checked_type = type;
+
+  TRY_CATCH (except, RETURN_MASK_ALL)
+    {
+      CHECK_TYPEDEF (checked_type);
+    }
+  GDB_PY_HANDLE_EXCEPTION (except);
+
+  if (checked_type != type)
+    py_type = type_to_type_object (checked_type);
+  iter = typy_make_iter (py_type, kind);
+  if (checked_type != type)
+    {
+      /* Need to wrap this in braces because Py_DECREF isn't wrapped
+	 in a do{}while(0).  */
+      Py_DECREF (py_type);
+    }
+  if (iter != NULL)
+    {
+      result = PySequence_List (iter);
+      Py_DECREF (iter);
+    }
+
   return result;
 }
 
Index: testsuite/gdb.python/py-type.c
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-type.c,v
retrieving revision 1.5
diff -u -p -r1.5 py-type.c
--- testsuite/gdb.python/py-type.c	28 Sep 2011 20:06:01 -0000	1.5
+++ testsuite/gdb.python/py-type.c	10 Nov 2011 18:48:52 -0000
@@ -21,6 +21,9 @@ struct s
   int b;
 };
 
+typedef struct s TS;
+TS ts;
+
 #ifdef __cplusplus
 struct C
 {
Index: testsuite/gdb.python/py-type.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.python/py-type.exp,v
retrieving revision 1.14
diff -u -p -r1.14 py-type.exp
--- testsuite/gdb.python/py-type.exp	28 Sep 2011 20:06:01 -0000	1.14
+++ testsuite/gdb.python/py-type.exp	10 Nov 2011 18:48:52 -0000
@@ -64,6 +64,10 @@ proc runto_bp {bp} {
 proc test_fields {lang} {
   global gdb_prompt
 
+  # .fields() of a typedef should still return the underlying field list
+  gdb_test "python print len(gdb.parse_and_eval('ts').type.fields())" "2" \
+      "$lang typedef field list"
+
   if {$lang == "c++"} {
       # Test usage with a class
       gdb_py_test_silent_cmd "print c" "print value" 1

Re: [RFA] [python] Fix field list of typedef regression

[Tom Tromey]

>>>>> "Doug" == Doug Evans <dje@google.com> writes:

Doug> I think this gets the ref-counting correct.
Doug> I singlestepped through it and it looked ok anyways.

Doug> Ok to check in?

Looks good to me.

Tom