Skip to content

Commit 3ef3c63

Browse files
markshannonmarkshannon
authored
GH-95707: Fix uses of Py_TPFLAGS_MANAGED_DICT (GH-95854)
* Make sure that tp_dictoffset is correct with Py_TPFLAGS_MANAGED_DICT is set. * Avoid traversing managed dict twice when subclassing class with Py_TPFLAGS_MANAGED_DICT set.
1 parent 4a7f5a5 commit 3ef3c63

File tree

7 files changed

+137
-25
lines changed

7 files changed

+137
-25
lines changed

‎Lib/test/test_capi.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -539,6 +539,30 @@ def test_heaptype_with_dict(self):
539539
inst = _testcapi.HeapCTypeWithDict()
540540
self.assertEqual({}, inst.__dict__)
541541

542+
def test_heaptype_with_managed_dict(self):
543+
inst = _testcapi.HeapCTypeWithManagedDict()
544+
inst.foo = 42
545+
self.assertEqual(inst.foo, 42)
546+
self.assertEqual(inst.__dict__, {"foo": 42})
547+
548+
inst = _testcapi.HeapCTypeWithManagedDict()
549+
self.assertEqual({}, inst.__dict__)
550+
551+
a = _testcapi.HeapCTypeWithManagedDict()
552+
b = _testcapi.HeapCTypeWithManagedDict()
553+
a.b = b
554+
b.a = a
555+
del a, b
556+
557+
def test_sublclassing_managed_dict(self):
558+
559+
class C(_testcapi.HeapCTypeWithManagedDict):
560+
pass
561+
562+
i = C()
563+
i.spam = i
564+
del i
565+
542566
def test_heaptype_with_negative_dict(self):
543567
inst = _testcapi.HeapCTypeWithNegativeDict()
544568
inst.foo = 42

‎Lib/test/test_sys.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1287,7 +1287,7 @@ class OverflowSizeof(int):
12871287
def __sizeof__(self):
12881288
return int(self)
12891289
self.assertEqual(sys.getsizeof(OverflowSizeof(sys.maxsize)),
1290-
sys.maxsize + self.gc_headsize)
1290+
sys.maxsize + self.gc_headsize*2)
12911291
with self.assertRaises(OverflowError):
12921292
sys.getsizeof(OverflowSizeof(sys.maxsize + 1))
12931293
with self.assertRaises(ValueError):

‎Misc/NEWS.d/next/C API/2022-08-03-13-01-57.gh-issue-92678.DLwONN.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Support C extensions using managed dictionaries by setting the
2+
``Py_TPFLAGS_MANAGED_DICT`` flag.

‎Modules/_testcapi/heaptype.c

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,45 @@ static PyType_Spec HeapCTypeWithDict2_spec = {
761761
HeapCTypeWithDict_slots
762762
};
763763

