Bug in NULL handling introduced 0.14.1-1
I have cython file which is using PyObject_GenericSetAttr which is defined as follows in my pyx file: cdef extern from "Python.h": int PyObject_GenericSetAttr(object, object, object) except -1 Now in my script I am using that function to generically delete an attribute by passing a NULL as the last value (this is proper way to trigger a generic delattr in the Python c-api) with the following line: PyObject_GenericSetAttr(self, name, <object>NULL) # need to cast to object or Cython won't compile In Cython 0.14.1-1 this generates the following code: __pyx_t_3 = __pyx_v_self; __Pyx_INCREF(__pyx_t_3); __pyx_t_2 = __pyx_v_name; __Pyx_INCREF(__pyx_t_2); __pyx_t_4 = ((PyObject *)NULL); __Pyx_INCREF(__pyx_t_4); __pyx_t_5 = PyObject_GenericSetAttr(__pyx_t_3, __pyx_t_2, __pyx_t_4); if (unlikely(__pyx_t_5 == -1)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 124; __pyx_clineno = __LINE__; goto __pyx_L1_error;} __Pyx_DECREF(__pyx_t_3); __pyx_t_3 = 0; __Pyx_DECREF(__pyx_t_2); __pyx_t_2 = 0; __Pyx_DECREF(__pyx_t_4); __pyx_t_4 = 0; This causes a segfault because the NULL is getting increfed via Py_INCREF instead of Py_XINCREF. This wasnt a problem in Cython 0.13-1 which generated the following code: __pyx_t_3 = PyObject_GenericSetAttr(__pyx_v_self, __pyx_v_name, ((PyObject *)NULL)); if (unlikely(__pyx_t_3 == -1)) {__pyx_filename = __pyx_f[0]; __pyx_lineno = 124; __pyx_clineno = __LINE__; goto __pyx_L1_error;} and doesn't attempt to INCREF the NULL. Is there a workaround for my current situation?
Chris Colbert wrote:
I have cython file which is using PyObject_GenericSetAttr
Now in my script I am using that function to generically delete an attribute by passing a NULL as the last value (this is proper way to trigger a generic delattr in the Python c-api)
I would have thought the proper way to do that was to use PyObject_DelAttr, which Pyrex exposes as delattr(). I don't think PyObject_GenericSetAttr is even meant to be called directly -- it's intended for filling the tp_setattr slot of type objects.
This causes a segfault because the NULL is getting increfed via Py_INCREF instead of Py_XINCREF.
I would recommend against trying to "fix" this. You got away with it before because you happened to be passing the NULL value directly to a function which is expecting it. But casting NULL to an object reference is not something that should be encouraged, because in any other context it would quickly lead to disaster. -- Greg
The problem with delattr (and thus PyObject_DelAttr) arises when you define a __delattr__ method on your class. There is not easy way to then call back into the "normal" python delattr semantics, except by doing object.__delattr__ (which is not optimized by Cython). Futher, calling PyObject_GenericSetattr(obj, name, NULL) appears to be the proper use, given that it properly follows the descriptor chain and will call __delete__ if obj.name is a descriptor. I would argue that there should be at least some way to pass a NULL pointer in Cython where a PyObject* is expected. On Sun, Feb 13, 2011 at 3:31 PM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Chris Colbert wrote:
I have cython file which is using PyObject_GenericSetAttr Now in my script I am using that function to generically delete an attribute by passing a NULL as the last value (this is proper way to trigger a generic delattr in the Python c-api)
I would have thought the proper way to do that was to use PyObject_DelAttr, which Pyrex exposes as delattr().
I don't think PyObject_GenericSetAttr is even meant to be called directly -- it's intended for filling the tp_setattr slot of type objects.
This causes a segfault because the NULL is getting increfed via Py_INCREF
instead of Py_XINCREF.
I would recommend against trying to "fix" this. You got away with it before because you happened to be passing the NULL value directly to a function which is expecting it. But casting NULL to an object reference is not something that should be encouraged, because in any other context it would quickly lead to disaster.
-- Greg
I should mention, that these PyObject_Generic* functions are being used from with __getattribute__, __setattr__, and __delattr__ methods. I should have added that context in the begining. On Sun, Feb 13, 2011 at 3:37 PM, Chris Colbert <sccolbert@gmail.com> wrote:
The problem with delattr (and thus PyObject_DelAttr) arises when you define a __delattr__ method on your class. There is not easy way to then call back into the "normal" python delattr semantics, except by doing object.__delattr__ (which is not optimized by Cython).
Futher, calling PyObject_GenericSetattr(obj, name, NULL) appears to be the proper use, given that it properly follows the descriptor chain and will call __delete__ if obj.name is a descriptor.
I would argue that there should be at least some way to pass a NULL pointer in Cython where a PyObject* is expected.
On Sun, Feb 13, 2011 at 3:31 PM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Chris Colbert wrote:
I have cython file which is using PyObject_GenericSetAttr Now in my script I am using that function to generically delete an attribute by passing a NULL as the last value (this is proper way to trigger a generic delattr in the Python c-api)
I would have thought the proper way to do that was to use PyObject_DelAttr, which Pyrex exposes as delattr().
I don't think PyObject_GenericSetAttr is even meant to be called directly -- it's intended for filling the tp_setattr slot of type objects.
This causes a segfault because the NULL is getting increfed via Py_INCREF
instead of Py_XINCREF.
I would recommend against trying to "fix" this. You got away with it before because you happened to be passing the NULL value directly to a function which is expecting it. But casting NULL to an object reference is not something that should be encouraged, because in any other context it would quickly lead to disaster.
-- Greg
changing the include definition to: cdef extern from "Python.h": int PyObject_GenericSetAttr(PyObject*, PyObject*, PyObject*) except -1 Seems to solve the problem, with the inconvenience of needing to explicitly cast to <PyObject*> before calling it. However, the objects are no longer incref'd before calling, so this may or not be an issue. On Sun, Feb 13, 2011 at 3:39 PM, Chris Colbert <sccolbert@gmail.com> wrote:
I should mention, that these PyObject_Generic* functions are being used from with __getattribute__, __setattr__, and __delattr__ methods. I should have added that context in the begining.
On Sun, Feb 13, 2011 at 3:37 PM, Chris Colbert <sccolbert@gmail.com>wrote:
The problem with delattr (and thus PyObject_DelAttr) arises when you define a __delattr__ method on your class. There is not easy way to then call back into the "normal" python delattr semantics, except by doing object.__delattr__ (which is not optimized by Cython).
Futher, calling PyObject_GenericSetattr(obj, name, NULL) appears to be the proper use, given that it properly follows the descriptor chain and will call __delete__ if obj.name is a descriptor.
I would argue that there should be at least some way to pass a NULL pointer in Cython where a PyObject* is expected.
On Sun, Feb 13, 2011 at 3:31 PM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Chris Colbert wrote:
I have cython file which is using PyObject_GenericSetAttr Now in my script I am using that function to generically delete an attribute by passing a NULL as the last value (this is proper way to trigger a generic delattr in the Python c-api)
I would have thought the proper way to do that was to use PyObject_DelAttr, which Pyrex exposes as delattr().
I don't think PyObject_GenericSetAttr is even meant to be called directly -- it's intended for filling the tp_setattr slot of type objects.
This causes a segfault because the NULL is getting increfed via
Py_INCREF instead of Py_XINCREF.
I would recommend against trying to "fix" this. You got away with it before because you happened to be passing the NULL value directly to a function which is expecting it. But casting NULL to an object reference is not something that should be encouraged, because in any other context it would quickly lead to disaster.
-- Greg
Chris Colbert wrote:
changing the include definition to:
cdef extern from "Python.h": int PyObject_GenericSetAttr(PyObject*, PyObject*, PyObject*) except -1
This suggests another possible workaround: cdef extern from "Python.h": int PyObject_GenericDelAttr "PyObject_GenericSetAttr" (object, object, PyObject*) This creates an alias for PyObject_GenericSetAttr with a different signature, which you then call as PyObject_GenericDelAttr(obj, name, NULL) If you're willing to write a tiny bit of C, you could make this tidier by using a .h file containing #define PyObject_GenericDelAttr(obj, name) PyObject_GenericSetAttr(obj, name, NULL) and then declare PyObject_GenericDelAttr to Cython with just two arguments. Maybe Cython should include something like this in its standard preamble, along with similar macros for other NULL-taking API functions. -- Greg
On Mon, Feb 14, 2011 at 1:50 AM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Chris Colbert wrote:
changing the include definition to:
cdef extern from "Python.h": int PyObject_GenericSetAttr(PyObject*, PyObject*, PyObject*) except -1
This suggests another possible workaround:
cdef extern from "Python.h": int PyObject_GenericDelAttr "PyObject_GenericSetAttr" (object, object, PyObject*)
This creates an alias for PyObject_GenericSetAttr with a different signature, which you then call as
Awesome, I didn't know you could do this. Thanks.
Chris Colbert wrote:
The problem with delattr (and thus PyObject_DelAttr) arises when you define a __delattr__ method on your class. There is not easy way to then call back into the "normal" python delattr semantics, except by doing object.__delattr__ (which is not optimized by Cython).
Hmmm, perhaps it should be? And similarly for all the other type slots.
I would argue that there should be at least some way to pass a NULL pointer in Cython where a PyObject* is expected.
The trouble is that supporting this in general both safely and efficiently would require keeping track of which object references are allowed to be NULL. Could be done, but might require quite a lot of work. Maybe something more limited could be done such as allowing a literal NULL to be passed to a function argument that is specially marked as permitting this. -- Greg
On Mon, Feb 14, 2011 at 1:49 AM, Greg Ewing <greg.ewing@canterbury.ac.nz>wrote:
Chris Colbert wrote:
The problem with delattr (and thus PyObject_DelAttr) arises when you define a __delattr__ method on your class. There is not easy way to then call back into the "normal" python delattr semantics, except by doing object.__delattr__ (which is not optimized by Cython).
Hmmm, perhaps it should be? And similarly for all the other type slots.
I would argue that there should be at least some way to pass a NULL
pointer in Cython where a PyObject* is expected.
The trouble is that supporting this in general both safely and efficiently would require keeping track of which object references are allowed to be NULL. Could be done, but might require quite a lot of work.
Maybe something more limited could be done such as allowing a literal NULL to be passed to a function argument that is specially marked as permitting this.
I had the same kind of thoughts. Perhaps something like: cdef extern from "Python.h": PyObject_GenericSetAttr(object, object, object or NULL)
participants (2)
-
Chris Colbert -
Greg Ewing