bpo-45383: Get metaclass from bases in PyType_From* (GH-28748)

This checks the bases of of a type created using the FromSpec
API to inherit the bases metaclasses.  The metaclass's alloc
function will be called as is done in `tp_new` for classes
created in Python.

Co-authored-by: Petr Viktorin <encukou@gmail.com>
Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@protonmail.com>

4 files changed, 323 insertions, 97 deletions

diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst
index 99b3845..fece3e6 100644
--- a/Doc/c-api/type.rst
+++ b/Doc/c-api/type.rst
@@ -193,11 +193,12 @@ The following functions and structs are used to create
 .. c:function:: PyObject* PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, PyType_Spec *spec, PyObject *bases)
 
    Create and return a :ref:`heap type <heap-types>` from the *spec*
-   (:const:`Py_TPFLAGS_HEAPTYPE`).
+   (see :const:`Py_TPFLAGS_HEAPTYPE`).
 
    The metaclass *metaclass* is used to construct the resulting type object.
-   When *metaclass* is ``NULL``, the default :c:type:`PyType_Type` is used
-   instead. Note that metaclasses that override
+   When *metaclass* is ``NULL``, the metaclass is derived from *bases*
+   (or *Py_tp_base[s]* slots if *bases* is ``NULL``, see below).
+   Note that metaclasses that override
    :c:member:`~PyTypeObject.tp_new` are not supported.
 
    The *bases* argument can be used to specify base classes; it can either
@@ -215,6 +216,19 @@ The following functions and structs are used to create
 
    This function calls :c:func:`PyType_Ready` on the new type.
 
+   Note that this function does *not* fully match the behavior of
+   calling :py:class:`type() <type>` or using the :keyword:`class` statement.
+   With user-provided base types or metaclasses, prefer
+   :ref:`calling <capi-call>` :py:class:`type` (or the metaclass)
+   over ``PyType_From*`` functions.
+   Specifically:
+
+   * :py:meth:`~object.__new__` is not called on the new class
+     (and it must be set to ``type.__new__``).
+   * :py:meth:`~object.__init__` is not called on the new class.
+   * :py:meth:`~object.__init_subclass__` is not called on any bases.
+   * :py:meth:`~object.__set_name__` is not called on new descriptors.
+
    .. versionadded:: 3.12
 
 .. c:function:: PyObject* PyType_FromModuleAndSpec(PyObject *module, PyType_Spec *spec, PyObject *bases)
@@ -228,6 +242,11 @@ The following functions and structs are used to create
       The function now accepts a single class as the *bases* argument and
       ``NULL`` as the ``tp_doc`` slot.
 
+   .. versionchanged:: 3.12
+
+      The function now finds and uses a metaclass corresponding to the provided
+      base classes.  Previously, only :class:`type` instances were returned.
+
 
 .. c:function:: PyObject* PyType_FromSpecWithBases(PyType_Spec *spec, PyObject *bases)
 
@@ -235,10 +254,21 @@ The following functions and structs are used to create
 
    .. versionadded:: 3.3
 
+   .. versionchanged:: 3.12
+
+      The function now finds and uses a metaclass corresponding to the provided
+      base classes.  Previously, only :class:`type` instances were returned.
+
 .. c:function:: PyObject* PyType_FromSpec(PyType_Spec *spec)
 
    Equivalent to ``PyType_FromMetaclass(NULL, NULL, spec, NULL)``.
 
+   .. versionchanged:: 3.12
+
+      The function now finds and uses a metaclass corresponding to the
+      base classes provided in *Py_tp_base[s]* slots.
+      Previously, only :class:`type` instances were returned.
+
 .. c:type:: PyType_Spec
 
    Structure defining a type's behavior.
diff --git a/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst
new file mode 100644
index 0000000..ca1b7a4
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2021-10-05-21-59-43.bpo-45383.TVClgf.rst
@@ -0,0 +1,3 @@
+The :c:func:`PyType_FromSpec` API will now find and use a metaclass
+based on the provided bases.
+An error will be raised if there is a metaclass conflict.
diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c
index ac0c96a..b75e03c 100644
--- a/Modules/_testcapimodule.c
+++ b/Modules/_testcapimodule.c
@@ -1208,6 +1208,161 @@ test_get_type_name(PyObject *self, PyObject *Py_UNUSED(ignored))
 }
 
 
