Potential problems for extern cdefs in argument parsing

[robertwb]

Reported by robertwb on 23 Jul 2008 07:57 UTC

Consider this code:

cdef extern from "t.h":
     ctypedef long hullo

def foo(hullo h): pass

I noticed that it creates code like this:
PyArg_ParseTupleAndKeywords(__pyx_args, __pyx_kwds, "l", __pyx_argnames, 
&__pyx_v_h))

Which could probably destroy the stack if we don't have the exact 
typedef, right? Which means: One should either write our own parsing 
code instead (generate for each parameter signature), or generate the 
typestring at runtime, or insert C checks for the sizeof(the typedefs). 
The latter will destroy the current approach for numpy.int64 and friends 
and remove what I consider a nice feature.

Just as I took all that care to allow approximate typedefs for buffers :-)

Just noting it down, I suppose for now we'll live with it. But need to 
be more careful about recommending approximate typedefs.

-- 
Dag Sverre

See thread at http://codespeak.net/pipermail/cython-dev/2008-July/001760.html

Migrated-From: http://trac.cython.org/ticket/28

[robertwb]

Comment by robertwb on 23 Jul 2008 07:58 UTC I haven't been able to use this to produce a crash on my machine, but it does look bad...

[robertwb]

Comment by robertwb on 19 Aug 2008 04:17 UTC Lets try to come up with some specific examples here...

[robertwb]

Comment by anonymous on 5 Sep 2008 13:15 UTC Without further verification, this problem is most likely resolved by the new argument parsing code.

[robertwb]

Comment by dagss on 5 Sep 2008 16:53 UTC The danger of a crash is now gone but there is still some inconsistency within Cython whether the size of the type plays a role for external typedefs, and I'd like the behaviour to change here rather than other places personally. Reopening but making it a minor wishlist issue.

Details:

Does anyone have any thoughts about the external typedef issue with 
argument unpacking? I see that with Stefan's code we get an exception 
rather than a segfault which is a big step forward, but one could still 
think a bit about what the behaviour should be (i.e. if people wants to 
use numpy.uint16_t as a function argument type I want a defined 
behaviour for that.)

Example:

test.h:
typedef long int my_int;

test.pyx:

cdef extern from "test.h":
    ctypedef char myint

def myfunc(myint x):
    pass

Now, myfunc(1000) will raise an overflow exception.

Granted, this is an extreme example. I just want a defined behaviour on 
this somehow, and documenting the existing one could be ok. If we want 
to support it without assumptions on the size, something like

#define __Pyx_FetchPyInt(result, from) switch(sizeof(result)) {\
  case sizeof(char): result = __pyx_PyInt_char(from); break;
  case sizeof(int):
  ....
   default: __Pyx_RaiseFetchPyIntError(from); result = -1;
}

should do the trick (and it is only needed when the typedef is extern). 
Will a patch like that be accepted? (It will have to wait a month or two 
if I'm the one doing it, but I am not expecting anybody else to in the 
meantime.)

[robertwb]

Comment by robertwb on 8 Nov 2008 22:51 UTC It should be noted that argument parsing has been entirely re-written and avoids explicit calls to PyArg_ParseTupleAndKeywords in most cases.

[robertwb]

Comment by scoder on 21 Dec 2008 20:25 UTC We can make that "all cases" now.

[robertwb]

Comment by scoder on 8 Mar 2009 12:48 UTC no idea when exactly this was fixed (is that still a problem in 0.10.x?), but argument parsing has its own unpacking code now, so this is no longer an issue.

[robertwb]

Modified by scoder on 14 Mar 2009 08:03 UTC

[robertwb]

Modified by scoder on 14 Mar 2009 08:03 UTC

[robertwb]

Modified by scoder on 14 Mar 2009 08:03 UTC