[Cython] segfault due to using DECREF instead of XDECREF

2014-01-21 Thread Syam Gadde

(apologies to the moderator for multiple postings -- I'm trying to figure out 
which of my email addresses is actually being exported to the public!)

Hi,

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()
#<<>>

#<>>
#>>

[prompt]$ python setup.py build_ext --inplace
[prompt]$ ls tmpnone.*
tmpnone.c  tmpnone.py  tmpnone.so
[prompt]$ python
Python 2.6.6 (r266:84292, Nov 21 2013, 12:39:37)
[GCC 4.4.7 20120313 (Red Hat 4.4.7-3)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
 >>> import tmpnone
Made it past myobject assignment
Segmentation fault




___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] segfault due to using DECREF instead of XDECREF

2014-01-23 Thread 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@python.org
https://mail.python.org/mailman/listinfo/cython-devel



___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


[Cython] memoryview refcount error

2014-01-28 Thread Syam Gadde

Hi again,

I'm exploring the typed memoryview feature with numpy and encountered a 
refcount error.  It seems to happen when I create a slice of a 
memoryview, and transpose it.  If I don't do both, I don't get the 
refcount error.  It happens in the error cleanup code.  I suspect that 
any error that causes the function to jump to the error cleanup code 
will do the same thing as the Exception below -- but that seemed the 
easiest way to demonstrate the problem. However, if there truly is an 
error in my code, please let me know! Thanks,


-syam

# BEGIN CODE
import numpy
cimport numpy

class MyException(Exception):
def __init__(self):
Exception.__init__(self)

def testfunc():
   a = numpy.arange(12).reshape([3,4])

   cdef numpy.int_t[:,:] a_view = a

   ## here is a slice followed by a transpose
   cdef numpy.int_t[:,:] b = a_view[:,:].T
   ## same thing happens with a more realistic slicing:
   #cdef numpy.int_t[:,:] b = a_view[1:2,:].T
   ## however, there is no error if I do this instead:
   #cdef numpy.int_t[:,:] b = a_view.T
   ## also no error if I do this instead:
   #cdef numpy.int_t[:,:] b = a_view[:,:]

   # The exception below is just to force the function to abort
   # and run the extraneous __PYX_XDEC_MEMVIEW in the error
   # cleanup code:
   #   Fatal Python error: Acquisition count is 0 (line )
   # Comment out this exception and we don't get the error
   raise MyException()

try:
testfunc()
except MyException:
pass
# END CODE



___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] memoryview refcount error

2014-01-31 Thread Syam Gadde
iew incref here:
-if self.obj.is_name or (self.obj.is_attribute and
- self.obj.is_memslice_transpose):
-code.put_xdecref_memoryviewslice(
+#if self.obj.is_name or (self.obj.is_attribute and
+# self.obj.is_memslice_transpose):
+#code.put_xdecref_memoryviewslice(
+#self.result(), have_gil=True)
+code.put_xdecref_memoryviewslice(
 self.result(), have_gil=True)
+code.putln("%s.memview = NULL;" % self.result())
+code.putln("%s.data = NULL;" % self.result())
 else:
 ExprNode.generate_disposal_code(self, code)



On 01/28/2014 01:47 PM, Syam Gadde wrote:

Hi again,

I'm exploring the typed memoryview feature with numpy and encountered a
refcount error.  It seems to happen when I create a slice of a
memoryview, and transpose it.  If I don't do both, I don't get the
refcount error.  It happens in the error cleanup code.  I suspect that
any error that causes the function to jump to the error cleanup code
will do the same thing as the Exception below -- but that seemed the
easiest way to demonstrate the problem. However, if there truly is an
error in my code, please let me know! Thanks,

-syam

# BEGIN CODE
import numpy
cimport numpy

class MyException(Exception):
  def __init__(self):
  Exception.__init__(self)

def testfunc():
 a = numpy.arange(12).reshape([3,4])

 cdef numpy.int_t[:,:] a_view = a

 ## here is a slice followed by a transpose
 cdef numpy.int_t[:,:] b = a_view[:,:].T
 ## same thing happens with a more realistic slicing:
 #cdef numpy.int_t[:,:] b = a_view[1:2,:].T
 ## however, there is no error if I do this instead:
 #cdef numpy.int_t[:,:] b = a_view.T
 ## also no error if I do this instead:
 #cdef numpy.int_t[:,:] b = a_view[:,:]

 # The exception below is just to force the function to abort
 # and run the extraneous __PYX_XDEC_MEMVIEW in the error
 # cleanup code:
 #   Fatal Python error: Acquisition count is 0 (line )
 # Comment out this exception and we don't get the error
 raise MyException()

try:
  testfunc()
except MyException:
  pass
# END CODE



___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel



___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


[Cython] line tracing/profiling code objects

2014-02-27 Thread Syam Gadde
Hi all,

I tried using line tracing in conjunction with line_profiler/kernprof to 
attempt to see if line-based profiling works in Cython.  I think it's 
pretty close.  But I think the problem is that line_profiler is getting 
a different PyCodeObject when it wraps the function and when it actually 
gets the trace call.  It adds the initial code object to a map, and 
later when it gets the trace call, decides that the trace call is not 
something it needs to pay attention to because the new code object that 
it gets is not the same as the original one.

The first code object is created at function declaration by 
__Pyx_PyCode_New (called in__Pyx_InitCachedConstants) and is assigned to 
a variable __pyx_k_codeobj_NNN.  The second code object is created, 
essentially, during the first entry into the function (in 
__Pyx_TraceCall, via __Pyx_TraceSetupAndCall).  It seems that setting 
__pyx_frame_code to the initial code object before calling TraceCall() 
would fix this.

Is this easy to do?  I'd do it myself, but I'd need help figuring out 
how to get the name of the appropriate __pyx_k_codeobj_NNN variable from 
within FuncDefNode.generate_function_definitions(), which calls 
put_trace_call().

Thanks for your help...

-syam
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] line tracing/profiling code objects

