Hello, according to valgrind the following Cython fragment causes a heap corruption (with Python 2.7.5 and Cython 0.19.1 on Ubuntu): cdef class MetaClass(type): cdef int i class MyClass(object): __metaclass__ = MetaClass Please find below the result of a many hour investigation (originally triggered by random crashes and heap corruptions of a bigger Cython program that uses metaclasses). MetaClass is compiled to a C structure that extends PyTypeObject (a cc. 200 byte structure on a 32-bit architecture): struct __pyx_obj_4meta_MetaClass { PyTypeObject __pyx_base; int i; }; Instances of PyTypeObject are supposed to be allocated statically when initializing extension modules, because the structure does not support (among others) garbage collection. However, when MyClass is created, an instance of struct __pyx_obj_4meta_MetaClass (cc. 200 + 4 bytes) is dynamically allocated by the Python memory management machinery. The machinery then tries to initialize the allocated memory. The problem is that it expects that the first member of struct __pyx_obj_4meta_MetaClass is of type PyHeapTypeObject (a cc. 500 byte structure) and after a type cast it writes to the tail members of the assumed PyHeapTypeObject, i.e. way beyond the allocated cc. 200 + 4 bytes, corrupting the object heap. This corruption is nicely reported by valgrind. The proposed fix is to make __pyx_obj_4meta_MetaClass extend PyHeapTypeObject: struct __pyx_obj_4meta_MetaClass { PyHeapTypeObject __pyx_base; int i; }; This can be achieved by adding the below 2 lines to Compiler/Builtin.py: 382a383,384 > elif name == 'type': > objstruct_cname = 'PyHeapTypeObject' So that the init_builtin_types function becomes: def init_builtin_types(): global builtin_types for name, cname, methods in builtin_types_table: utility = builtin_utility_code.get(name) if name == 'frozenset': objstruct_cname = 'PySetObject' elif name == 'bool': objstruct_cname = None * elif name == 'type':* * objstruct_cname = 'PyHeapTypeObject'* else: objstruct_cname = 'Py%sObject' % name.capitalize() the_type = builtin_scope.declare_builtin_type(name, cname, utility, objstruct_cname) builtin_types[name] = the_type for method in methods: method.declare_in_type(the_type) After patching my Cython installation with the above, valgrind stopped to complain and there were no more crashes. Please consider adding this fix in the next release of Cython. Regards Dénes Vadász
Thanks for the careful analysis. See https://github.com/cython/cython/commit/e8b54dec4cdc94764abda4a107bc37dcd58e... On Tue, Sep 10, 2013 at 3:26 PM, Dénes Vadász <python2@dvadasz.hu> wrote:
Hello,
according to valgrind the following Cython fragment causes a heap corruption (with Python 2.7.5 and Cython 0.19.1 on Ubuntu):
cdef class MetaClass(type): cdef int i
class MyClass(object): __metaclass__ = MetaClass
Please find below the result of a many hour investigation (originally triggered by random crashes and heap corruptions of a bigger Cython program that uses metaclasses).
MetaClass is compiled to a C structure that extends PyTypeObject (a cc. 200 byte structure on a 32-bit architecture):
struct __pyx_obj_4meta_MetaClass { PyTypeObject __pyx_base; int i; };
Instances of PyTypeObject are supposed to be allocated statically when initializing extension modules, because the structure does not support (among others) garbage collection. However, when MyClass is created, an instance of struct __pyx_obj_4meta_MetaClass (cc. 200 + 4 bytes) is dynamically allocated by the Python memory management machinery. The machinery then tries to initialize the allocated memory. The problem is that it expects that the first member of struct __pyx_obj_4meta_MetaClass is of type PyHeapTypeObject (a cc. 500 byte structure) and after a type cast it writes to the tail members of the assumed PyHeapTypeObject, i.e. way beyond the allocated cc. 200 + 4 bytes, corrupting the object heap. This corruption is nicely reported by valgrind.
The proposed fix is to make __pyx_obj_4meta_MetaClass extend PyHeapTypeObject:
struct __pyx_obj_4meta_MetaClass { PyHeapTypeObject __pyx_base; int i; };
This can be achieved by adding the below 2 lines to Compiler/Builtin.py:
382a383,384
elif name == 'type': objstruct_cname = 'PyHeapTypeObject'
So that the init_builtin_types function becomes:
def init_builtin_types(): global builtin_types for name, cname, methods in builtin_types_table: utility = builtin_utility_code.get(name) if name == 'frozenset': objstruct_cname = 'PySetObject' elif name == 'bool': objstruct_cname = None elif name == 'type': objstruct_cname = 'PyHeapTypeObject' else: objstruct_cname = 'Py%sObject' % name.capitalize() the_type = builtin_scope.declare_builtin_type(name, cname, utility, objstruct_cname) builtin_types[name] = the_type for method in methods: method.declare_in_type(the_type)
After patching my Cython installation with the above, valgrind stopped to complain and there were no more crashes.
Please consider adding this fix in the next release of Cython.
Regards
Dénes Vadász
_______________________________________________ cython-devel mailing list cython-devel@python.org https://mail.python.org/mailman/listinfo/cython-devel
Robert Bradshaw, 12.09.2013 23:02:
Thanks for the careful analysis. See https://github.com/cython/cython/commit/e8b54dec4cdc94764abda4a107bc37dcd58e...
Might be possible to merge it with this special case: https://github.com/cython/cython/blob/master/Cython/Compiler/Code.py#L50 and/or make it a part of BuiltinObjectType somehow. Stefan
participants (3)
-
Dénes Vadász -
Robert Bradshaw -
Stefan Behnel