+static PyType_Slot empty_type_slots[] = {
+    {0, 0},
+};
+
+static PyType_Spec MinimalMetaclass_spec = {
+    .name = "_testcapi.MinimalMetaclass",
+    .basicsize = sizeof(PyHeapTypeObject),
+    .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+    .slots = empty_type_slots,
+};
+
+static PyType_Spec MinimalType_spec = {
+    .name = "_testcapi.MinimalSpecType",
+    .basicsize = sizeof(PyObject),
+    .flags = Py_TPFLAGS_DEFAULT,
+    .slots = empty_type_slots,
+};
+
+static PyObject *
+test_from_spec_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+    PyObject *metaclass = NULL;
+    PyObject *class = NULL;
+    PyObject *new = NULL;
+    PyObject *subclasses = NULL;
+    PyObject *result = NULL;
+    int r;
+
+    metaclass = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type);
+    if (metaclass == NULL) {
+        goto finally;
+    }
+    class = PyObject_CallFunction(metaclass, "s(){}", "TestClass");
+    if (class == NULL) {
+        goto finally;
+    }
+
+    new = PyType_FromSpecWithBases(&MinimalType_spec, class);
+    if (new == NULL) {
+        goto finally;
+    }
+    if (Py_TYPE(new) != (PyTypeObject*)metaclass) {
+        PyErr_SetString(PyExc_AssertionError,
+                "Metaclass not set properly!");
+        goto finally;
+    }
+
+    /* Assert that __subclasses__ is updated */
+    subclasses = PyObject_CallMethod(class, "__subclasses__", "");
+    if (!subclasses) {
+        goto finally;
+    }
+    r = PySequence_Contains(subclasses, new);
+    if (r < 0) {
+        goto finally;
+    }
+    if (r == 0) {
+        PyErr_SetString(PyExc_AssertionError,
+                "subclasses not set properly!");
+        goto finally;
+    }
+
+    result = Py_NewRef(Py_None);
+
+finally:
+    Py_XDECREF(metaclass);
+    Py_XDECREF(class);
+    Py_XDECREF(new);
+    Py_XDECREF(subclasses);
+    return result;
+}
+
+
+static PyObject *
+test_from_spec_invalid_metatype_inheritance(PyObject *self, PyObject *Py_UNUSED(ignored))
+{
+    PyObject *metaclass_a = NULL;
+    PyObject *metaclass_b = NULL;
+    PyObject *class_a = NULL;
+    PyObject *class_b = NULL;
+    PyObject *bases = NULL;
+    PyObject *new = NULL;
+    PyObject *meta_error_string = NULL;
+    PyObject *exc_type = NULL;
+    PyObject *exc_value = NULL;
+    PyObject *exc_traceback = NULL;
+    PyObject *result = NULL;
+
+    metaclass_a = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type);
+    if (metaclass_a == NULL) {
+        goto finally;
+    }
+    metaclass_b = PyType_FromSpecWithBases(&MinimalMetaclass_spec, (PyObject*)&PyType_Type);
+    if (metaclass_b == NULL) {
+        goto finally;
+    }
+    class_a = PyObject_CallFunction(metaclass_a, "s(){}", "TestClassA");
+    if (class_a == NULL) {
+        goto finally;
+    }
+
+    class_b = PyObject_CallFunction(metaclass_b, "s(){}", "TestClassB");
+    if (class_b == NULL) {
+        goto finally;
+    }
+
+    bases = PyTuple_Pack(2, class_a, class_b);
+    if (bases == NULL) {
+        goto finally;
+    }
+
+    /*
+     * The following should raise a TypeError due to a MetaClass conflict.
+     */
+    new = PyType_FromSpecWithBases(&MinimalType_spec, bases);
+    if (new != NULL) {
+        PyErr_SetString(PyExc_AssertionError,
+                "MetaType conflict not recognized by PyType_FromSpecWithBases");
+            goto finally;
+    }
+
+    // Assert that the correct exception was raised
+    if (PyErr_ExceptionMatches(PyExc_TypeError)) {
+        PyErr_Fetch(&exc_type, &exc_value, &exc_traceback);
+
+        meta_error_string = PyUnicode_FromString("metaclass conflict:");
+        if (meta_error_string == NULL) {
+            goto finally;
+        }
+        int res = PyUnicode_Contains(exc_value, meta_error_string);
+        if (res < 0) {
+            goto finally;
+        }
+        if (res == 0) {
+            PyErr_SetString(PyExc_AssertionError,
+                    "TypeError did not inlclude expected message.");
+            goto finally;
+        }
+        result = Py_NewRef(Py_None);
+    }
+finally:
+    Py_XDECREF(metaclass_a);
+    Py_XDECREF(metaclass_b);
+    Py_XDECREF(bases);
+    Py_XDECREF(new);
+    Py_XDECREF(meta_error_string);
+    Py_XDECREF(exc_type);
+    Py_XDECREF(exc_value);
+    Py_XDECREF(exc_traceback);
+    Py_XDECREF(class_a);
+    Py_XDECREF(class_b);
+    return result;
+}
+
+
 static PyObject *
 simple_str(PyObject *self) {
     return PyUnicode_FromString("<test>");
@@ -5952,6 +6107,11 @@ static PyMethodDef TestMethods[] = {
     {"test_get_type_name",        test_get_type_name,            METH_NOARGS},
     {"test_get_type_qualname",    test_get_type_qualname,        METH_NOARGS},
     {"test_type_from_ephemeral_spec", test_type_from_ephemeral_spec, 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},
     {"get_kwargs", _PyCFunction_CAST(get_kwargs),
       METH_VARARGS|METH_KEYWORDS},
     {"getargs_tuple",           getargs_tuple,                   METH_VARARGS},
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 2566d21..51dc5e3 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -3361,13 +3361,51 @@ static const PySlot_Offset pyslot_offsets[] = {
 #include "typeslots.inc"
 };
 
+/* Given a PyType_FromMetaclass `bases` argument (NULL, type, or tuple of
+ * types), return a tuple of types.
+ */
+inline static PyObject *
+get_bases_tuple(PyObject *bases_in, PyType_Spec *spec)
+{
+    if (!bases_in) {
+        /* Default: look in the spec, fall back to (type,). */
+        PyTypeObject *base = &PyBaseObject_Type;  // borrowed ref
+        PyObject *bases = NULL;  // borrowed ref
+        const PyType_Slot *slot;
+        for (slot = spec->slots; slot->slot; slot++) {
+            switch (slot->slot) {
+                case Py_tp_base:
+                    base = slot->pfunc;
+                    break;
+                case Py_tp_bases:
+                    bases = slot->pfunc;
+                    break;
+            }
+        }
+        if (!bases) {
+            return PyTuple_Pack(1, base);
+        }
+        if (PyTuple_Check(bases)) {
+            return Py_NewRef(bases);
+        }
+        PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple");
+        return NULL;
+    }
+    if (PyTuple_Check(bases_in)) {
+        return Py_NewRef(bases_in);
+    }
+    // Not a tuple, should be a single type
+    return PyTuple_Pack(1, bases_in);
+}
+
 PyObject *
 PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
-                     PyType_Spec *spec, PyObject *bases)
+                     PyType_Spec *spec, PyObject *bases_in)
 {
-    PyHeapTypeObject *res;
-    PyObject *modname;
-    PyTypeObject *type, *base;
+    PyHeapTypeObject *res = NULL;
+    PyObject *modname = NULL;
+    PyTypeObject *type;
+    PyObject *bases = NULL;
     int r;
 
     const PyType_Slot *slot;
@@ -3375,16 +3413,6 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
     char *res_start;
     short slot_offset, subslot_offset;
 
-    if (!metaclass) {
-        metaclass = &PyType_Type;
-    }
-
-    if (metaclass->tp_new != PyType_Type.tp_new) {
-        PyErr_SetString(PyExc_TypeError,
-                        "Metaclasses with custom tp_new are not supported.");
-        return NULL;
-    }
-
     nmembers = weaklistoffset = dictoffset = vectorcalloffset = 0;
     for (slot = spec->slots; slot->slot; slot++) {
         if (slot->slot == Py_tp_members) {
@@ -3413,16 +3441,57 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
         }
     }
 
-    res = (PyHeapTypeObject*)metaclass->tp_alloc(metaclass, 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;
+        goto finally;
+    }
+
+    /* Get a tuple of bases.
+     * bases is a strong reference (unlike bases_in).
+     */
+    bases = get_bases_tuple(bases_in, spec);
+    if (!bases) {
+        goto finally;
+    }
+
+    /* Calculate the metaclass */
+
+    if (!metaclass) {
+        metaclass = &PyType_Type;
+    }
+    metaclass = _PyType_CalculateMetaclass(metaclass, bases);
+    if (metaclass == NULL) {
+        goto finally;
+    }
+    if (!PyType_Check(metaclass)) {
+        PyErr_Format(PyExc_TypeError,
+                     "Metaclass '%R' is not a subclass of 'type'.",
+                     metaclass);
+        goto finally;
+    }
+    if (metaclass->tp_new != PyType_Type.tp_new) {
+        PyErr_SetString(PyExc_TypeError,
+                        "Metaclasses with custom tp_new are not supported.");
+        goto finally;
+    }
+
+    /* Calculate best base, and check that all bases are type objects */
+    PyTypeObject *base = best_base(bases);  // borrowed ref
+    if (base == NULL) {
+        goto finally;
+    }
+    // best_base should check Py_TPFLAGS_BASETYPE & raise a proper exception,
+    // here we just check its work
+    assert(_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE));
+
+    /* Allocate the new type */
+
+    res = (PyHeapTypeObject*)metaclass->tp_alloc(metaclass, nmembers);
+    if (res == NULL) {
+        goto finally;
     }
