bpo-42266: Handle monkey-patching descriptors in LOAD_ATTR cache (GH-23157)

pablogsal · web-flow · commit 80449f243b13 · 2020-11-05T09:23:15.000Z
diff --git a/Lib/test/test_opcache.py b/Lib/test/test_opcache.py
@@ -0,0 +1,23 @@
+import unittest
+
+class TestLoadAttrCache(unittest.TestCase):
+    def test_descriptor_added_after_optimization(self):
+        class Descriptor:
+            pass
+
+        class C:
+            def __init__(self):
+                self.x = 1
+            x = Descriptor()
+
+        def f(o):
+            return o.x
+
+        o = C()
+        for i in range(1025):
+            assert f(o) == 1
+
+        Descriptor.__get__ = lambda self, instance, value: 2
+        Descriptor.__set__ = lambda *args: None
+
+        self.assertEqual(f(o), 2)
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-11-04-23-03-25.bpo-42266.G4hGDe.rst b/Misc/NEWS.d/next/Core and Builtins/2020-11-04-23-03-25.bpo-42266.G4hGDe.rst
@@ -0,0 +1,3 @@
+Fixed a bug with the LOAD_ATTR opcode cache that was not respecting
+monkey-patching a class-level attribute to make it a descriptor. Patch by
+Pablo Galindo.
diff --git a/PCbuild/lib.pyproj b/PCbuild/lib.pyproj
@@ -1196,6 +1196,7 @@
     <Compile Include="test\test_nntplib.py" />
     <Compile Include="test\test_ntpath.py" />
     <Compile Include="test\test_numeric_tower.py" />
+    <Compile Include="test\test_opcache.py" />
     <Compile Include="test\test_opcodes.py" />
     <Compile Include="test\test_openpty.py" />
     <Compile Include="test\test_operator.py" />
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -3179,7 +3179,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
                 if (co_opcache != NULL && /* co_opcache can be NULL after a DEOPT() call. */
                     type->tp_getattro == PyObject_GenericGetAttr)
                 {
-                    PyObject *descr;
                     Py_ssize_t ret;
 
                     if (type->tp_dictoffset > 0) {
@@ -3190,12 +3189,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, PyFrameObject *f, int throwflag)
                                 goto error;
                             }
                         }
-
-                        descr = _PyType_Lookup(type, name);
-                        if (descr == NULL ||
-                            Py_TYPE(descr)->tp_descr_get == NULL ||
-                            !PyDescr_IsData(descr))
-                        {
+                        if (_PyType_Lookup(type, name) == NULL) {
                             dictptr = (PyObject **) ((char *)owner + type->tp_dictoffset);
                             dict = *dictptr;
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+Fixed a bug with the LOAD_ATTR opcode cache that was not respecting`
	`2`	`+monkey-patching a class-level attribute to make it a descriptor. Patch by`
	`3`	`+Pablo Galindo.`
Original file line number	Diff line number	Diff line change
`@@ -3179,7 +3179,6 @@ _PyEval_EvalFrameDefault(PyThreadState tstate, PyFrameObject f, int throwflag)`
`3179`	`3179`	`if (co_opcache != NULL && /* co_opcache can be NULL after a DEOPT() call. */`
`3180`	`3180`	`type->tp_getattro == PyObject_GenericGetAttr)`
`3181`	`3181`	`{`
`3182`		`- PyObject *descr;`
`3183`	`3182`	`Py_ssize_t ret;`
`3184`	`3183`
`3185`	`3184`	`if (type->tp_dictoffset > 0) {`
`@@ -3190,12 +3189,7 @@ _PyEval_EvalFrameDefault(PyThreadState tstate, PyFrameObject f, int throwflag)`
`3190`	`3189`	`goto error;`
`3191`	`3190`	`}`
`3192`	`3191`	`}`
`3193`		`-`
`3194`		`- descr = _PyType_Lookup(type, name);`
`3195`		`- if (descr == NULL \|\|`
`3196`		`- Py_TYPE(descr)->tp_descr_get == NULL \|\|`
`3197`		`- !PyDescr_IsData(descr))`
`3198`		`- {`
	`3192`	`+ if (_PyType_Lookup(type, name) == NULL) {`
`3199`	`3193`	`dictptr = (PyObject *) ((char )owner + type->tp_dictoffset);`
`3200`	`3194`	`dict = *dictptr;`
`3201`	`3195`