FYI: fix python-value reference counting

Tom Tromey tromey@redhat.com
Thu Aug 13 21:41:00 GMT 2009


I'm checking this in.

This updates the Python Value code to properly use the new value
reference counting.

This patch is an improvement over the current situation, but not ideal.
We still have to use value_copy in a couple of places -- which is a
shame, because value_incref is much cheaper.  This change is somewhat
more involved; I put it on my to-do list for later.

Built and regtested on x86-64 (compile farm).

Tom

2009-08-13  Tom Tromey  <tromey@redhat.com>

	* varobj.c (update_dynamic_varobj_children): Don't use
	value_copy.
	* value.h: (preserve_one_value): Declare.
	(value_prepend_to_list, value_remove_from_list): Remove.
	* value.c (preserve_one_value): No longer static.
	(preserve_values): Call preserve_python_values.
	(value_prepend_to_list): Remove.
	(value_remove_from_list): Remove.
	* python/python.h (values_in_python): Don't declare.
	(preserve_python_values): Declare.
	* python/python-value.c (values_in_python): Change type.  Move
	lower.  Now static.
	(struct value_object): Add struct tag.
	<next, prev>: New fields.
	(valpy_dealloc): Update.
	(note_value): New function.
	(valpy_new): Use value_incref, note_value.
	(preserve_python_values): New function.
	(valpy_positive): Don't use value_copy.
	(value_to_value_object): Use value_incref, note_value.
	(convert_value_from_python): Update comment.

Index: value.c
===================================================================
RCS file: /cvs/src/src/gdb/value.c,v
retrieving revision 1.91
diff -u -r1.91 value.c
--- value.c	21 Jul 2009 18:15:32 -0000	1.91
+++ value.c	13 Aug 2009 18:33:21 -0000
@@ -312,31 +312,6 @@
   return allocate_value (array_type);
 }
 
-/* Needed if another module needs to maintain its on list of values.  */
-void
-value_prepend_to_list (struct value **head, struct value *val)
-{
-  val->next = *head;
-  *head = val;
-}
-
-/* Needed if another module needs to maintain its on list of values.  */
-void
-value_remove_from_list (struct value **head, struct value *val)
-{
-  struct value *prev;
-
-  if (*head == val)
-    *head = (*head)->next;
-  else
-    for (prev = *head; prev->next; prev = prev->next)
-      if (prev->next == val)
-      {
-	prev->next = val->next;
-	break;
-      }
-}
-
 struct value *
 allocate_computed_value (struct type *type,
                          struct lval_funcs *funcs,
@@ -1430,7 +1405,7 @@
 /* Update VALUE before discarding OBJFILE.  COPIED_TYPES is used to
    prevent cycles / duplicates.  */
 
-static void
+void
 preserve_one_value (struct value *value, struct objfile *objfile,
 		    htab_t copied_types)
 {
@@ -1490,8 +1465,7 @@
   for (var = internalvars; var; var = var->next)
     preserve_one_internalvar (var, objfile, copied_types);
 
-  for (val = values_in_python; val; val = val->next)
-    preserve_one_value (val, objfile, copied_types);
+  preserve_python_values (objfile, copied_types);
 
   htab_delete (copied_types);
 }
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.147
diff -u -r1.147 value.h
--- value.h	21 Jul 2009 18:15:32 -0000	1.147
+++ value.h	13 Aug 2009 18:33:21 -0000
@@ -41,11 +41,6 @@
 
 struct value;
 
-/* Needed if another module needs to maintain its own list of values.  */
-
-void value_prepend_to_list (struct value **head, struct value *val);
-void value_remove_from_list (struct value **head, struct value *val);
-
 /* Values are stored in a chain, so that they can be deleted easily
    over calls to the inferior.  Values assigned to internal variables,
    put into the value history or exposed to Python are taken off this
@@ -664,6 +659,8 @@
 
 extern struct value *value_copy (struct value *);
 
+extern void preserve_one_value (struct value *, struct objfile *, htab_t);
+
 /* From valops.c */
 
 extern struct value *varying_to_slice (struct value *);
Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.144
diff -u -r1.144 varobj.c
--- varobj.c	30 Jul 2009 13:12:54 -0000	1.144
+++ varobj.c	13 Aug 2009 18:33:21 -0000
@@ -899,16 +899,7 @@
       if (!PyArg_ParseTuple (item, "sO", &name, &py_v))
 	error (_("Invalid item from the child list"));
       
-      if (PyObject_TypeCheck (py_v, &value_object_type))
-	{
-	  /* If we just call convert_value_from_python for this type,
-	     we won't know who owns the result.  For this one case we
-	     need to copy the resulting value.  */
-	  v = value_object_to_value (py_v);
-	  v = value_copy (v);
-	}
-      else
-	v = convert_value_from_python (py_v);
+      v = convert_value_from_python (py_v);
 
       /* TODO: This assume the name of the i-th child never changes.  */
 
Index: python/python-value.c
===================================================================
RCS file: /cvs/src/src/gdb/python/python-value.c,v
retrieving revision 1.24
diff -u -r1.24 python-value.c
--- python/python-value.c	10 Jul 2009 10:35:17 -0000	1.24
+++ python/python-value.c	13 Aug 2009 18:33:21 -0000
@@ -26,15 +26,6 @@
 #include "dfp.h"
 #include "valprint.h"
 
-/* List of all values which are currently exposed to Python. It is
-   maintained so that when an objfile is discarded, preserve_values
-   can copy the values' types if needed.  This is declared
-   unconditionally to reduce the number of uses of HAVE_PYTHON in the
-   generic code.  */
-/* This variable is unnecessarily initialized to NULL in order to 
-   work around a linker bug on MacOS.  */
-struct value *values_in_python = NULL;
-
 #ifdef HAVE_PYTHON
 
 #include "python-internal.h"
@@ -58,20 +49,38 @@
 #define builtin_type_pychar \
   language_string_char_type (python_language, python_gdbarch)
 
-typedef struct {
+typedef struct value_object {
   PyObject_HEAD
+  struct value_object *next;
+  struct value_object *prev;
   struct value *value;
   PyObject *address;
   PyObject *type;
 } value_object;
 
+/* List of all values which are currently exposed to Python. It is
+   maintained so that when an objfile is discarded, preserve_values
+   can copy the values' types if needed.  */
+/* This variable is unnecessarily initialized to NULL in order to
+   work around a linker bug on MacOS.  */
+static value_object *values_in_python = NULL;
+
 /* Called by the Python interpreter when deallocating a value object.  */
 static void
 valpy_dealloc (PyObject *obj)
 {
   value_object *self = (value_object *) obj;
 
-  value_remove_from_list (&values_in_python, self->value);
+  /* Remove SELF from the global list.  */
+  if (self->prev)
+    self->prev->next = self->next;
+  else
+    {
+      gdb_assert (values_in_python == self);
+      values_in_python = self->next;
+    }
+  if (self->next)
+    self->next->prev = self->prev;
 
   value_free (self->value);
 
@@ -89,6 +98,17 @@
   self->ob_type->tp_free (self);
 }
 
+/* Helper to push a Value object on the global list.  */
+static void
+note_value (value_object *value_obj)
+{
+  value_obj->next = values_in_python;
+  if (value_obj->next)
+    value_obj->next->prev = value_obj;
+  value_obj->prev = NULL;
+  values_in_python = value_obj;
+}
+
 /* Called when a new gdb.Value object needs to be allocated.  */
 static PyObject *
 valpy_new (PyTypeObject *subtype, PyObject *args, PyObject *keywords)
@@ -119,14 +139,25 @@
     }
 
   value_obj->value = value;
+  value_incref (value);
   value_obj->address = NULL;
   value_obj->type = NULL;
-  release_value (value);
-  value_prepend_to_list (&values_in_python, value);
+  note_value (value_obj);
 
   return (PyObject *) value_obj;
 }
 
