• Kirill Smelkov's avatar
    includes/cpython: Fix newfunc to use PyObject* for args/kwargs instead of object (#4823) · 7c789034
    Kirill Smelkov authored
    object means the argument is always non-NULL valid Python object, while
    PyObject* argument can be generally NULL. If the argument is indeed
    passed as NULL, and we declare it as object, generated code will crash
    while trying to incref it.
    
    Quoting https://github.com/cython/cython/issues/4822:
    
        object.pxd currently declares `newfunc` as follows:
    
        ```pyx
        ctypedef object (*newfunc)(cpython.type.type, object, object)  # (type, args, kwargs)
        ```
    
        which implies that `args` and `kwargs` are always live objects and cannot be NULL.
    
        However Python can, and does, call tp_new with either args=NULL, or kwargs=NULL or both. And in such cases this leads to segfault in automatically-generated __Pyx_INCREF for args or kw.
    
        The fix is to change `object` to `PyObject*` for both args and kwargs.
    
        Please see below for details:
    
        ```cython
        # cython: language_level=3
        from cpython cimport newfunc, type as cpytype, Py_TYPE
    
        cdef class X:
            cdef int i
            def __init__(self, i):
                self.i = i
            def __repr__(self):
                return 'X(%d)' % self.i
    
        cdef newfunc _orig_tp_new = Py_TYPE(X(0)).tp_new
    
        cdef object _trace_tp_new(cpytype cls, object args, object kw):
            print('_trace_tp_new', cls, args, kw)
            return _orig_tp_new(cls, args, kw)
    
        Py_TYPE(X(0)).tp_new = _trace_tp_new
    
        x = X(123)
        print(x)
        ```
    
        ```console
        (neo) (py3.venv) (g.env) kirr@deca:~/src/tools/go/pygolang$ cythonize -i x.pyx
        Compiling /home/kirr/src/tools/go/pygolang/x.pyx because it changed.
        [1/1] Cythonizing /home/kirr/src/tools/go/pygolang/x.pyx
        running build_ext
        building 'x' extension
        ...
        x86_64-linux-gnu-gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O2 -Wall -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=. -fstack-protector-strong -Wformat -Werror=format-security -g -fwrapv -O2 -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -fPIC -I/home/kirr/src/wendelin/venv/py3.venv/include -I/usr/include/python3.9 -c /home/kirr/src/tools/go/pygolang/x.c -o /home/kirr/src/tools/go/pygolang/tmpqkz1r96s/home/kirr/src/tools/go/pygolang/x.o
        x86_64-linux-gnu-gcc -pthread -shared -Wl,-O1 -Wl,-Bsymbolic-functions -Wl,-z,relro -g -fwrapv -O2 -Wl,-z,relro -g -fwrapv -O2 -g -ffile-prefix-map=/build/python3.9-RNBry6/python3.9-3.9.2=. -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 /home/kirr/src/tools/go/pygolang/tmpqkz1r96s/home/kirr/src/tools/go/pygolang/x.o -o /home/kirr/src/tools/go/pygolang/x.cpython-39-x86_64-linux-gnu.so
        ```
    
        ```console
        (neo) (py3.venv) (g.env) kirr@deca:~/src/tools/go/pygolang$ python -c 'import x'
        Ошибка сегментирования (стек памяти сброшен на диск)
        ```
    
        ```console
        (neo) (py3.venv) (g.env) kirr@deca:~/src/tools/go/pygolang$ gdb python core
        ...
        Reading symbols from python...
        Reading symbols from /usr/lib/debug/.build-id/f9/02f8a561c3abdb9c8d8c859d4243bd8c3f928f.debug...
        [New LWP 218557]
        [Thread debugging using libthread_db enabled]
        Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
        Core was generated by `python -c import x'.
        Program terminated with signal SIGSEGV, Segmentation fault.
        #0  _Py_INCREF (op=0x0) at /usr/include/python3.9/object.h:408
        408         op->ob_refcnt++;
    
        (gdb) bt 5
        #0  _Py_INCREF (op=0x0) at /usr/include/python3.9/object.h:408
        #1  __pyx_f_1x__trace_tp_new (__pyx_v_cls=0x7f5ce75e6880 <__pyx_type_1x_X>, __pyx_v_args=(123,), __pyx_v_kw=0x0) at /home/kirr/src/tools/go/pygolang/x.c:1986
        #2  0x000000000051dd7e in type_call (type=type@entry=0x7f5ce75e6880 <__pyx_type_1x_X>, args=args@entry=(123,), kwds=kwds@entry=0x0)
            at ../Objects/typeobject.c:1014
        #3  0x00007f5ce75df8d4 in __Pyx_PyObject_Call (func=<type at remote 0x7f5ce75e6880>, arg=(123,), kw=0x0) at /home/kirr/src/tools/go/pygolang/x.c:3414
        #4  0x00007f5ce75df276 in __pyx_pymod_exec_x (__pyx_pyinit_module=<optimized out>) at /home/kirr/src/tools/go/pygolang/x.c:3017
        (More stack frames follow...)
    
        (gdb) f 1
        #1  __pyx_f_1x__trace_tp_new (__pyx_v_cls=0x7f5ce75e6880 <__pyx_type_1x_X>, __pyx_v_args=(123,), __pyx_v_kw=0x0) at /home/kirr/src/tools/go/pygolang/x.c:1986
        1986      __Pyx_INCREF(__pyx_v_kw);
        ```
    
    -> Change newfunc signature to use PyObject* instead of object to fix it.
    
    With this fix, and test example updates to account for object -> PyObject* change as follows ...
    
        --- a/x.pyx.kirr
        +++ b/x.pyx
        @@ -1,5 +1,5 @@
         # cython: language_level=3
        -from cpython cimport newfunc, type as cpytype, Py_TYPE
        +from cpython cimport newfunc, type as cpytype, Py_TYPE, PyObject
    
         cdef class X:
             cdef int i
        @@ -10,8 +10,12 @@ cdef class X:
    
         cdef newfunc _orig_tp_new = Py_TYPE(X(0)).tp_new
    
        -cdef object _trace_tp_new(cpytype cls, object args, object kw):
        -    print('_trace_tp_new', cls, args, kw)
        +cdef object xobject(PyObject* x):
        +    return "null"  if x == NULL  else \
        +           <object>x
        +
        +cdef object _trace_tp_new(cpytype cls, PyObject* args, PyObject* kw):
        +    print('_trace_tp_new', cls, xobject(args), xobject(kw))
             return _orig_tp_new(cls, args, kw)
    
         Py_TYPE(X(0)).tp_new = _trace_tp_new
    
    ... it works as expected without crashing:
    
        $ python -c 'import x'
        _trace_tp_new <type 'x.X'> (123,) null
        X(123)
    
    Fixes: https://github.com/cython/cython/issues/4822
    7c789034
object.pxd 18 KB