Mailing List Archive

https://github.com/python/cpython/commit/3ef3c6306def489ba9115f0a8a57ab1e99795a5c
commit: 3ef3c6306def489ba9115f0a8a57ab1e99795a5c
branch: main
author: Mark Shannon <mark@hotpy.org>
committer: markshannon <mark@hotpy.org>
date: 2022-08-15T12:29:27+01:00
summary:

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.

files:
A Misc/NEWS.d/next/C API/2022-08-03-13-01-57.gh-issue-92678.DLwONN.rst
M Lib/test/test_capi.py
M Lib/test/test_sys.py
M Modules/_testcapi/heaptype.c
M Objects/object.c
M Objects/typeobject.c
M Tools/gdb/libpython.py

diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py
index 1ff14e7bc56f..e4d20355d430 100644
--- a/Lib/test/test_capi.py
+++ b/Lib/test/test_capi.py
@@ -539,6 +539,30 @@ def test_heaptype_with_dict(self):
inst = _testcapi.HeapCTypeWithDict()
self.assertEqual({}, inst.__dict__)

+ def test_heaptype_with_managed_dict(self):
+ inst = _testcapi.HeapCTypeWithManagedDict()
+ inst.foo = 42
+ self.assertEqual(inst.foo, 42)
+ self.assertEqual(inst.__dict__, {"foo": 42})
+
+ inst = _testcapi.HeapCTypeWithManagedDict()
+ self.assertEqual({}, inst.__dict__)
+
+ a = _testcapi.HeapCTypeWithManagedDict()
+ b = _testcapi.HeapCTypeWithManagedDict()
+ a.b = b
+ b.a = a
+ del a, b
+
+ def test_sublclassing_managed_dict(self):
+
+ class C(_testcapi.HeapCTypeWithManagedDict):
+ pass
+
+ i = C()
+ i.spam = i
+ del i
+
def test_heaptype_with_negative_dict(self):
inst = _testcapi.HeapCTypeWithNegativeDict()
inst.foo = 42
diff --git a/Lib/test/test_sys.py b/Lib/test/test_sys.py
index 05fbcc19b6b7..78da09d6abc0 100644
--- a/Lib/test/test_sys.py
+++ b/Lib/test/test_sys.py
@@ -1287,7 +1287,7 @@ class OverflowSizeof(int):
def __sizeof__(self):
return int(self)
self.assertEqual(sys.getsizeof(OverflowSizeof(sys.maxsize)),
- sys.maxsize + self.gc_headsize)
+ sys.maxsize + self.gc_headsize*2)
with self.assertRaises(OverflowError):
sys.getsizeof(OverflowSizeof(sys.maxsize + 1))
with self.assertRaises(ValueError):
diff --git a/Misc/NEWS.d/next/C API/2022-08-03-13-01-57.gh-issue-92678.DLwONN.rst b/Misc/NEWS.d/next/C API/2022-08-03-13-01-57.gh-issue-92678.DLwONN.rst
new file mode 100644
index 000000000000..a3a209f5e413
--- /dev/null
+++ b/Misc/NEWS.d/next/C API/2022-08-03-13-01-57.gh-issue-92678.DLwONN.rst
@@ -0,0 +1,2 @@
+Support C extensions using managed dictionaries by setting the
+``Py_TPFLAGS_MANAGED_DICT`` flag.
diff --git a/Modules/_testcapi/heaptype.c b/Modules/_testcapi/heaptype.c
index 514541ce348d..de990bac51d6 100644
--- a/Modules/_testcapi/heaptype.c
+++ b/Modules/_testcapi/heaptype.c
@@ -761,6 +761,45 @@ static PyType_Spec HeapCTypeWithDict2_spec = {
HeapCTypeWithDict_slots
};

