bpo-46433: _PyType_GetModuleByDef: handle static types in MRO (GH-30696)

Automerge-Triggered-By: GH:encukou

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()

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.

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)

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]*/

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;