[Cython] segfault due to using DECREF instead of XDECREF

[Stefan Behnel]

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