Skip to content

Commit

Permalink
bpo-41295: Reimplement the Carlo Verre "hackcheck" (pythonGH-21528)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
scoder authored and arun-mani-j committed Jul 21, 2020
1 parent 39373d2 commit 056364a
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 7 deletions.
36 changes: 36 additions & 0 deletions Lib/test/test_descr.py
Original file line number Diff line number Diff line change
Expand Up @@ -4308,6 +4308,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
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
27 changes: 20 additions & 7 deletions Objects/typeobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -5961,14 +5961,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,
Expand All @@ -5978,8 +5993,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;
}

Expand Down

0 comments on commit 056364a

Please sign in to comment.