Issue #26168: Fixed possible refleaks in failing Py_BuildValue() with the "N"
format unit.
diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 2029265..944e960 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -16,6 +16,11 @@
# Skip this test if the _testcapi module isn't available.
_testcapi = support.import_module('_testcapi')
+class CAPITest(unittest.TestCase):
+
+ def test_buildvalue_N(self):
+ _testcapi.test_buildvalue_N()
+
@unittest.skipUnless(threading, 'Threading required for this test.')
class TestPendingCalls(unittest.TestCase):
@@ -132,7 +137,7 @@
except _testcapi.error:
raise support.TestFailed, sys.exc_info()[1]
- support.run_unittest(TestPendingCalls, TestThreadState)
+ support.run_unittest(CAPITest, TestPendingCalls, TestThreadState)
if __name__ == "__main__":
test_main()
diff --git a/Misc/NEWS b/Misc/NEWS
index e825a2e..86fef45 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@
Core and Builtins
-----------------
+- Issue #26168: Fixed possible refleaks in failing Py_BuildValue() with the "N"
+ format unit.
+
- Issue #27039: Fixed bytearray.remove() for values greater than 127. Patch by
Joe Jevnik.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index 95a0920..69f59fc 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -931,6 +931,100 @@
#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)
{
if (args == NULL) {
@@ -2414,6 +2508,7 @@
{"test_with_docstring", (PyCFunction)test_with_docstring, METH_NOARGS,
PyDoc_STR("This is a pretty normal docstring.")},
+ {"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 8bdec8b..ebe62c3 100644
--- a/Python/modsupport.c
+++ b/Python/modsupport.c
@@ -156,48 +156,83 @@
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;
}
@@ -207,29 +242,24 @@
{
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,
@@ -257,27 +287,23 @@
{
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 = 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,