gh-127773: Disable attribute cache on incompatible MRO entries (GH-127924)

encukou · web-flow · commit aa6579cb60b4 · 2025-01-13T14:10:41.000+01:00
diff --git a/Include/cpython/object.h b/Include/cpython/object.h
@@ -221,17 +221,27 @@ struct _typeobject {
     PyObject *tp_weaklist; /* not used for static builtin types */
     destructor tp_del;
 
-    /* Type attribute cache version tag. Added in version 2.6 */
+    /* Type attribute cache version tag. Added in version 2.6.
+     * If zero, the cache is invalid and must be initialized.
+     */
     unsigned int tp_version_tag;
 
     destructor tp_finalize;
     vectorcallfunc tp_vectorcall;
 
     /* bitset of which type-watchers care about this type */
     unsigned char tp_watched;
+
+    /* Number of tp_version_tag values used.
+     * Set to _Py_ATTR_CACHE_UNUSED if the attribute cache is
+     * disabled for this type (e.g. due to custom MRO entries).
+     * Otherwise, limited to MAX_VERSIONS_PER_CLASS (defined elsewhere).
+     */
     uint16_t tp_versions_used;
 };
 
+#define _Py_ATTR_CACHE_UNUSED (30000)  // (see tp_versions_used)
+
 /* This struct is used by the specializer
  * It should be treated as an opaque blob
  * by code other than the specializer and interpreter. */
diff --git a/Lib/test/test_metaclass.py b/Lib/test/test_metaclass.py
@@ -254,6 +254,33 @@
     [...]
     test.test_metaclass.ObscureException
 
+Test setting attributes with a non-base type in mro() (gh-127773).
+
+    >>> class Base:
+    ...     value = 1
+    ...
+    >>> class Meta(type):
+    ...     def mro(cls):
+    ...         return (cls, Base, object)
+    ...
+    >>> class WeirdClass(metaclass=Meta):
+    ...     pass
+    ...
+    >>> Base.value
+    1
+    >>> WeirdClass.value
+    1
+    >>> Base.value = 2
+    >>> Base.value
+    2
+    >>> WeirdClass.value
+    2
+    >>> Base.value = 3
+    >>> Base.value
+    3
+    >>> WeirdClass.value
+    3
+
 """
 
 import sys
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-12-13-15-21-45.gh-issue-127773.E-DZR4.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-12-13-15-21-45.gh-issue-127773.E-DZR4.rst
@@ -0,0 +1 @@
+Do not use the type attribute cache for types with incompatible :term:`MRO`.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -992,6 +992,7 @@ static void
 set_version_unlocked(PyTypeObject *tp, unsigned int version)
 {
     ASSERT_TYPE_LOCK_HELD();
+    assert(version == 0 || (tp->tp_versions_used != _Py_ATTR_CACHE_UNUSED));
 #ifndef Py_GIL_DISABLED
     PyInterpreterState *interp = _PyInterpreterState_GET();
     // lookup the old version and set to null
@@ -1148,6 +1149,10 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) {
         PyObject *b = PyTuple_GET_ITEM(bases, i);
         PyTypeObject *cls = _PyType_CAST(b);
 
+        if (cls->tp_versions_used >= _Py_ATTR_CACHE_UNUSED) {
+            goto clear;
+        }
+
         if (!is_subtype_with_mro(lookup_tp_mro(type), type, cls)) {
             goto clear;
         }
@@ -1156,7 +1161,8 @@ type_mro_modified(PyTypeObject *type, PyObject *bases) {
 
  clear:
     assert(!(type->tp_flags & _Py_TPFLAGS_STATIC_BUILTIN));
-    set_version_unlocked(type, 0); /* 0 is not a valid version tag */
+    set_version_unlocked(type, 0);  /* 0 is not a valid version tag */
+    type->tp_versions_used = _Py_ATTR_CACHE_UNUSED;
     if (PyType_HasFeature(type, Py_TPFLAGS_HEAPTYPE)) {
         // This field *must* be invalidated if the type is modified (see the
         // comment on struct _specialization_cache):
@@ -1208,6 +1214,9 @@ _PyType_GetVersionForCurrentState(PyTypeObject *tp)
 
 
 #define MAX_VERSIONS_PER_CLASS 1000
+#if _Py_ATTR_CACHE_UNUSED < MAX_VERSIONS_PER_CLASS
+#error "_Py_ATTR_CACHE_UNUSED must be bigger than max"
+#endif
 
 static int
 assign_version_tag(PyInterpreterState *interp, PyTypeObject *type)
@@ -1225,6 +1234,7 @@ assign_version_tag(PyInterpreterState *interp, PyTypeObject *type)
         return 0;
     }
     if (type->tp_versions_used >= MAX_VERSIONS_PER_CLASS) {
+        /* (this includes `tp_versions_used == _Py_ATTR_CACHE_UNUSED`) */
         return 0;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Do not use the type attribute cache for types with incompatible :term:`MRO`.