764+
static int
765+
heapmanaged_traverse(HeapCTypeObject *self, visitproc visit, void *arg)
766+
{
767+
Py_VISIT(Py_TYPE(self));
768+
return _PyObject_VisitManagedDict((PyObject *)self, visit, arg);
769+
}
770+
771+
static void
772+
heapmanaged_clear(HeapCTypeObject *self)
773+
{
774+
_PyObject_ClearManagedDict((PyObject *)self);
775+
}
776+
777+
static void
778+
heapmanaged_dealloc(HeapCTypeObject *self)
779+
{
780+
PyTypeObject *tp = Py_TYPE(self);
781+
_PyObject_ClearManagedDict((PyObject *)self);
782+
PyObject_GC_UnTrack(self);
783+
PyObject_GC_Del(self);
784+
Py_DECREF(tp);
785+
}
786+
787+
static PyType_Slot HeapCTypeWithManagedDict_slots[] = {
788+
{Py_tp_traverse, heapmanaged_traverse},
789+
{Py_tp_getset, heapctypewithdict_getsetlist},
790+
{Py_tp_clear, heapmanaged_clear},
791+
{Py_tp_dealloc, heapmanaged_dealloc},
792+
{0, 0},
793+
};
794+
795+
static PyType_Spec HeapCTypeWithManagedDict_spec = {
796+
"_testcapi.HeapCTypeWithManagedDict",
797+
sizeof(PyObject),
798+
0,
799+
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_MANAGED_DICT,
800+
HeapCTypeWithManagedDict_slots
801+
};
802+
764803
static struct PyMemberDef heapctypewithnegativedict_members[] = {
765804
{"dictobj", T_OBJECT, offsetof(HeapCTypeWithDictObject, dict)},
766805
{"__dictoffset__", T_PYSSIZET, -(Py_ssize_t)sizeof(void*), READONLY},
@@ -963,6 +1002,12 @@ _PyTestCapi_Init_Heaptype(PyObject *m) {
9631002
}
9641003
PyModule_AddObject(m, "HeapCTypeWithNegativeDict", HeapCTypeWithNegativeDict);
9651004

1005+
PyObject *HeapCTypeWithManagedDict = PyType_FromSpec(&HeapCTypeWithManagedDict_spec);
1006+
if (HeapCTypeWithManagedDict == NULL) {
1007+
return -1;
1008+
}
1009+
PyModule_AddObject(m, "HeapCTypeWithManagedDict", HeapCTypeWithManagedDict);
1010+
9661011
PyObject *HeapCTypeWithWeakref = PyType_FromSpec(&HeapCTypeWithWeakref_spec);
9671012
if (HeapCTypeWithWeakref == NULL) {
9681013
return -1;

‎Objects/object.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1064,6 +1064,7 @@ _PyObject_ComputedDictPointer(PyObject *obj)
10641064
if (dictoffset == 0)
10651065
return NULL;
10661066
if (dictoffset < 0) {
1067+
assert(dictoffset != -1);
10671068
Py_ssize_t tsize = Py_SIZE(obj);
10681069
if (tsize < 0) {
10691070
tsize = -tsize;

‎Objects/typeobject.c

Lines changed: 53 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,16 +1312,21 @@ subtype_traverse(PyObject *self, visitproc visit, void *arg)
13121312
assert(base);
13131313
}
13141314

1315-
if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
1316-
int err = _PyObject_VisitManagedDict(self, visit, arg);
1317-
if (err) {
1318-
return err;
1315+
if (type->tp_dictoffset != base->tp_dictoffset) {
1316+
assert(base->tp_dictoffset == 0);
1317+
if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
1318+
assert(type->tp_dictoffset == -1);
1319+
int err = _PyObject_VisitManagedDict(self, visit, arg);
1320+
if (err) {
1321+
return err;
1322+
}
1323+
}
1324+
else {
1325+
PyObject **dictptr = _PyObject_ComputedDictPointer(self);
1326+
if (dictptr && *dictptr) {
1327+
Py_VISIT(*dictptr);
1328+
}
13191329
}
1320-
}
1321-
else if (type->tp_dictoffset != base->tp_dictoffset) {
1322-
PyObject **dictptr = _PyObject_ComputedDictPointer(self);
1323-
if (dictptr && *dictptr)
1324-
Py_VISIT(*dictptr);
13251330
}
13261331

13271332
if (type->tp_flags & Py_TPFLAGS_HEAPTYPE
@@ -1380,7 +1385,9 @@ subtype_clear(PyObject *self)
13801385
/* Clear the instance dict (if any), to break cycles involving only
13811386
__dict__ slots (as in the case 'self.__dict__ is self'). */
13821387
if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
1383-
_PyObject_ClearManagedDict(self);
1388+
if ((base->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0) {
1389+
_PyObject_ClearManagedDict(self);
1390+
}
13841391
}
13851392
else if (type->tp_dictoffset != base->tp_dictoffset) {
13861393
PyObject **dictptr = _PyObject_ComputedDictPointer(self);
@@ -3085,20 +3092,15 @@ type_new_descriptors(const type_new_ctx *ctx, PyTypeObject *type)
30853092
}
30863093
}
30873094

3088-
if (ctx->add_dict && ctx->base->tp_itemsize) {
3089-
type->tp_dictoffset = -(long)sizeof(PyObject *);
3090-
slotoffset += sizeof(PyObject *);
3091-
}
3092-
30933095
if (ctx->add_weak) {
30943096
assert(!ctx->base->tp_itemsize);
30953097
type->tp_weaklistoffset = slotoffset;
30963098
slotoffset += sizeof(PyObject *);
30973099
}
3098-
if (ctx->add_dict && ctx->base->tp_itemsize == 0) {
3100+
if (ctx->add_dict) {
30993101
assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0);
31003102
type->tp_flags |= Py_TPFLAGS_MANAGED_DICT;
3101-
type->tp_dictoffset = -slotoffset - sizeof(PyObject *)*3;
3103+
type->tp_dictoffset = -1;
31023104
}
31033105

31043106
type->tp_basicsize = slotoffset;
@@ -6161,6 +6163,7 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
61616163
COPYVAL(tp_itemsize);
61626164
COPYVAL(tp_weaklistoffset);
61636165
COPYVAL(tp_dictoffset);
6166+
61646167
#undef COPYVAL
61656168

61666169
/* Setup fast subclass flags */
@@ -6567,6 +6570,21 @@ type_ready_fill_dict(PyTypeObject *type)
65676570
return 0;
65686571
}
65696572

6573+
static int
6574+
type_ready_dict_offset(PyTypeObject *type)
6575+
{
6576+
if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
6577+
if (type->tp_dictoffset > 0 || type->tp_dictoffset < -1) {
6578+
PyErr_Format(PyExc_TypeError,
6579+
"type %s has the Py_TPFLAGS_MANAGED_DICT flag "
6580+
"but tp_dictoffset is set",
6581+
type->tp_name);
6582+
return -1;
6583+
}
6584+
type->tp_dictoffset = -1;
6585+
}
6586+
return 0;
6587+
}
65706588

65716589
static int
65726590
type_ready_mro(PyTypeObject *type)
@@ -6775,6 +6793,21 @@ type_ready_post_checks(PyTypeObject *type)
67756793
type->tp_name);
67766794
return -1;
67776795
}
6796+
if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
6797+
if (type->tp_dictoffset != -1) {
6798+
PyErr_Format(PyExc_SystemError,
6799+
"type %s has the Py_TPFLAGS_MANAGED_DICT flag "
6800+
"but tp_dictoffset is set to incompatible value",
6801+
type->tp_name);
6802+
return -1;
6803+
}
6804+
}
6805+
else if (type->tp_dictoffset < sizeof(PyObject)) {
6806+
if (type->tp_dictoffset + type->tp_basicsize <= 0) {
6807+
PyErr_Format(PyExc_SystemError,
6808+
"type %s has a tp_dictoffset that is too small");
6809+
}
6810+
}
67786811
return 0;
67796812
}
67806813

