From d49b4f83fbdcbf1fd7c121a4b3c69388695c9065 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 3 Jun 2022 14:46:05 +0200 Subject: [PATCH 1/3] Document PyType_Spec doesn't accept repeated slot IDs; raise where this was problematic --- Doc/c-api/type.rst | 2 ++ Lib/test/test_capi.py | 6 ++++ Modules/_testcapimodule.c | 59 +++++++++++++++++++++++++++++++++++++++ Objects/typeobject.c | 16 +++++++++-- 4 files changed, 81 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 99b3845237d868..189eb6f03e478c 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -266,6 +266,8 @@ The following functions and structs are used to create Array of :c:type:`PyType_Slot` structures. Terminated by the special slot value ``{0, NULL}``. + Each slot ID should be specified at most once. + .. c:type:: PyType_Slot Structure defining optional functionality of a type, containing a slot ID diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 95930ba40ceb51..085f2ca4a3634a 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -618,6 +618,12 @@ def test_heaptype_with_custom_metaclass(self): with self.assertRaisesRegex(TypeError, msg): t = _testcapi.pytype_fromspec_meta(_testcapi.HeapCTypeMetaclassCustomNew) + def test_pytype_fromspec_with_repeated_slots(self): + for variant in range(2): + with self.subTest(variant=variant): + with self.assertRaises(RuntimeError): + _testcapi.create_type_from_repeated_slots(variant) + def test_pynumber_tobase(self): from _testcapi import pynumber_tobase self.assertEqual(pynumber_tobase(123, 2), '0b1111011') diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index ac0c96a11d3f4a..35917c3628e3a3 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -1327,6 +1327,63 @@ test_type_from_ephemeral_spec(PyObject *self, PyObject *Py_UNUSED(ignored)) return result; } +PyType_Slot repeated_doc_slots[] = { + {Py_tp_doc, "A class used for tests·"}, + {Py_tp_doc, "A class used for tests"}, + {0, 0}, +}; + +PyType_Spec repeated_doc_slots_spec = { + .name = "RepeatedDocSlotClass", + .basicsize = sizeof(PyObject), + .slots = repeated_doc_slots, +}; + +typedef struct { + PyObject_HEAD + int data; +} HeapCTypeWithDataObject; + + +static struct PyMemberDef members_to_repeat[] = { + {"T_INT", T_INT, offsetof(HeapCTypeWithDataObject, data), 0, NULL}, + {NULL} +}; + +PyType_Slot repeated_members_slots[] = { + {Py_tp_members, members_to_repeat}, + {Py_tp_members, members_to_repeat}, + {0, 0}, +}; + +PyType_Spec repeated_members_slots_spec = { + .name = "RepeatedMembersSlotClass", + .basicsize = sizeof(HeapCTypeWithDataObject), + .slots = repeated_members_slots, +}; + +static PyObject * +create_type_from_repeated_slots(PyObject *self, PyObject *variant_obj) +{ + PyObject *class = NULL; + int variant = PyLong_AsLong(variant_obj); + if (PyErr_Occurred()) { + return NULL; + } + switch (variant) { + case 0: + class = PyType_FromSpec(&repeated_doc_slots_spec); + break; + case 1: + class = PyType_FromSpec(&repeated_members_slots_spec); + break; + default: + PyErr_SetString(PyExc_ValueError, "bad test variant"); + break; + } + return class; +} + static PyObject * test_get_type_qualname(PyObject *self, PyObject *Py_UNUSED(ignored)) @@ -5952,6 +6009,8 @@ static PyMethodDef TestMethods[] = { {"test_get_type_name", test_get_type_name, METH_NOARGS}, {"test_get_type_qualname", test_get_type_qualname, METH_NOARGS}, {"test_type_from_ephemeral_spec", test_type_from_ephemeral_spec, METH_NOARGS}, + {"create_type_from_repeated_slots", + create_type_from_repeated_slots, METH_O}, {"get_kwargs", _PyCFunction_CAST(get_kwargs), METH_VARARGS|METH_KEYWORDS}, {"getargs_tuple", getargs_tuple, METH_VARARGS}, diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 2566d217a0f3be..56272b44bf0477 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3371,7 +3371,8 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, int r; const PyType_Slot *slot; - Py_ssize_t nmembers, weaklistoffset, dictoffset, vectorcalloffset; + Py_ssize_t nmembers = 0; + Py_ssize_t weaklistoffset, dictoffset, vectorcalloffset; char *res_start; short slot_offset, subslot_offset; @@ -3388,7 +3389,12 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, nmembers = weaklistoffset = dictoffset = vectorcalloffset = 0; for (slot = spec->slots; slot->slot; slot++) { if (slot->slot == Py_tp_members) { - nmembers = 0; + if (nmembers != 0) { + PyErr_SetString( + PyExc_RuntimeError, + "Multiple Py_tp_members slots are not supported."); + return NULL; + } for (const PyMemberDef *memb = slot->pfunc; memb->name != NULL; memb++) { nmembers++; if (strcmp(memb->name, "__weaklistoffset__") == 0) { @@ -3535,6 +3541,12 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, else if (slot->slot == Py_tp_doc) { /* For the docstring slot, which usually points to a static string literal, we need to make a copy */ + if (type->tp_doc != NULL) { + PyErr_SetString( + PyExc_RuntimeError, + "Multiple Py_tp_doc slots are not supported."); + return NULL; + } if (slot->pfunc == NULL) { type->tp_doc = NULL; continue; From b87b1f2a62f1da9f273f8e9a9046530230bd5016 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 3 Jun 2022 14:54:49 +0200 Subject: [PATCH 2/3] Add blurb --- .../next/C API/2022-06-03-14-54-41.gh-issue-93466.DDtH0X.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2022-06-03-14-54-41.gh-issue-93466.DDtH0X.rst diff --git a/Misc/NEWS.d/next/C API/2022-06-03-14-54-41.gh-issue-93466.DDtH0X.rst b/Misc/NEWS.d/next/C API/2022-06-03-14-54-41.gh-issue-93466.DDtH0X.rst new file mode 100644 index 00000000000000..0bb65ea2b38786 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2022-06-03-14-54-41.gh-issue-93466.DDtH0X.rst @@ -0,0 +1,3 @@ +Slot IDs in PyType_Spec may not be repeated. The documentation was updated +to mention this. For some cases of repeated slots, PyType_FromSpec and +related functions will now raise an exception. From d115147bf3577ae6051e4df4692a887e42b9f325 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 10 Jun 2022 10:01:46 +0200 Subject: [PATCH 3/3] Use RuntimeError --- Lib/test/test_capi.py | 2 +- Objects/typeobject.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_capi.py b/Lib/test/test_capi.py index 085f2ca4a3634a..cd6a4de47a73d8 100644 --- a/Lib/test/test_capi.py +++ b/Lib/test/test_capi.py @@ -621,7 +621,7 @@ def test_heaptype_with_custom_metaclass(self): def test_pytype_fromspec_with_repeated_slots(self): for variant in range(2): with self.subTest(variant=variant): - with self.assertRaises(RuntimeError): + with self.assertRaises(SystemError): _testcapi.create_type_from_repeated_slots(variant) def test_pynumber_tobase(self): diff --git a/Objects/typeobject.c b/Objects/typeobject.c index a54fe08e621865..e57651446f888f 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3419,7 +3419,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, if (slot->slot == Py_tp_members) { if (nmembers != 0) { PyErr_SetString( - PyExc_RuntimeError, + PyExc_SystemError, "Multiple Py_tp_members slots are not supported."); return NULL; } @@ -3567,7 +3567,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, literal, we need to make a copy */ if (type->tp_doc != NULL) { PyErr_SetString( - PyExc_RuntimeError, + PyExc_SystemError, "Multiple Py_tp_doc slots are not supported."); return NULL; }