From 06b9d4ab7d81a8b1f7b47c793542fb419147ea22 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Tue, 8 Nov 2022 15:43:57 +0800 Subject: [PATCH 01/11] Fix double-free bug in Argument Clinic str_converter generated code --- Lib/test/clinic.test | 15 +++++++-------- Tools/clinic/clinic.py | 24 +++++++++++++++++++++++- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/Lib/test/clinic.test b/Lib/test/clinic.test index 47e3e02490c816..fd63d71668342f 100644 --- a/Lib/test/clinic.test +++ b/Lib/test/clinic.test @@ -1740,29 +1740,28 @@ test_str_converter_encoding(PyObject *module, PyObject *const *args, Py_ssize_t goto exit; } return_value = test_str_converter_encoding_impl(module, a, b, c, d, d_length, e, e_length); - -exit: - /* Cleanup for a */ + /* Post operation for a */ if (a) { PyMem_FREE(a); } - /* Cleanup for b */ + /* Post operation for b */ if (b) { PyMem_FREE(b); } - /* Cleanup for c */ + /* Post operation for c */ if (c) { PyMem_FREE(c); } - /* Cleanup for d */ + /* Post operation for d */ if (d) { PyMem_FREE(d); } - /* Cleanup for e */ + /* Post operation for e */ if (e) { PyMem_FREE(e); } +exit: return return_value; } @@ -1770,7 +1769,7 @@ static PyObject * test_str_converter_encoding_impl(PyObject *module, char *a, char *b, char *c, char *d, Py_ssize_t d_length, char *e, Py_ssize_t e_length) -/*[clinic end generated code: output=8acb886a3843f3bc input=eb4c38e1f898f402]*/ +/*[clinic end generated code: output=5dfef501bb5f5e82 input=eb4c38e1f898f402]*/ /*[clinic input] diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index a8687e3470a185..34e52cdb7a4e9f 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -348,6 +348,12 @@ def __init__(self): # "goto exit" if there are any. self.return_conversion = [] + # The C statements required to do some operations + # after the end of parsing but before cleaning up. + # These operations may be, for example, memory deallocations which + # can only be done without any error happening during argument parsing. + self.post_operations = [] + # The C statements required to clean up after the impl call. self.cleanup = [] @@ -820,6 +826,7 @@ def parser_body(prototype, *fields, declarations=''): {modifications} {return_value} = {c_basename}_impl({impl_arguments}); {return_conversion} + {post_operations} {exit_label} {cleanup} @@ -1460,6 +1467,7 @@ def render_function(self, clinic, f): template_dict['impl_parameters'] = ", ".join(data.impl_parameters) template_dict['impl_arguments'] = ", ".join(data.impl_arguments) template_dict['return_conversion'] = format_escape("".join(data.return_conversion).rstrip()) + template_dict['post_operations'] = format_escape("".join(data.post_operations).rstrip()) template_dict['cleanup'] = format_escape("".join(data.cleanup)) template_dict['return_value'] = data.return_value @@ -1484,6 +1492,7 @@ def render_function(self, clinic, f): return_conversion=template_dict['return_conversion'], initializers=template_dict['initializers'], modifications=template_dict['modifications'], + post_operations=template_dict['post_operations'], cleanup=template_dict['cleanup'], ) @@ -2725,6 +2734,11 @@ def _render_non_self(self, parameter, data): # parse_arguments self.parse_argument(data.parse_arguments) + # post_operations + post_operations = self.post_operations() + if post_operations: + data.post_operations.append('/* Post operation for ' + name + ' */\n' + post_operations.rstrip() + "\n") + # cleanup cleanup = self.cleanup() if cleanup: @@ -2820,6 +2834,14 @@ def modify(self): """ return "" + def post_operations(self): + """ + The C statements required to do some operations after the end of parsing but before cleaning up. + Returns a string containing this code indented at column 0. + If no operation is necessary, returns an empty string. + """ + return "" + def cleanup(self): """ The C statements required to clean up after this variable. @@ -3416,7 +3438,7 @@ def converter_init(self, *, accept={str}, encoding=None, zeroes=False): if NoneType in accept and self.c_default == "Py_None": self.c_default = "NULL" - def cleanup(self): + def post_operations(self): if self.encoding: name = self.name return "".join(["if (", name, ") {\n PyMem_FREE(", name, ");\n}\n"]) From 67c307807324c3b8fd5dc220fed581966288adec Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Tue, 8 Nov 2022 15:55:44 +0800 Subject: [PATCH 02/11] Add News --- .../next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst diff --git a/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst b/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst new file mode 100644 index 00000000000000..4d941f96bfdf2e --- /dev/null +++ b/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst @@ -0,0 +1,3 @@ +Add "post_operations" section to Argument Clinic generated code. +Fix double-free bug in Argument Clinic str_converter generated code by +moving memory clean up out of "exit" label. From cf16400d551ff0405d8c0d50d3a219bbdd037354 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Wed, 9 Nov 2022 10:32:41 +0800 Subject: [PATCH 03/11] Use walrus operator --- Tools/clinic/clinic.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 34e52cdb7a4e9f..2f9e5d9a4eb69e 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2735,8 +2735,7 @@ def _render_non_self(self, parameter, data): self.parse_argument(data.parse_arguments) # post_operations - post_operations = self.post_operations() - if post_operations: + if post_operations := self.post_operations(): data.post_operations.append('/* Post operation for ' + name + ' */\n' + post_operations.rstrip() + "\n") # cleanup From 3ae2aeaf8f0572de1343078d860675787953c948 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 15:26:55 +0800 Subject: [PATCH 04/11] Rename "post_operations" to "post_parsing" --- ...22-11-08-15-54-43.gh-issue-99240.MhYwcz.rst | 2 +- Tools/clinic/clinic.py | 18 +++++++++--------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst b/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst index 4d941f96bfdf2e..8eceadbd86211c 100644 --- a/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst +++ b/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst @@ -1,3 +1,3 @@ -Add "post_operations" section to Argument Clinic generated code. +Add "post_parsing" section to Argument Clinic generated code. Fix double-free bug in Argument Clinic str_converter generated code by moving memory clean up out of "exit" label. diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 2f9e5d9a4eb69e..8cc272cd578e49 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -352,7 +352,7 @@ def __init__(self): # after the end of parsing but before cleaning up. # These operations may be, for example, memory deallocations which # can only be done without any error happening during argument parsing. - self.post_operations = [] + self.post_parsing = [] # The C statements required to clean up after the impl call. self.cleanup = [] @@ -826,7 +826,7 @@ def parser_body(prototype, *fields, declarations=''): {modifications} {return_value} = {c_basename}_impl({impl_arguments}); {return_conversion} - {post_operations} + {post_parsing} {exit_label} {cleanup} @@ -1467,7 +1467,7 @@ def render_function(self, clinic, f): template_dict['impl_parameters'] = ", ".join(data.impl_parameters) template_dict['impl_arguments'] = ", ".join(data.impl_arguments) template_dict['return_conversion'] = format_escape("".join(data.return_conversion).rstrip()) - template_dict['post_operations'] = format_escape("".join(data.post_operations).rstrip()) + template_dict['post_parsing'] = format_escape("".join(data.post_parsing).rstrip()) template_dict['cleanup'] = format_escape("".join(data.cleanup)) template_dict['return_value'] = data.return_value @@ -1492,7 +1492,7 @@ def render_function(self, clinic, f): return_conversion=template_dict['return_conversion'], initializers=template_dict['initializers'], modifications=template_dict['modifications'], - post_operations=template_dict['post_operations'], + post_parsing=template_dict['post_parsing'], cleanup=template_dict['cleanup'], ) @@ -2734,9 +2734,9 @@ def _render_non_self(self, parameter, data): # parse_arguments self.parse_argument(data.parse_arguments) - # post_operations - if post_operations := self.post_operations(): - data.post_operations.append('/* Post operation for ' + name + ' */\n' + post_operations.rstrip() + "\n") + # post_parsing + if post_parsing := self.post_parsing(): + data.post_parsing.append('/* Post operation for ' + name + ' */\n' + post_parsing.rstrip() + "\n") # cleanup cleanup = self.cleanup() @@ -2833,7 +2833,7 @@ def modify(self): """ return "" - def post_operations(self): + def post_parsing(self): """ The C statements required to do some operations after the end of parsing but before cleaning up. Returns a string containing this code indented at column 0. @@ -3437,7 +3437,7 @@ def converter_init(self, *, accept={str}, encoding=None, zeroes=False): if NoneType in accept and self.c_default == "Py_None": self.c_default = "NULL" - def post_operations(self): + def post_parsing(self): if self.encoding: name = self.name return "".join(["if (", name, ") {\n PyMem_FREE(", name, ");\n}\n"]) From c3595e1ac9efe5c2e86f3f1411b4f1ea7187b7ad Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 15:38:46 +0800 Subject: [PATCH 05/11] Add Argument Clinic functional test cases --- Lib/test/test_clinic.py | 15 ++++++ Modules/_testclinic.c | 88 ++++++++++++++++++++++++++++++++ Modules/clinic/_testclinic.c.h | 91 +++++++++++++++++++++++++++++++++- 3 files changed, 193 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_clinic.py b/Lib/test/test_clinic.py index 7c1bd1c10d2ab6..63ce5c46839645 100644 --- a/Lib/test/test_clinic.py +++ b/Lib/test/test_clinic.py @@ -1045,6 +1045,17 @@ def test_str_converter(self): self.assertEqual(ac_tester.str_converter('a', b'b', b'c'), ('a', 'b', 'c')) self.assertEqual(ac_tester.str_converter('a', b'b', 'c\0c'), ('a', 'b', 'c\0c')) + def test_str_converter_encoding(self): + with self.assertRaises(TypeError): + ac_tester.str_converter_encoding(1) + self.assertEqual(ac_tester.str_converter_encoding('a', 'b', 'c'), ('a', 'b', 'c')) + with self.assertRaises(TypeError): + ac_tester.str_converter_encoding('a', b'b\0b', 'c') + self.assertEqual(ac_tester.str_converter_encoding('a', b'b', bytearray([ord('c')])), ('a', 'b', 'c')) + self.assertEqual(ac_tester.str_converter_encoding('a', b'b', bytearray([ord('c'), 0, ord('c')])), + ('a', 'b', 'c\x00c')) + self.assertEqual(ac_tester.str_converter_encoding('a', b'b', b'c\x00c'), ('a', 'b', 'c\x00c')) + def test_py_buffer_converter(self): with self.assertRaises(TypeError): ac_tester.py_buffer_converter('a', 'b') @@ -1211,6 +1222,10 @@ def test_keyword_only_parameter(self): ac_tester.keyword_only_parameter(1) self.assertEqual(ac_tester.keyword_only_parameter(a=1), (1,)) + def test_gh_99240_double_free(self): + expected_error = r'gh_99240_double_free\(\) argument 2 must be encoded string without null bytes, not str' + with self.assertRaisesRegex(TypeError, expected_error): + ac_tester.gh_99240_double_free('a', '\0b') if __name__ == "__main__": unittest.main() diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index c9858e96445714..935746c6abc896 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -551,6 +551,64 @@ str_converter_impl(PyObject *module, const char *a, const char *b, } +/*[clinic input] +str_converter_encoding + + a: str(encoding="idna") + b: str(encoding="idna", accept={bytes, bytearray, str}) + c: str(encoding="idna", accept={bytes, bytearray, str}, zeroes=True) + / + +[clinic start generated code]*/ + +static PyObject * +str_converter_encoding_impl(PyObject *module, char *a, char *b, char *c, + Py_ssize_t c_length) +/*[clinic end generated code: output=af68766049248a1c input=0c5cf5159d0e870d]*/ +{ + assert(!PyErr_Occurred()); + PyObject *out[3] = {NULL,}; + int i = 0; + PyObject *arg; + + arg = PyUnicode_FromString(a); + assert(arg || PyErr_Occurred()); + if (!arg) { + goto error; + } + out[i++] = arg; + + arg = PyUnicode_FromString(b); + assert(arg || PyErr_Occurred()); + if (!arg) { + goto error; + } + out[i++] = arg; + + arg = PyUnicode_FromStringAndSize(c, c_length); + assert(arg || PyErr_Occurred()); + if (!arg) { + goto error; + } + out[i++] = arg; + + PyObject *tuple = PyTuple_New(3); + if (!tuple) { + goto error; + } + for (int j = 0; j < 3; j++) { + PyTuple_SET_ITEM(tuple, j, out[j]); + } + return tuple; + +error: + for (int j = 0; j < i; j++) { + Py_DECREF(out[j]); + } + return NULL; +} + + static PyObject * bytes_from_buffer(Py_buffer *buf) { @@ -892,6 +950,34 @@ keyword_only_parameter_impl(PyObject *module, PyObject *a) } +/*[clinic input] +gh_99240_double_free + + a: str(encoding="idna") + b: str(encoding="idna") + / + +Proof-of-concept of GH-99240 double-free bug. + +If parsing `a` successes, `a` will be assigned an address points to an allocated memory. +After that, if parsing `b` fails, the memory which `a` points to is freed by function `_PyArg_ParseStack`, +and `_PyArg_ParseStack` returns 0, then control flow goes to label "exit". +At this time, `a` is not NULL, so the memory it points to is freed again, +which cause a double-free problem and a runtime crash. + +Calling this function by gh_99240_double_free('a', '\0b') +to trigger this bug (crash). + +[clinic start generated code]*/ + +static PyObject * +gh_99240_double_free_impl(PyObject *module, char *a, char *b) +/*[clinic end generated code: output=586dc714992fe2ed input=419d3a3790de435e]*/ +{ + Py_RETURN_NONE; +} + + static PyMethodDef tester_methods[] = { TEST_EMPTY_FUNCTION_METHODDEF OBJECTS_CONVERTER_METHODDEF @@ -916,6 +1002,7 @@ static PyMethodDef tester_methods[] = { DOUBLE_CONVERTER_METHODDEF PY_COMPLEX_CONVERTER_METHODDEF STR_CONVERTER_METHODDEF + STR_CONVERTER_ENCODING_METHODDEF PY_BUFFER_CONVERTER_METHODDEF KEYWORDS_METHODDEF KEYWORDS_KWONLY_METHODDEF @@ -933,6 +1020,7 @@ static PyMethodDef tester_methods[] = { POSONLY_KEYWORDS_OPT_KWONLY_OPT_METHODDEF POSONLY_OPT_KEYWORDS_OPT_KWONLY_OPT_METHODDEF KEYWORD_ONLY_PARAMETER_METHODDEF + GH_99240_DOUBLE_FREE_METHODDEF {NULL, NULL} }; diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index b0ac4c2eef8340..1b9032856eee1a 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -1165,6 +1165,49 @@ str_converter(PyObject *module, PyObject *const *args, Py_ssize_t nargs) return return_value; } +PyDoc_STRVAR(str_converter_encoding__doc__, +"str_converter_encoding($module, a, b, c, /)\n" +"--\n" +"\n"); + +#define STR_CONVERTER_ENCODING_METHODDEF \ + {"str_converter_encoding", _PyCFunction_CAST(str_converter_encoding), METH_FASTCALL, str_converter_encoding__doc__}, + +static PyObject * +str_converter_encoding_impl(PyObject *module, char *a, char *b, char *c, + Py_ssize_t c_length); + +static PyObject * +str_converter_encoding(PyObject *module, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + char *a = NULL; + char *b = NULL; + char *c = NULL; + Py_ssize_t c_length; + + if (!_PyArg_ParseStack(args, nargs, "esetet#:str_converter_encoding", + "idna", &a, "idna", &b, "idna", &c, &c_length)) { + goto exit; + } + return_value = str_converter_encoding_impl(module, a, b, c, c_length); + /* Post operation for a */ + if (a) { + PyMem_FREE(a); + } + /* Post operation for b */ + if (b) { + PyMem_FREE(b); + } + /* Post operation for c */ + if (c) { + PyMem_FREE(c); + } + +exit: + return return_value; +} + PyDoc_STRVAR(py_buffer_converter__doc__, "py_buffer_converter($module, a, b, /)\n" "--\n" @@ -2288,4 +2331,50 @@ 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(gh_99240_double_free__doc__, +"gh_99240_double_free($module, a, b, /)\n" +"--\n" +"\n" +"Proof-of-concept of GH-99240 double-free bug.\n" +"\n" +"If parsing `a` successes, `a` will be assigned an address points to an allocated memory.\n" +"After that, if parsing `b` fails, the memory which `a` points to is freed by function `_PyArg_ParseStack`,\n" +"and `_PyArg_ParseStack` returns 0, then control flow goes to label \"exit\".\n" +"At this time, `a` is not NULL, so the memory it points to is freed again,\n" +"which cause a double-free problem and a runtime crash.\n" +"\n" +"Calling this function by gh_99240_double_free(\'a\', \'\\0b\')\n" +"to trigger this bug (crash)."); + +#define GH_99240_DOUBLE_FREE_METHODDEF \ + {"gh_99240_double_free", _PyCFunction_CAST(gh_99240_double_free), METH_FASTCALL, gh_99240_double_free__doc__}, + +static PyObject * +gh_99240_double_free_impl(PyObject *module, char *a, char *b); + +static PyObject * +gh_99240_double_free(PyObject *module, PyObject *const *args, Py_ssize_t nargs) +{ + PyObject *return_value = NULL; + char *a = NULL; + char *b = NULL; + + if (!_PyArg_ParseStack(args, nargs, "eses:gh_99240_double_free", + "idna", &a, "idna", &b)) { + goto exit; + } + return_value = gh_99240_double_free_impl(module, a, b); + /* Post operation for a */ + if (a) { + PyMem_FREE(a); + } + /* Post operation for b */ + if (b) { + PyMem_FREE(b); + } + +exit: + return return_value; +} +/*[clinic end generated code: output=a2e5c02750be8f94 input=a9049054013a1b77]*/ From 946ad3f8f009f0ec2009f291989aafe8a4472c43 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 19:21:55 +0800 Subject: [PATCH 06/11] Simplify news Co-authored-by: Erlend E. Aasland --- .../Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst b/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst index 8eceadbd86211c..0a4db052755f87 100644 --- a/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst +++ b/Misc/NEWS.d/next/Library/2022-11-08-15-54-43.gh-issue-99240.MhYwcz.rst @@ -1,3 +1,2 @@ -Add "post_parsing" section to Argument Clinic generated code. -Fix double-free bug in Argument Clinic str_converter generated code by -moving memory clean up out of "exit" label. +Fix double-free bug in Argument Clinic ``str_converter`` by +extracting memory clean up to a new ``post_parsing`` section. From 7767139f8c6a9ae58181ebc998b0b61a54717dd7 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 19:23:38 +0800 Subject: [PATCH 07/11] Delete unnecessary function doc Co-authored-by: Erlend E. Aasland --- Modules/_testclinic.c | 11 +---------- Modules/clinic/_testclinic.c.h | 13 ++----------- 2 files changed, 3 insertions(+), 21 deletions(-) diff --git a/Modules/_testclinic.c b/Modules/_testclinic.c index 935746c6abc896..bde174301d65b5 100644 --- a/Modules/_testclinic.c +++ b/Modules/_testclinic.c @@ -959,20 +959,11 @@ gh_99240_double_free Proof-of-concept of GH-99240 double-free bug. -If parsing `a` successes, `a` will be assigned an address points to an allocated memory. -After that, if parsing `b` fails, the memory which `a` points to is freed by function `_PyArg_ParseStack`, -and `_PyArg_ParseStack` returns 0, then control flow goes to label "exit". -At this time, `a` is not NULL, so the memory it points to is freed again, -which cause a double-free problem and a runtime crash. - -Calling this function by gh_99240_double_free('a', '\0b') -to trigger this bug (crash). - [clinic start generated code]*/ static PyObject * gh_99240_double_free_impl(PyObject *module, char *a, char *b) -/*[clinic end generated code: output=586dc714992fe2ed input=419d3a3790de435e]*/ +/*[clinic end generated code: output=586dc714992fe2ed input=23db44aa91870fc7]*/ { Py_RETURN_NONE; } diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index 1b9032856eee1a..efc8a91c06e536 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2336,16 +2336,7 @@ PyDoc_STRVAR(gh_99240_double_free__doc__, "gh_99240_double_free($module, a, b, /)\n" "--\n" "\n" -"Proof-of-concept of GH-99240 double-free bug.\n" -"\n" -"If parsing `a` successes, `a` will be assigned an address points to an allocated memory.\n" -"After that, if parsing `b` fails, the memory which `a` points to is freed by function `_PyArg_ParseStack`,\n" -"and `_PyArg_ParseStack` returns 0, then control flow goes to label \"exit\".\n" -"At this time, `a` is not NULL, so the memory it points to is freed again,\n" -"which cause a double-free problem and a runtime crash.\n" -"\n" -"Calling this function by gh_99240_double_free(\'a\', \'\\0b\')\n" -"to trigger this bug (crash)."); +"Proof-of-concept of GH-99240 double-free bug."); #define GH_99240_DOUBLE_FREE_METHODDEF \ {"gh_99240_double_free", _PyCFunction_CAST(gh_99240_double_free), METH_FASTCALL, gh_99240_double_free__doc__}, @@ -2377,4 +2368,4 @@ gh_99240_double_free(PyObject *module, PyObject *const *args, Py_ssize_t nargs) exit: return return_value; } -/*[clinic end generated code: output=a2e5c02750be8f94 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=a63e7a4294003ad4 input=a9049054013a1b77]*/ From 9d08af55ccae6706c340f3ba872540e2267800df Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 19:24:18 +0800 Subject: [PATCH 08/11] Simplify function doc Co-authored-by: Erlend E. Aasland --- Tools/clinic/clinic.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index 8cc272cd578e49..b8b2e4b392dc66 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2836,8 +2836,8 @@ def modify(self): def post_parsing(self): """ The C statements required to do some operations after the end of parsing but before cleaning up. - Returns a string containing this code indented at column 0. - If no operation is necessary, returns an empty string. + Return a string containing this code indented at column 0. + If no operation is necessary, return an empty string. """ return "" From aa218c4638bb929f23ed07d42646b1cee860ebcd Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 20:06:09 +0800 Subject: [PATCH 09/11] Do not generate redundant NULL check --- Lib/test/clinic.test | 22 ++++++---------------- Modules/clinic/_testclinic.c.h | 22 ++++++---------------- Tools/clinic/clinic.py | 2 +- 3 files changed, 13 insertions(+), 33 deletions(-) diff --git a/Lib/test/clinic.test b/Lib/test/clinic.test index fd63d71668342f..d1cff9036b057c 100644 --- a/Lib/test/clinic.test +++ b/Lib/test/clinic.test @@ -1741,25 +1741,15 @@ test_str_converter_encoding(PyObject *module, PyObject *const *args, Py_ssize_t } return_value = test_str_converter_encoding_impl(module, a, b, c, d, d_length, e, e_length); /* Post operation for a */ - if (a) { - PyMem_FREE(a); - } + PyMem_FREE(a); /* Post operation for b */ - if (b) { - PyMem_FREE(b); - } + PyMem_FREE(b); /* Post operation for c */ - if (c) { - PyMem_FREE(c); - } + PyMem_FREE(c); /* Post operation for d */ - if (d) { - PyMem_FREE(d); - } + PyMem_FREE(d); /* Post operation for e */ - if (e) { - PyMem_FREE(e); - } + PyMem_FREE(e); exit: return return_value; @@ -1769,7 +1759,7 @@ static PyObject * test_str_converter_encoding_impl(PyObject *module, char *a, char *b, char *c, char *d, Py_ssize_t d_length, char *e, Py_ssize_t e_length) -/*[clinic end generated code: output=5dfef501bb5f5e82 input=eb4c38e1f898f402]*/ +/*[clinic end generated code: output=a9f5b30da7f9bfaf input=eb4c38e1f898f402]*/ /*[clinic input] diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index efc8a91c06e536..1e95e65fc409ab 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -1192,17 +1192,11 @@ str_converter_encoding(PyObject *module, PyObject *const *args, Py_ssize_t nargs } return_value = str_converter_encoding_impl(module, a, b, c, c_length); /* Post operation for a */ - if (a) { - PyMem_FREE(a); - } + PyMem_FREE(a); /* Post operation for b */ - if (b) { - PyMem_FREE(b); - } + PyMem_FREE(b); /* Post operation for c */ - if (c) { - PyMem_FREE(c); - } + PyMem_FREE(c); exit: return return_value; @@ -2357,15 +2351,11 @@ gh_99240_double_free(PyObject *module, PyObject *const *args, Py_ssize_t nargs) } return_value = gh_99240_double_free_impl(module, a, b); /* Post operation for a */ - if (a) { - PyMem_FREE(a); - } + PyMem_FREE(a); /* Post operation for b */ - if (b) { - PyMem_FREE(b); - } + PyMem_FREE(b); exit: return return_value; } -/*[clinic end generated code: output=a63e7a4294003ad4 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=4e20fb88b2861703 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index b8b2e4b392dc66..f840d397f559e4 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -3440,7 +3440,7 @@ def converter_init(self, *, accept={str}, encoding=None, zeroes=False): def post_parsing(self): if self.encoding: name = self.name - return "".join(["if (", name, ") {\n PyMem_FREE(", name, ");\n}\n"]) + return f"PyMem_FREE({name});\n" def parse_arg(self, argname, displayname): if self.format_unit == 's': From 485c7aae69a4ee9273111f5d7eada07d701095b8 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 21:32:02 +0800 Subject: [PATCH 10/11] Polish comment --- Lib/test/clinic.test | 12 ++++++------ Modules/clinic/_testclinic.c.h | 12 ++++++------ Tools/clinic/clinic.py | 2 +- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Lib/test/clinic.test b/Lib/test/clinic.test index d1cff9036b057c..80f96e0b41c860 100644 --- a/Lib/test/clinic.test +++ b/Lib/test/clinic.test @@ -1740,15 +1740,15 @@ test_str_converter_encoding(PyObject *module, PyObject *const *args, Py_ssize_t goto exit; } return_value = test_str_converter_encoding_impl(module, a, b, c, d, d_length, e, e_length); - /* Post operation for a */ + /* Post parse cleanup for a */ PyMem_FREE(a); - /* Post operation for b */ + /* Post parse cleanup for b */ PyMem_FREE(b); - /* Post operation for c */ + /* Post parse cleanup for c */ PyMem_FREE(c); - /* Post operation for d */ + /* Post parse cleanup for d */ PyMem_FREE(d); - /* Post operation for e */ + /* Post parse cleanup for e */ PyMem_FREE(e); exit: @@ -1759,7 +1759,7 @@ static PyObject * test_str_converter_encoding_impl(PyObject *module, char *a, char *b, char *c, char *d, Py_ssize_t d_length, char *e, Py_ssize_t e_length) -/*[clinic end generated code: output=a9f5b30da7f9bfaf input=eb4c38e1f898f402]*/ +/*[clinic end generated code: output=999c1deecfa15b0a input=eb4c38e1f898f402]*/ /*[clinic input] diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index 1e95e65fc409ab..a84d3d693d6baa 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -1191,11 +1191,11 @@ str_converter_encoding(PyObject *module, PyObject *const *args, Py_ssize_t nargs goto exit; } return_value = str_converter_encoding_impl(module, a, b, c, c_length); - /* Post operation for a */ + /* Post parse cleanup for a */ PyMem_FREE(a); - /* Post operation for b */ + /* Post parse cleanup for b */ PyMem_FREE(b); - /* Post operation for c */ + /* Post parse cleanup for c */ PyMem_FREE(c); exit: @@ -2350,12 +2350,12 @@ gh_99240_double_free(PyObject *module, PyObject *const *args, Py_ssize_t nargs) goto exit; } return_value = gh_99240_double_free_impl(module, a, b); - /* Post operation for a */ + /* Post parse cleanup for a */ PyMem_FREE(a); - /* Post operation for b */ + /* Post parse cleanup for b */ PyMem_FREE(b); exit: return return_value; } -/*[clinic end generated code: output=4e20fb88b2861703 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=d66fb0c51d666229 input=a9049054013a1b77]*/ diff --git a/Tools/clinic/clinic.py b/Tools/clinic/clinic.py index f840d397f559e4..1d587cef7b0432 100755 --- a/Tools/clinic/clinic.py +++ b/Tools/clinic/clinic.py @@ -2736,7 +2736,7 @@ def _render_non_self(self, parameter, data): # post_parsing if post_parsing := self.post_parsing(): - data.post_parsing.append('/* Post operation for ' + name + ' */\n' + post_parsing.rstrip() + "\n") + data.post_parsing.append('/* Post parse cleanup for ' + name + ' */\n' + post_parsing.rstrip() + '\n') # cleanup cleanup = self.cleanup() From aea779fbf0dcd91b29fb5c931dcb2c73e01f5b92 Mon Sep 17 00:00:00 2001 From: colorfulappl Date: Thu, 24 Nov 2022 21:35:04 +0800 Subject: [PATCH 11/11] Rerun make clinic --- Modules/clinic/_testclinic.c.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Modules/clinic/_testclinic.c.h b/Modules/clinic/_testclinic.c.h index e606262278daeb..9aad44566bcde0 100644 --- a/Modules/clinic/_testclinic.c.h +++ b/Modules/clinic/_testclinic.c.h @@ -2390,7 +2390,6 @@ gh_99233_refcount(PyObject *module, PyObject *const *args, Py_ssize_t nargs) Py_XDECREF(__clinic_args); return return_value; } -/*[clinic end generated code: output=a5c9f181f3a32d85 input=a9049054013a1b77]*/ PyDoc_STRVAR(gh_99240_double_free__doc__, "gh_99240_double_free($module, a, b, /)\n" @@ -2424,4 +2423,4 @@ gh_99240_double_free(PyObject *module, PyObject *const *args, Py_ssize_t nargs) exit: return return_value; } -/*[clinic end generated code: output=d66fb0c51d666229 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=49dced2c99bcd0fb input=a9049054013a1b77]*/