2012/2/28 Vitja Makarov <[email protected]>: > 2012/2/28 mark florisson <[email protected]>: >> On 28 February 2012 18:57, Vitja Makarov <[email protected]> wrote: >>> 2012/2/28 mark florisson <[email protected]>: >>>> On 28 February 2012 18:19, Lisandro Dalcin <[email protected]> wrote: >>>>> This is something I really have no idea about how to fix, so I'll ask >>>>> any of you to do it. >>>>> >>>>> How to reproduce. The quick example below should fail in the second to >>>>> last line in test_cinit.py, but it succeeds: >>>>> >>>>> $ cat cinit.pyx >>>>> cdef class A: >>>>> def __cinit__(self, A a=None): >>>>> pass >>>>> >>>>> $ cat test_cinit.py >>>>> import pyximport; pyximport.install() >>>>> from cinit import A >>>>> a = A(123) >>>>> print (a) >>>>> >>>>> $ python test_cinit.py >>>>> /home/dalcinl/.pyxbld/temp.linux-x86_64-2.7/pyrex/cinit.c:1429:13: >>>>> warning: ‘__pyx_clear_code_object_cache’ defined but not used >>>>> [-Wunused-function] >>>>> <cinit.A object at 0x7fd28f1b7920> >>>>> >>>>> >>>>> I think the issue is bad code generation. Please try to follow the >>>>> flow below, when ArgTypeTest fails, then "goto __pyx_L1_error" but >>>>> __pyx_r is never initialized to -1, so the function (accidentally) >>>>> returns 0 indicating success. BTW, GCC warns about __pyx_r used before >>>>> initialization is some other code of mine (more complex, involving >>>>> inheritance, and the C compiler inlining code), but not for this >>>>> simple example. >>>>> >>>>> >>>>> static int __pyx_pw_5cinit_1A_1__cinit__(PyObject *__pyx_v_self, >>>>> PyObject *__pyx_args, PyObject *__pyx_kwds) { >>>>> ... >>>>> int __pyx_r; >>>>> ... >>>>> { >>>>> .... <arg unpacking code>.... >>>>> } >>>>> goto __pyx_L4_argument_unpacking_done; >>>>> ... >>>>> __pyx_L4_argument_unpacking_done:; >>>>> if (unlikely(!__Pyx_ArgTypeTest(((PyObject *)__pyx_v_a), >>>>> __pyx_ptype_5cinit_A, 1, "a", 0))) {__pyx_filename = __pyx_f[0]; >>>>> __pyx_lineno = 2; __pyx_clineno = __LINE__; goto __pyx_L1_error;} >>>>> ... >>>>> __pyx_L1_error:; >>>>> __pyx_L0:; >>>>> __Pyx_RefNannyFinishContext(); >>>>> return __pyx_r; >>>>> } >>>>> >>>>> >>>>> BTW, valgrind is able to catch the issue: >>>>> >>>>> >>>>> $ touch cinit.pyx >>>>> $ CFLAGS=-O0 valgrind python test_cinit.py >>>>> ==6735== Memcheck, a memory error detector >>>>> ==6735== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al. >>>>> ==6735== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info >>>>> ==6735== Command: python test_cinit.py >>>>> ==6735== >>>>> /home/dalcinl/.pyxbld/temp.linux-x86_64-2.7/pyrex/cinit.c:1429:13: >>>>> warning: ‘__pyx_clear_code_object_cache’ defined but not used >>>>> [-Wunused-function] >>>>> ==6735== Conditional jump or move depends on uninitialised value(s) >>>>> ==6735== at 0x12DA4635: __pyx_tp_new_5cinit_A (cinit.c:548) >>>>> ==6735== by 0x3A87C9DCF2: ??? (in /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87C49192: PyObject_Call (in >>>>> /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87CDE794: PyEval_EvalFrameEx (in >>>>> /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87CE15A4: PyEval_EvalCodeEx (in >>>>> /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87CE16D1: PyEval_EvalCode (in >>>>> /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87CFB9EB: ??? (in /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87CFC7EF: PyRun_FileExFlags (in >>>>> /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87CFD26E: PyRun_SimpleFileExFlags (in >>>>> /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x3A87D0E744: Py_Main (in /usr/lib64/libpython2.7.so.1.0) >>>>> ==6735== by 0x56F969C: (below main) (in /lib64/libc-2.14.90.so) >>>>> ==6735== >>>>> <cinit.A object at 0xec47430> >>>>> ==6735== >>>>> ==6735== HEAP SUMMARY: >>>>> ==6735== in use at exit: 8,870,441 bytes in 53,569 blocks >>>>> ==6735== total heap usage: 391,334 allocs, 337,765 frees, 94,515,287 >>>>> bytes allocated >>>>> ==6735== >>>>> ==6735== LEAK SUMMARY: >>>>> ==6735== definitely lost: 0 bytes in 0 blocks >>>>> ==6735== indirectly lost: 0 bytes in 0 blocks >>>>> ==6735== possibly lost: 2,319,018 bytes in 15,424 blocks >>>>> ==6735== still reachable: 6,551,423 bytes in 38,145 blocks >>>>> ==6735== suppressed: 0 bytes in 0 blocks >>>>> ==6735== Rerun with --leak-check=full to see details of leaked memory >>>>> ==6735== >>>>> ==6735== For counts of detected and suppressed errors, rerun with: -v >>>>> ==6735== Use --track-origins=yes to see where uninitialised values come >>>>> from >>>>> ==6735== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2) >>>>> >>>>> >>>>> >>>>> -- >>>>> Lisandro Dalcin >>>>> --------------- >>>>> CIMEC (INTEC/CONICET-UNL) >>>>> Predio CONICET-Santa Fe >>>>> Colectora RN 168 Km 472, Paraje El Pozo >>>>> 3000 Santa Fe, Argentina >>>>> Tel: +54-342-4511594 (ext 1011) >>>>> Tel/Fax: +54-342-4511169 >>>>> _______________________________________________ >>>>> cython-devel mailing list >>>>> [email protected] >>>>> http://mail.python.org/mailman/listinfo/cython-devel >>>> >>>> Thanks, I fixed it here: https://github.com/markflorisson88/cython >>>> >>>> It should probably have more tests, also for other special methods. >>> >>> >>> This bug was introduced by DefNode refactoring. >>> >>> Mark, your fix isn't correct __pyx_r should be set to error_value() on >>> error just like DefNode does. >> >> Right, that would be work for all cases. Could you fix it and push to master? >> > > Sure. >
I fixed it here: https://github.com/cython/cython/commit/bef95224ad130b4b4680c283c12bea7be79328e5 Thanks! -- vitja. _______________________________________________ cython-devel mailing list [email protected] http://mail.python.org/mailman/listinfo/cython-devel