+    res_start = (char*)res;
 
     type = &res->ht_type;
     /* The flags must be initialized early, before the GC traverses us */
@@ -3439,7 +3508,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
 
     res->ht_name = PyUnicode_FromString(s);
     if (!res->ht_name) {
-        goto fail;
+        goto finally;
     }
     res->ht_qualname = Py_NewRef(res->ht_name);
 
@@ -3455,70 +3524,25 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
     Py_ssize_t name_buf_len = strlen(spec->name) + 1;
     res->_ht_tpname = PyMem_Malloc(name_buf_len);
     if (res->_ht_tpname == NULL) {
-        goto fail;
+        goto finally;
     }
     type->tp_name = memcpy(res->_ht_tpname, spec->name, name_buf_len);
 
     res->ht_module = Py_XNewRef(module);
 
-    /* Adjust for empty tuple bases */
-    if (!bases) {
-        base = &PyBaseObject_Type;
-        /* See whether Py_tp_base(s) was specified */
-        for (slot = spec->slots; slot->slot; slot++) {
-            if (slot->slot == Py_tp_base)
-                base = slot->pfunc;
-            else if (slot->slot == Py_tp_bases) {
-                bases = slot->pfunc;
-            }
-        }
-        if (!bases) {
-            bases = PyTuple_Pack(1, base);
-            if (!bases)
-                goto fail;
-        }
-        else if (!PyTuple_Check(bases)) {
-            PyErr_SetString(PyExc_SystemError, "Py_tp_bases is not a tuple");
-            goto fail;
-        }
-        else {
-            Py_INCREF(bases);
-        }
-    }
-    else if (!PyTuple_Check(bases)) {
-        bases = PyTuple_Pack(1, bases);
-        if (!bases)
-            goto fail;
-    }
-    else {
-        Py_INCREF(bases);
-    }
-
-    /* Calculate best base, and check that all bases are type objects */
-    base = best_base(bases);
-    if (base == NULL) {
-        Py_DECREF(bases);
-        goto fail;
-    }
-    if (!_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)) {
-        PyErr_Format(PyExc_TypeError,
-                     "type '%.100s' is not an acceptable base type",
-                     base->tp_name);
-        Py_DECREF(bases);
-        goto fail;
-    }
-
     /* Initialize essential fields */
     type->tp_as_async = &res->as_async;
     type->tp_as_number = &res->as_number;
     type->tp_as_sequence = &res->as_sequence;
     type->tp_as_mapping = &res->as_mapping;
     type->tp_as_buffer = &res->as_buffer;