2014-03-27 Thread Syam Gadde
Stefan,

I'm sorry to be so long in responding.  Your response was very helpful, 
but I had to suddenly change my focus to other projects. I've been 
looking at it again.

I took the easy way out for now and attempted to name the codeobj 
objects deterministically, as you suggested at the end of your message:

https://github.com/SyamGadde/cython/commit/b56135154f546eec34d88342dc1698cf1803492e

The assumption is that only one codeobj is ever generated for each named 
function within a module.  I'm afraid I may have taken a hammer to the 
code instead of a chisel, so your comments are welcome.

This is actually sufficient for using the kernprof/line_profiler 
decorator @profile as-is on Cython-generated non-cdef functions/methods 
as long as you are using the following cython directives:

   --directive profile=true
   --directive linetrace=true
   --directive binding=true

The binding=true is necessary so that the function objects get the 
func_code attribute set to the codeobj above.  Actually line_profiler 
just uses func_code as a unique identifier for the function and doesn't 
really look at the contents of func_code except to determine if a 
function is a generator.  If there is another attribute available for 
uniquely identifying function objects that don't have func_code, then 
line_profiler could potentially use that too and it could work on cdef- 
and cpdef-declared functions...

...except for the fact that cdef and cpdef functions don't accept 
arbitrary decorators, which is the typical way to use 
kernprof/line_profiler.  But I'm pretty happy to be able to use 
kernprof/line_profiler on compiled pure-Python functions for now.

Thanks for your help!

-syam

