This is the mail archive of the
archer@sourceware.org
mailing list for the Archer project.
[python] don't try to free "independent" types
- From: Tom Tromey <tromey at redhat dot com>
- To: Project Archer <archer at sourceware dot org>
- Date: Tue, 25 Nov 2008 14:16:35 -0700
- Subject: [python] don't try to free "independent" types
- Reply-to: Tom Tromey <tromey at redhat dot com>
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