BUG: Inherit MetaClass from bases in FromSpec API

This checks the bases of of a type created using the FromSpec API to inherit the bases metaclasses. The MetaClasses alloc function will be called as is done in `tp_new` for classes created in Python.
python · encukou · Jun 9, 2022 · Oct 5, 2021 · Oct 5, 2021 · Oct 20, 2021
commit 67c8a97affcb9dbb12f44fdb7767e756b82ea34b
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
@@ -1173,6 +1173,143 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored))
 }
 
 
+/*
+ * Small helper to import abc.ABC and ctypes.Array for testing.  Both
+ * are (incompatible) MetaClass instances.  If Array is NULL it is not filled.
+ */
+static int
+import_abc_and_array(PyObject **ABC, PyObject **Array)
+{
+    PyObject *abc_mod = PyImport_ImportModule("abc");
+    if (abc_mod == NULL) {
+        return -1;
+    }
+    *ABC = PyObject_GetAttrString(abc_mod, "ABC");
+    Py_DECREF(abc_mod);
+    if (*ABC == NULL) {
+        return -1;
+    }
+    if (Array == NULL) {
+        return 0;
+    }
+
+    PyObject *ctypes_mod = PyImport_ImportModule("ctypes");
+    if (ctypes_mod == NULL) {
+        Py_CLEAR(*ABC);
+        return -1;
+    }
+    *Array = PyObject_GetAttrString(ctypes_mod, "Array");
+    Py_DECREF(ctypes_mod);
+    if (*Array == NULL) {
+        Py_CLEAR(*ABC);
+        return -1;
+    }
+    return 0;
+}
+
+
+static PyType_Slot MinimalType_slots[] = {
+    {0, 0},
+};
+
+static PyType_Spec MinimalType_spec = {
+    "_testcapi.MinimalSpecType",
+    0,
+    0,
+    Py_TPFLAGS_DEFAULT,
+    MinimalType_slots
+};
+
+
+static PyObject *
+test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+    /* Get two (incompatible) MetaTypes */
+    PyObject *ABC;
+    if (import_abc_and_array(&ABC, NULL) < 0) {
+        return NULL;
+    }
+
+    PyObject *bases = PyTuple_Pack(1, ABC);
+    if (bases == NULL) {
+        Py_DECREF(ABC);
+        return NULL;
+    }
+    PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases);
+    Py_DECREF(bases);
+    if (new == NULL) {
+        Py_DECREF(ABC);
+        return NULL;
+    }
+    if (Py_TYPE(new) != Py_TYPE(ABC)) {
+        PyErr_SetString(PyExc_AssertionError,
+                "MetaType appears not correctly inherited from ABC!");
+        Py_DECREF(ABC);
+        Py_DECREF(new);
+        return NULL;
+    }
+    Py_DECREF(ABC);
+    Py_DECREF(new);
+    Py_RETURN_NONE;
+}
+
+
+static PyObject *
+test_from_spec_invalid_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+    /* Get two (incompatible) MetaTypes */
+    PyObject *ABC, *Array;
+
+    if (import_abc_and_array(&ABC, &Array) < 0) {
+        return NULL;
+    }
+
+    PyObject *bases = PyTuple_Pack(2, ABC, Array);
+    Py_DECREF(ABC);
+    Py_DECREF(Array);
+    if (bases == NULL) {
+        return NULL;
+    }
+    /*
+     * The following should raise a TypeError due to a MetaClass conflict.
+     */
+    PyObject *new = PyType_FromSpecWithBases(&MinimalType_spec, bases);
+    Py_DECREF(bases);
+    if (new != NULL) {
+        Py_DECREF(new);
+        PyErr_SetString(PyExc_AssertionError,
+                "MetaType conflict not recognized by PyType_FromSpecWithBases");
+        return NULL;
+    }
+    if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+        PyObject *type, *value, *traceback, *meta_error_string;
+
+        PyErr_Fetch(&type, &value, &traceback);
+        Py_DECREF(type);
+        Py_XDECREF(traceback);
+
+        meta_error_string = PyUnicode_FromString("metaclass conflict:");
+        if (meta_error_string == NULL) {
+            Py_DECREF(value);
+            return NULL;
+        }
+        int res = PyUnicode_Contains(value, meta_error_string);
+        Py_DECREF(value);
+        Py_DECREF(meta_error_string);
+        if (res < 0) {
+            return NULL;
+        }
+        if (res == 0) {
+            PyErr_SetString(PyExc_AssertionError,
+                    "TypeError did not inlclude expected message.");
+            return NULL;
+        }
+        Py_RETURN_NONE;
+    }
+    return NULL;
+}
+
+
 static PyObject *
 test_get_type_qualname(PyObject *self, PyObject *Py_UNUSED(ignored))
 {
@@ -5722,6 +5859,11 @@ static PyMethodDef TestMethods[] = {
     {"get_args",                get_args,                        METH_VARARGS},
     {"test_get_statictype_slots", test_get_statictype_slots,     METH_NOARGS},
     {"test_get_type_name",        test_get_type_name,            METH_NOARGS},
+    {"test_from_spec_metatype_inheritance", test_from_spec_metatype_inheritance,
+     METH_NOARGS},
+    {"test_from_spec_invalid_metatype_inheritance",
+     test_from_spec_invalid_metatype_inheritance,
+     METH_NOARGS},
     {"test_get_type_qualname",   test_get_type_qualname,       METH_NOARGS},
     {"get_kwargs", (PyCFunction)(void(*)(void))get_kwargs,
       METH_VARARGS|METH_KEYWORDS},

@@ -3398,37 +3398,12 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
         }
     }
 
