GH-94822: Don't specialize when metaclasses are involved (GH-94892)

python · Jul 18, 2022 · daf68ba · daf68ba
1 parent c41d4d0
commit daf68ba
Show file tree

Hide file tree

Showing 3 changed files with 351 additions and 6 deletions.
diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py
@@ -1,5 +1,6 @@
 import unittest
 
+
 class TestLoadAttrCache(unittest.TestCase):
     def test_descriptor_added_after_optimization(self):
         class Descriptor:
@@ -21,3 +22,346 @@ def f(o):
         Descriptor.__set__ = lambda *args: None
 
         self.assertEqual(f(o), 2)
+
+    def test_metaclass_descriptor_added_after_optimization(self):
+        class Descriptor:
+            pass
+
+        class Metaclass(type):
+            attribute = Descriptor()
+
+        class Class(metaclass=Metaclass):
+            attribute = True
+
+        def __get__(self, instance, owner):
+            return False
+
+        def __set__(self, instance, value):
+            return None
+
+        def f():
+            return Class.attribute
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Descriptor.__get__ = __get__
+        Descriptor.__set__ = __set__
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_metaclass_descriptor_shadows_class_attribute(self):
+        class Metaclass(type):
+            @property
+            def attribute(self):
+                return True
+
+        class Class(metaclass=Metaclass):
+            attribute = False
+
+        def f():
+            return Class.attribute
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+    def test_metaclass_set_descriptor_after_optimization(self):
+        class Metaclass(type):
+            pass
+
+        class Class(metaclass=Metaclass):
+            attribute = True
+
+        @property
+        def attribute(self):
+            return False
+
+        def f():
+            return Class.attribute
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Metaclass.attribute = attribute
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_metaclass_del_descriptor_after_optimization(self):
+        class Metaclass(type):
+            @property
+            def attribute(self):
+                return True
+
+        class Class(metaclass=Metaclass):
+            attribute = False
+
+        def f():
+            return Class.attribute
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        del Metaclass.attribute
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_type_descriptor_shadows_attribute_method(self):
+        class Class:
+            mro = None
+
+        def f():
+            return Class.mro
+
+        for _ in range(1025):
+            self.assertIsNone(f())
+
+    def test_type_descriptor_shadows_attribute_member(self):
+        class Class:
+            __base__ = None
+
+        def f():
+            return Class.__base__
+
+        for _ in range(1025):
+            self.assertIs(f(), object)
+
+    def test_type_descriptor_shadows_attribute_getset(self):
+        class Class:
+            __name__ = "Spam"
+
+        def f():
+            return Class.__name__
+
+        for _ in range(1025):
+            self.assertEqual(f(), "Class")
+
+    def test_metaclass_getattribute(self):
+        class Metaclass(type):
+            def __getattribute__(self, name):
+                return True
+
+        class Class(metaclass=Metaclass):
+            attribute = False
+
+        def f():
+            return Class.attribute
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+    def test_metaclass_swap(self):
+        class OldMetaclass(type):
+            @property
+            def attribute(self):
+                return True
+
+        class NewMetaclass(type):
+            @property
+            def attribute(self):
+                return False
+
+        class Class(metaclass=OldMetaclass):
+            pass
+
+        def f():
+            return Class.attribute
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Class.__class__ = NewMetaclass
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+
+class TestLoadMethodCache(unittest.TestCase):
+    def test_descriptor_added_after_optimization(self):
+        class Descriptor:
+            pass
+
+        class Class:
+            attribute = Descriptor()
+
+        def __get__(self, instance, owner):
+            return lambda: False
+
+        def __set__(self, instance, value):
+            return None
+
+        def attribute():
+            return True
+
+        instance = Class()
+        instance.attribute = attribute
+
+        def f():
+            return instance.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Descriptor.__get__ = __get__
+        Descriptor.__set__ = __set__
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_metaclass_descriptor_added_after_optimization(self):
+        class Descriptor:
+            pass
+
+        class Metaclass(type):
+            attribute = Descriptor()
+
+        class Class(metaclass=Metaclass):
+            def attribute():
+                return True
+
+        def __get__(self, instance, owner):
+            return lambda: False
+
+        def __set__(self, instance, value):
+            return None
+
+        def f():
+            return Class.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Descriptor.__get__ = __get__
+        Descriptor.__set__ = __set__
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_metaclass_descriptor_shadows_class_attribute(self):
+        class Metaclass(type):
+            @property
+            def attribute(self):
+                return lambda: True
+
+        class Class(metaclass=Metaclass):
+            def attribute():
+                return False
+
+        def f():
+            return Class.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+    def test_metaclass_set_descriptor_after_optimization(self):
+        class Metaclass(type):
+            pass
+
+        class Class(metaclass=Metaclass):
+            def attribute():
+                return True
+
+        @property
+        def attribute(self):
+            return lambda: False
+
+        def f():
+            return Class.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Metaclass.attribute = attribute
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_metaclass_del_descriptor_after_optimization(self):
+        class Metaclass(type):
+            @property
+            def attribute(self):
+                return lambda: True
+
+        class Class(metaclass=Metaclass):
+            def attribute():
+                return False
+
+        def f():
+            return Class.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        del Metaclass.attribute
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+    def test_type_descriptor_shadows_attribute_method(self):
+        class Class:
+            def mro():
+                return ["Spam", "eggs"]
+
+        def f():
+            return Class.mro()
+
+        for _ in range(1025):
+            self.assertEqual(f(), ["Spam", "eggs"])
+
+    def test_type_descriptor_shadows_attribute_member(self):
+        class Class:
+            def __base__():
+                return "Spam"
+
+        def f():
+            return Class.__base__()
+
+        for _ in range(1025):
+            self.assertNotEqual(f(), "Spam")
+
+    def test_metaclass_getattribute(self):
+        class Metaclass(type):
+            def __getattribute__(self, name):
+                return lambda: True
+
+        class Class(metaclass=Metaclass):
+            def attribute():
+                return False
+
+        def f():
+            return Class.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+    def test_metaclass_swap(self):
+        class OldMetaclass(type):
+            @property
+            def attribute(self):
+                return lambda: True
+
+        class NewMetaclass(type):
+            @property
+            def attribute(self):
+                return lambda: False
+
+        class Class(metaclass=OldMetaclass):
+            pass
+
+        def f():
+            return Class.attribute()
+
+        for _ in range(1025):
+            self.assertTrue(f())
+
+        Class.__class__ = NewMetaclass
+
+        for _ in range(1025):
+            self.assertFalse(f())
+
+
+if __name__ == "__main__":
+    import unittest
+    unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-07-15-22-16-08.gh-issue-94822.zRRzBN.rst b/Misc/NEWS.d/next/Core and Builtins/2022-07-15-22-16-08.gh-issue-94822.zRRzBN.rst
@@ -0,0 +1,2 @@
+Fix an issue where lookups of metaclass descriptors may be ignored when an
+identically-named attribute also exists on the class itself.
diff --git a/Python/specialize.c b/Python/specialize.c
@@ -945,6 +945,10 @@ specialize_class_load_attr(PyObject *owner, _Py_CODEUNIT *instr,
                              PyObject *name)
 {
     _PyLoadMethodCache *cache = (_PyLoadMethodCache *)(instr + 1);
+    if (!PyType_CheckExact(owner) || _PyType_Lookup(Py_TYPE(owner), name)) {
+        SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_ATTR_METACLASS_ATTRIBUTE);
+        return -1;
+    }
     PyObject *descr = NULL;
     DescriptorClassification kind = 0;
     kind = analyze_descriptor((PyTypeObject *)owner, name, &descr, 0);
@@ -957,12 +961,7 @@ specialize_class_load_attr(PyObject *owner, _Py_CODEUNIT *instr,
             return 0;
 #ifdef Py_STATS
         case ABSENT:
-            if (_PyType_Lookup(Py_TYPE(owner), name) != NULL) {
-                SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_ATTR_METACLASS_ATTRIBUTE);
-            }
-            else {
-                SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_EXPECTED_ERROR);
-            }
+            SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_EXPECTED_ERROR);
             return -1;
 #endif
         default:
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		Fix an issue where lookups of metaclass descriptors may be ignored when an
		identically-named attribute also exists on the class itself.