This is the mail archive of the archer@sourceware.org mailing list for the Archer project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[python] don't try to free "independent" types


Last night I thought of another scenario whereby we could end up
referring to a free'd "struct type".  This script shows the problem:

    break 44
    run
    print &s
    python s = gdb.get_value_from_history(0)
    python t = gdb.Type ('PTR')
    kill
    file
    python s = s.cast(t.pointer())
    python t = None
    python print s.type()

When we evaluate "t = None", we delete the underlying struct type.
However, "s" still refers to it by virtue of the cast operation.

After considering this for a while I arrived at the conclusion that,
short of a value-and-type garbage collector, there is not much that
can be done.

I think this would be pretty controversial.  So, instead of doing this
on the branch, I am checking in this patch, which removes freeing of
objfile-less types.  This is a memory leak, but I think such types are
likely to be relatively rare, and this is preferable to a crash.  I
will raise this issue when I submit the Type work upstream.

I added a new test case for this particular scenario.  More losing
cases exist though; basically any operation on the Value resulting
from Value.cast can cause problems, including things like extracting
the underlying struct value for MI pretty-printing.

Tom

2008-11-25  Tom Tromey  <tromey@redhat.com>

	* python/python-value.c (valpy_type): Update.
	* python/python-internal.h (type_to_type_object): Update.
	* python/python-type.c (pyty_type_object) <owned, originator>:
	Remove.
	(clean_up_objfile_types): Update.
	(typy_dealloc): Update.
	(set_type): Update.  Remove "parent" argument.
	(type_to_type_object): Likewise.
	(convert_field): Update.  Remove "self" argument.
	(typy_fields): Update.
	(typy_pointer): Update.
	(typy_reference): Update.
	(typy_target): Update.
	(typy_template_argument): Update.
	(typy_new): Update.

2008-11-25  Tom Tromey  <tromey@redhat.com>

	* gdb.python/python-value.c (PTR): New typedef.
	* gdb.python/python-value.exp (test_value_after_death): New proc.
	Call it.

diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 4603b18..83fc1e2 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -84,7 +84,7 @@ PyObject *symbol_to_symbol_object (struct symbol *sym);
 PyObject *block_to_block_object (struct block *block);
 PyObject *value_to_value_object (struct value *v);
 PyObject *gdb_owned_value_to_value_object (struct value *v);
-PyObject *type_to_type_object (PyObject *, struct type *);
+PyObject *type_to_type_object (struct type *);
 PyObject *objfile_to_objfile_object (struct objfile *);
 
 PyObject *objfpy_get_printers (PyObject *, void *);
diff --git a/gdb/python/python-type.c b/gdb/python/python-type.c
index cf24a75..0df5252 100644
--- a/gdb/python/python-type.c
+++ b/gdb/python/python-type.c
@@ -37,15 +37,6 @@ typedef struct pyty_type_object
      underlying struct type when the objfile is deleted.  */
   struct pyty_type_object *prev;
   struct pyty_type_object *next;
-
-  /* This is nonzero if the type is owned by this object and should be
-     freed when the object is deleted.  */
-  int owned;
-
-  /* A Type derived from an 'owned' type will refer to its originator.
-     This prevents us from freeing the underlying 'type' too
-     early.  In other cases, this is NULL.  */
-  PyObject *originator;
 } type_object;
 
 static PyTypeObject type_object_type;
@@ -143,7 +134,7 @@ typy_code (PyObject *self, PyObject *args)
 /* Helper function for typy_fields which converts a single field to a
    dictionary.  Returns NULL on error.  */
 static PyObject *
