[Cython] __cinit__ swallowing exceptions

Vitja Makarov vitja.makarov at gmail.com
Tue Feb 28 20:35:54 CET 2012


2012/2/28 Vitja Makarov <vitja.makarov at gmail.com>:
> 2012/2/28 mark florisson <markflorisson88 at gmail.com>:
>> On 28 February 2012 18:57, Vitja Makarov <vitja.makarov at gmail.com> wrote:
>>> 2012/2/28 mark florisson <markflorisson88 at gmail.com>:
>>>> On 28 February 2012 18:19, Lisandro Dalcin <dalcinl at gmail.com> 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
>>>>> cython-devel at python.org
>>>>> 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.


More information about the cython-devel mailing list