Ticket #28 (reopened defect)

Opened 6 months ago

Last modified 2 weeks ago

Potential problems for extern cdefs in argument parsing

Reported by: robertwb Assigned to: somebody
Priority: minor Milestone: wishlist
Component: Code Generation Version:
Keywords: Cc:

Description 

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

Change History

07/23/2008 12:58:17 AM changed by robertwb

I haven't been able to use this to produce a crash on my machine, but it does look bad...

08/18/2008 09:17:58 PM changed by robertwb

  • milestone set to 0.9.8.2.

Lets try to come up with some specific examples here...

09/05/2008 06:15:42 AM changed by anonymous

  • status changed from new to closed.
  • resolution set to fixed.

Without further verification, this problem is most likely resolved by the new argument parsing code.

09/05/2008 09:53:15 AM changed by dagss

  • priority changed from major to minor.
  • status changed from closed to reopened.
  • resolution deleted.
  • milestone changed from 0.9.8.2 to wishlist.

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.)

11/08/2008 02:51:24 PM changed by robertwb

It should be noted that argument parsing has been entirely re-written and avoids explicit calls to PyArg?_ParseTupleAndKeywords in most cases.

12/21/2008 12:25:57 PM changed by scoder

We can make that "all cases" now.