On 02/27/2014 04:06 PM, Stefan Behnel wrote:
> Hi!
>
> Syam Gadde, 27.02.2014 16:22:
>> I tried using line tracing in conjunction with line_profiler/kernprof to
>> attempt to see if line-based profiling works in Cython.  I think it's
>> pretty close.  But I think the problem is that line_profiler is getting
>> a different PyCodeObject when it wraps the function and when it actually
>> gets the trace call.  It adds the initial code object to a map, and
>> later when it gets the trace call, decides that the trace call is not
>> something it needs to pay attention to because the new code object that
>> it gets is not the same as the original one.
>>
>> The first code object is created at function declaration by
>> __Pyx_PyCode_New (called in__Pyx_InitCachedConstants) and is assigned to
>> a variable __pyx_k_codeobj_NNN.  The second code object is created,
>> essentially, during the first entry into the function (in
>> __Pyx_TraceCall, via __Pyx_TraceSetupAndCall).  It seems that setting
>> __pyx_frame_code to the initial code object before calling TraceCall()
>> would fix this.
> That's a part of it, yes. Here's another bit in the puzzle:
>
> https://github.com/cython/cython/pull/93
>
> The problem is that the code objects that we currently create along the way
> have a fixed Python code line number (because that was the simplest way to
> get it working). The two changes above will get us pretty far. What remains
> to be done then is to enable line number calculation at runtime by
> emulating byte code offsets in the frame object instead of using absolute
> line numbers. The pull request refers to CPython's
> Objects/lnotab_notes.txt, which has some details on the inner workings.
>
> Properly calculating line numbers at runtime would also have other nice
> side effects, because it would remove the need to create multiple code
> objects for a single function in the first place. So it's very desirable.
>
>
>> Is this easy to do?  I'd do it myself, but I'd need help figuring out
>> how to get the name of the appropriate __pyx_k_codeobj_NNN variable from
>> within FuncDefNode.generate_function_definitions(), which calls
>> put_trace_call().
> The best way is usually to run it through a debugger and see what you can
> get your hands on. :) The constant names (__pyx_k_...) are generated on the
> fly at C code generation time, so you can't get them before that. Two
> possible solutions: either change the way how the C names of code objects
> are being generated (e.g. by making their C name depend on the mangled C
> name of the function so that they can be deterministically named at
> analysis time), or make the code object node remember its generated
> constant name and reuse it in other places.
>
> As you can see, it's not entirely trivial overall, but we've already been
> piling up the bits and pieces so that the only remaining effort is to put
> everything together. If you could give it a try, I'm sure you'd make a lot
> of people hap

[Cython] memoryview slice transpose

2014-03-27 Thread Syam Gadde
I wanted to re-offer for discussion a fix for a bug I reported a while 
back.  The code that breaks is this:

import numpy
cimport numpy

def testfunc():
a = numpy.arange(12).reshape([3,4])
cdef numpy.int_t[:,:] a_view = a
cdef numpy.int_t[:,:] b = a_view[:,:].T

One problem is that when a memoryview slice is decremented, the .memview 
and .data fields are not set to NULL, and any error that causes a jump 
to cleanup code will attempt to decref it again.

The second problem (I think) is that assignment of a transpose of 
memoryview slices to a variable is not resulting in an incref on the 
underlying slice, even though the variable is not a temporary and 
continues to persist.

Here is a patch that solves the problem for me.

https://github.com/SyamGadde/cython/commit/5739d8b908f18c4fc9103ef04e39964d61af3495

The part I am least sure about is removing the "if" qualifications on 
the incref and decref:

   if self.obj.is_name  or self.obj.is_attribute and 
self.obj.is_memslice_transpose:

that was obviously there for a reason, but it causes problems in the above 
case.  If there is a less extreme solution I'd be happy to take it.

Thanks,

-syam
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel


Re: [Cython] memoryview slice transpose

2014-04-07 Thread Syam Gadde
My patch actually addressed two slightly different issues, both of which 
have to do with refcounts of transposed slices.  I'll address the first 
one below since that's the one that's not so obvious.

Here's the code that causes an "Acquisition count is 0" error:

import numpy
cimport numpy

def testfunc():
a = numpy.arange(12).reshape([3,4])
cdef numpy.int_t[:,:] a_view = a
cdef numpy.int_t[:,:] b = a_view[:,:].T
raise Exception()  # comment this out and you still get a refcount 
error at a different line

testfunc()


On 04/06/2014 06:12 AM, Stefan Behnel wrote:
> Syam Gadde, 27.03.2014 21:29:
>> One problem is that when a memoryview slice is decremented, the .memview
>> and .data fields are not set to NULL, and any error that causes a jump
>> to cleanup code will attempt to decref it again.
> Hmm, I can't see why the additional NULL setting would change anything.
> That's what the call to put_xdecref_memoryviewslice() already did that
> immediately precedes them (it actually calls "__Pyx_XDEC_MEMVIEW()" in
> MemoryView_C.c).

But only if the refcount of the memview goes to zero.Maybe that's what 
needs to change?

