From c3f52b4d707a78eb342372a2be00f3eb846a05b9 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 23 Jun 2021 10:00:43 +0100 Subject: bpo-44486: Make sure that modules always have a dictionary. (GH-26847) * Make sure that modules always have a dictionary. --- Lib/test/test_module.py | 13 +-- .../2021-06-22-10-55-23.bpo-44486.wct-9X.rst | 2 + Objects/moduleobject.c | 102 ++++++++++++--------- Python/ceval.c | 1 + 4 files changed, 66 insertions(+), 52 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2021-06-22-10-55-23.bpo-44486.wct-9X.rst diff --git a/Lib/test/test_module.py b/Lib/test/test_module.py index 65319d5..6608dbc 100644 --- a/Lib/test/test_module.py +++ b/Lib/test/test_module.py @@ -22,8 +22,8 @@ class ModuleTests(unittest.TestCase): # An uninitialized module has no __dict__ or __name__, # and __doc__ is None foo = ModuleType.__new__(ModuleType) - self.assertTrue(foo.__dict__ is None) - self.assertRaises(TypeError, dir, foo) + self.assertTrue(isinstance(foo.__dict__, dict)) + self.assertEqual(dir(foo), []) try: s = foo.__name__ self.fail("__name__ = %s" % repr(s)) @@ -318,15 +318,6 @@ a = A(destroyed)""" del foo.__dict__['__annotations__'] def test_annotations_getset_raises(self): - # module has no dict, all operations fail - foo = ModuleType.__new__(ModuleType) - with self.assertRaises(TypeError): - print(foo.__annotations__) - with self.assertRaises(TypeError): - foo.__annotations__ = {} - with self.assertRaises(TypeError): - del foo.__annotations__ - # double delete foo = ModuleType("foo") foo.__annotations__ = {} diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-06-22-10-55-23.bpo-44486.wct-9X.rst b/Misc/NEWS.d/next/Core and Builtins/2021-06-22-10-55-23.bpo-44486.wct-9X.rst new file mode 100644 index 0000000..cc41960 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2021-06-22-10-55-23.bpo-44486.wct-9X.rst @@ -0,0 +1,2 @@ +Modules will always have a dictionary, even when created by +``types.ModuleType.__new__()`` diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c index b69e5ce..61478a1 100644 --- a/Objects/moduleobject.c +++ b/Objects/moduleobject.c @@ -64,8 +64,7 @@ module_init_dict(PyModuleObject *mod, PyObject *md_dict, _Py_IDENTIFIER(__package__); _Py_IDENTIFIER(__loader__); - if (md_dict == NULL) - return -1; + assert(md_dict != NULL); if (doc == NULL) doc = Py_None; @@ -87,12 +86,11 @@ module_init_dict(PyModuleObject *mod, PyObject *md_dict, return 0; } - -PyObject * -PyModule_NewObject(PyObject *name) +static PyModuleObject * +new_module_notrack(PyTypeObject *mt) { PyModuleObject *m; - m = PyObject_GC_New(PyModuleObject, &PyModule_Type); + m = PyObject_GC_New(PyModuleObject, mt); if (m == NULL) return NULL; m->md_def = NULL; @@ -100,6 +98,29 @@ PyModule_NewObject(PyObject *name) m->md_weaklist = NULL; m->md_name = NULL; m->md_dict = PyDict_New(); + if (m->md_dict != NULL) { + return m; + } + Py_DECREF(m); + return NULL; +} + +static PyObject * +new_module(PyTypeObject *mt, PyObject *args, PyObject *kws) +{ + PyObject *m = (PyObject *)new_module_notrack(mt); + if (m != NULL) { + PyObject_GC_Track(m); + } + return m; +} + +PyObject * +PyModule_NewObject(PyObject *name) +{ + PyModuleObject *m = new_module_notrack(&PyModule_Type); + if (m == NULL) + return NULL; if (module_init_dict(m, m->md_dict, name, NULL) != 0) goto fail; PyObject_GC_Track(m); @@ -728,43 +749,42 @@ module_getattro(PyModuleObject *m, PyObject *name) return attr; } PyErr_Clear(); - if (m->md_dict) { - _Py_IDENTIFIER(__getattr__); - getattr = _PyDict_GetItemIdWithError(m->md_dict, &PyId___getattr__); - if (getattr) { - return PyObject_CallOneArg(getattr, name); - } - if (PyErr_Occurred()) { - return NULL; - } - mod_name = _PyDict_GetItemIdWithError(m->md_dict, &PyId___name__); - if (mod_name && PyUnicode_Check(mod_name)) { - Py_INCREF(mod_name); - PyObject *spec = _PyDict_GetItemIdWithError(m->md_dict, &PyId___spec__); - if (spec == NULL && PyErr_Occurred()) { - Py_DECREF(mod_name); - return NULL; - } - Py_XINCREF(spec); - if (_PyModuleSpec_IsInitializing(spec)) { - PyErr_Format(PyExc_AttributeError, - "partially initialized " - "module '%U' has no attribute '%U' " - "(most likely due to a circular import)", - mod_name, name); - } - else { - PyErr_Format(PyExc_AttributeError, - "module '%U' has no attribute '%U'", - mod_name, name); - } - Py_XDECREF(spec); + assert(m->md_dict != NULL); + _Py_IDENTIFIER(__getattr__); + getattr = _PyDict_GetItemIdWithError(m->md_dict, &PyId___getattr__); + if (getattr) { + return PyObject_CallOneArg(getattr, name); + } + if (PyErr_Occurred()) { + return NULL; + } + mod_name = _PyDict_GetItemIdWithError(m->md_dict, &PyId___name__); + if (mod_name && PyUnicode_Check(mod_name)) { + Py_INCREF(mod_name); + PyObject *spec = _PyDict_GetItemIdWithError(m->md_dict, &PyId___spec__); + if (spec == NULL && PyErr_Occurred()) { Py_DECREF(mod_name); return NULL; } - else if (PyErr_Occurred()) { - return NULL; + Py_XINCREF(spec); + if (_PyModuleSpec_IsInitializing(spec)) { + PyErr_Format(PyExc_AttributeError, + "partially initialized " + "module '%U' has no attribute '%U' " + "(most likely due to a circular import)", + mod_name, name); } + else { + PyErr_Format(PyExc_AttributeError, + "module '%U' has no attribute '%U'", + mod_name, name); + } + Py_XDECREF(spec); + Py_DECREF(mod_name); + return NULL; + } + else if (PyErr_Occurred()) { + return NULL; } PyErr_Format(PyExc_AttributeError, "module has no attribute '%U'", name); @@ -948,7 +968,7 @@ PyTypeObject PyModule_Type = { 0, /* tp_descr_set */ offsetof(PyModuleObject, md_dict), /* tp_dictoffset */ module___init__, /* tp_init */ - PyType_GenericAlloc, /* tp_alloc */ - PyType_GenericNew, /* tp_new */ + 0, /* tp_alloc */ + new_module, /* tp_new */ PyObject_GC_Del, /* tp_free */ }; diff --git a/Python/ceval.c b/Python/ceval.c index 9c11640..3f961f6 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3337,6 +3337,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag) _PyLoadAttrCache *cache1 = &caches[-1].load_attr; DEOPT_IF(!PyModule_CheckExact(owner), LOAD_ATTR); PyDictObject *dict = (PyDictObject *)((PyModuleObject *)owner)->md_dict; + assert(dict != NULL); DEOPT_IF(dict->ma_keys->dk_version != cache1->dk_version_or_hint, LOAD_ATTR); assert(dict->ma_keys->dk_kind == DICT_KEYS_UNICODE); assert(cache0->index < dict->ma_keys->dk_nentries); -- cgit v0.12