-convert_field (PyObject *self, struct type *type, int field)
+convert_field (struct type *type, int field)
 {
   PyObject *result = field_new ();
   PyObject *arg;
@@ -184,7 +175,7 @@ convert_field (PyObject *self, struct type *type, int field)
   if (PyObject_SetAttrString (result, "bitsize", arg) < 0)
     goto failarg;
 
-  arg = type_to_type_object (self, TYPE_FIELD_TYPE (type, field));
+  arg = type_to_type_object (TYPE_FIELD_TYPE (type, field));
   if (!arg)
     goto fail;
   if (PyObject_SetAttrString (result, "type", arg) < 0)
@@ -215,7 +206,7 @@ typy_fields (PyObject *self, PyObject *args)
 
   for (i = 0; i < TYPE_NFIELDS (type); ++i)
     {
-      PyObject *dict = convert_field (self, type, i);
+      PyObject *dict = convert_field (type, i);
       if (!dict)
 	{
 	  Py_DECREF (result);
@@ -255,7 +246,7 @@ typy_pointer (PyObject *self, PyObject *args)
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-  return type_to_type_object (self, type);
+  return type_to_type_object (type);
 }
 
 /* Return a Type object which represents a reference to SELF.  */
@@ -271,7 +262,7 @@ typy_reference (PyObject *self, PyObject *args)
     }
   GDB_PY_HANDLE_EXCEPTION (except);
 
-  return type_to_type_object (self, type);
+  return type_to_type_object (type);
 }
 
 /* Return a Type object which represents the target type of SELF.  */
@@ -286,7 +277,7 @@ typy_target (PyObject *self, PyObject *args)
       return NULL;
     }
 
-  return type_to_type_object (self, TYPE_TARGET_TYPE (type));
+  return type_to_type_object (TYPE_TARGET_TYPE (type));
 }
 
 /* Return the size of the type represented by SELF, in bytes.  */
@@ -444,7 +435,7 @@ typy_template_argument (PyObject *self, PyObject *args)
   if (! argtype)
     return NULL;
 
-  return type_to_type_object (self, argtype);
+  return type_to_type_object (argtype);
 }
 
 static PyObject *
@@ -504,11 +495,13 @@ clean_up_objfile_types (struct objfile *objfile, void *datum)
       type_object *next = obj->next;
 
       htab_empty (copied_types);
+      /* Note that we leak this memory.  There is no good way to
+	 handle freeing this, given things like Value.cast and other
+	 unconstrained operations on values.  */
       obj->type = copy_type_recursive (objfile, obj->type, copied_types);
 
       obj->next = NULL;
       obj->prev = NULL;
-      obj->owned = 1;
 
       obj = next;
     }
@@ -519,10 +512,9 @@ clean_up_objfile_types (struct objfile *objfile, void *datum)
 }
 
 static void
-set_type (type_object *obj, struct type *type, type_object *parent)
+set_type (type_object *obj, struct type *type)
 {
   obj->type = type;
-  obj->owned = 0;
   obj->prev = NULL;
   if (type && TYPE_OBJFILE (type))
     {
@@ -535,21 +527,6 @@ set_type (type_object *obj, struct type *type, type_object *parent)
     }
   else
     obj->next = NULL;
-
-  obj->originator = NULL;
-  if (type && parent)
-    {
-      if (parent->owned)
-	{
-	  Py_INCREF (parent);
-	  obj->originator = (PyObject *) parent;
-	}
-      else if (parent->originator)
-	{
-	  Py_INCREF (parent->originator);
-	  obj->originator = parent->originator;
-	}
-    }
 }
 
 static PyObject *
@@ -588,7 +565,7 @@ typy_new (PyTypeObject *subtype, PyObject *args, PyObject *kwargs)
   if (! result)
     return NULL;
 
-  set_type (result, type, NULL);
+  set_type (result, type);
 
   return (PyObject *) result;
 }
@@ -598,52 +575,30 @@ typy_dealloc (PyObject *obj)
 {
   type_object *type = (type_object *) obj;
 
-  if (type->type)
+  if (type->prev)
+    type->prev->next = type->next;
+  else if (type->type && TYPE_OBJFILE (type->type))
     {
-      if (type->owned)
-	{
-	  /* We own the type, so delete it.  */
-	  htab_t deleted_types;
-
-	  deleted_types = create_deleted_types_hash ();
-	  delete_type_recursive (type->type, deleted_types);
-	  htab_delete (deleted_types);
-	}
-      else if (type->originator)
-	{
-	  Py_DECREF (type->originator);
-	}
-      else
-	{
-	  if (type->prev)
-	    type->prev->next = type->next;
-	  else
-	    {
-	      /* Must reset head of list.  */
-	      struct objfile *objfile = TYPE_OBJFILE (type->type);
-	      if (objfile)
-		set_objfile_data (objfile, typy_objfile_data_key, type->next);
-	    }
-	  if (type->next)
-	    type->next->prev = type->prev;
-	}
+      /* Must reset head of list.  */
+      struct objfile *objfile = TYPE_OBJFILE (type->type);
+      if (objfile)
+	set_objfile_data (objfile, typy_objfile_data_key, type->next);
     }
+  if (type->next)
+    type->next->prev = type->prev;
 
   type->ob_type->tp_free (type);
 }
 