+/* Iterate over all the Value objects, calling preserve_one_value on
+   each.  */
+void
+preserve_python_values (struct objfile *objfile, htab_t copied_types)
+{
+  value_object *iter;
+
+  for (iter = values_in_python; iter; iter = iter->next)
+    preserve_one_value (iter->value, objfile, copied_types);
+}
+
 /* Given a value of a pointer type, apply the C unary * operator to it.  */
 static PyObject *
 valpy_dereference (PyObject *self, PyObject *args)
@@ -543,9 +574,7 @@
 static PyObject *
 valpy_positive (PyObject *self)
 {
-  struct value *copy = value_copy (((value_object *) self)->value);
-
-  return value_to_value_object (copy);
+  return value_to_value_object (((value_object *) self)->value);
 }
 
 static PyObject *
@@ -800,16 +829,17 @@
   if (val_obj != NULL)
     {
       val_obj->value = val;
+      value_incref (val);
       val_obj->address = NULL;
       val_obj->type = NULL;
-      release_value (val);
-      value_prepend_to_list (&values_in_python, val);
+      note_value (val_obj);
     }
 
   return (PyObject *) val_obj;
 }
 
-/* Returns value structure corresponding to the given value object.  */
+/* Returns a borrowed reference to the struct value corresponding to
+   the given value object.  */
 struct value *
 value_object_to_value (PyObject *self)
 {
@@ -821,7 +851,8 @@
 }
 
 /* Try to convert a Python value to a gdb value.  If the value cannot
-   be converted, set a Python exception and return NULL.  */
+   be converted, set a Python exception and return NULL.  Returns a
+   reference to a new value on the all_values chain.  */
 
 struct value *
 convert_value_from_python (PyObject *obj)
@@ -1019,4 +1050,12 @@
   valpy_new			  /* tp_new */
 };
 
+#else
+
+void
+preserve_python_values (struct objfile *objfile, htab_t copied_types)
+{
+  /* Nothing.  */
+}
+
 #endif /* HAVE_PYTHON */
Index: python/python.h
===================================================================
RCS file: /cvs/src/src/gdb/python/python.h,v
retrieving revision 1.4
diff -u -r1.4 python.h
--- python/python.h	28 May 2009 01:05:14 -0000	1.4
+++ python/python.h	13 Aug 2009 18:33:21 -0000
@@ -22,8 +22,6 @@
 
 #include "value.h"
 
-extern struct value *values_in_python;
-
 void eval_python_from_control_command (struct command_line *);
 
 int apply_val_pretty_printer (struct type *type, const gdb_byte *valaddr,
@@ -32,4 +30,6 @@
 			      const struct value_print_options *options,
 			      const struct language_defn *language);
 
+void preserve_python_values (struct objfile *objfile, htab_t copied_types);
+
 #endif /* GDB_PYTHON_H */



More information about the Gdb-patches mailing list