-    res = (PyHeapTypeObject*)PyType_GenericAlloc(&PyType_Type, nmembers);
-    if (res == NULL)
-        return NULL;
-    res_start = (char*)res;
-
     if (spec->name == NULL) {
         PyErr_SetString(PyExc_SystemError,
                         "Type spec does not define the name field.");
-        goto fail;
+        return NULL;
     }
 
-    /* Set the type name and qualname */
-    const char *s = strrchr(spec->name, '.');
-    if (s == NULL)
-        s = spec->name;
-    else
-        s++;
-
-    type = &res->ht_type;
-    /* The flags must be initialized early, before the GC traverses us */
-    type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
-    res->ht_name = PyUnicode_FromString(s);
-    if (!res->ht_name)
-        goto fail;
-    res->ht_qualname = res->ht_name;
-    Py_INCREF(res->ht_qualname);
-    type->tp_name = spec->name;
-
-    Py_XINCREF(module);
-    res->ht_module = module;
-
     /* Adjust for empty tuple bases */
     if (!bases) {
         base = &PyBaseObject_Type;
@@ -3443,11 +3418,11 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
         if (!bases) {
             bases = PyTuple_Pack(1, base);
             if (!bases)
-                goto fail;
+                return NULL;
         }
         else if (!PyTuple_Check(bases)) {
             PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple");
-            goto fail;
+            return NULL;
         }
         else {
             Py_INCREF(bases);
@@ -3456,12 +3431,47 @@ PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
     else if (!PyTuple_Check(bases)) {
         bases = PyTuple_Pack(1, bases);
         if (!bases)
-            goto fail;
+            return NULL;
     }
     else {
         Py_INCREF(bases);
     }
 
+    /* NOTE: Missing API to replace `&PyType_Type` below, see bpo-15870 */
+    PyTypeObject *metatype = _PyType_CalculateMetaclass(&PyType_Type, bases);
+    if (metatype == NULL) {
+        Py_DECREF(bases);
+        return NULL;
+    }
+    res = (PyHeapTypeObject*)metatype->tp_alloc(metatype, nmembers);
+    if (res == NULL) {
+        Py_DECREF(bases);
+        return NULL;
+    }
+    res_start = (char*)res;
+
+    /* Set the type name and qualname */
+    const char *s = strrchr(spec->name, '.');
+    if (s == NULL)
+        s = spec->name;
+    else
+        s++;
+
+    type = &res->ht_type;
+    /* The flags must be initialized early, before the GC traverses us */
+    type->tp_flags = spec->flags | Py_TPFLAGS_HEAPTYPE;
+    res->ht_name = PyUnicode_FromString(s);
+    if (!res->ht_name) {
+        Py_DECREF(bases);
+        goto fail;
+    }
+    res->ht_qualname = res->ht_name;
+    Py_INCREF(res->ht_qualname);
+    type->tp_name = spec->name;
+
+    Py_XINCREF(module);
+    res->ht_module = module;
+
     /* Calculate best base, and check that all bases are type objects */
     base = best_base(bases);
     if (base == NULL) {