From 0ef08530124c5ca13a9394f4ac18bee8e6c66409 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 2 Feb 2022 16:57:51 +0100 Subject: [PATCH] bpo-46433: _PyType_GetModuleByDef: handle static types in MRO (GH-30696) Automerge-Triggered-By: GH:encukou --- Lib/test/test_capi.py | 16 ++++++++++ .../2022-01-19-16-51-54.bpo-46433.Er9ApS.rst | 2 ++ Modules/_testmultiphase.c | 24 ++++++++++++-- Modules/clinic/_testmultiphase.c.h | 32 ++++++++++++++++++- Objects/typeobject.c | 9 +++--- 5 files changed, 74 insertions(+), 9 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2022-01-19-16-51-54.bpo-46433.Er9ApS.rst diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index a5db8a11c5f..ccf8ceda498 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -1068,6 +1068,22 @@ class Test_ModuleStateAccess(unittest.TestCase): with self.assertRaises(TypeError): increment_count(1, 2, 3) + def test_get_module_bad_def(self): + # _PyType_GetModuleByDef fails gracefully if it doesn't + # find what it's looking for. + # see bpo-46433 + instance = self.module.StateAccessType() + with self.assertRaises(TypeError): + instance.getmodulebydef_bad_def() + + def test_get_module_static_in_mro(self): + # Here, the class _PyType_GetModuleByDef is looking for + # appears in the MRO after a static type (Exception). + # see bpo-46433 + class Subclass(BaseException, self.module.StateAccessType): + pass + self.assertIs(Subclass().get_defining_module(), self.module) + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/C API/2022-01-19-16-51-54.bpo-46433.Er9ApS.rst b/Misc/NEWS.d/next/C API/2022-01-19-16-51-54.bpo-46433.Er9ApS.rst new file mode 100644 index 00000000000..e1987c4536b --- /dev/null +++ b/Misc/NEWS.d/next/C API/2022-01-19-16-51-54.bpo-46433.Er9ApS.rst @@ -0,0 +1,2 @@ +The internal function _PyType_GetModuleByDef now correctly handles +inheritance patterns involving static types. diff --git a/Modules/_testmultiphase.c b/Modules/_testmultiphase.c index ee69c423361..f7bde9895eb 100644 --- a/Modules/_testmultiphase.c +++ b/Modules/_testmultiphase.c @@ -126,6 +126,8 @@ static PyType_Spec Example_Type_spec = { static PyModuleDef def_meth_state_access; +static PyModuleDef def_nonmodule; +static PyModuleDef def_nonmodule_with_methods; /*[clinic input] _testmultiphase.StateAccessType.get_defining_module @@ -153,6 +155,24 @@ _testmultiphase_StateAccessType_get_defining_module_impl(StateAccessTypeObject * return retval; } +/*[clinic input] +_testmultiphase.StateAccessType.getmodulebydef_bad_def + + cls: defining_class + +Test that result of _PyType_GetModuleByDef with a bad def is NULL. +[clinic start generated code]*/ + +static PyObject * +_testmultiphase_StateAccessType_getmodulebydef_bad_def_impl(StateAccessTypeObject *self, + PyTypeObject *cls) +/*[clinic end generated code: output=64509074dfcdbd31 input=906047715ee293cd]*/ +{ + _PyType_GetModuleByDef(Py_TYPE(self), &def_nonmodule); // should raise + assert(PyErr_Occurred()); + return NULL; +} + /*[clinic input] _testmultiphase.StateAccessType.increment_count_clinic @@ -249,6 +269,7 @@ _testmultiphase_StateAccessType_get_count_impl(StateAccessTypeObject *self, static PyMethodDef StateAccessType_methods[] = { _TESTMULTIPHASE_STATEACCESSTYPE_GET_DEFINING_MODULE_METHODDEF + _TESTMULTIPHASE_STATEACCESSTYPE_GETMODULEBYDEF_BAD_DEF_METHODDEF _TESTMULTIPHASE_STATEACCESSTYPE_GET_COUNT_METHODDEF _TESTMULTIPHASE_STATEACCESSTYPE_INCREMENT_COUNT_CLINIC_METHODDEF { @@ -437,9 +458,6 @@ PyInit__testmultiphase(PyObject *spec) /**** Importing a non-module object ****/ -static PyModuleDef def_nonmodule; -static PyModuleDef def_nonmodule_with_methods; - /* Create a SimpleNamespace(three=3) */ static PyObject* createfunc_nonmodule(PyObject *spec, PyModuleDef *def) diff --git a/Modules/clinic/_testmultiphase.c.h b/Modules/clinic/_testmultiphase.c.h index 55f934be8c6..17c28d54ce5 100644 --- a/Modules/clinic/_testmultiphase.c.h +++ b/Modules/clinic/_testmultiphase.c.h @@ -35,6 +35,36 @@ exit: return return_value; } +PyDoc_STRVAR(_testmultiphase_StateAccessType_getmodulebydef_bad_def__doc__, +"getmodulebydef_bad_def($self, /)\n" +"--\n" +"\n" +"Test that result of _PyType_GetModuleByDef with a bad def is NULL."); + +#define _TESTMULTIPHASE_STATEACCESSTYPE_GETMODULEBYDEF_BAD_DEF_METHODDEF \ + {"getmodulebydef_bad_def", (PyCFunction)(void(*)(void))_testmultiphase_StateAccessType_getmodulebydef_bad_def, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, _testmultiphase_StateAccessType_getmodulebydef_bad_def__doc__}, + +static PyObject * +_testmultiphase_StateAccessType_getmodulebydef_bad_def_impl(StateAccessTypeObject *self, + PyTypeObject *cls); + +static PyObject * +_testmultiphase_StateAccessType_getmodulebydef_bad_def(StateAccessTypeObject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + static const char * const _keywords[] = { NULL}; + static _PyArg_Parser _parser = {":getmodulebydef_bad_def", _keywords, 0}; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser + )) { + goto exit; + } + return_value = _testmultiphase_StateAccessType_getmodulebydef_bad_def_impl(self, cls); + +exit: + return return_value; +} + PyDoc_STRVAR(_testmultiphase_StateAccessType_increment_count_clinic__doc__, "increment_count_clinic($self, /, n=1, *, twice=False)\n" "--\n" @@ -101,4 +131,4 @@ _testmultiphase_StateAccessType_get_count(StateAccessTypeObject *self, PyTypeObj exit: return return_value; } -/*[clinic end generated code: output=f01137bb3b373e14 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=eb1b8c2ee6290be3 input=a9049054013a1b77]*/ diff --git a/Objects/typeobject.c b/Objects/typeobject.c index f7e0775e222..0c893eaa75b 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3756,11 +3756,10 @@ _PyType_GetModuleByDef(PyTypeObject *type, struct PyModuleDef *def) Py_ssize_t n = PyTuple_GET_SIZE(mro); for (Py_ssize_t i = 0; i < n; i++) { PyObject *super = PyTuple_GET_ITEM(mro, i); - // _PyType_GetModuleByDef() must only be called on a heap type created - // by PyType_FromModuleAndSpec() or on its subclasses. - // type_ready_mro() ensures that a static type cannot inherit from a - // heap type. - assert(_PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)); + if(!_PyType_HasFeature((PyTypeObject *)super, Py_TPFLAGS_HEAPTYPE)) { + // Static types in the MRO need to be skipped + continue; + } PyHeapTypeObject *ht = (PyHeapTypeObject*)super; PyObject *module = ht->ht_module;