diff options
| -rw-r--r-- | Doc/c-api/module.rst | 12 | ||||
| -rw-r--r-- | Lib/test/test_importlib/extension/test_loader.py | 16 | ||||
| -rw-r--r-- | Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst | 2 | ||||
| -rw-r--r-- | Modules/_testmultiphase.c | 52 | ||||
| -rw-r--r-- | Objects/moduleobject.c | 21 |
5 files changed, 96 insertions, 7 deletions
diff --git a/Doc/c-api/module.rst b/Doc/c-api/module.rst index 7efab28..017b656 100644 --- a/Doc/c-api/module.rst +++ b/Doc/c-api/module.rst @@ -196,17 +196,23 @@ or request "multi-phase initialization" by returning the definition struct itsel .. c:member:: traverseproc m_traverse A traversal function to call during GC traversal of the module object, or - *NULL* if not needed. + *NULL* if not needed. This function may be called before module state + is allocated (:c:func:`PyModule_GetState()` may return `NULL`), + and before the :c:member:`Py_mod_exec` function is executed. .. c:member:: inquiry m_clear A clear function to call during GC clearing of the module object, or - *NULL* if not needed. + *NULL* if not needed. This function may be called before module state + is allocated (:c:func:`PyModule_GetState()` may return `NULL`), + and before the :c:member:`Py_mod_exec` function is executed. .. c:member:: freefunc m_free A function to call during deallocation of the module object, or *NULL* if - not needed. + not needed. This function may be called before module state + is allocated (:c:func:`PyModule_GetState()` may return `NULL`), + and before the :c:member:`Py_mod_exec` function is executed. Single-phase initialization ........................... diff --git a/Lib/test/test_importlib/extension/test_loader.py b/Lib/test/test_importlib/extension/test_loader.py index 8d20040..53ac3c7 100644 --- a/Lib/test/test_importlib/extension/test_loader.py +++ b/Lib/test/test_importlib/extension/test_loader.py @@ -9,7 +9,7 @@ import types import unittest import importlib.util import importlib - +from test.support.script_helper import assert_python_failure class LoaderTests(abc.LoaderTests): @@ -267,6 +267,20 @@ class MultiPhaseExtensionModuleTests(abc.LoaderTests): self.assertEqual(module.__name__, name) self.assertEqual(module.__doc__, "Module named in %s" % lang) + @unittest.skipIf(not hasattr(sys, 'gettotalrefcount'), + '--with-pydebug has to be enabled for this test') + def test_bad_traverse(self): + ''' Issue #32374: Test that traverse fails when accessing per-module + state before Py_mod_exec was executed. + (Multiphase initialization modules only) + ''' + script = """if True: + import importlib.util as util + spec = util.find_spec('_testmultiphase') + spec.name = '_testmultiphase_with_bad_traverse' + m = spec.loader.create_module(spec)""" + assert_python_failure("-c", script) + (Frozen_MultiPhaseExtensionModuleTests, Source_MultiPhaseExtensionModuleTests diff --git a/Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst b/Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst new file mode 100644 index 0000000..f9cf6d6 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2018-01-09-17-03-54.bpo-32374.SwwLoz.rst @@ -0,0 +1,2 @@ +Document that m_traverse for multi-phase initialized modules can be called +with m_state=NULL, and add a sanity check diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index 9b04ebf..8090a48 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -10,6 +10,10 @@ typedef struct { PyObject *x_attr; /* Attributes dictionary */ } ExampleObject; +typedef struct { + PyObject *integer; +} testmultiphase_state; + /* Example methods */ static int @@ -218,18 +222,21 @@ static int execfunc(PyObject *m) } /* Helper for module definitions; there'll be a lot of them */ -#define TEST_MODULE_DEF(name, slots, methods) { \ + +#define TEST_MODULE_DEF_EX(name, slots, methods, statesize, traversefunc) { \ PyModuleDef_HEAD_INIT, /* m_base */ \ name, /* m_name */ \ PyDoc_STR("Test module " name), /* m_doc */ \ - 0, /* m_size */ \ + statesize, /* m_size */ \ methods, /* m_methods */ \ slots, /* m_slots */ \ - NULL, /* m_traverse */ \ + traversefunc, /* m_traverse */ \ NULL, /* m_clear */ \ NULL, /* m_free */ \ } +#define TEST_MODULE_DEF(name, slots, methods) TEST_MODULE_DEF_EX(name, slots, methods, 0, NULL) + PyModuleDef_Slot main_slots[] = { {Py_mod_exec, execfunc}, {0, NULL}, @@ -613,6 +620,44 @@ PyInit__testmultiphase_exec_unreported_exception(PyObject *spec) return PyModuleDef_Init(&def_exec_unreported_exception); } +static int +bad_traverse(PyObject *self, visitproc visit, void *arg) { + testmultiphase_state *m_state; + + m_state = PyModule_GetState(self); + Py_VISIT(m_state->integer); + return 0; +} + +static int +execfunc_with_bad_traverse(PyObject *mod) { + testmultiphase_state *m_state; + + m_state = PyModule_GetState(mod); + if (m_state == NULL) { + return -1; + } + + m_state->integer = PyLong_FromLong(0x7fffffff); + Py_INCREF(m_state->integer); + + return 0; +} + +static PyModuleDef_Slot slots_with_bad_traverse[] = { + {Py_mod_exec, execfunc_with_bad_traverse}, + {0, NULL} +}; + +static PyModuleDef def_with_bad_traverse = TEST_MODULE_DEF_EX( + "_testmultiphase_with_bad_traverse", slots_with_bad_traverse, NULL, + sizeof(testmultiphase_state), bad_traverse); + +PyMODINIT_FUNC +PyInit__testmultiphase_with_bad_traverse(PyObject *spec) { + return PyModuleDef_Init(&def_with_bad_traverse); +} + /*** Helper for imp test ***/ static PyModuleDef imp_dummy_def = TEST_MODULE_DEF("imp_dummy", main_slots, testexport_methods); @@ -622,3 +667,4 @@ PyInit_imp_dummy(PyObject *spec) { return PyModuleDef_Init(&imp_dummy_def); } + diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index d6cde40..8fb368e 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -21,6 +21,17 @@ static PyMemberDef module_members[] = { {0} }; + +/* Helper for sanity check for traverse not handling m_state == NULL + * Issue #32374 */ +#ifdef Py_DEBUG +static int +bad_traverse_test(PyObject *self, void *arg) { + assert(self != NULL); + return 0; +} +#endif + PyTypeObject PyModuleDef_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) "moduledef", /* tp_name */ @@ -345,6 +356,16 @@ PyModule_FromDefAndSpec2(struct PyModuleDef* def, PyObject *spec, int module_api } } + /* Sanity check for traverse not handling m_state == NULL + * This doesn't catch all possible cases, but in many cases it should + * make many cases of invalid code crash or raise Valgrind issues + * sooner than they would otherwise. + * Issue #32374 */ +#ifdef Py_DEBUG + if (def->m_traverse != NULL) { + def->m_traverse(m, bad_traverse_test, NULL); + } +#endif Py_DECREF(nameobj); return m; |