From f95777b7baaade42027c9cdca5a0a7a88c23a070 Mon Sep 17 00:00:00 2001 From: Matthieu Dartiailh Date: Mon, 16 Jan 2023 10:33:03 +0100 Subject: [PATCH 1/5] do not ignore default argument values in _PyFunction_FromConstructor --- Objects/funcobject.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Objects/funcobject.c b/Objects/funcobject.c index baa360381a7724..91a6b3dd40a232 100644 --- a/Objects/funcobject.c +++ b/Objects/funcobject.c @@ -87,8 +87,8 @@ _PyFunction_FromConstructor(PyFrameConstructor *constr) op->func_name = Py_NewRef(constr->fc_name); op->func_qualname = Py_NewRef(constr->fc_qualname); op->func_code = Py_NewRef(constr->fc_code); - op->func_defaults = NULL; - op->func_kwdefaults = NULL; + op->func_defaults = Py_XNewRef(constr->fc_defaults); + op->func_kwdefaults = Py_XNewRef(constr->fc_kwdefaults); op->func_closure = Py_XNewRef(constr->fc_closure); op->func_doc = Py_NewRef(Py_None); op->func_dict = NULL; From 0bb1c96502d9e4a5d4cda7306fd23d5173ec848e Mon Sep 17 00:00:00 2001 From: Matthieu Dartiailh Date: Wed, 18 Jan 2023 10:42:04 +0100 Subject: [PATCH 2/5] test: add a test for PyEval_EvalCodeEx --- Lib/test/test_capi/test_eval_code_ex.py | 56 +++++++++++ Modules/_testcapimodule.c | 125 ++++++++++++++++++++++++ 2 files changed, 181 insertions(+) create mode 100644 Lib/test/test_capi/test_eval_code_ex.py diff --git a/Lib/test/test_capi/test_eval_code_ex.py b/Lib/test/test_capi/test_eval_code_ex.py new file mode 100644 index 00000000000000..02cf8639689bf1 --- /dev/null +++ b/Lib/test/test_capi/test_eval_code_ex.py @@ -0,0 +1,56 @@ +import unittest + +from test.support import import_helper + + +# Skip this test if the _testcapi module isn't available. +_testcapi = import_helper.import_module('_testcapi') + + +class PyEval_EvalCodeExTests(unittest.TestCase): + + def test_simple(self): + def f(): + return a + + self.assertEqual(_testcapi.eval_code_ex(f.__code__, dict(a=1)), 1) + + # Need to force the compiler to use LOAD_NAME + # def test_custom_locals(self): + # def f(): + # return + + def test_with_args(self): + def f(a): + return a + + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (1,)), 1) + + def test_with_kwargs(self): + def f(a): + return a + + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), dict(a=1)), 1) + + def test_with_default(self): + def f(a): + return a + + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), {}, (1,)), 1) + + def test_with_kwarg_default(self): + def f(*, a): + return a + + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), {}, (), dict(a=1)), 1) + + def test_with_closure(self): + a = 1 + def f(): + return a + + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), {}, (), {}, f.__closure__), 1) + + +if __name__ == "__main__": + unittest.main() diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 486988b4e34cdd..05e2dd680fd28b 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2973,6 +2973,130 @@ eval_get_func_desc(PyObject *self, PyObject *func) return PyUnicode_FromString(PyEval_GetFuncDesc(func)); } +static PyObject * +eval_eval_code_ex(PyObject *mod, PyObject *pos_args) +{ + PyObject* result = NULL; + PyObject* code; + PyObject* globals; + PyObject* locals = NULL; + PyObject* args = NULL; + PyObject* kwargs = NULL; + PyObject* defaults = NULL; + PyObject* kw_defaults = NULL; + PyObject* closure = NULL; + + PyObject** c_kwargs = NULL; + + if( !PyArg_UnpackTuple( pos_args, "eval_code_ex", 2, 8, &code, &globals, &locals, &args, &kwargs, &defaults, &kw_defaults, &closure) ) + { + goto exit; + } + + if( !PyCode_Check( code ) ) + { + PyErr_SetString( PyExc_TypeError, "code must be a Python code object" ); + goto exit; + } + + if( !PyDict_Check( globals ) ) + { + PyErr_SetString( PyExc_TypeError, "globals must be a dict" ); + goto exit; + } + + if( locals && !PyMapping_Check( locals ) ) + { + PyErr_SetString( PyExc_TypeError, "locals must be a mapping" ); + goto exit; + } + if( locals == Py_None ) + locals = NULL; + + PyObject** c_args = NULL; + Py_ssize_t c_args_len = 0; + if( args ) + { + if ( !PyTuple_Check( args ) ) + { + PyErr_SetString( PyExc_TypeError, "args must be a tuple" ); + goto exit; + } else { + c_args = &PyTuple_GET_ITEM( args, 0 ); + c_args_len = PyTuple_Size( args ); + } + } + + Py_ssize_t c_kwargs_len = 0; + if( kwargs ) + { + if( !PyDict_Check( kwargs ) ) + { + PyErr_SetString( PyExc_TypeError, "keywords must be a dict" ); + goto exit; + } else { + c_kwargs_len = PyDict_Size( kwargs ); + if( c_kwargs_len > 0 ) + { + c_kwargs = PyMem_NEW( PyObject*, 2 * c_kwargs_len ); + if( !c_kwargs ) + { + PyErr_NoMemory(); + goto exit; + } + Py_ssize_t i = 0; + Py_ssize_t pos = 0; + while( PyDict_Next( kwargs, &pos, &c_kwargs[ i ], &c_kwargs[ i + 1 ] ) ) + i += 2; + c_kwargs_len = i / 2; + /* XXX This is broken if the caller deletes dict items! */ + } + } + } + + + PyObject** c_defaults = NULL; + Py_ssize_t c_defaults_len = 0; + if( ( defaults ) && PyTuple_Check( defaults ) ) + { + c_defaults = &PyTuple_GET_ITEM( defaults, 0 ); + c_defaults_len = PyTuple_Size( defaults ); + } + + if( ( kw_defaults ) && !PyDict_Check( kw_defaults ) ) + { + PyErr_SetString( PyExc_TypeError, "kw_defaults must be a dict" ); + goto exit; + } + + if( ( closure ) && !PyTuple_Check( closure ) ) + { + PyErr_SetString( PyExc_TypeError, "closure must be a tuple of cells" ); + goto exit; + } + + + result = PyEval_EvalCodeEx( + code, + globals, + locals, + c_args, + c_args_len, + c_kwargs, + c_kwargs_len, + c_defaults, + c_defaults_len, + kw_defaults, + closure + ); + +exit: + if( c_kwargs ) + PyMem_DEL( c_kwargs ); + + return result; +} + static PyObject * get_feature_macros(PyObject *self, PyObject *Py_UNUSED(args)) { @@ -3294,6 +3418,7 @@ static PyMethodDef TestMethods[] = { {"set_exc_info", test_set_exc_info, METH_VARARGS}, {"argparsing", argparsing, METH_VARARGS}, {"code_newempty", code_newempty, METH_VARARGS}, + {"eval_code_ex", eval_eval_code_ex, METH_VARARGS}, {"make_exception_with_doc", _PyCFunction_CAST(make_exception_with_doc), METH_VARARGS | METH_KEYWORDS}, {"make_memoryview_from_NULL_pointer", make_memoryview_from_NULL_pointer, From ce402c3877bc14606ee36dc53b92c9d66f9a1997 Mon Sep 17 00:00:00 2001 From: Matthieu Dartiailh Date: Sun, 5 Feb 2023 21:42:14 +0100 Subject: [PATCH 3/5] test: attempt to abide by PEP 7 --- Modules/_testcapimodule.c | 80 +++++++++++++++++++++++---------------- 1 file changed, 47 insertions(+), 33 deletions(-) diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 05e2dd680fd28b..29c9200ec7b673 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2988,66 +2988,81 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) PyObject** c_kwargs = NULL; - if( !PyArg_UnpackTuple( pos_args, "eval_code_ex", 2, 8, &code, &globals, &locals, &args, &kwargs, &defaults, &kw_defaults, &closure) ) + if (!PyArg_UnpackTuple(pos_args, + "eval_code_ex", + 2, + 8, + &code, + &globals, + &locals, + &args, + &kwargs, + &defaults, + &kw_defaults, + &closure)) { goto exit; } - if( !PyCode_Check( code ) ) - { - PyErr_SetString( PyExc_TypeError, "code must be a Python code object" ); + if (!PyCode_Check(code)) { + PyErr_SetString(PyExc_TypeError, + "code must be a Python code object"); goto exit; } - if( !PyDict_Check( globals ) ) - { - PyErr_SetString( PyExc_TypeError, "globals must be a dict" ); + if (!PyDict_Check(globals)) { + PyErr_SetString(PyExc_TypeError, "globals must be a dict"); goto exit; } - if( locals && !PyMapping_Check( locals ) ) - { - PyErr_SetString( PyExc_TypeError, "locals must be a mapping" ); + if (locals && !PyMapping_Check(locals)) { + PyErr_SetString(PyExc_TypeError, "locals must be a mapping"); goto exit; } - if( locals == Py_None ) + if (locals == Py_None) { locals = NULL; + } PyObject** c_args = NULL; Py_ssize_t c_args_len = 0; - if( args ) + + if (args) { - if ( !PyTuple_Check( args ) ) - { + if (!PyTuple_Check( args )) { PyErr_SetString( PyExc_TypeError, "args must be a tuple" ); goto exit; } else { - c_args = &PyTuple_GET_ITEM( args, 0 ); - c_args_len = PyTuple_Size( args ); + c_args = &PyTuple_GET_ITEM(args, 0); + c_args_len = PyTuple_Size(args); } } Py_ssize_t c_kwargs_len = 0; + if( kwargs ) { - if( !PyDict_Check( kwargs ) ) - { - PyErr_SetString( PyExc_TypeError, "keywords must be a dict" ); + if (!PyDict_Check(kwargs)) { + PyErr_SetString(PyExc_TypeError, "keywords must be a dict"); goto exit; } else { - c_kwargs_len = PyDict_Size( kwargs ); - if( c_kwargs_len > 0 ) - { - c_kwargs = PyMem_NEW( PyObject*, 2 * c_kwargs_len ); - if( !c_kwargs ) - { + c_kwargs_len = PyDict_Size(kwargs); + if (c_kwargs_len > 0) { + c_kwargs = PyMem_NEW(PyObject*, 2 * c_kwargs_len); + if (!c_kwargs) { PyErr_NoMemory(); goto exit; } + Py_ssize_t i = 0; Py_ssize_t pos = 0; - while( PyDict_Next( kwargs, &pos, &c_kwargs[ i ], &c_kwargs[ i + 1 ] ) ) + + while (PyDict_Next(kwargs, + &pos, + &c_kwargs[ i ], + &c_kwargs[ i + 1 ])) + { i += 2; + } c_kwargs_len = i / 2; /* XXX This is broken if the caller deletes dict items! */ } @@ -3057,20 +3072,18 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) PyObject** c_defaults = NULL; Py_ssize_t c_defaults_len = 0; - if( ( defaults ) && PyTuple_Check( defaults ) ) - { + + if (defaults && PyTuple_Check(defaults)) { c_defaults = &PyTuple_GET_ITEM( defaults, 0 ); c_defaults_len = PyTuple_Size( defaults ); } - if( ( kw_defaults ) && !PyDict_Check( kw_defaults ) ) - { + if (kw_defaults && !PyDict_Check(kw_defaults)) { PyErr_SetString( PyExc_TypeError, "kw_defaults must be a dict" ); goto exit; } - if( ( closure ) && !PyTuple_Check( closure ) ) - { + if (closure && !PyTuple_Check(closure)) { PyErr_SetString( PyExc_TypeError, "closure must be a tuple of cells" ); goto exit; } @@ -3091,8 +3104,9 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) ); exit: - if( c_kwargs ) + if (c_kwargs) { PyMem_DEL( c_kwargs ); + } return result; } From a5ece93cdedcbf8160688bfb2cb8a64ff4da846f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Mon, 6 Feb 2023 20:13:39 +0100 Subject: [PATCH 4/5] Add Blurb --- .../2023-02-06-20-13-36.gh-issue-92173.RQE0mk.rst | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-02-06-20-13-36.gh-issue-92173.RQE0mk.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-02-06-20-13-36.gh-issue-92173.RQE0mk.rst b/Misc/NEWS.d/next/Core and Builtins/2023-02-06-20-13-36.gh-issue-92173.RQE0mk.rst new file mode 100644 index 00000000000000..2d991f6ca21b6f --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-02-06-20-13-36.gh-issue-92173.RQE0mk.rst @@ -0,0 +1,8 @@ +macOS #.. section: IDLE #.. section: Tools/Demos #.. section: C API + +# Write your Misc/NEWS entry below. It should be a simple ReST paragraph. # +Don't start with "- Issue #: " or "- gh-issue-: " or that sort of +stuff. +########################################################################### + +Fix the ``defs`` and ``kwdefs`` arguments to :c:func:`PyEval_EvalCodeEx`. From f14a0e58fbccc57da0b4ebc436246bdbf91cc5be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20Langa?= Date: Mon, 6 Feb 2023 21:22:15 +0100 Subject: [PATCH 5/5] Fix refleak and formatting --- Lib/test/test_capi/test_eval_code_ex.py | 8 ++--- Modules/_testcapimodule.c | 46 ++++++++++++------------- Python/ceval.c | 3 -- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_capi/test_eval_code_ex.py b/Lib/test/test_capi/test_eval_code_ex.py index 02cf8639689bf1..2d28e5289eff94 100644 --- a/Lib/test/test_capi/test_eval_code_ex.py +++ b/Lib/test/test_capi/test_eval_code_ex.py @@ -21,16 +21,16 @@ def f(): # return def test_with_args(self): - def f(a): + def f(a, b, c): return a - self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (1,)), 1) + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (1, 2, 3)), 1) def test_with_kwargs(self): - def f(a): + def f(a, b, c): return a - self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), dict(a=1)), 1) + self.assertEqual(_testcapi.eval_code_ex(f.__code__, {}, {}, (), dict(a=1, b=2, c=3)), 1) def test_with_default(self): def f(a): diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index d3106a6215a0a1..dc73a850777f00 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -2201,7 +2201,7 @@ dict_get_version(PyObject *self, PyObject *args) return NULL; _Py_COMP_DIAG_PUSH - _Py_COMP_DIAG_IGNORE_DEPR_DECLS + _Py_COMP_DIAG_IGNORE_DEPR_DECLS version = dict->ma_version_tag; _Py_COMP_DIAG_POP @@ -3031,17 +3031,17 @@ eval_get_func_desc(PyObject *self, PyObject *func) static PyObject * eval_eval_code_ex(PyObject *mod, PyObject *pos_args) { - PyObject* result = NULL; - PyObject* code; - PyObject* globals; - PyObject* locals = NULL; - PyObject* args = NULL; - PyObject* kwargs = NULL; - PyObject* defaults = NULL; - PyObject* kw_defaults = NULL; - PyObject* closure = NULL; + PyObject *result = NULL; + PyObject *code; + PyObject *globals; + PyObject *locals = NULL; + PyObject *args = NULL; + PyObject *kwargs = NULL; + PyObject *defaults = NULL; + PyObject *kw_defaults = NULL; + PyObject *closure = NULL; - PyObject** c_kwargs = NULL; + PyObject **c_kwargs = NULL; if (!PyArg_UnpackTuple(pos_args, "eval_code_ex", @@ -3078,13 +3078,13 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) locals = NULL; } - PyObject** c_args = NULL; + PyObject **c_args = NULL; Py_ssize_t c_args_len = 0; if (args) { - if (!PyTuple_Check( args )) { - PyErr_SetString( PyExc_TypeError, "args must be a tuple" ); + if (!PyTuple_Check(args)) { + PyErr_SetString(PyExc_TypeError, "args must be a tuple"); goto exit; } else { c_args = &PyTuple_GET_ITEM(args, 0); @@ -3094,7 +3094,7 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) Py_ssize_t c_kwargs_len = 0; - if( kwargs ) + if (kwargs) { if (!PyDict_Check(kwargs)) { PyErr_SetString(PyExc_TypeError, "keywords must be a dict"); @@ -3113,8 +3113,8 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) while (PyDict_Next(kwargs, &pos, - &c_kwargs[ i ], - &c_kwargs[ i + 1 ])) + &c_kwargs[i], + &c_kwargs[i + 1])) { i += 2; } @@ -3125,21 +3125,21 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) } - PyObject** c_defaults = NULL; + PyObject **c_defaults = NULL; Py_ssize_t c_defaults_len = 0; if (defaults && PyTuple_Check(defaults)) { - c_defaults = &PyTuple_GET_ITEM( defaults, 0 ); - c_defaults_len = PyTuple_Size( defaults ); + c_defaults = &PyTuple_GET_ITEM(defaults, 0); + c_defaults_len = PyTuple_Size(defaults); } if (kw_defaults && !PyDict_Check(kw_defaults)) { - PyErr_SetString( PyExc_TypeError, "kw_defaults must be a dict" ); + PyErr_SetString(PyExc_TypeError, "kw_defaults must be a dict"); goto exit; } if (closure && !PyTuple_Check(closure)) { - PyErr_SetString( PyExc_TypeError, "closure must be a tuple of cells" ); + PyErr_SetString(PyExc_TypeError, "closure must be a tuple of cells"); goto exit; } @@ -3160,7 +3160,7 @@ eval_eval_code_ex(PyObject *mod, PyObject *pos_args) exit: if (c_kwargs) { - PyMem_DEL( c_kwargs ); + PyMem_DEL(c_kwargs); } return result; diff --git a/Python/ceval.c b/Python/ceval.c index 2e6fed580dede4..ecb5bf9655553e 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -1761,9 +1761,6 @@ PyEval_EvalCodeEx(PyObject *_co, PyObject *globals, PyObject *locals, } allargs = newargs; } - for (int i = 0; i < kwcount; i++) { - PyTuple_SET_ITEM(kwnames, i, Py_NewRef(kws[2*i])); - } PyFrameConstructor constr = { .fc_globals = globals, .fc_builtins = builtins,