diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 96666d1d167..d262d991a25 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -236,6 +236,9 @@ class CAPITest(unittest.TestCase): 'return_result_with_error.* ' 'returned a result with an error set') + def test_buildvalue_N(self): + _testcapi.test_buildvalue_N() + @unittest.skipUnless(threading, 'Threading required for this test.') class TestPendingCalls(unittest.TestCase): diff --git a/Misc/NEWS b/Misc/NEWS index 303563f6132..ef1a65fdb82 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -10,6 +10,9 @@ Release date: tba Core and Builtins ----------------- +- Issue #26168: Fixed possible refleaks in failing Py_BuildValue() with the "N" + format unit. + - Issue #26991: Fix possible refleak when creating a function with annotations. - Issue #27039: Fixed bytearray.remove() for values greater than 127. Patch by diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index d86af158a29..3810e944ce4 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -872,6 +872,100 @@ test_L_code(PyObject *self) #endif /* ifdef HAVE_LONG_LONG */ +static PyObject * +return_none(void *unused) +{ + Py_RETURN_NONE; +} + +static PyObject * +raise_error(void *unused) +{ + PyErr_SetNone(PyExc_ValueError); + return NULL; +} + +static int +test_buildvalue_N_error(const char *fmt) +{ + PyObject *arg, *res; + + arg = PyList_New(0); + if (arg == NULL) { + return -1; + } + + Py_INCREF(arg); + res = Py_BuildValue(fmt, return_none, NULL, arg); + if (res == NULL) { + return -1; + } + Py_DECREF(res); + if (Py_REFCNT(arg) != 1) { + PyErr_Format(TestError, "test_buildvalue_N: " + "arg was not decrefed in successful " + "Py_BuildValue(\"%s\")", fmt); + return -1; + } + + Py_INCREF(arg); + res = Py_BuildValue(fmt, raise_error, NULL, arg); + if (res != NULL || !PyErr_Occurred()) { + PyErr_Format(TestError, "test_buildvalue_N: " + "Py_BuildValue(\"%s\") didn't complain", fmt); + return -1; + } + PyErr_Clear(); + if (Py_REFCNT(arg) != 1) { + PyErr_Format(TestError, "test_buildvalue_N: " + "arg was not decrefed in failed " + "Py_BuildValue(\"%s\")", fmt); + return -1; + } + Py_DECREF(arg); + return 0; +} + +static PyObject * +test_buildvalue_N(PyObject *self, PyObject *noargs) +{ + PyObject *arg, *res; + + arg = PyList_New(0); + if (arg == NULL) { + return NULL; + } + Py_INCREF(arg); + res = Py_BuildValue("N", arg); + if (res == NULL) { + return NULL; + } + if (res != arg) { + return raiseTestError("test_buildvalue_N", + "Py_BuildValue(\"N\") returned wrong result"); + } + if (Py_REFCNT(arg) != 2) { + return raiseTestError("test_buildvalue_N", + "arg was not decrefed in Py_BuildValue(\"N\")"); + } + Py_DECREF(res); + Py_DECREF(arg); + + if (test_buildvalue_N_error("O&N") < 0) + return NULL; + if (test_buildvalue_N_error("(O&N)") < 0) + return NULL; + if (test_buildvalue_N_error("[O&N]") < 0) + return NULL; + if (test_buildvalue_N_error("{O&N}") < 0) + return NULL; + if (test_buildvalue_N_error("{()O&(())N}") < 0) + return NULL; + + Py_RETURN_NONE; +} + + static PyObject * get_args(PyObject *self, PyObject *args) { @@ -3728,6 +3822,7 @@ static PyMethodDef TestMethods[] = { {"test_pep3118_obsolete_write_locks", (PyCFunction)test_pep3118_obsolete_write_locks, METH_NOARGS}, #endif {"getbuffer_with_null_view", getbuffer_with_null_view, METH_O}, + {"test_buildvalue_N", test_buildvalue_N, METH_NOARGS}, {"get_args", get_args, METH_VARARGS}, {"get_kwargs", (PyCFunction)get_kwargs, METH_VARARGS|METH_KEYWORDS}, {"getargs_tuple", getargs_tuple, METH_VARARGS}, diff --git a/Python/modsupport.c b/Python/modsupport.c index 6c938ddd797..0d093711f5f 100644 --- a/Python/modsupport.c +++ b/Python/modsupport.c @@ -63,48 +63,84 @@ static PyObject *do_mkdict(const char**, va_list *, int, int, int); static PyObject *do_mkvalue(const char**, va_list *, int); +static void +do_ignore(const char **p_format, va_list *p_va, int endchar, int n, int flags) +{ + PyObject *v; + int i; + assert(PyErr_Occurred()); + v = PyTuple_New(n); + for (i = 0; i < n; i++) { + PyObject *exception, *value, *tb, *w; + + PyErr_Fetch(&exception, &value, &tb); + w = do_mkvalue(p_format, p_va, flags); + PyErr_Restore(exception, value, tb); + if (w != NULL) { + if (v != NULL) { + PyTuple_SET_ITEM(v, i, w); + } + else { + Py_DECREF(w); + } + } + } + Py_XDECREF(v); + if (**p_format != endchar) { + PyErr_SetString(PyExc_SystemError, + "Unmatched paren in format"); + return; + } + if (endchar) + ++*p_format; +} + static PyObject * do_mkdict(const char **p_format, va_list *p_va, int endchar, int n, int flags) { PyObject *d; int i; - int itemfailed = 0; if (n < 0) return NULL; - if ((d = PyDict_New()) == NULL) + if (n % 2) { + PyErr_SetString(PyExc_SystemError, + "Bad dict format"); + do_ignore(p_format, p_va, endchar, n, flags); return NULL; + } /* Note that we can't bail immediately on error as this will leak refcounts on any 'N' arguments. */ + if ((d = PyDict_New()) == NULL) { + do_ignore(p_format, p_va, endchar, n, flags); + return NULL; + } for (i = 0; i < n; i+= 2) { PyObject *k, *v; - int err; + k = do_mkvalue(p_format, p_va, flags); if (k == NULL) { - itemfailed = 1; - Py_INCREF(Py_None); - k = Py_None; - } - v = do_mkvalue(p_format, p_va, flags); - if (v == NULL) { - itemfailed = 1; - Py_INCREF(Py_None); - v = Py_None; - } - err = PyDict_SetItem(d, k, v); - Py_DECREF(k); - Py_DECREF(v); - if (err < 0 || itemfailed) { + do_ignore(p_format, p_va, endchar, n - i - 1, flags); Py_DECREF(d); return NULL; } + v = do_mkvalue(p_format, p_va, flags); + if (v == NULL || PyDict_SetItem(d, k, v) < 0) { + do_ignore(p_format, p_va, endchar, n - i - 2, flags); + Py_DECREF(k); + Py_XDECREF(v); + Py_DECREF(d); + return NULL; + } + Py_DECREF(k); + Py_DECREF(v); } - if (d != NULL && **p_format != endchar) { + if (**p_format != endchar) { Py_DECREF(d); - d = NULL; PyErr_SetString(PyExc_SystemError, "Unmatched paren in format"); + return NULL; } - else if (endchar) + if (endchar) ++*p_format; return d; } @@ -114,29 +150,24 @@ do_mklist(const char **p_format, va_list *p_va, int endchar, int n, int flags) { PyObject *v; int i; - int itemfailed = 0; if (n < 0) return NULL; - v = PyList_New(n); - if (v == NULL) - return NULL; /* Note that we can't bail immediately on error as this will leak refcounts on any 'N' arguments. */ + v = PyList_New(n); + if (v == NULL) { + do_ignore(p_format, p_va, endchar, n, flags); + return NULL; + } for (i = 0; i < n; i++) { PyObject *w = do_mkvalue(p_format, p_va, flags); if (w == NULL) { - itemfailed = 1; - Py_INCREF(Py_None); - w = Py_None; + do_ignore(p_format, p_va, endchar, n - i - 1, flags); + Py_DECREF(v); + return NULL; } PyList_SET_ITEM(v, i, w); } - - if (itemfailed) { - /* do_mkvalue() should have already set an error */ - Py_DECREF(v); - return NULL; - } if (**p_format != endchar) { Py_DECREF(v); PyErr_SetString(PyExc_SystemError, @@ -153,37 +184,23 @@ do_mktuple(const char **p_format, va_list *p_va, int endchar, int n, int flags) { PyObject *v; int i; - int itemfailed = 0; if (n < 0) return NULL; - if ((v = PyTuple_New(n)) == NULL) - return NULL; /* Note that we can't bail immediately on error as this will leak refcounts on any 'N' arguments. */ + if ((v = PyTuple_New(n)) == NULL) { + do_ignore(p_format, p_va, endchar, n, flags); + return NULL; + } for (i = 0; i < n; i++) { - PyObject *w; - - if (itemfailed) { - PyObject *exception, *value, *tb; - PyErr_Fetch(&exception, &value, &tb); - w = do_mkvalue(p_format, p_va, flags); - PyErr_Restore(exception, value, tb); - } - else { - w = do_mkvalue(p_format, p_va, flags); - } + PyObject *w = do_mkvalue(p_format, p_va, flags); if (w == NULL) { - itemfailed = 1; - Py_INCREF(Py_None); - w = Py_None; + do_ignore(p_format, p_va, endchar, n - i - 1, flags); + Py_DECREF(v); + return NULL; } PyTuple_SET_ITEM(v, i, w); } - if (itemfailed) { - /* do_mkvalue() should have already set an error */ - Py_DECREF(v); - return NULL; - } if (**p_format != endchar) { Py_DECREF(v); PyErr_SetString(PyExc_SystemError,