+
     /* Set tp_base and tp_bases */
+    type->tp_base = (PyTypeObject *)Py_NewRef(base);
     type->tp_bases = bases;
-    Py_INCREF(base);
-    type->tp_base = base;
+    bases = NULL;  // We give our reference to bases to the type
 
+    /* Copy the sizes */
     type->tp_basicsize = spec->basicsize;
     type->tp_itemsize = spec->itemsize;
 
@@ -3526,7 +3550,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
         if (slot->slot < 0
             || (size_t)slot->slot >= Py_ARRAY_LENGTH(pyslot_offsets)) {
             PyErr_SetString(PyExc_RuntimeError, "invalid slot offset");
-            goto fail;
+            goto finally;
         }
         else if (slot->slot == Py_tp_base || slot->slot == Py_tp_bases) {
             /* Processed above */
@@ -3544,7 +3568,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
             if (tp_doc == NULL) {
                 type->tp_doc = NULL;
                 PyErr_NoMemory();
-                goto fail;
+                goto finally;
             }
             memcpy(tp_doc, slot->pfunc, len);
             type->tp_doc = tp_doc;
@@ -3579,8 +3603,9 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
         type->tp_vectorcall_offset = vectorcalloffset;
     }
 
-    if (PyType_Ready(type) < 0)
-        goto fail;
+    if (PyType_Ready(type) < 0) {
+        goto finally;
+    }
 
     if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
         res->ht_cached_keys = _PyDict_NewKeysForClass();
