From e81f30d96c975e0a0f1067879ddbd39f84fd0b3e Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Mon, 7 Jun 2021 23:18:53 -0700 Subject: [PATCH 1/8] Add cleanup at end of init function Add decrefs to the end of the generated init function to make sure we free everything we need to in the case of an error. --- mypyc/codegen/emitmodule.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 68f5c9968aca..9f822db7e9fa 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -899,9 +899,11 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module ' goto fail;') # HACK: Manually instantiate generated classes here + type_structs = [] # type: List[str] for cl in module.classes: if cl.is_generated: type_struct = emitter.type_struct_name(cl) + type_structs.append(type_struct) emitter.emit_lines( '{t} = (PyTypeObject *)CPyType_FromTemplate(' '(PyObject *){t}_template, NULL, modname);' @@ -918,7 +920,13 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module emitter.emit_line('return {};'.format(module_static)) emitter.emit_lines('fail:', - '{} = NULL;'.format(module_static), + 'Py_XDECREF({});'.format(module_static), + 'Py_XDECREF(modname);') + # the type objects returned from CPyType_FromTemplate are all new references + # so we have to decref them + for t in type_structs: + emitter.emit_line('Py_XDECREF({});'.format(t)) + emitter.emit_lines('{} = NULL;'.format(module_static), 'return NULL;') emitter.emit_line('}') From 2f24fe6063319a21e145d13c87c1de999f1e7e0d Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 13:06:55 -0700 Subject: [PATCH 2/8] Fix 'modname may be uninitialized' error modname might be uninitialized if PyModule_Create fails with an error, so initialize modname to NULL at the beginning of the function so that Py_XDECREF does nothing. --- mypyc/codegen/emitmodule.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 9f822db7e9fa..720705511433 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -874,6 +874,7 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module declaration = 'PyObject *CPyInit_{}(void)'.format(exported_name(module_name)) emitter.emit_lines(declaration, '{') + emitter.emit_line('PyObject* modname = NULL;') # Store the module reference in a static and return it when necessary. # This is separate from the *global* reference to the module that will # be populated when it is imported by a compiled module. We want that @@ -890,7 +891,7 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module 'if (unlikely({} == NULL))'.format(module_static), ' goto fail;') emitter.emit_line( - 'PyObject *modname = PyObject_GetAttrString((PyObject *){}, "__name__");'.format( + 'modname = PyObject_GetAttrString((PyObject *){}, "__name__");'.format( module_static)) module_globals = emitter.static_name('globals', module_name) From 24f5ce334d3c88c4eeb1a27820bf70910b5ecbea Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 14:28:29 -0700 Subject: [PATCH 3/8] Free type structs for non generated classes Free the type structs associated with all classes in the case of an error, regardless of whether those classes are generated. --- mypyc/codegen/emitmodule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 720705511433..fc4ca9fa0412 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -902,9 +902,9 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module # HACK: Manually instantiate generated classes here type_structs = [] # type: List[str] for cl in module.classes: + type_struct = emitter.type_struct_name(cl) + type_structs.append(type_struct) if cl.is_generated: - type_struct = emitter.type_struct_name(cl) - type_structs.append(type_struct) emitter.emit_lines( '{t} = (PyTypeObject *)CPyType_FromTemplate(' '(PyObject *){t}_template, NULL, modname);' From ff3d71778b97217ed1a50cce1620513b866b74a4 Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 14:36:48 -0700 Subject: [PATCH 4/8] Change Py_XDECREF to Py_CLEAR Use Py_CLEAR instead of using Py_XDECREF on the module object we created and then setting it to NULL because Py_CLEAR does the same thing. --- mypyc/codegen/emitmodule.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index fc4ca9fa0412..b9ab38545446 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -921,14 +921,13 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module emitter.emit_line('return {};'.format(module_static)) emitter.emit_lines('fail:', - 'Py_XDECREF({});'.format(module_static), + 'Py_CLEAR({});'.format(module_static), 'Py_XDECREF(modname);') # the type objects returned from CPyType_FromTemplate are all new references # so we have to decref them for t in type_structs: emitter.emit_line('Py_XDECREF({});'.format(t)) - emitter.emit_lines('{} = NULL;'.format(module_static), - 'return NULL;') + emitter.emit_line('return NULL;') emitter.emit_line('}') def generate_top_level_call(self, module: ModuleIR, emitter: Emitter) -> None: From ea8b1b20e643a3873f6c00929c1b96995c658ed3 Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 15:56:23 -0700 Subject: [PATCH 5/8] Clean up final types in the case of an error Make sure we clean up finals if we run into an error while running the initialization code. --- mypyc/codegen/emitmodule.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index b9ab38545446..d6714fe5bbd5 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -36,7 +36,7 @@ generate_legacy_wrapper_function, legacy_wrapper_function_header, ) from mypyc.ir.ops import DeserMaps, LoadLiteral -from mypyc.ir.rtypes import RType, RTuple +from mypyc.ir.rtypes import RType, RTuple, is_tagged from mypyc.ir.func_ir import FuncIR from mypyc.ir.class_ir import ClassIR from mypyc.ir.module_ir import ModuleIR, ModuleIRs, deserialize_modules @@ -925,6 +925,12 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module 'Py_XDECREF(modname);') # the type objects returned from CPyType_FromTemplate are all new references # so we have to decref them + for name, typ in module.final_names: + static_name = emitter.static_name(name, module_name) + if emitter.ctype(typ) == 'PyObject *': + emitter.emit_line('Py_CLEAR({});'.format(static_name)) + elif is_tagged(typ): + emitter.emit_line('CPyTagged_XDecRef({});'.format(static_name)) for t in type_structs: emitter.emit_line('Py_XDECREF({});'.format(t)) emitter.emit_line('return NULL;') From 9420cecec2a0189b4a7d3d61926605579eb7a0e0 Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 16:15:01 -0700 Subject: [PATCH 6/8] Use Py_CLEAR in more places in clean up Use Py_CLEAR on the module name and all of the type structs to make sure that those are set to NULL if we run into an error. --- mypyc/codegen/emitmodule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index d6714fe5bbd5..dd6eeda97c0f 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -922,7 +922,7 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module emitter.emit_line('return {};'.format(module_static)) emitter.emit_lines('fail:', 'Py_CLEAR({});'.format(module_static), - 'Py_XDECREF(modname);') + 'Py_CLEAR(modname);') # the type objects returned from CPyType_FromTemplate are all new references # so we have to decref them for name, typ in module.final_names: @@ -932,7 +932,7 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module elif is_tagged(typ): emitter.emit_line('CPyTagged_XDecRef({});'.format(static_name)) for t in type_structs: - emitter.emit_line('Py_XDECREF({});'.format(t)) + emitter.emit_line('Py_CLEAR({});'.format(t)) emitter.emit_line('return NULL;') emitter.emit_line('}') From ff8d287f196b98cc55809ecb72a4f4b674bc09bc Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 22:57:20 -0700 Subject: [PATCH 7/8] Move comment to correct place The comment about why we have to decref and clear type structs got moved in one of the previous commits, so move it back to the right place. --- mypyc/codegen/emitmodule.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index dd6eeda97c0f..1ef4bc0fa123 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -923,14 +923,14 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module emitter.emit_lines('fail:', 'Py_CLEAR({});'.format(module_static), 'Py_CLEAR(modname);') - # the type objects returned from CPyType_FromTemplate are all new references - # so we have to decref them for name, typ in module.final_names: static_name = emitter.static_name(name, module_name) if emitter.ctype(typ) == 'PyObject *': emitter.emit_line('Py_CLEAR({});'.format(static_name)) elif is_tagged(typ): emitter.emit_line('CPyTagged_XDecRef({});'.format(static_name)) + # the type objects returned from CPyType_FromTemplate are all new references + # so we have to decref them for t in type_structs: emitter.emit_line('Py_CLEAR({});'.format(t)) emitter.emit_line('return NULL;') From ebe20ed7d03802584fdd593ac77224e9be3f8022 Mon Sep 17 00:00:00 2001 From: Pranav Rajpal <78008260+pranavrajpal@users.noreply.github.com> Date: Tue, 8 Jun 2021 23:22:17 -0700 Subject: [PATCH 8/8] Improve handling of other final types in cleanup Improve the handling of final values for types other than python objects and tagged integers by using emit_dec_ref and c_undefined_value. In particular, this should handle tuples correctly and make sure to clear tagged integers correctly. --- mypyc/codegen/emitmodule.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/mypyc/codegen/emitmodule.py b/mypyc/codegen/emitmodule.py index 1ef4bc0fa123..14f9b3f8f467 100644 --- a/mypyc/codegen/emitmodule.py +++ b/mypyc/codegen/emitmodule.py @@ -36,7 +36,7 @@ generate_legacy_wrapper_function, legacy_wrapper_function_header, ) from mypyc.ir.ops import DeserMaps, LoadLiteral -from mypyc.ir.rtypes import RType, RTuple, is_tagged +from mypyc.ir.rtypes import RType, RTuple from mypyc.ir.func_ir import FuncIR from mypyc.ir.class_ir import ClassIR from mypyc.ir.module_ir import ModuleIR, ModuleIRs, deserialize_modules @@ -925,10 +925,9 @@ def generate_module_def(self, emitter: Emitter, module_name: str, module: Module 'Py_CLEAR(modname);') for name, typ in module.final_names: static_name = emitter.static_name(name, module_name) - if emitter.ctype(typ) == 'PyObject *': - emitter.emit_line('Py_CLEAR({});'.format(static_name)) - elif is_tagged(typ): - emitter.emit_line('CPyTagged_XDecRef({});'.format(static_name)) + emitter.emit_dec_ref(static_name, typ, is_xdec=True) + undef = emitter.c_undefined_value(typ) + emitter.emit_line('{} = {};'.format(static_name, undef)) # the type objects returned from CPyType_FromTemplate are all new references # so we have to decref them for t in type_structs: