Commit 7c789034 authored by Kirill Smelkov's avatar Kirill Smelkov Committed by GitHub

includes/cpython: Fix newfunc to use PyObject* for args/kwargs instead of object (#4823)

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
parent 1c0691f7
......@@ -5,7 +5,7 @@ cdef extern from "Python.h":
ctypedef struct PyObject # forward declaration
ctypedef object (*newfunc)(cpython.type.type, object, object) # (type, args, kwargs)
ctypedef object (*newfunc)(cpython.type.type, PyObject*, PyObject*) # (type, args|NULL, kwargs|NULL)
ctypedef object (*unaryfunc)(object)
ctypedef object (*binaryfunc)(object, object)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment