[Cython] segfault due to using DECREF instead of XDECREF

[Syam Gadde]

On 01/23/2014 01:04 PM, Stefan Behnel wrote:
> BTW, thanks for cutting down the code to a short and easy to analyse 
> test case, but if the original code really has a control flow like 
> that, there might be some space left for improved clarity.

Absolutely.  In the wild, it manifested itself in PyFFTW.  It turns out 
I will likely not be using it, but thought it was an interesting edge 
case, and was what looked like valid Python, though it plays hard and 
fast with Python's runtime binding...

-syam

On 01/23/2014 01:04 PM, Stefan Behnel wrote:
> Syam Gadde, 21.01.2014 23:00:
>> It seems that cython's static code analysis is failing on the following
>> code.  It thinks that a particular variable (myobject2) must not be
>> NULL, but in fact it is, so the __Pyx_DECREF_SET segfaults.  This came
>> from third-party code, so I could fix it with an explicit initialization
>> to None, but I'd love to have Cython deal with it directly.  Thanks for
>> your help!
>>
>> # When compiled with Cython 0.19.2 or 0.20dev, the following code
>> # segfaults on import
>>
>> import random
>>
>> def myfunc():
>>       ## uncommenting the following fixes things
>>       #myobject2 = None
>>
>>       myfalse = random.random() > 2
>>
>>       if myfalse:
>>           myobject = None
>>           myobject2 = None
>>
>>       if not myfalse:
>>           # here Cython uses __Pyx_XDECREF_SET
>>           myobject = None
>>
>>       print "Made it past myobject assignment"
>>
>>       if not myfalse or myobject2 is None:
>>           # here Cython uses Pyx_DECREF_SET because it has determined
>>           # that __pyx_v_myobject2 can't be null, but it really, really can!
>>           # (if no one assigned myobject2 yet)
>>           myobject2 = None
>>
>>       print "Made it past myobject2 assignment"
>>
>> myfunc()
> Thanks for the report. The generated C code currently looks like this:
>
> ===================
> /*
>   *      if not myfalse or myobject2 is None:             # <<<<<<<<<<<<<<
>   */
>    __pyx_t_4 = __Pyx_PyObject_IsTrue(__pyx_v_myfalse); if /*error*/
>    __pyx_t_3 = ((!__pyx_t_4) != 0);
>    if (!__pyx_t_3) {
>      if (unlikely(!__pyx_v_myobject2)) {
>                       __Pyx_RaiseUnboundLocalError("myobject2"); {...} }
>      __pyx_t_4 = (__pyx_v_myobject2 == Py_None);
>      __pyx_t_5 = (__pyx_t_4 != 0);
>    } else {
>      __pyx_t_5 = __pyx_t_3;
>    }
>    if (__pyx_t_5) {
>
> /*
>   *          myobject2 = None             # <<<<<<<<<<<<<<
>   */
>      __Pyx_INCREF(Py_None);
>      __Pyx_DECREF_SET(__pyx_v_myobject2, Py_None);
> ===================
>
> So, it's smart enough to figure out that it has to raise an
> UnboundLocalError on the "if" test if the variable hasn't been set yet, but
> then fails to see that in the case that the condition short-circuits, this
> test hasn't been executed yet.
>
> BTW, thanks for cutting down the code to a short and easy to analyse test
> case, but if the original code really has a control flow like that, there
> might be some space left for improved clarity.
>
> Stefan
>
> _______________________________________________
> cython-devel mailing list
> cython-devel at python.org
> https://mail.python.org/mailman/listinfo/cython-devel
>