[Python-checkins] bpo-41295: Reimplement the Carlo Verre "hackcheck" (GH-21528)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/38d930f2ccbff6f93c4c54a7a6a1759266136504
commit: 38d930f2ccbff6f93c4c54a7a6a1759266136504
branch: 3.8
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2020-07-18T14:37:43-07:00
summary:

bpo-41295: Reimplement the Carlo Verre "hackcheck" (GH-21528)


Walk down the MRO backwards to find the type that originally defined the final `tp_setattro`, then make sure we are not jumping over intermediate C-level bases with the Python-level call.

Automerge-Triggered-By: @gvanrossum
(cherry picked from commit c53b310e5926266ce267c44a168165cacd786d6e)

Co-authored-by: scoder <stefan_ml at behnel.de>

files:
A Misc/NEWS.d/next/Core and Builtins/2020-07-18-08-15-32.bpo-41295.pu8Ezo.rst
M Lib/test/test_descr.py
M Objects/typeobject.c

diff --git a/Lib/test/test_descr.py b/Lib/test/test_descr.py
index c4d900a8c1bcb..18397a86b14c5 100644
--- a/Lib/test/test_descr.py
+++ b/Lib/test/test_descr.py
@@ -4315,6 +4315,42 @@ def test_carloverre(self):
         else:
             self.fail("Carlo Verre __delattr__ succeeded!")
 
+    def test_carloverre_multi_inherit_valid(self):
+        class A(type):
+            def __setattr__(cls, key, value):
+                type.__setattr__(cls, key, value)
+
+        class B:
+            pass
+
+        class C(B, A):
+            pass
+
+        obj = C('D', (object,), {})
+        try:
+            obj.test = True
+        except TypeError:
+            self.fail("setattr through direct base types should be legal")
+
+    def test_carloverre_multi_inherit_invalid(self):
+        class A(type):
+            def __setattr__(cls, key, value):
+                object.__setattr__(cls, key, value)  # this should fail!
+
+        class B:
+            pass
+
+        class C(B, A):
+            pass
+
+        obj = C('D', (object,), {})
+        try:
+            obj.test = True
+        except TypeError:
+            pass
+        else:
+            self.fail("setattr through indirect base types should be rejected")
+
     def test_weakref_segfault(self):
         # Testing weakref segfault...
         # SF 742911
diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-07-18-08-15-32.bpo-41295.pu8Ezo.rst b/Misc/NEWS.d/next/Core and Builtins/2020-07-18-08-15-32.bpo-41295.pu8Ezo.rst
new file mode 100644
index 0000000000000..d61fd8f0a2968
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2020-07-18-08-15-32.bpo-41295.pu8Ezo.rst	
@@ -0,0 +1,3 @@
+Resolve a regression in CPython 3.8.4 where defining "__setattr__" in a
+multi-inheritance setup and calling up the hierarchy chain could fail
+if builtins/extension types were involved in the base types.
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 00128698cd018..0acde714a87b1 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -5822,14 +5822,29 @@ hackcheck(PyObject *self, setattrofunc func, const char *what)
         return 1;
     }
     assert(PyTuple_Check(mro));
-    Py_ssize_t i, n;
-    n = PyTuple_GET_SIZE(mro);
-    for (i = 0; i < n; i++) {
+
+    /* Find the (base) type that defined the type's slot function. */
+    PyTypeObject *defining_type = type;
+    Py_ssize_t i;
+    for (i = PyTuple_GET_SIZE(mro) - 1; i >= 0; i--) {
         PyTypeObject *base = (PyTypeObject*) PyTuple_GET_ITEM(mro, i);
+        if (base->tp_setattro == slot_tp_setattro) {
+            /* Ignore Python classes:
+               they never define their own C-level setattro. */
+        }
+        else if (base->tp_setattro == type->tp_setattro) {
+            defining_type = base;
+            break;
+        }
+    }
+
+    /* Reject calls that jump over intermediate C-level overrides. */
+    for (PyTypeObject *base = defining_type; base; base = base->tp_base) {
         if (base->tp_setattro == func) {
-            /* 'func' is the earliest non-Python implementation in the MRO. */
+            /* 'func' is the right slot function to call. */
             break;
-        } else if (base->tp_setattro != slot_tp_setattro) {
+        }
+        else if (base->tp_setattro != slot_tp_setattro) {
             /* 'base' is not a Python class and overrides 'func'.
                Its tp_setattro should be called instead. */
             PyErr_Format(PyExc_TypeError,
@@ -5839,8 +5854,6 @@ hackcheck(PyObject *self, setattrofunc func, const char *what)
             return 0;
         }
     }
-    /* Either 'func' is not in the mro (which should fail when checking 'self'),
-       or it's the right slot function to call. */
     return 1;
 }