From b2e72562707b5724917f59ffae6a5dd3931a086d Mon Sep 17 00:00:00 2001 From: Bill Fisher <william.w.fisher@gmail.com> Date: Sun, 11 Dec 2022 17:32:36 -0700 Subject: [PATCH 1/9] Initialize frame->previous in _PyFrame_InitializeSpecials. --- Include/internal/pycore_frame.h | 1 + Lib/test/test_ctypes/test_python_api.py | 31 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 7fa410d288c33a..8de752b47d7045 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -109,6 +109,7 @@ _PyFrame_InitializeSpecials( frame->f_locals = locals; frame->stacktop = code->co_nlocalsplus; frame->frame_obj = NULL; + frame->previous = NULL; frame->prev_instr = _PyCode_CODE(code) - 1; frame->yield_offset = 0; frame->owner = FRAME_OWNED_BY_THREAD; diff --git a/Lib/test/test_ctypes/test_python_api.py b/Lib/test/test_ctypes/test_python_api.py index 49571f97bbe152..38b94bfc377f94 100644 --- a/Lib/test/test_ctypes/test_python_api.py +++ b/Lib/test/test_ctypes/test_python_api.py @@ -81,5 +81,36 @@ def test_pyobject_repr(self): self.assertEqual(repr(py_object(42)), "py_object(42)") self.assertEqual(repr(py_object(object)), "py_object(%r)" % object) + def test_PyFrame_New_f_back(self): + """Test that accessing `f_back` does not cause a segmentation fault on + a frame created with ctypes (GH-99110).""" + # Adapted from: + # https://naleraphael.github.io/blog/posts/devlog_create_a_builtin_frame_object/ + + p_memtype = POINTER(c_ulong if sizeof(c_void_p) == 8 else c_uint) + pythonapi.PyFrame_New.argtypes = ( + p_memtype, # PyThreadState *tstate + p_memtype, # PyCodeObject *code + py_object, # PyObject *globals + py_object # PyObject *locals + ) + pythonapi.PyFrame_New.restype = py_object # PyFrameObject* + pythonapi.PyThreadState_Get.argtypes = None + pythonapi.PyThreadState_Get.restype = p_memtype + + def dummy(): + pass + + frame = pythonapi.PyFrame_New( + pythonapi.PyThreadState_Get(), # thread state + cast(id(dummy.__code__), p_memtype), # a code object + globals(), + locals(), + ) + + # The following line should not cause a segmentation fault. + assert frame.f_back is None + + if __name__ == "__main__": unittest.main() From b6e290d6c9d05d8c489d7e70b3e75ab51dcfb9c7 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 12 Dec 2022 01:05:17 +0000 Subject: [PATCH 2/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst new file mode 100644 index 00000000000000..9acd5dd3de1de3 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst @@ -0,0 +1 @@ +Initialize frame->previous in _PyFrame_InitializeSpecials. From f667d8fb603a0ac6044c7e988e7d4d38464ded02 Mon Sep 17 00:00:00 2001 From: Bill Fisher <william.w.fisher@gmail.com> Date: Sun, 11 Dec 2022 22:46:24 -0700 Subject: [PATCH 3/9] Fixing Python file whitespace. (patchcheck) --- Lib/test/test_ctypes/test_python_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_python_api.py b/Lib/test/test_ctypes/test_python_api.py index 38b94bfc377f94..de6093d5ae5f89 100644 --- a/Lib/test/test_ctypes/test_python_api.py +++ b/Lib/test/test_ctypes/test_python_api.py @@ -82,7 +82,7 @@ def test_pyobject_repr(self): self.assertEqual(repr(py_object(object)), "py_object(%r)" % object) def test_PyFrame_New_f_back(self): - """Test that accessing `f_back` does not cause a segmentation fault on + """Test that accessing `f_back` does not cause a segmentation fault on a frame created with ctypes (GH-99110).""" # Adapted from: # https://naleraphael.github.io/blog/posts/devlog_create_a_builtin_frame_object/ From d6797833bcb0fb6195de0fb23b4c2643122af69f Mon Sep 17 00:00:00 2001 From: Bill Fisher <william.w.fisher@gmail.com> Date: Tue, 13 Dec 2022 13:53:18 -0700 Subject: [PATCH 4/9] Move initialization of frame->previous to init_frame. Use self.assertEquals instead of assert. --- Include/internal/pycore_frame.h | 2 +- Lib/test/test_ctypes/test_python_api.py | 2 +- .../2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst | 3 ++- Objects/frameobject.c | 1 + 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 8de752b47d7045..8b5b2274ecb14f 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -109,10 +109,10 @@ _PyFrame_InitializeSpecials( frame->f_locals = locals; frame->stacktop = code->co_nlocalsplus; frame->frame_obj = NULL; - frame->previous = NULL; frame->prev_instr = _PyCode_CODE(code) - 1; frame->yield_offset = 0; frame->owner = FRAME_OWNED_BY_THREAD; + // frame->previous: initialized when frame is linked into the frame stack } /* Gets the pointer to the locals array diff --git a/Lib/test/test_ctypes/test_python_api.py b/Lib/test/test_ctypes/test_python_api.py index de6093d5ae5f89..17ef2be84c6e70 100644 --- a/Lib/test/test_ctypes/test_python_api.py +++ b/Lib/test/test_ctypes/test_python_api.py @@ -109,7 +109,7 @@ def dummy(): ) # The following line should not cause a segmentation fault. - assert frame.f_back is None + self.assertEqual(frame.f_back, None) if __name__ == "__main__": diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst index 9acd5dd3de1de3..54c0c8dc9a8fce 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst @@ -1 +1,2 @@ -Initialize frame->previous in _PyFrame_InitializeSpecials. +Initialize frame->previous in frameobject.c to fix a segmentation fault when +accessing frames created by ctypes. diff --git a/Objects/frameobject.c b/Objects/frameobject.c index 74c26d8d4d96a5..eab85c08fc0165 100644 --- a/Objects/frameobject.c +++ b/Objects/frameobject.c @@ -1013,6 +1013,7 @@ init_frame(_PyInterpreterFrame *frame, PyFunctionObject *func, PyObject *locals) PyCodeObject *code = (PyCodeObject *)func->func_code; _PyFrame_InitializeSpecials(frame, (PyFunctionObject*)Py_NewRef(func), Py_XNewRef(locals), code); + frame->previous = NULL; for (Py_ssize_t i = 0; i < code->co_nlocalsplus; i++) { frame->localsplus[i] = NULL; } From 230b80e2c5a7da57283f219f94a879602edf29ed Mon Sep 17 00:00:00 2001 From: Bill Fisher <william.w.fisher@gmail.com> Date: Sat, 17 Dec 2022 19:24:05 -0700 Subject: [PATCH 5/9] Move comment above function. --- Include/internal/pycore_frame.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_frame.h b/Include/internal/pycore_frame.h index 8b5b2274ecb14f..f18723b303224f 100644 --- a/Include/internal/pycore_frame.h +++ b/Include/internal/pycore_frame.h @@ -96,7 +96,10 @@ static inline void _PyFrame_StackPush(_PyInterpreterFrame *f, PyObject *value) { void _PyFrame_Copy(_PyInterpreterFrame *src, _PyInterpreterFrame *dest); -/* Consumes reference to func and locals */ +/* Consumes reference to func and locals. + Does not initialize frame->previous, which happens + when frame is linked into the frame stack. + */ static inline void _PyFrame_InitializeSpecials( _PyInterpreterFrame *frame, PyFunctionObject *func, @@ -112,7 +115,6 @@ _PyFrame_InitializeSpecials( frame->prev_instr = _PyCode_CODE(code) - 1; frame->yield_offset = 0; frame->owner = FRAME_OWNED_BY_THREAD; - // frame->previous: initialized when frame is linked into the frame stack } /* Gets the pointer to the locals array From 4e70dc50580212e74aac4c12dd3255933fe85224 Mon Sep 17 00:00:00 2001 From: Bill Fisher <william.w.fisher@gmail.com> Date: Sat, 17 Dec 2022 19:25:28 -0700 Subject: [PATCH 6/9] Add frame_new function to _testcapimodule.c. Move frame test from test_python_api.py to test_frame.py. --- Lib/test/test_ctypes/test_python_api.py | 30 ------------------------- Lib/test/test_frame.py | 9 ++++++++ Modules/_testcapimodule.c | 18 +++++++++++++++ 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_ctypes/test_python_api.py b/Lib/test/test_ctypes/test_python_api.py index 17ef2be84c6e70..5c1da8b2633978 100644 --- a/Lib/test/test_ctypes/test_python_api.py +++ b/Lib/test/test_ctypes/test_python_api.py @@ -81,36 +81,6 @@ def test_pyobject_repr(self): self.assertEqual(repr(py_object(42)), "py_object(42)") self.assertEqual(repr(py_object(object)), "py_object(%r)" % object) - def test_PyFrame_New_f_back(self): - """Test that accessing `f_back` does not cause a segmentation fault on - a frame created with ctypes (GH-99110).""" - # Adapted from: - # https://naleraphael.github.io/blog/posts/devlog_create_a_builtin_frame_object/ - - p_memtype = POINTER(c_ulong if sizeof(c_void_p) == 8 else c_uint) - pythonapi.PyFrame_New.argtypes = ( - p_memtype, # PyThreadState *tstate - p_memtype, # PyCodeObject *code - py_object, # PyObject *globals - py_object # PyObject *locals - ) - pythonapi.PyFrame_New.restype = py_object # PyFrameObject* - pythonapi.PyThreadState_Get.argtypes = None - pythonapi.PyThreadState_Get.restype = p_memtype - - def dummy(): - pass - - frame = pythonapi.PyFrame_New( - pythonapi.PyThreadState_Get(), # thread state - cast(id(dummy.__code__), p_memtype), # a code object - globals(), - locals(), - ) - - # The following line should not cause a segmentation fault. - self.assertEqual(frame.f_back, None) - if __name__ == "__main__": unittest.main() diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index ed413f105e5b17..b230143732f9ac 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -408,6 +408,15 @@ def test_frame_get_generator(self): frame = next(gen) self.assertIs(gen, _testcapi.frame_getgenerator(frame)) + def test_frame_fback_api(self): + """Test that accessing `f_back` does not cause a segmentation fault on + a frame created with `PyFrame_New` (GH-99110).""" + def dummy(): + pass + + frame = _testcapi.frame_new(dummy.__code__, globals(), locals()) + # The following line should not cause a segmentation fault. + self.assertEqual(frame.f_back, None) if __name__ == "__main__": unittest.main() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 83eef73a875d9d..e442675967e079 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -20,6 +20,7 @@ #define PY_SSIZE_T_CLEAN #include "Python.h" +#include "frameobject.h" // PyFrame_New #include "marshal.h" // PyMarshal_WriteLongToFile #include "structmember.h" // for offsetof(), T_OBJECT #include <float.h> // FLT_MAX @@ -2911,6 +2912,22 @@ frame_getlasti(PyObject *self, PyObject *frame) return PyLong_FromLong(lasti); } +static PyObject * +frame_new(PyObject *self, PyObject *args) +{ + PyObject *code, *globals, *locals; + if (!PyArg_ParseTuple(args, "OOO", &code, &globals, &locals)) { + return NULL; + } + if (!PyCode_Check(code)) { + PyErr_SetString(PyExc_TypeError, "argument must be a code object"); + return NULL; + } + PyThreadState *tstate = PyThreadState_Get(); + + return (PyObject *)PyFrame_New(tstate, (PyCodeObject *)code, globals, locals); +} + static PyObject * test_frame_getvar(PyObject *self, PyObject *args) { @@ -3352,6 +3369,7 @@ static PyMethodDef TestMethods[] = { {"frame_getgenerator", frame_getgenerator, METH_O, NULL}, {"frame_getbuiltins", frame_getbuiltins, METH_O, NULL}, {"frame_getlasti", frame_getlasti, METH_O, NULL}, + {"frame_new", frame_new, METH_VARARGS, NULL}, {"frame_getvar", test_frame_getvar, METH_VARARGS, NULL}, {"frame_getvarstring", test_frame_getvarstring, METH_VARARGS, NULL}, {"eval_get_func_name", eval_get_func_name, METH_O, NULL}, From 627e8d5a177ae4566b18128dc6e90b85d3bb803c Mon Sep 17 00:00:00 2001 From: Bill Fisher <william.w.fisher@gmail.com> Date: Sat, 17 Dec 2022 20:08:42 -0700 Subject: [PATCH 7/9] Remove blank line. --- Lib/test/test_ctypes/test_python_api.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_python_api.py b/Lib/test/test_ctypes/test_python_api.py index 5c1da8b2633978..49571f97bbe152 100644 --- a/Lib/test/test_ctypes/test_python_api.py +++ b/Lib/test/test_ctypes/test_python_api.py @@ -81,6 +81,5 @@ def test_pyobject_repr(self): self.assertEqual(repr(py_object(42)), "py_object(42)") self.assertEqual(repr(py_object(object)), "py_object(%r)" % object) - if __name__ == "__main__": unittest.main() From 35a4c5820d3d7eb41995f49f5140249145aa8743 Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Fri, 23 Dec 2022 16:38:22 +0530 Subject: [PATCH 8/9] Use assertIsNone --- Lib/test/test_frame.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/test/test_frame.py b/Lib/test/test_frame.py index b230143732f9ac..40c734b6e33abe 100644 --- a/Lib/test/test_frame.py +++ b/Lib/test/test_frame.py @@ -416,7 +416,7 @@ def dummy(): frame = _testcapi.frame_new(dummy.__code__, globals(), locals()) # The following line should not cause a segmentation fault. - self.assertEqual(frame.f_back, None) + self.assertIsNone(frame.f_back) if __name__ == "__main__": unittest.main() From 2c1db9974a8aea47f38609cf2821b077f211877e Mon Sep 17 00:00:00 2001 From: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Date: Fri, 23 Dec 2022 18:41:43 +0530 Subject: [PATCH 9/9] Update Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst --- .../2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst b/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst index 54c0c8dc9a8fce..175740dfca07ec 100644 --- a/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst +++ b/Misc/NEWS.d/next/Core and Builtins/2022-12-12-01-05-16.gh-issue-99110.1JqtIg.rst @@ -1,2 +1,2 @@ Initialize frame->previous in frameobject.c to fix a segmentation fault when -accessing frames created by ctypes. +accessing frames created by :c:func:`PyFrame_New`.