From b73e3b6d4a6c505f2869ae4d0a4a93241450cf32 Mon Sep 17 00:00:00 2001 From: Mark Shannon Date: Wed, 17 Aug 2022 12:50:53 +0100 Subject: [PATCH] GH-95589: Dont crash when subclassing extension classes with multiple inheritance (GH-96028) * Treat tp_weakref and tp_dictoffset like other opaque slots for multiple inheritance. * Document Py_TPFLAGS_MANAGED_DICT and Py_TPFLAGS_MANAGED_WEAKREF in what's new. --- Doc/whatsnew/3.12.rst | 17 +++++++ Lib/test/test_capi.py | 44 +++++++++++++----- ...2-08-16-16-54-42.gh-issue-95589.6xE1ar.rst | 4 ++ Objects/typeobject.c | 45 ++++++------------- 4 files changed, 67 insertions(+), 43 deletions(-) create mode 100644 Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 5926205ce5c07d..9689d9df9dfc45 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -450,6 +450,11 @@ New Features inherit the ``Py_TPFLAGS_HAVE_VECTORCALL`` flag. (Contributed by Petr Viktorin in :gh:`93274`.) + The :const:`Py_TPFLAGS_MANAGED_DICT` and :const:`Py_TPFLAGS_MANAGED_WEAKREF` + flags have been added. This allows extensions classes to support object + ``__dict__`` and weakrefs with less bookkeeping, + using less memory and with faster access. + Porting to Python 3.12 ---------------------- @@ -486,6 +491,18 @@ Porting to Python 3.12 :c:func:`PyUnicode_FromFormatV`. (Contributed by Philip Georgi in :gh:`95504`.) +* Extension classes wanting to add a ``__dict__`` or weak reference slot + should use :const:`Py_TPFLAGS_MANAGED_DICT` and + :const:`Py_TPFLAGS_MANAGED_WEAKREF` instead of ``tp_dictoffset`` and + ``tp_weaklistoffset``, respectively. + The use of ``tp_dictoffset`` and ``tp_weaklistoffset`` is still + supported, but does not fully support multiple inheritance + (:gh: `95589`), and performance may be worse. + Classes declaring :const:`Py_TPFLAGS_MANAGED_DICT` should call + :c:func:`_PyObject_VisitManagedDict` and :c:func:`_PyObject_ClearManagedDict` + to traverse and clear their instance's dictionaries. + To clear weakrefs, call :c:func:`PyObject_ClearWeakRefs`, as before. + Deprecated ---------- diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index e6516b33aec018..b698e34dec4e22 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -677,21 +677,43 @@ def test_heaptype_with_custom_metaclass(self): def test_multiple_inheritance_ctypes_with_weakref_or_dict(self): - class Both1(_testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithDict): + with self.assertRaises(TypeError): + class Both1(_testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithDict): + pass + with self.assertRaises(TypeError): + class Both2(_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref): + pass + + def test_multiple_inheritance_ctypes_with_weakref_or_dict_and_other_builtin(self): + + with self.assertRaises(TypeError): + class C1(_testcapi.HeapCTypeWithDict, list): + pass + + with self.assertRaises(TypeError): + class C2(_testcapi.HeapCTypeWithWeakref, list): + pass + + class C3(_testcapi.HeapCTypeWithManagedDict, list): pass - class Both2(_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref): + class C4(_testcapi.HeapCTypeWithManagedWeakref, list): pass - for cls in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithDict2, - _testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithWeakref2): - for cls2 in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithDict2, - _testcapi.HeapCTypeWithWeakref, _testcapi.HeapCTypeWithWeakref2): - if cls is not cls2: - class S(cls, cls2): - pass - class B1(Both1, cls): + inst = C3() + inst.append(0) + str(inst.__dict__) + + inst = C4() + inst.append(0) + str(inst.__weakref__) + + for cls in (_testcapi.HeapCTypeWithManagedDict, _testcapi.HeapCTypeWithManagedWeakref): + for cls2 in (_testcapi.HeapCTypeWithDict, _testcapi.HeapCTypeWithWeakref): + class S(cls, cls2): + pass + class B1(C3, cls): pass - class B2(Both1, cls): + class B2(C4, cls): pass def test_pytype_fromspec_with_repeated_slots(self): diff --git a/Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst b/Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst new file mode 100644 index 00000000000000..696e3c80db62cf --- /dev/null +++ b/Misc/NEWS.d/next/C API/2022-08-16-16-54-42.gh-issue-95589.6xE1ar.rst @@ -0,0 +1,4 @@ +Extensions classes that set ``tp_dictoffset`` and ``tp_weaklistoffset`` +lose the support for multiple inheritance, but are now safe. Extension +classes should use :const:`Py_TPFLAGS_MANAGED_DICT` and +:const:`Py_TPFLAGS_MANAGED_WEAKREF` instead. diff --git a/Objects/typeobject.c b/Objects/typeobject.c index f796a91b3c278b..e8c36cf1539911 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -2330,36 +2330,13 @@ best_base(PyObject *bases) return base; } -#define ADDED_FIELD_AT_OFFSET(name, offset) \ - (type->tp_ ## name && (base->tp_ ##name == 0) && \ - type->tp_ ## name + sizeof(PyObject *) == (offset) && \ - type->tp_flags & Py_TPFLAGS_HEAPTYPE) - static int -extra_ivars(PyTypeObject *type, PyTypeObject *base) +shape_differs(PyTypeObject *t1, PyTypeObject *t2) { - size_t t_size = type->tp_basicsize; - size_t b_size = base->tp_basicsize; - - assert(t_size >= b_size); /* Else type smaller than base! */ - if (type->tp_itemsize || base->tp_itemsize) { - /* If itemsize is involved, stricter rules */ - return t_size != b_size || - type->tp_itemsize != base->tp_itemsize; - } - /* Check for __dict__ and __weakrefs__ slots in either order */ - if (ADDED_FIELD_AT_OFFSET(weaklistoffset, t_size)) { - t_size -= sizeof(PyObject *); - } - if ((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0 && - ADDED_FIELD_AT_OFFSET(dictoffset, t_size)) { - t_size -= sizeof(PyObject *); - } - /* Check __weakrefs__ again, in case it precedes __dict__ */ - if (ADDED_FIELD_AT_OFFSET(weaklistoffset, t_size)) { - t_size -= sizeof(PyObject *); - } - return t_size != b_size; + return ( + t1->tp_basicsize != t2->tp_basicsize || + t1->tp_itemsize != t2->tp_itemsize + ); } static PyTypeObject * @@ -2367,14 +2344,18 @@ solid_base(PyTypeObject *type) { PyTypeObject *base; - if (type->tp_base) + if (type->tp_base) { base = solid_base(type->tp_base); - else + } + else { base = &PyBaseObject_Type; - if (extra_ivars(type, base)) + } + if (shape_differs(type, base)) { return type; - else + } + else { return base; + } } static void object_dealloc(PyObject *);