diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py index 61f337d70ea787..550c430b24e17b 100644 --- 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,296 @@ 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()) + + +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()) + + +if __name__ == "__main__": + import unittest + unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-07-13-16-42-57.gh-issue-94822.zRRzBN.rst b/Misc/NEWS.d/next/Core and Builtins/2022-07-13-16-42-57.gh-issue-94822.zRRzBN.rst new file mode 100644 index 00000000000000..5b24918e497706 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-07-13-16-42-57.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/ceval.c b/Python/ceval.c index 2a0ce63d435d7c..8091bf43c2b76f 100644 --- a/Python/ceval.c +++ b/Python/ceval.c @@ -3672,7 +3672,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int DEOPT_IF(((PyTypeObject *)cls)->tp_version_tag != type_version, LOAD_ATTR); assert(type_version != 0); - + uint32_t meta_version = read_u32(cache->keys_version); + DEOPT_IF(Py_TYPE(cls)->tp_version_tag != meta_version, LOAD_ATTR); + assert(meta_version != 0); STAT_INC(LOAD_ATTR, hit); PyObject *res = read_obj(cache->descr); assert(res != NULL); diff --git a/Python/specialize.c b/Python/specialize.c index 66cae44fa8f3d7..63fcda99dbb97b 100644 --- a/Python/specialize.c +++ b/Python/specialize.c @@ -945,6 +945,20 @@ specialize_class_load_attr(PyObject *owner, _Py_CODEUNIT *instr, PyObject *name) { _PyLoadMethodCache *cache = (_PyLoadMethodCache *)(instr + 1); + PyTypeObject *metaclass = Py_TYPE(owner); + if (_PyType_Lookup(metaclass, name)) { + // The metaclass has the named attribute. This breaks LOAD_ATTR_CLASS + // when the attribute *also* exists on the class... but it also means we + // can't specialize for things like Class.__doc__, Class.__name__, etc. + // Perhaps LOAD_ATTR_METACLASS may be worth adding in the future? + SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_ATTR_METACLASS_ATTRIBUTE); + return -1; + } + if (metaclass->tp_getattro != PyType_Type.tp_getattro) { + // The metaclass defines __getattribute__. All bets are off: + SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OVERRIDDEN); + return -1; + } PyObject *descr = NULL; DescriptorClassification kind = 0; kind = analyze_descriptor((PyTypeObject *)owner, name, &descr, 0); @@ -952,17 +966,13 @@ specialize_class_load_attr(PyObject *owner, _Py_CODEUNIT *instr, case METHOD: case NON_DESCRIPTOR: write_u32(cache->type_version, ((PyTypeObject *)owner)->tp_version_tag); + write_u32(cache->keys_version, metaclass->tp_version_tag); write_obj(cache->descr, descr); _Py_SET_OPCODE(*instr, LOAD_ATTR_CLASS); 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: