includes/cpython: Fix newfunc to use PyObject* for args/kwargs instead of object#4823
Merged
da-woods merged 1 commit intocython:0.29.xfrom Jul 3, 2022
Merged
includes/cpython: Fix newfunc to use PyObject* for args/kwargs instead of object#4823da-woods merged 1 commit intocython:0.29.xfrom
da-woods merged 1 commit intocython:0.29.xfrom
Conversation
…d of object 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 cython#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 cython#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 cython#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 cython#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 cython#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 cython#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: cython#4822
Contributor
|
This looks good to me (although I'll wait for other comments). My feeling is that your "reassign |
Contributor
Author
|
@da-woods, thanks for feedback. I share your feelings. And btw, the |
Contributor
|
Thanks @navytux |
Contributor
Author
|
@da-woods, thanks for merging. |
scoder
pushed a commit
that referenced
this pull request
Jul 4, 2022
…d 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 #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: #4822
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 #4822:
-> 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 ...
... it works as expected without crashing:
Fixes: #4822