+static int
+heapmanaged_traverse(HeapCTypeObject *self, visitproc visit, void *arg)
+{
+ Py_VISIT(Py_TYPE(self));
+ return _PyObject_VisitManagedDict((PyObject *)self, visit, arg);
+}
+
+static void
+heapmanaged_clear(HeapCTypeObject *self)
+{
+ _PyObject_ClearManagedDict((PyObject *)self);
+}
+
+static void
+heapmanaged_dealloc(HeapCTypeObject *self)
+{
+ PyTypeObject *tp = Py_TYPE(self);
+ _PyObject_ClearManagedDict((PyObject *)self);
+ PyObject_GC_UnTrack(self);
+ PyObject_GC_Del(self);
+ Py_DECREF(tp);
+}
+
+static PyType_Slot HeapCTypeWithManagedDict_slots[] = {
+ {Py_tp_traverse, heapmanaged_traverse},
+ {Py_tp_getset, heapctypewithdict_getsetlist},
+ {Py_tp_clear, heapmanaged_clear},
+ {Py_tp_dealloc, heapmanaged_dealloc},
+ {0, 0},
+};
+
+static PyType_Spec HeapCTypeWithManagedDict_spec = {
+ "_testcapi.HeapCTypeWithManagedDict",
+ sizeof(PyObject),
+ 0,
+ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_MANAGED_DICT,
+ HeapCTypeWithManagedDict_slots
+};
+
static struct PyMemberDef heapctypewithnegativedict_members[] = {
{"dictobj", T_OBJECT, offsetof(HeapCTypeWithDictObject, dict)},
{"__dictoffset__", T_PYSSIZET, -(Py_ssize_t)sizeof(void*), READONLY},
@@ -963,6 +1002,12 @@ _PyTestCapi_Init_Heaptype(PyObject *m) {
}
PyModule_AddObject(m, "HeapCTypeWithNegativeDict", HeapCTypeWithNegativeDict);

+ PyObject *HeapCTypeWithManagedDict = PyType_FromSpec(&HeapCTypeWithManagedDict_spec);
+ if (HeapCTypeWithManagedDict == NULL) {
+ return -1;
+ }
+ PyModule_AddObject(m, "HeapCTypeWithManagedDict", HeapCTypeWithManagedDict);
+
PyObject *HeapCTypeWithWeakref = PyType_FromSpec(&HeapCTypeWithWeakref_spec);
if (HeapCTypeWithWeakref == NULL) {
return -1;
diff --git a/Objects/object.c b/Objects/object.c
index a90c6faf99db..9bbe0eef308f 100644
--- a/Objects/object.c
+++ b/Objects/object.c
@@ -1064,6 +1064,7 @@ _PyObject_ComputedDictPointer(PyObject *obj)
if (dictoffset == 0)
return NULL;
if (dictoffset < 0) {
+ assert(dictoffset != -1);
Py_ssize_t tsize = Py_SIZE(obj);
if (tsize < 0) {
tsize = -tsize;
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
index 27b12a00d77f..67dfc6f8483d 100644
--- a/Objects/typeobject.c
+++ b/Objects/typeobject.c
@@ -1312,16 +1312,21 @@ subtype_traverse(PyObject *self, visitproc visit, void *arg)
assert(base);
}

- if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
- int err = _PyObject_VisitManagedDict(self, visit, arg);
- if (err) {
- return err;
+ if (type->tp_dictoffset != base->tp_dictoffset) {
+ assert(base->tp_dictoffset == 0);
+ if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
+ assert(type->tp_dictoffset == -1);
+ int err = _PyObject_VisitManagedDict(self, visit, arg);
+ if (err) {
+ return err;
+ }
+ }
+ else {
+ PyObject **dictptr = _PyObject_ComputedDictPointer(self);
+ if (dictptr && *dictptr) {
+ Py_VISIT(*dictptr);
+ }
}
- }
- else if (type->tp_dictoffset != base->tp_dictoffset) {
- PyObject **dictptr = _PyObject_ComputedDictPointer(self);
- if (dictptr && *dictptr)
- Py_VISIT(*dictptr);
}

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

- if (ctx->add_dict && ctx->base->tp_itemsize) {
- type->tp_dictoffset = -(long)sizeof(PyObject *);
- slotoffset += sizeof(PyObject *);
- }
-
if (ctx->add_weak) {
assert(!ctx->base->tp_itemsize);
type->tp_weaklistoffset = slotoffset;
slotoffset += sizeof(PyObject *);
}
- if (ctx->add_dict && ctx->base->tp_itemsize == 0) {
+ if (ctx->add_dict) {
assert((type->tp_flags & Py_TPFLAGS_MANAGED_DICT) == 0);
type->tp_flags |= Py_TPFLAGS_MANAGED_DICT;
- type->tp_dictoffset = -slotoffset - sizeof(PyObject *)*3;
+ type->tp_dictoffset = -1;
}

type->tp_basicsize = slotoffset;
@@ -6161,6 +6163,7 @@ inherit_special(PyTypeObject *type, PyTypeObject *base)
COPYVAL(tp_itemsize);
COPYVAL(tp_weaklistoffset);
COPYVAL(tp_dictoffset);
+
#undef COPYVAL

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

+static int
+type_ready_dict_offset(PyTypeObject *type)
+{
+ if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
+ if (type->tp_dictoffset > 0 || type->tp_dictoffset < -1) {
+ PyErr_Format(PyExc_TypeError,
+ "type %s has the Py_TPFLAGS_MANAGED_DICT flag "
+ "but tp_dictoffset is set",
+ type->tp_name);
+ return -1;
+ }
+ type->tp_dictoffset = -1;
+ }
+ return 0;
+}

static int
type_ready_mro(PyTypeObject *type)
@@ -6775,6 +6793,21 @@ type_ready_post_checks(PyTypeObject *type)
type->tp_name);
return -1;
}
+ if (type->tp_flags & Py_TPFLAGS_MANAGED_DICT) {
+ if (type->tp_dictoffset != -1) {
+ PyErr_Format(PyExc_SystemError,
+ "type %s has the Py_TPFLAGS_MANAGED_DICT flag "
+ "but tp_dictoffset is set to incompatible value",
+ type->tp_name);
+ return -1;
+ }
+ }
+ else if (type->tp_dictoffset < sizeof(PyObject)) {
+ if (type->tp_dictoffset + type->tp_basicsize <= 0) {
+ PyErr_Format(PyExc_SystemError,
+ "type %s has a tp_dictoffset that is too small");
+ }
+ }
return 0;
}