See the case below (look for the *'s).  memviewslice objects are 
defined at the top of the function and on the stack, so they persist 
throughout the function.  Several memviewslice objects may share the 
same base memview object.  So if a memviewslice object finished doing 
what it needs to do, it needs to decrement the refcount of the memview 
object it's pointing to using __PYX_XDEC_MEMVIEW, *and* detach itself 
from the memview (that's the setting .memview = NULL part).  The reason 
the explicit detaching is important is that there are extra 
__PYX_XDEC_MEMVIEW() calls in the error cleanup code, presumably to 
decrement the reference counts for still attached memviews in the case 
there is an error/exception.  If the memview field is set to NULL 
whenever a slice is done with it, the extra calls to __PYX_XDEC_MEMVIEW 
are OK because it checks to see whether .memview is NULL before going 
forward.  But currently, if an error/exception occurs while the slice is 
still attached to the memview, you can end up with a refcount < 0.

If __PYX_XDEC_MEMVIEW did indeed set .memview to NULL on every call, 
then this wouldn't be necessary.  But I have not tried that to see if it 
breaks something else.


   /* "testtranspose.pyx":7
  *a = numpy.arange(12).reshape([3,4])
  *cdef numpy.int_t[:,:] a_view = a
  *cdef numpy.int_t[:,:] b = a_view[:,:].T # <<<<<<<<<<<<<<
  *raise Exception()
  *
  */
   __pyx_t_6 = -1;
   __pyx_t_5.data = __pyx_v_a_view.data;
   __pyx_t_5.memview = __pyx_v_a_view.memview;
   __PYX_INC_MEMVIEW(&__pyx_t_5, 0);
   __pyx_t_5.shape[0] = __pyx_v_a_view.shape[0];
__pyx_t_5.strides[0] = __pyx_v_a_view.strides[0];
 __pyx_t_5.suboffsets[0] = -1;

__pyx_t_5.shape[1] = __pyx_v_a_view.shape[1];
__pyx_t_5.strides[1] = __pyx_v_a_view.strides[1];
 __pyx_t_5.suboffsets[1] = -1;

__pyx_t_7 = __pyx_t_5;
   if (unlikely(__pyx_memslice_transpose(&__pyx_t_7) == 0)) 
{__pyx_filename = __p
yx_f[0]; __pyx_lineno = 7; __pyx_clineno = __LINE__; goto __pyx_L1_error;}
   __PYX_XDEC_MEMVIEW(&__pyx_t_5, 1);//**
   __pyx_v_b = __pyx_t_7;
   __pyx_t_7.memview = NULL;
   __pyx_t_7.data = NULL;

   /* "testtranspose.pyx":8
  *cdef numpy.int_t[:,:] a_view = a
  *cdef numpy.int_t[:,:] b = a_view[:,:].T
  *raise Exception() # <<<<<<<<<<<<<<
  *
  * testfunc()
  */
   __pyx_t_1 = __Pyx_PyObject_Call(__pyx_builtin_Exception, 
__pyx_empty_tuple, NULL); if (unlikely(!__pyx_t_1)) {__pyx_filename = 
__pyx_f[0]; __pyx_lineno = 8; __pyx_clineno = __LINE__; goto 
__pyx_L1_error;}
   __Pyx_GOTREF(__pyx_t_1);
   __Pyx_Raise(__pyx_t_1, 0, 0, 0);
   __Pyx_DECREF(__pyx_t_1); __pyx_t_1 = 0;
   {__pyx_filename = __pyx_f[0]; __pyx_lineno = 8; __pyx_clineno = 
__LINE__; goto __pyx_L1_error;}

   /* "testtranspose.pyx":4
  * cimport numpy
  *
  * def testfunc(): # <<<<<<<<<<<<<<
  *a = numpy.arange(12).reshape([3,4])
  *cdef numpy.int_t[:,:] a_view = a
  */

   /* function exit code */
   __pyx_L1_error:;  // ** exception causes a jump to here *
   __Pyx_XDECREF(__pyx_t_1);
   __Pyx_XDECREF(__pyx_t_2);
   __Pyx_XDECREF(__pyx_t_3);
   __PYX_XDEC_MEMVIEW(&__pyx_t_4, 1);
   __PYX_XDEC_MEMVIEW(&__pyx_t_5, 1); //
   __PYX_XDEC_MEMVIEW(&__pyx_t_7, 1);
   __Pyx_AddTraceback("testtranspose.testfunc", __pyx_clineno, 
__pyx_lineno, __pyx_filename);
   __pyx_r = NULL;
   __Pyx_XDECREF(__pyx_v_a);
   __PYX_XDEC_MEMVIEW(&__pyx_v_a_view, 1);
   __PYX_XDEC_MEMVIEW(&__pyx_v_b, 1);
   __Pyx_XGIVEREF(__pyx_r);
   __Pyx_RefNannyFinishContext();
   return __pyx_r;
___
cython-devel mailing list
cython-devel@python.org
https://mail.python.org/mailman/listinfo/cython-devel