-/* Create a new Type referring to TYPE.  PARENT is the Type from which
-   TYPE is derived; this is needed to handle reference counting for
-   derived 'owned' types.  PARENT can be NULL if you know that this is
-   not a problem; otherwise it must be a Type object.  */
+/* Create a new Type referring to TYPE.  */
 PyObject *
-type_to_type_object (PyObject *parent, struct type *type)
+type_to_type_object (struct type *type)
 {
   type_object *type_obj;
 
   type_obj = PyObject_New (type_object, &type_object_type);
   if (type_obj)
-    set_type (type_obj, type, (type_object *) parent);
+    set_type (type_obj, type);
 
   return (PyObject *) type_obj;
 }
diff --git a/gdb/python/python-value.c b/gdb/python/python-value.c
index 807f914..2a6af14 100644
--- a/gdb/python/python-value.c
+++ b/gdb/python/python-value.c
@@ -151,7 +151,7 @@ static PyObject *
 valpy_type (PyObject *self, PyObject *args)
 {
   struct value *value = ((value_object *) self)->value;
-  return type_to_type_object (NULL, value_type (value));
+  return type_to_type_object (value_type (value));
 }
 
 /* Return Unicode string with value contents (assumed to be encoded in the
diff --git a/gdb/testsuite/gdb.python/python-value.c b/gdb/testsuite/gdb.python/python-value.c
index 0180ed5..95d9e03 100644
--- a/gdb/testsuite/gdb.python/python-value.c
+++ b/gdb/testsuite/gdb.python/python-value.c
@@ -27,12 +27,15 @@ union u
   float b;
 };
 
+typedef struct s *PTR;
+
 int
 main (int argc, char *argv[])
 {
   char *cp = argv[0]; /* Prevent gcc from optimizing argv[] out.  */
   struct s s;
   union u u;
+  PTR x = &s;
 
   s.a = 3;
   s.b = 5;
diff --git a/gdb/testsuite/gdb.python/python-value.exp b/gdb/testsuite/gdb.python/python-value.exp
index 99b576a..a34af9b 100644
--- a/gdb/testsuite/gdb.python/python-value.exp
+++ b/gdb/testsuite/gdb.python/python-value.exp
@@ -222,6 +222,33 @@ proc test_value_in_inferior {} {
   gdb_test "python print arg0" "0x.*$testfile\"" "verify dereferenced value"
 }
 
+proc test_value_after_death {} {
+  # Construct a type while the inferior is still running.
+  gdb_py_test_silent_cmd "python ptrtype = gdb.Type('PTR')" \
+    "create PTR type" 1
+
+  # Kill the inferior and remove the symbols.
+  gdb_test "kill" "" "kill the inferior" \
+    "Kill the program being debugged. .y or n. $" \
+    "y"
+  gdb_test "file" "" "Discard the symbols" \
+    "Discard symbol table from.*y or n. $" \
+    "y"
+
+  # Now create a value using that type.  Relies on arg0, created by
+  # test_value_in_inferior.
+  gdb_py_test_silent_cmd "python castval = arg0.cast(ptrtype.pointer())" \
+    "cast arg0 to PTR" 1
+
+  # Make sure the type is deleted.
+  gdb_py_test_silent_cmd "python ptrtype = None" \
+    "delete PTR type" 1
+
+  # Now see if the value's type is still valid.
+  gdb_test "python print castval.type()" "struct s .." \
+    "print value's type"
+}
+
 
 # Start with a fresh gdb.
 
@@ -251,3 +278,4 @@ if ![runto_main] then {
 }
 
 test_value_in_inferior
+test_value_after_death


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]