@@ -6814,6 +6847,9 @@ type_ready(PyTypeObject *type)
if (type_ready_inherit(type) < 0) {
return -1;
}
+ if (type_ready_dict_offset(type) < 0) {
+ return -1;
+ }
if (type_ready_set_hash(type) < 0) {
return -1;
}
diff --git a/Tools/gdb/libpython.py b/Tools/gdb/libpython.py
index d03c63791258..899cb6c7dea0 100755
--- a/Tools/gdb/libpython.py
+++ b/Tools/gdb/libpython.py
@@ -478,13 +478,17 @@ def get_attr_dict(self):
dictoffset = int_from_int(typeobj.field('tp_dictoffset'))
if dictoffset != 0:
if dictoffset < 0:
- type_PyVarObject_ptr = gdb.lookup_type('PyVarObject').pointer()
- tsize = int_from_int(self._gdbval.cast(type_PyVarObject_ptr)['ob_size'])
- if tsize < 0:
- tsize = -tsize
- size = _PyObject_VAR_SIZE(typeobj, tsize)
- dictoffset += size
- assert dictoffset % _sizeof_void_p() == 0
+ if int_from_int(typeobj.field('tp_flags')) & Py_TPFLAGS_MANAGED_DICT:
+ assert dictoffset == -1
+ dictoffset = -3 * _sizeof_void_p()
+ else:
+ type_PyVarObject_ptr = gdb.lookup_type('PyVarObject').pointer()
+ tsize = int_from_int(self._gdbval.cast(type_PyVarObject_ptr)['ob_size'])
+ if tsize < 0:
+ tsize = -tsize
+ size = _PyObject_VAR_SIZE(typeobj, tsize)
+ dictoffset += size
+ assert dictoffset % _sizeof_void_p() == 0

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

_______________________________________________
Python-checkins mailing list
Python-checkins@python.org
https://mail.python.org/mailman/listinfo/python-checkins