@@ -6814,6 +6847,9 @@ type_ready(PyTypeObject *type)
68146847
if (type_ready_inherit(type) < 0) {
68156848
return -1;
68166849
}
6850+
if (type_ready_dict_offset(type) < 0) {
6851+
return -1;
6852+
}
68176853
if (type_ready_set_hash(type) < 0) {
68186854
return -1;
68196855
}

‎Tools/gdb/libpython.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -478,13 +478,17 @@ def get_attr_dict(self):
478478
dictoffset = int_from_int(typeobj.field('tp_dictoffset'))
479479
if dictoffset != 0:
480480
if dictoffset < 0:
481-
type_PyVarObject_ptr = gdb.lookup_type('PyVarObject').pointer()
482-
tsize = int_from_int(self._gdbval.cast(type_PyVarObject_ptr)['ob_size'])
483-
if tsize < 0:
484-
tsize = -tsize
485-
size = _PyObject_VAR_SIZE(typeobj, tsize)
486-
dictoffset += size
487-
assert dictoffset % _sizeof_void_p() == 0
481+
if int_from_int(typeobj.field('tp_flags')) & Py_TPFLAGS_MANAGED_DICT:
482+
assert dictoffset == -1
483+
dictoffset = -3 * _sizeof_void_p()
484+
else:
485+
type_PyVarObject_ptr = gdb.lookup_type('PyVarObject').pointer()
486+
tsize = int_from_int(self._gdbval.cast(type_PyVarObject_ptr)['ob_size'])
487+
if tsize < 0:
488+
tsize = -tsize
489+
size = _PyObject_VAR_SIZE(typeobj, tsize)
490+
dictoffset += size
491+
assert dictoffset % _sizeof_void_p() == 0
488492

489493
dictptr = self._gdbval.cast(_type_char_ptr()) + dictoffset
490494
PyObjectPtrPtr = PyObjectPtr.get_gdb_type().pointer()

0 commit comments

Comments
 (0)