From ce0b4abff8bf3362570ff0d40730ea6dd4fa5656 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Mon, 7 Nov 2022 19:49:54 +0800 Subject: [PATCH 1/6] Do INCREF to tuple-packed arguments in Argument Clinic --- Lib/test/clinic.test | 3 ++- Tools/clinic/clinic.py | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/Lib/test/clinic.test b/Lib/test/clinic.test index 47e3e02490c816..f4e842f68056bd 100644 --- a/Lib/test/clinic.test +++ b/Lib/test/clinic.test @@ -3793,6 +3793,7 @@ test_vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t narg a = args[0]; __clinic_args = PyTuple_New(nargs - 1); for (Py_ssize_t i = 0; i < nargs - 1; ++i) { + Py_INCREF(args[1 + i]); PyTuple_SET_ITEM(__clinic_args, i, args[1 + i]); } return_value = test_vararg_and_posonly_impl(module, a, __clinic_args); @@ -3804,7 +3805,7 @@ exit: static PyObject * test_vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args) -/*[clinic end generated code: output=548bca3a127c22c1 input=08dc2bf7afbf1613]*/ +/*[clinic end generated code: output=c6d0dd2d43a88a7f input=08dc2bf7afbf1613]*/ /*[clinic input] test_vararg diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index a8687e3470a185..f1c1d8ea769c3c 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -956,12 +956,14 @@ def parser_body(prototype, *fields, declarations=''): parser_code.append(normalize_snippet(""" %s = PyTuple_New(%s); for (Py_ssize_t i = 0; i < %s; ++i) {{ + Py_INCREF(args[%d + i]); PyTuple_SET_ITEM(%s, i, args[%d + i]); }} """ % ( p.converter.parser_name, left_args, left_args, + max_pos, p.converter.parser_name, max_pos ), indent=4)) From 2230e7b4f8c4cfae0a4e0d254ed1e04524796dcf Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Tue, 8 Nov 2022 11:19:49 +0800 Subject: [PATCH 2/6] Add News --- .../next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst diff --git a/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst b/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst new file mode 100644 index 00000000000000..6901f27e3f6b26 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst @@ -0,0 +1 @@ +Fix refcount error when arguments are packed to tuple in ArgumentCclinic. From c17b6bcc4202b3ce4f0f392cac65b70c5dd7b27b Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 14:56:07 +0800 Subject: [PATCH 3/6] Fix typo --- .../next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst b/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst index 6901f27e3f6b26..f98c181cc9c54b 100644 --- a/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst +++ b/Misc/NEWS.d/next/Library/2022-11-08-11-18-51.gh-issue-64490.VcBgrN.rst @@ -1 +1 @@ -Fix refcount error when arguments are packed to tuple in ArgumentCclinic. +Fix refcount error when arguments are packed to tuple in Argument Clinic. From ecba0f06d471189701c95a3b8d489175c82ddce0 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 15:12:18 +0800 Subject: [PATCH 4/6] Add Argument Clinic functional test cases --- Lib/test/test_clinic.py | 14 +++++++ Modules/_testclinic.c | 42 +++++++++++++++++++ Modules/clinic/_testclinic.c.h | 74 +++++++++++++++++++++++++++++++++- 3 files changed, 129 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 7c1bd1c10d2ab6..a590fa50aab04f 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1211,6 +1211,20 @@ def test_keyword_only_parameter(self): ac_tester.keyword_only_parameter(1) self.assertEqual(ac_tester.keyword_only_parameter(a=1), (1,)) + def test_vararg_and_posonly(self): + with self.assertRaises(TypeError): + ac_tester.vararg_and_posonly() + with self.assertRaises(TypeError): + ac_tester.vararg_and_posonly(1, b=2) + self.assertEqual(ac_tester.vararg_and_posonly(1, 2, 3, 4), (1, (2, 3, 4))) + + def test_gh_99233_refcount(self): + arg = '*A unique string is not referenced by anywhere else.*' + arg_refcount_origin = sys.getrefcount(arg) + ac_tester.gh_99233_refcount(arg) + arg_refcount_after = sys.getrefcount(arg) + self.assertEqual(arg_refcount_origin, arg_refcount_after) + if __name__ == "__main__": unittest.main() diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index c9858e96445714..1c02056c7d3c0c 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -892,6 +892,46 @@ keyword_only_parameter_impl(PyObject *module, PyObject *a) } +/*[clinic input] +vararg_and_posonly + + a: object + *args: object + / + +[clinic start generated code]*/ + +static PyObject * +vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args) +/*[clinic end generated code: output=42792f799465a14d input=defe017b19ba52e8]*/ +{ + return pack_arguments_newref(2, a, args); +} + + +/*[clinic input] +gh_99233_refcount + + *args: object + / + +Proof-of-concept of GH-99233 refcount error bug. + +While AC-generated code is packing varargs to a tuple, the arguments' refcounts are not increased. +So all the packed arguments' refcounts are decreased 1 improperly when the tuple is released later. + +Call this function with whatever arguments and check if the arguments' refcount is correct. + +[clinic start generated code]*/ + +static PyObject * +gh_99233_refcount_impl(PyObject *module, PyObject *args) +/*[clinic end generated code: output=585855abfbca9a7f input=d34627c52b39ed17]*/ +{ + Py_RETURN_NONE; +} + + static PyMethodDef tester_methods[] = { TEST_EMPTY_FUNCTION_METHODDEF OBJECTS_CONVERTER_METHODDEF @@ -933,6 +973,8 @@ static PyMethodDef tester_methods[] = { POSONLY_KEYWORDS_OPT_KWONLY_OPT_METHODDEF POSONLY_OPT_KEYWORDS_OPT_KWONLY_OPT_METHODDEF KEYWORD_ONLY_PARAMETER_METHODDEF + VARARG_AND_POSONLY_METHODDEF + GH_99233_REFCOUNT_METHODDEF {NULL, NULL} }; diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index b0ac4c2eef8340..eedd10ec8e0039 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2288,4 +2288,76 @@ keyword_only_parameter(PyObject *module, PyObject *const *args, Py_ssize_t nargs exit: return return_value; } -/*[clinic end generated code: output=a9212f8e6ba18bba input=a9049054013a1b77]*/ + +PyDoc_STRVAR(vararg_and_posonly__doc__, +"vararg_and_posonly($module, a, /, *args)\n" +"--\n" +"\n"); + +#define VARARG_AND_POSONLY_METHODDEF \ + {"vararg_and_posonly", _PyCFunction_CAST(vararg_and_posonly), METH_FASTCALL, vararg_and_posonly__doc__}, + +static PyObject * +vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args); + +static PyObject * +vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + PyObject *a; + PyObject *__clinic_args = NULL; + + if (!_PyArg_CheckPositional("vararg_and_posonly", nargs, 1, PY_SSIZE_T_MAX)) { + goto exit; + } + a = args[0]; + __clinic_args = PyTuple_New(nargs - 1); + for (Py_ssize_t i = 0; i < nargs - 1; ++i) { + Py_INCREF(args[1 + i]); + PyTuple_SET_ITEM(__clinic_args, i, args[1 + i]); + } + return_value = vararg_and_posonly_impl(module, a, __clinic_args); + +exit: + Py_XDECREF(__clinic_args); + return return_value; +} + +PyDoc_STRVAR(gh_99233_refcount__doc__, +"gh_99233_refcount($module, /, *args)\n" +"--\n" +"\n" +"Proof-of-concept of GH-99233 refcount error bug.\n" +"\n" +"While AC-generated code is packing varargs to a tuple, the arguments\' refcounts are not increased.\n" +"So all the packed arguments\' refcounts are decreased 1 improperly when the tuple is released later.\n" +"\n" +"Call this function with whatever arguments and check if the arguments\' refcount is correct."); + +#define GH_99233_REFCOUNT_METHODDEF \ + {"gh_99233_refcount", _PyCFunction_CAST(gh_99233_refcount), METH_FASTCALL, gh_99233_refcount__doc__}, + +static PyObject * +gh_99233_refcount_impl(PyObject *module, PyObject *args); + +static PyObject * +gh_99233_refcount(PyObject *module, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + PyObject *__clinic_args = NULL; + + if (!_PyArg_CheckPositional("gh_99233_refcount", nargs, 0, PY_SSIZE_T_MAX)) { + goto exit; + } + __clinic_args = PyTuple_New(nargs - 0); + for (Py_ssize_t i = 0; i < nargs - 0; ++i) { + Py_INCREF(args[0 + i]); + PyTuple_SET_ITEM(__clinic_args, i, args[0 + i]); + } + return_value = gh_99233_refcount_impl(module, __clinic_args); + +exit: + Py_XDECREF(__clinic_args); + return return_value; +} +/*[clinic end generated code: output=be283ebc1e845c8d input=a9049054013a1b77]*/ From b652c0c10edd0cf95a234ecb9f2011f6f02b63dc Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 19:31:35 +0800 Subject: [PATCH 5/6] Delete unnecessary function doc --- Modules/_testclinic.c | 7 +------ Modules/clinic/_testclinic.c.h | 9 ++------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index 1c02056c7d3c0c..a23ece2ae0355b 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -917,16 +917,11 @@ gh_99233_refcount Proof-of-concept of GH-99233 refcount error bug. -While AC-generated code is packing varargs to a tuple, the arguments' refcounts are not increased. -So all the packed arguments' refcounts are decreased 1 improperly when the tuple is released later. - -Call this function with whatever arguments and check if the arguments' refcount is correct. - [clinic start generated code]*/ static PyObject * gh_99233_refcount_impl(PyObject *module, PyObject *args) -/*[clinic end generated code: output=585855abfbca9a7f input=d34627c52b39ed17]*/ +/*[clinic end generated code: output=585855abfbca9a7f input=85f5fb47ac91a626]*/ { Py_RETURN_NONE; } diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index eedd10ec8e0039..78b361f19c61eb 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2327,12 +2327,7 @@ PyDoc_STRVAR(gh_99233_refcount__doc__, "gh_99233_refcount($module, /, *args)\n" "--\n" "\n" -"Proof-of-concept of GH-99233 refcount error bug.\n" -"\n" -"While AC-generated code is packing varargs to a tuple, the arguments\' refcounts are not increased.\n" -"So all the packed arguments\' refcounts are decreased 1 improperly when the tuple is released later.\n" -"\n" -"Call this function with whatever arguments and check if the arguments\' refcount is correct."); +"Proof-of-concept of GH-99233 refcount error bug."); #define GH_99233_REFCOUNT_METHODDEF \ {"gh_99233_refcount", _PyCFunction_CAST(gh_99233_refcount), METH_FASTCALL, gh_99233_refcount__doc__}, @@ -2360,4 +2355,4 @@ gh_99233_refcount(PyObject *module, PyObject *const *args, Py_ssize_t nargs) Py_XDECREF(__clinic_args); return return_value; } -/*[clinic end generated code: output=be283ebc1e845c8d input=a9049054013a1b77]*/ +/*[clinic end generated code: output=2ca3fb3a99fe5800 input=a9049054013a1b77]*/ From a876e94a31e425b360cb0f18bb0895a67670af88 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 19:52:37 +0800 Subject: [PATCH 6/6] Replace Py_INCREF with Py_NewRef --- Lib/test/clinic.test | 5 ++--- Modules/clinic/_testclinic.c.h | 8 +++----- Tools/clinic/clinic.py | 4 +--- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/Lib/test/clinic.test b/Lib/test/clinic.test index f4e842f68056bd..7b804e8576aedc 100644 --- a/Lib/test/clinic.test +++ b/Lib/test/clinic.test @@ -3793,8 +3793,7 @@ test_vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t narg a = args[0]; __clinic_args = PyTuple_New(nargs - 1); for (Py_ssize_t i = 0; i < nargs - 1; ++i) { - Py_INCREF(args[1 + i]); - PyTuple_SET_ITEM(__clinic_args, i, args[1 + i]); + PyTuple_SET_ITEM(__clinic_args, i, Py_NewRef(args[1 + i])); } return_value = test_vararg_and_posonly_impl(module, a, __clinic_args); @@ -3805,7 +3804,7 @@ exit: static PyObject * test_vararg_and_posonly_impl(PyObject *module, PyObject *a, PyObject *args) -/*[clinic end generated code: output=c6d0dd2d43a88a7f input=08dc2bf7afbf1613]*/ +/*[clinic end generated code: output=081a953b8cbe7617 input=08dc2bf7afbf1613]*/ /*[clinic input] test_vararg diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index 78b361f19c61eb..eb425821e9cb3f 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2313,8 +2313,7 @@ vararg_and_posonly(PyObject *module, PyObject *const *args, Py_ssize_t nargs) a = args[0]; __clinic_args = PyTuple_New(nargs - 1); for (Py_ssize_t i = 0; i < nargs - 1; ++i) { - Py_INCREF(args[1 + i]); - PyTuple_SET_ITEM(__clinic_args, i, args[1 + i]); + PyTuple_SET_ITEM(__clinic_args, i, Py_NewRef(args[1 + i])); } return_value = vararg_and_posonly_impl(module, a, __clinic_args); @@ -2346,8 +2345,7 @@ gh_99233_refcount(PyObject *module, PyObject *const *args, Py_ssize_t nargs) } __clinic_args = PyTuple_New(nargs - 0); for (Py_ssize_t i = 0; i < nargs - 0; ++i) { - Py_INCREF(args[0 + i]); - PyTuple_SET_ITEM(__clinic_args, i, args[0 + i]); + PyTuple_SET_ITEM(__clinic_args, i, Py_NewRef(args[0 + i])); } return_value = gh_99233_refcount_impl(module, __clinic_args); @@ -2355,4 +2353,4 @@ gh_99233_refcount(PyObject *module, PyObject *const *args, Py_ssize_t nargs) Py_XDECREF(__clinic_args); return return_value; } -/*[clinic end generated code: output=2ca3fb3a99fe5800 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=a5c9f181f3a32d85 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f1c1d8ea769c3c..94e17ee9c7dfda 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -956,14 +956,12 @@ def parser_body(prototype, *fields, declarations=''): parser_code.append(normalize_snippet(""" %s = PyTuple_New(%s); for (Py_ssize_t i = 0; i < %s; ++i) {{ - Py_INCREF(args[%d + i]); - PyTuple_SET_ITEM(%s, i, args[%d + i]); + PyTuple_SET_ITEM(%s, i, Py_NewRef(args[%d + i])); }} """ % ( p.converter.parser_name, left_args, left_args, - max_pos, p.converter.parser_name, max_pos ), indent=4))