@@ -3588,29 +3613,33 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
 
     if (type->tp_doc) {
         PyObject *__doc__ = PyUnicode_FromString(_PyType_DocWithoutSignature(type->tp_name, type->tp_doc));
-        if (!__doc__)
-            goto fail;
+        if (!__doc__) {
+            goto finally;
+        }
         r = PyDict_SetItem(type->tp_dict, &_Py_ID(__doc__), __doc__);
         Py_DECREF(__doc__);
-        if (r < 0)
-            goto fail;
+        if (r < 0) {
+            goto finally;
+        }
     }
 
     if (weaklistoffset) {
         type->tp_weaklistoffset = weaklistoffset;
-        if (PyDict_DelItemString((PyObject *)type->tp_dict, "__weaklistoffset__") < 0)
-            goto fail;
+        if (PyDict_DelItemString((PyObject *)type->tp_dict, "__weaklistoffset__") < 0) {
+            goto finally;
+        }
     }
     if (dictoffset) {
         type->tp_dictoffset = dictoffset;
-        if (PyDict_DelItemString((PyObject *)type->tp_dict, "__dictoffset__") < 0)
-            goto fail;
+        if (PyDict_DelItemString((PyObject *)type->tp_dict, "__dictoffset__") < 0) {
+            goto finally;
+        }
     }
 
     /* Set type.__module__ */
     r = PyDict_Contains(type->tp_dict, &_Py_ID(__module__));
     if (r < 0) {
-        goto fail;
+        goto finally;
     }
     if (r == 0) {
         s = strrchr(spec->name, '.');
@@ -3618,26 +3647,30 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module,
             modname = PyUnicode_FromStringAndSize(
                     spec->name, (Py_ssize_t)(s - spec->name));
             if (modname == NULL) {
-                goto fail;
+                goto finally;
             }
             r = PyDict_SetItem(type->tp_dict, &_Py_ID(__module__), modname);
-            Py_DECREF(modname);
-            if (r != 0)
-                goto fail;
-        } else {
+            if (r != 0) {
+                goto finally;
+            }
+        }
+        else {
             if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
                     "builtin type %.200s has no __module__ attribute",
                     spec->name))
-                goto fail;
+                goto finally;
         }
     }
 
     assert(_PyType_CheckConsistency(type));
-    return (PyObject*)res;
 
- fail:
-    Py_DECREF(res);
-    return NULL;
+ finally:
+    if (PyErr_Occurred()) {
+        Py_CLEAR(res);
+    }
+    Py_XDECREF(bases);
+    Py_XDECREF(modname);
+    return (PyObject*)res;
 }
 
 PyObject *