summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Malcolm <dmalcolm@redhat.com>2012-01-18 17:04:59 (GMT)
committerDavid Malcolm <dmalcolm@redhat.com>2012-01-18 17:04:59 (GMT)
commiteef8891a6efd628e9d5b9f47b7335be31b25c6e1 (patch)
tree9afcad0a3add82a4e516c3323a38f2f9a825b8e1
parent5e1157c35c3ef57cba1615ffe706583d80f4a04d (diff)
downloadgcc-python-plugin-eef8891a6efd628e9d5b9f47b7335be31b25c6e1.zip
gcc-python-plugin-eef8891a6efd628e9d5b9f47b7335be31b25c6e1.tar.gz
gcc-python-plugin-eef8891a6efd628e9d5b9f47b7335be31b25c6e1.tar.xz
add metaclass to PyGccWrapper to fix garbage-collection segfault on user-defined classes
Previously, all PyGccWrapper instances assumed that their ob_type was a PyTypeObject that was also a PyGccWrapperTypeObject, a PyTypeObject extended to hold a wrtp_mark callback. This worked for predefined types, where the type object was statically allocated, but led to crashes with user-defined types, where the new type is a PyHeapTypeObject: I believe that upon casting to *(PyGccWrapperTypeObject*)typeobj)->wrtp_mark we were actually accessing the as_number field: /* The *real* layout of a type object when allocated on the heap */ typedef struct _heaptypeobject { PyTypeObject ht_type; PyNumberMethods as_number; <--- this field i.e. the: [...snip...] } PyHeapTypeObject; thus receiving the first field of PyNumberMethods: binaryfunc nb_add; (casting it with the wrong signature) which happened to be NULL. With this change, we increase PyGccWrapperTypeObject to be a PyHeapTypeObject (even for those that aren't on the heap) so that user-defined types have enough room. We introduce a metaclass: PyGccWrapperMetaType, the ob_type of our various PyGccWrapperTypeObject instances. This has a tp_basicsize that ensures that subclasses have room for the extra field in PyGccWrapperTypeObject, and a tp_new implementation that propagates the wrtp_mark callback when creating new subtypes. Fixes the issue reported as: https://fedorahosted.org/pipermail/gcc-python-plugin/2012-January/000152.html and adds a reproducer for it as: tests/plugin/gc/segfault-on-instance-of-pass-subclass/
-rw-r--r--cpybuilder.py6
-rw-r--r--gcc-python-wrapper.c110
-rw-r--r--gcc-python.h3
-rw-r--r--tests/plugin/callgraph/stdout.txt4
-rw-r--r--tests/plugin/gc/segfault-on-instance-of-pass-subclass/input.c1
-rw-r--r--tests/plugin/gc/segfault-on-instance-of-pass-subclass/script.py10
-rw-r--r--tests/plugin/options/stdout.txt2
-rw-r--r--tests/plugin/parameters/stdout.txt2
-rw-r--r--tests/plugin/rtl/stdout.txt2
-rw-r--r--wrapperbuilder.py9
10 files changed, 140 insertions, 9 deletions
diff --git a/cpybuilder.py b/cpybuilder.py
index ec22706..41f613d 100644
--- a/cpybuilder.py
+++ b/cpybuilder.py
@@ -246,7 +246,11 @@ class PyTypeObject(NamedEntity):
return result
def c_initializer(self):
- result = ' PyVarObject_HEAD_INIT(0, 0)\n'
+ if hasattr(self, 'ob_type'):
+ ob_type_str = getattr(self, 'ob_type')
+ else:
+ ob_type_str = 'NULL'
+ result = ' PyVarObject_HEAD_INIT(%s, 0)\n' % ob_type_str
result += ' "%(tp_name)s", /*tp_name*/\n' % self.__dict__
result += ' sizeof(%(struct_name)s), /*tp_basicsize*/\n' % self.__dict__
result += ' 0, /*tp_itemsize*/\n'
diff --git a/gcc-python-wrapper.c b/gcc-python-wrapper.c
index 93234b0..a4a394c 100644
--- a/gcc-python-wrapper.c
+++ b/gcc-python-wrapper.c
@@ -20,6 +20,74 @@
/*
Low-level wrapper support, with integration with GCC's garbage
collector (aka "GGC")
+
+ High-level overview
+ ===================
+
+ We keep track of all "live" wrapper objects, and each one knows how
+ to mark the object it wraps; we register a hook into GCC"s GC so that
+ when it starts marking, we iterate through all our live wrapper objects,
+ marking the underlying GCC objects.
+
+
+ Implementation details
+ ======================
+
+ All of our wrapper types are subclasses of PyGccWrapper, which adds
+ a doubly-linked list to the top of the objects, so that we can track
+ all live wrapper objects. This list is updated via their PyTypeObject's
+ tp_alloc and tp_dealloc.
+
+ Each has a PyTypeObject that's actually a PyGccWrapperTypeObject, which adds
+ a "wrtp_mark" hook to a PyTypeObject, so that it can participate in
+ GCC's garbage collector. It actually has to add it to PyHeapTypeObject,
+ given that that's the (larger) in-memory layout of a user-defined type.
+
+ In order to correctly inherit this wrtp_mark slot, we need to override the
+ creation of any user-defined subclases of these PyTypeObjects, adding the
+ "inherit the wrtp_mark callback" logic. We do this by setting the ob_type
+ of our PyGccWrapperTypeObject to be PyGccWrapperMetaType, which supplies
+ a custom tp_new hook: analogous to setting a __metaclass__:
+
+ http://docs.python.org/reference/datamodel.html#customizing-class-creation
+
+ Thus, given e.g.
+
+ class MyPass(gcc.GimplePass):
+ ...etc...
+ p = MyPass('mypass')
+
+ p is a PyGccPass
+ ob_type == address on heap of user-defined type ("MyPass")
+
+ "MyPass" is this heap-allocated PyGccWrapperTypeObject:
+ ob_type == &PyGccWrapperMetaType
+ tp_base == &gcc_GimplePassType (the parent class: "gcc.GimplePass")
+ wrtp_mark == wrtp_mark_for_PyGccPass (inherited from tp_base via
+ PyGccWrapperMetaType.tp_new)
+
+ gcc_GimplePassType is a statically-allocated PyGccWrapperTypeObject (in
+ autogenerated-pass.c):
+ ob_type == &PyGccWrapperMetaType
+ tp_base == &gcc_PassType (the parent class: "gcc.Pass")
+ wrtp_mark == wrtp_mark_for_PyGccPass ("manually" set up by
+ generate-pass-c.py)
+
+ gcc_PassType is a statically-allocated PyGccWrapperTypeObject (in
+ autogenerated-pass.c):
+ ob_type == &PyGccWrapperMetaType
+ tp_base == NULL, but becomes &PyBaseObject_Type aka "object"
+ wrtp_mark == wrtp_mark_for_PyGccPass ("manually" set up by
+ generate-pass-c.py)
+
+ PyGccWrapperMetaType is the statically-allocated metaclass for the above
+ type objects ("gcc.WrapperMeta")
+
+ (as it happens, gcc.Pass wraps an object that doesn't participate in the
+ GCC garbage collector, so this is somewhat redundant. However, it should
+ matter when creating user-defined subclasses of types that *do* wrap a
+ GC-managed object)
+
*/
#include <Python.h>
@@ -27,9 +95,49 @@
#include "gcc-python-wrappers.h"
#include "gcc-python-compat.h"
+static void
+force_gcc_gc(void);
+
/* Debugging, for use by selftest routine */
static int debug_gcc_python_wrapper = 0;
+/*
+ This is the overriden tp_new in our metatype: when we create new subtypes,
+ it copies up our added slots:
+*/
+static PyObject*
+gcc_python_wrapper_meta_tp_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
+{
+ PyTypeObject *new_type;
+
+ /* Use PyType_Type's tp_new */
+ new_type = (PyTypeObject*)PyType_Type.tp_new(type, args, kwds);
+ if (!new_type) {
+ return NULL;
+ }
+
+ /* Verify that the metaclass is sane, and that the created type object
+ will thus be large enough for our extra information: */
+ assert(Py_TYPE(new_type)->tp_basicsize >= sizeof(PyGccWrapperTypeObject));
+
+ /* Inherit wrtp_mark: */
+ ((PyGccWrapperTypeObject*)new_type)->wrtp_mark = \
+ ((PyGccWrapperTypeObject*)new_type->tp_base)->wrtp_mark;
+
+ return (PyObject*)new_type;
+}
+
+PyTypeObject PyGccWrapperMetaType = {
+ PyVarObject_HEAD_INIT(&PyType_Type, 0)
+ "gcc.WrapperMeta", /*tp_name*/
+ sizeof(PyGccWrapperTypeObject), /*tp_basicsize*/
+ 0, /*tp_itemsize*/
+
+ .tp_flags = Py_TPFLAGS_DEFAULT,
+ .tp_base = &PyType_Type,
+ .tp_new = gcc_python_wrapper_meta_tp_new,
+};
+
/* Maintain a circular linked list of PyGccWrapper instances: */
static struct PyGccWrapper sentinel = {
PyObject_HEAD_INIT(NULL)
@@ -157,6 +265,8 @@ gcc_python_wrapper_init(void)
{
/* Register our GC root-walking callback: */
ggc_register_root_tab(&myroot);
+
+ PyType_Ready(&PyGccWrapperMetaType);
}
static void
diff --git a/gcc-python.h b/gcc-python.h
index d57316d..ffb88a8 100644
--- a/gcc-python.h
+++ b/gcc-python.h
@@ -79,7 +79,7 @@ typedef void (*wrtp_marker) (PyGccWrapper *wrapper);
typedef struct PyGccWrapperTypeObject
{
- PyTypeObject wrtp_base;
+ PyHeapTypeObject wrtp_base;
/* Callback for marking the wrapped objects when GCC's garbage
collector runs: */
@@ -107,6 +107,7 @@ gcc_python_wrapper_track(PyGccWrapper *obj);
extern void
gcc_python_wrapper_dealloc(PyObject *obj);
+extern PyTypeObject PyGccWrapperMetaType;
/*
Macro DECLARE_SIMPLE_WRAPPER():
ARG_structname:
diff --git a/tests/plugin/callgraph/stdout.txt b/tests/plugin/callgraph/stdout.txt
index f9560e3..2c7f7f9 100644
--- a/tests/plugin/callgraph/stdout.txt
+++ b/tests/plugin/callgraph/stdout.txt
@@ -24,7 +24,7 @@ class CallgraphNode(__builtin__.object)
| ----------------------------------------------------------------------
| Data and other attributes defined here:
|
- | __new__ = <built-in method __new__ of type object>
+ | __new__ = <built-in method __new__ of gcc.WrapperMeta object>
| T.__new__(S, ...) -> a new object with type S, a subtype of T
Help on class CallgraphEdge in module gcc:
@@ -53,7 +53,7 @@ class CallgraphEdge(__builtin__.object)
| ----------------------------------------------------------------------
| Data and other attributes defined here:
|
- | __new__ = <built-in method __new__ of type object>
+ | __new__ = <built-in method __new__ of gcc.WrapperMeta object>
| T.__new__(S, ...) -> a new object with type S, a subtype of T
cgn:
diff --git a/tests/plugin/gc/segfault-on-instance-of-pass-subclass/input.c b/tests/plugin/gc/segfault-on-instance-of-pass-subclass/input.c
new file mode 100644
index 0000000..40a8c17
--- /dev/null
+++ b/tests/plugin/gc/segfault-on-instance-of-pass-subclass/input.c
@@ -0,0 +1 @@
+/* empty */
diff --git a/tests/plugin/gc/segfault-on-instance-of-pass-subclass/script.py b/tests/plugin/gc/segfault-on-instance-of-pass-subclass/script.py
new file mode 100644
index 0000000..d447bf8
--- /dev/null
+++ b/tests/plugin/gc/segfault-on-instance-of-pass-subclass/script.py
@@ -0,0 +1,10 @@
+import gcc
+
+class UnusedArg(gcc.GimplePass):
+ def execute(self, fun):
+ pass
+
+ps = UnusedArg(name='UnusedArg')
+ps.register_after('lower')
+
+gcc._force_garbage_collection()
diff --git a/tests/plugin/options/stdout.txt b/tests/plugin/options/stdout.txt
index 14a92f4..81cc833 100644
--- a/tests/plugin/options/stdout.txt
+++ b/tests/plugin/options/stdout.txt
@@ -36,7 +36,7 @@ class Option(__builtin__.object)
| ----------------------------------------------------------------------
| Data and other attributes defined here:
|
- | __new__ = <built-in method __new__ of type object>
+ | __new__ = <built-in method __new__ of gcc.WrapperMeta object>
| T.__new__(S, ...) -> a new object with type S, a subtype of T
gcc.Option('-funroll-loops')
diff --git a/tests/plugin/parameters/stdout.txt b/tests/plugin/parameters/stdout.txt
index 3bb724f..bdae602 100644
--- a/tests/plugin/parameters/stdout.txt
+++ b/tests/plugin/parameters/stdout.txt
@@ -24,7 +24,7 @@ class Parameter(__builtin__.object)
| ----------------------------------------------------------------------
| Data and other attributes defined here:
|
- | __new__ = <built-in method __new__ of type object>
+ | __new__ = <built-in method __new__ of gcc.WrapperMeta object>
| T.__new__(S, ...) -> a new object with type S, a subtype of T
struct-reorg-cold-struct-ratio
diff --git a/tests/plugin/rtl/stdout.txt b/tests/plugin/rtl/stdout.txt
index 2720e05..46f5ad7 100644
--- a/tests/plugin/rtl/stdout.txt
+++ b/tests/plugin/rtl/stdout.txt
@@ -21,6 +21,6 @@ class Rtl(__builtin__.object)
| ----------------------------------------------------------------------
| Data and other attributes defined here:
|
- | __new__ = <built-in method __new__ of type object>
+ | __new__ = <built-in method __new__ of gcc.WrapperMeta object>
| T.__new__(S, ...) -> a new object with type S, a subtype of T
diff --git a/wrapperbuilder.py b/wrapperbuilder.py
index 01cab62..d369f31 100644
--- a/wrapperbuilder.py
+++ b/wrapperbuilder.py
@@ -28,15 +28,20 @@ class PyGccWrapperTypeObject(PyTypeObject):
A PyTypeObject that's also a PyGccWrapperTypeObject
(with metaclass PyGccWrapperMetaType)
"""
+ def __init__(self, *args, **kwargs):
+ PyTypeObject.__init__(self, *args, **kwargs)
+ self.ob_type = '&PyGccWrapperMetaType'
+
def c_defn(self):
result = '\n'
result += 'PyGccWrapperTypeObject %(identifier)s = {\n' % self.__dict__
result += self.c_src_field_value('wrtp_base',
- '{\n%s}' % indent(self.c_initializer()))
+ '{\n .ht_type = {\n%s}' % indent(indent(self.c_initializer())))
+ result += ' },\n'
result += self.c_src_field_value('wrtp_mark',
'wrtp_mark_for_%s' % self.struct_name,
cast='wrtp_marker')
- result += '};\n' % self.__dict__
+ result += '};\n'
result +='\n'
return result