Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-41295: Reimplement the Carlo Verre "hackcheck" #21528

Merged
merged 4 commits into from
Jul 18, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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