gh-155962: Specialize LOAD_ATTR after replacing an instance's __dict__

Author: koxudaxi

Lib/test/test_opcache.py

@@ -64,6 +64,75 @@ def f(self):
 
 
 class TestLoadAttrCache(unittest.TestCase):
+    @requires_specialization
+    def test_load_attr_replaced_dict_specializes(self):
+        class C:
+            class_value = "class"
+
+            def method(self):
+                return "class method"
+
+        c = C()
+        c.__dict__ = {
+            "instance_value": 42,
+            "class_value": "instance",
+            "method": "instance method",
+        }
+
+        def get_values():
+            return c.instance_value, c.class_value, c.method
+
+        expected = (42, "instance", "instance method")
+        for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
+            self.assertEqual(get_values(), expected)
+
+        opnames = [
+            instruction.opname
+            for instruction in dis.get_instructions(get_values, adaptive=True)
+        ]
+        self.assertEqual(opnames.count("LOAD_ATTR_WITH_HINT"), 3)
+
+    @requires_specialization
+    def test_load_attr_replaced_general_dict_does_not_specialize(self):
+        class C:
+            pass
+
+        c = C()
+        c.__dict__ = {"x": 42, 1: None}
+
+        def get_x():
+            return c.x
+
+        for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
+            self.assertEqual(get_x(), 42)
+
+        opnames = {
+            instruction.opname
+            for instruction in dis.get_instructions(get_x, adaptive=True)
+        }
+        self.assertNotIn("LOAD_ATTR_WITH_HINT", opnames)
+
+    @requires_specialization
+    def test_load_attr_replaced_dict_ignores_stale_shared_key(self):
+        class C:
+            x = "class"
+
+        c = C()
+        c.x = "instance"
+        c.__dict__ = {}
+
+        def get_x():
+            return c.x
+
+        for _ in range(_testinternalcapi.SPECIALIZATION_THRESHOLD):
+            self.assertEqual(get_x(), "class")
+
+        opnames = {
+            instruction.opname
+            for instruction in dis.get_instructions(get_x, adaptive=True)
+        }
+        self.assertNotIn("LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES", opnames)
+
     def test_descriptor_added_after_optimization(self):
         class Descriptor:
             pass
@@ -1086,6 +1155,30 @@ def write(items):
         opname = "LOAD_ATTR_WITH_HINT"
         self.assert_races_do_not_crash(opname, get_items, read, write)
 
+    @requires_specialization
+    def test_load_attr_with_hint_replaced_dict(self):
+        def get_items():
+            class C:
+                pass
+
+            items = []
+            for _ in range(self.ITEMS):
+                item = C()
+                item.__dict__ = {"a": None}
+                items.append(item)
+            return items
+
+        def read(items):
+            for item in items:
+                item.a
+
+        def write(items):
+            for item in items:
+                item.__dict__ = {"padding": None, "a": None}
+
+        opname = "LOAD_ATTR_WITH_HINT"
+        self.assert_races_do_not_crash(opname, get_items, read, write)
+
     @requires_specialization
     def test_load_global_module(self):
         if not have_dict_key_versions():

Python/specialize.c

@@ -677,6 +677,10 @@ specialize_dict_access_hint(
         SPECIALIZATION_FAIL(base_op, SPEC_FAIL_ATTR_SPLIT_DICT);
         return 0;
     }
+    if (dict->ma_keys->dk_kind != DICT_KEYS_UNICODE) {
+        SPECIALIZATION_FAIL(base_op, SPEC_FAIL_ATTR_NON_STRING);
+        return 0;
+    }
     Py_ssize_t index = _PyDict_LookupIndex(dict, name);
     if (index != (uint16_t)index) {
         SPECIALIZATION_FAIL(base_op,
@@ -750,10 +754,9 @@ specialize_attr_loadclassattr(PyObject *owner, _Py_CODEUNIT *instr,
                               uint32_t shared_keys_version);
 static int specialize_class_load_attr(PyObject* owner, _Py_CODEUNIT* instr, PyObject* name);
 
-/* Returns true if instances of obj's class are
- * likely to have `name` in their __dict__.
- * For objects with inline values, we check in the shared keys.
- * For other objects, we check their actual dictionary.
+/* Returns true if obj is likely to have `name` in its __dict__.
+ * For objects with valid inline values, we check in the shared keys.
+ * Otherwise, we check their actual dictionary.
  */
 static bool
 instance_has_key(PyObject *obj, PyObject *name, uint32_t *shared_keys_version)
@@ -762,7 +765,8 @@ instance_has_key(PyObject *obj, PyObject *name, uint32_t *shared_keys_version)
     if ((cls->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
         return false;
     }
-    if (cls->tp_flags & Py_TPFLAGS_INLINE_VALUES) {
+    if ((cls->tp_flags & Py_TPFLAGS_INLINE_VALUES) &&
+        FT_ATOMIC_LOAD_UINT8(_PyObject_InlineValues(obj)->valid)) {
         PyDictKeysObject *keys = ((PyHeapTypeObject *)cls)->ht_cached_keys;
         Py_ssize_t index =
             _PyDictKeys_StringLookupAndVersion(keys, name, shared_keys_version);
@@ -1281,14 +1285,14 @@ specialize_attr_loadclassattr(PyObject *owner, _Py_CODEUNIT *instr,
 
     unsigned long tp_flags = PyType_GetFlags(owner_cls);
     if (tp_flags & Py_TPFLAGS_INLINE_VALUES) {
-        #ifndef Py_GIL_DISABLED
-        assert(_PyDictKeys_StringLookup(
-                   ((PyHeapTypeObject *)owner_cls)->ht_cached_keys, name) < 0);
-        #endif
         if (shared_keys_version == 0) {
             SPECIALIZATION_FAIL(LOAD_ATTR, SPEC_FAIL_OUT_OF_VERSIONS);
             return 0;
         }
+        #ifndef Py_GIL_DISABLED
+        assert(_PyDictKeys_StringLookup(
+                   ((PyHeapTypeObject *)owner_cls)->ht_cached_keys, name) < 0);
+        #endif
         specialize(instr, is_method ? LOAD_ATTR_METHOD_WITH_VALUES : LOAD_ATTR_NONDESCRIPTOR_WITH_VALUES);
     }
     else {