Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-99240: Fix double-free bug in Argument Clinic str_converter generated code #99241

Merged
merged 13 commits into from
Nov 24, 2022
15 changes: 7 additions & 8 deletions Lib/test/clinic.test
Original file line number Diff line number Diff line change
@@ -1740,37 +1740,36 @@ 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;
}

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]
Original file line number Diff line number Diff line change
@@ -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.
23 changes: 22 additions & 1 deletion Tools/clinic/clinic.py
Original file line number Diff line number Diff line change
@@ -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,10 @@ 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")

# cleanup
cleanup = self.cleanup()
if cleanup:
@@ -2820,6 +2833,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 +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 cleanup(self):
def post_operations(self):
if self.encoding:
name = self.name
return "".join(["if (", name, ") {\n PyMem_FREE(", name, ");\n}\n"])