From a9679fa3165e5bce5a44c83fd57226850b327fc7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 10 Oct 2022 15:17:53 +0200 Subject: [PATCH 01/24] gh-103509: PEP 697 -- Limited C API for Extending Opaque Types --- Doc/c-api/object.rst | 39 ++++ Doc/c-api/structures.rst | 16 ++ Doc/c-api/type.rst | 48 ++++- Doc/c-api/typeobj.rst | 20 ++ Doc/data/stable_abi.dat | 2 + Include/cpython/object.h | 1 + Include/descrobject.h | 1 + Include/internal/pycore_object.h | 5 - Include/object.h | 5 + Lib/test/test_capi/test_misc.py | 114 +++++++++++ Lib/test/test_stable_abi_ctypes.py | 2 + Misc/stable_abi.toml | 9 + Modules/Setup.stdlib.in | 2 +- Modules/_testcapi/heaptype.c | 111 +++++++++++ Modules/_testcapi/heaptype_relative.c | 270 ++++++++++++++++++++++++++ Modules/_testcapi/parts.h | 1 + Modules/_testcapimodule.c | 3 + Objects/descrobject.c | 6 + Objects/typeobject.c | 102 +++++++++- PC/python3dll.c | 2 + PCbuild/_testcapi.vcxproj | 1 + PCbuild/_testcapi.vcxproj.filters | 3 + Python/structmember.c | 12 ++ 23 files changed, 758 insertions(+), 17 deletions(-) create mode 100644 Modules/_testcapi/heaptype_relative.c diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 0a12bb9e8c54f0..ed64f0576ff5a7 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -395,3 +395,42 @@ Object Protocol returns ``NULL`` if the object cannot be iterated. .. versionadded:: 3.10 + +.. c:function:: PyObject* PyObject_GetTypeData(PyObject *o, PyTypeObject *cls) + + Get a pointer to subclass-specific data reserved for *cls*. + + The object *o* **must** be an instance of *cls*, and *cls* must have been + created using negative :c:member:`PyType_Spec.basicsize`. + Python does not check this. + + On error, set an exception and return ``NULL``. + + .. versionadded:: 3.12 + +.. c:function:: Py_ssize_t PyType_GetTypeDataSize(PyTypeObject *cls) + + Return the size of the memory reserved for *cls*, i.e. the size of the + memory :c:func:`PyObject_GetTypeData` returns. + + This may be larger than requested using :c:member:`-PyType_Spec.basicsize `; + it is safe to use this larger size (e.g. with :c:func:`!memset`). + + The type *cls* **must** have been created using + negative :c:member:`PyType_Spec.basicsize`. + Python does not check this. + + On error, set an exception and return a negative value. + + .. versionadded:: 3.12 + +.. c:function:: PyObject* PyObject_GetItemData(PyObject *o) + + Get a pointer to per-item data for a class with + :c:macro:`Py_TPFLAGS_ITEMS_AT_END`. + + On error, set an exception and return ``NULL``. + :py:exc:`TypeError` is raised if *o* does not have + :c:macro:`Py_TPFLAGS_ITEMS_AT_END` set. + + .. versionadded:: 3.12 diff --git a/Doc/c-api/structures.rst b/Doc/c-api/structures.rst index 9618a0cf676972..338db6378d240e 100644 --- a/Doc/c-api/structures.rst +++ b/Doc/c-api/structures.rst @@ -486,6 +486,22 @@ The following flags can be used with :c:member:`PyMemberDef.flags`: Emit an ``object.__getattr__`` :ref:`audit event ` before reading. +.. c:macro:: Py_RELATIVE_OFFSET + + Indicates that the :c:member:`~PyMemberDef.offset` of this ``PyMemberDef`` + entry indicates an offset from the subclass-specific data, rather than + from ``PyObject``. + + Can only be used as part of :c:member:`Py_tp_members ` + :c:type:`slot ` when creating a class using negative + :c:member:`~PyTypeDef.basicsize`. + It is mandatory in that case. + + This flag is only used in :c:type:`PyTypeSlot`. + When setting :c:member:`~PyTypeObject.tp_members` during + class creation, Python clears it and sets + :c:member:`PyMemberDef.offset` to the offset from the ``PyObject`` struct. + .. index:: single: READ_RESTRICTED single: WRITE_RESTRICTED diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 7b5d1fac40ed87..bb133f7f98a84f 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -322,25 +322,57 @@ The following functions and structs are used to create Structure defining a type's behavior. - .. c:member:: const char* PyType_Spec.name + .. c:member:: const char* name Name of the type, used to set :c:member:`PyTypeObject.tp_name`. - .. c:member:: int PyType_Spec.basicsize - .. c:member:: int PyType_Spec.itemsize + .. c:member:: int basicsize - Size of the instance in bytes, used to set - :c:member:`PyTypeObject.tp_basicsize` and - :c:member:`PyTypeObject.tp_itemsize`. + If positive, specifies the size of the instance in bytes. + It is used to set :c:member:`PyTypeObject.tp_basicsize`. - .. c:member:: int PyType_Spec.flags + If zero, specifies that :c:member:`~PyTypeObject.tp_basicsize` + should be inherited. + + If negative, the absolute value specifies how much space instances of the + class need *in addition* to the superclass. + Use :c:func:`PyObject_GetTypeData` to get a pointer to subclass-specific + memory reserved this way. + + .. versionchanged:: 3.12 + + Previously, this field could not be negative. + + .. c:member:: int itemsize + + Size of one element of a variable-size type, in bytes + Used to set :c:member:`PyTypeObject.tp_itemsize`. + See ``tp_itemsize`` documentation for caveats. + + If zero, :c:member:`~PyTypeObject.tp_itemsize` is inherited. + Extending arbitrary variable-sized classes is dangerous, + since some types use a fixed offset for variable-sized memory, + which can then overlap fixed-sized memory used by a subclass. + To help prevent mistakes, inheriting ``itemsize`` is only possible + in the following situations: + + - The base is not variable-sized (its + :c:member:`~PyTypeObject.tp_itemsize`). + - The requested :c:member:`PyType_Spec.basicsize` is positive, + suggesting that the memory layout of the base class is known. + - The requested :c:member:`PyType_Spec.basicsize` is zero, + suggesting that the subclass does not access the instance's memory + directly. + - With the :c:macro:`Py_TPFLAGS_ITEMS_AT_END` flag. + + .. c:member:: int flags Type flags, used to set :c:member:`PyTypeObject.tp_flags`. If the ``Py_TPFLAGS_HEAPTYPE`` flag is not set, :c:func:`PyType_FromSpecWithBases` sets it automatically. - .. c:member:: PyType_Slot *PyType_Spec.slots + .. c:member:: PyType_Slot *slots Array of :c:type:`PyType_Slot` structures. Terminated by the special slot value ``{0, NULL}``. diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index fd8f49ccb1caab..2a6a28c2459ba5 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -1171,6 +1171,26 @@ and :c:type:`PyType_Type` effectively act as defaults.) :c:member:`~PyTypeObject.tp_weaklistoffset` field is set in a superclass. + .. c:macro:: Py_TPFLAGS_ITEMS_AT_END + + Only usable with variable-size types, i.e. ones with non-zero + :c:member:`~PyObject.tp_itemsize`. + + Indicates that the variable-sized portion of an instance of this type is + at the end of the instance's memory area, at an offset of + :c:expr:`Py_TYPE(obj)->tp_basicsize` (which may be different in each + subclass). + + When setting this flag, be sure that all superclasses either + use this memory layout, or are not variable-sized. + Python does not check this. + + .. versionadded:: 3.12 + + **Inheritance:** + + This flag is inherited. + .. XXX Document more flags here? diff --git a/Doc/data/stable_abi.dat b/Doc/data/stable_abi.dat index 4cc06d22baaa93..f112d268129fd1 100644 --- a/Doc/data/stable_abi.dat +++ b/Doc/data/stable_abi.dat @@ -521,6 +521,7 @@ function,PyObject_GetAttrString,3.2,, function,PyObject_GetBuffer,3.11,, function,PyObject_GetItem,3.2,, function,PyObject_GetIter,3.2,, +function,PyObject_GetTypeData,3.12,, function,PyObject_HasAttr,3.2,, function,PyObject_HasAttrString,3.2,, function,PyObject_Hash,3.2,, @@ -675,6 +676,7 @@ function,PyType_GetModuleState,3.10,, function,PyType_GetName,3.11,, function,PyType_GetQualName,3.11,, function,PyType_GetSlot,3.4,, +function,PyType_GetTypeDataSize,3.12,, function,PyType_IsSubtype,3.2,, function,PyType_Modified,3.2,, function,PyType_Ready,3.2,, diff --git a/Include/cpython/object.h b/Include/cpython/object.h index 98cc51cd7fee49..446a3ebb5de164 100644 --- a/Include/cpython/object.h +++ b/Include/cpython/object.h @@ -553,6 +553,7 @@ Py_DEPRECATED(3.11) typedef int UsingDeprecatedTrashcanMacro; Py_TRASHCAN_END; \ } while(0); +PyAPI_FUNC(void *) PyObject_GetItemData(PyObject *obj); PyAPI_FUNC(int) _PyObject_VisitManagedDict(PyObject *obj, visitproc visit, void *arg); PyAPI_FUNC(void) _PyObject_ClearManagedDict(PyObject *obj); diff --git a/Include/descrobject.h b/Include/descrobject.h index 0a420b865dfd1b..fd66d17b497a31 100644 --- a/Include/descrobject.h +++ b/Include/descrobject.h @@ -83,6 +83,7 @@ struct PyMemberDef { #define Py_READONLY 1 #define Py_AUDIT_READ 2 // Added in 3.10, harmless no-op before that #define _Py_WRITE_RESTRICTED 4 // Deprecated, no-op. Do not reuse the value. +#define Py_RELATIVE_OFFSET 8 PyAPI_FUNC(PyObject *) PyMember_GetOne(const char *, PyMemberDef *); PyAPI_FUNC(int) PyMember_SetOne(char *, PyMemberDef *, PyObject *); diff --git a/Include/internal/pycore_object.h b/Include/internal/pycore_object.h index b3d496ed6fc240..5d756c897ff452 100644 --- a/Include/internal/pycore_object.h +++ b/Include/internal/pycore_object.h @@ -376,11 +376,6 @@ extern int _PyObject_IsInstanceDictEmpty(PyObject *); extern int _PyType_HasSubclasses(PyTypeObject *); extern PyObject* _PyType_GetSubclasses(PyTypeObject *); -// Access macro to the members which are floating "behind" the object -static inline PyMemberDef* _PyHeapType_GET_MEMBERS(PyHeapTypeObject *etype) { - return (PyMemberDef*)((char*)etype + Py_TYPE(etype)->tp_basicsize); -} - PyAPI_FUNC(PyObject *) _PyObject_LookupSpecial(PyObject *, PyObject *); /* C function call trampolines to mitigate bad function pointer casts. diff --git a/Include/object.h b/Include/object.h index 2943a6066818cd..57cf4a06044ba3 100644 --- a/Include/object.h +++ b/Include/object.h @@ -270,6 +270,8 @@ PyAPI_FUNC(PyObject *) PyType_GetQualName(PyTypeObject *); #endif #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x030C0000 PyAPI_FUNC(PyObject *) PyType_FromMetaclass(PyTypeObject*, PyObject*, PyType_Spec*, PyObject*); +PyAPI_FUNC(void *) PyObject_GetTypeData(PyObject *obj, PyTypeObject *cls); +PyAPI_FUNC(Py_ssize_t) PyType_GetTypeDataSize(PyTypeObject *cls); #endif /* Generic type check */ @@ -436,6 +438,9 @@ given type object has a specified feature. // subject itself (rather than a mapped attribute on it): #define _Py_TPFLAGS_MATCH_SELF (1UL << 22) +/* Items (ob_size*tp_itemsize) are found at the end of an instance's memory */ +#define Py_TPFLAGS_ITEMS_AT_END (1UL << 23) + /* These flags are used to determine if a type is a subclass. */ #define Py_TPFLAGS_LONG_SUBCLASS (1UL << 24) #define Py_TPFLAGS_LIST_SUBCLASS (1UL << 25) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 637adc01a331ce..bb15eff76507de 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -16,11 +16,13 @@ import unittest import warnings import weakref +import operator from test import support from test.support import MISSING_C_DOCSTRINGS from test.support import import_helper from test.support import threading_helper from test.support import warnings_helper +from test.support import requires_limited_api from test.support.script_helper import assert_python_failure, assert_python_ok, run_python_until_end try: import _posixsubprocess @@ -756,6 +758,118 @@ def meth(self): MutableBase.meth = lambda self: 'changed' self.assertEqual(instance.meth(), 'changed') + @requires_limited_api + def test_heaptype_relative_sizes(self): + # Test subclassing using "relative" basicsize, see PEP 697 + def check(extra_base_size, extra_size): + Base, Sub, instance, data_ptr, data_size = ( + _testcapi.make_sized_heaptypes( + extra_base_size, -extra_size)) + + # no alignment shenanigans when inheriting directly + if extra_size == 0: + self.assertEqual(Base.__basicsize__, Sub.__basicsize__) + self.assertEqual(data_size, 0) + + else: + # The following offsets should be in increasing order: + data_offset = data_ptr - id(instance) + offsets = [ + (0, 'start of object'), + (Base.__basicsize__, 'end of base data'), + (data_offset, 'subclass data'), + (data_offset + extra_size, 'end of requested subcls data'), + (data_offset + data_size, 'end of reserved subcls data'), + (Sub.__basicsize__, 'end of object'), + ] + ordered_offsets = sorted(offsets, key=operator.itemgetter(0)) + self.assertEqual( + offsets, ordered_offsets, + msg=f'Offsets not in expected order, got: {ordered_offsets}') + + # end of reserved subcls data == end of object + self.assertEqual(Sub.__basicsize__, data_offset + data_size) + + # we don't reserve (requested + alignment) or more data + self.assertLess(data_size - extra_size, + _testcapi.alignof_max_align_t) + + # Everything should be aligned + self.assertEqual(data_ptr % _testcapi.alignof_max_align_t, 0) + self.assertEqual(data_size % _testcapi.alignof_max_align_t, 0) + + sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, + object.__basicsize__, + object.__basicsize__-1, + object.__basicsize__+1}) + for extra_base_size in sizes: + for extra_size in sizes: + args = dict(extra_base_size=extra_base_size, + extra_size=extra_size) + with self.subTest(**args): + check(**args) + + @requires_limited_api + def test_HeapCCollection(self): + """Make sure HeapCCollection works properly by itself""" + collection = _testcapi.HeapCCollection(1, 2, 3) + self.assertEqual(list(collection), [1, 2, 3]) + + @requires_limited_api + def test_heaptype_inherit_itemsize(self): + """Test HeapCCollection subclasses work properly""" + sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, + object.__basicsize__, + object.__basicsize__-1, + object.__basicsize__+1}) + for extra_size in sizes: + with self.subTest(extra_size=extra_size): + Sub = _testcapi.subclass_var_heaptype( + _testcapi.HeapCCollection, -extra_size, 0, 0) + collection = Sub(1, 2, 3) + collection.set_data_to_3s() + + self.assertEqual(list(collection), [1, 2, 3]) + mem = collection.get_data() + self.assertGreaterEqual(len(mem), extra_size) + self.assertTrue(set(mem) <= {3}, f'got {mem!r}') + + @requires_limited_api + def test_heaptype_relative_members(self): + """Test HeapCCollection subclasses work properly""" + sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, + object.__basicsize__, + object.__basicsize__-1, + object.__basicsize__+1}) + for extra_base_size in sizes: + for extra_size in sizes: + for offset in sizes: + with self.subTest(extra_base_size=extra_base_size, extra_size=extra_size, offset=offset): + if offset < extra_size: + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, -extra_size, offset, True) + Base = Sub.mro()[1] + instance = Sub() + self.assertEqual(instance.memb, instance.get_memb()) + instance.set_memb(13) + self.assertEqual(instance.memb, instance.get_memb()) + self.assertEqual(instance.get_memb(), 13) + instance.memb = 14 + self.assertEqual(instance.memb, instance.get_memb()) + self.assertEqual(instance.get_memb(), 14) + self.assertGreaterEqual(instance.get_memb_offset(), Base.__basicsize__) + self.assertLess(instance.get_memb_offset(), Sub.__basicsize__) + else: + with self.assertRaises(SystemError): + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, -extra_size, offset, True) + with self.assertRaises(SystemError): + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, extra_size, offset, True) + with self.subTest(extra_base_size=extra_base_size, extra_size=extra_size): + with self.assertRaises(SystemError): + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, -extra_size, -1, True) def test_pynumber_tobase(self): from _testcapi import pynumber_tobase diff --git a/Lib/test/test_stable_abi_ctypes.py b/Lib/test/test_stable_abi_ctypes.py index 2feaaf8603b831..4ca39d85e5460c 100644 --- a/Lib/test/test_stable_abi_ctypes.py +++ b/Lib/test/test_stable_abi_ctypes.py @@ -529,6 +529,7 @@ def test_windows_feature_macros(self): "PyObject_GetBuffer", "PyObject_GetItem", "PyObject_GetIter", + "PyObject_GetTypeData", "PyObject_HasAttr", "PyObject_HasAttrString", "PyObject_Hash", @@ -679,6 +680,7 @@ def test_windows_feature_macros(self): "PyType_GetName", "PyType_GetQualName", "PyType_GetSlot", + "PyType_GetTypeDataSize", "PyType_IsSubtype", "PyType_Modified", "PyType_Ready", diff --git a/Misc/stable_abi.toml b/Misc/stable_abi.toml index 23baeeeae79193..48299e9b35ff97 100644 --- a/Misc/stable_abi.toml +++ b/Misc/stable_abi.toml @@ -2397,3 +2397,12 @@ added = '3.12' # Before 3.12, available in "structmember.h" w/o Py_ prefix [const.Py_AUDIT_READ] added = '3.12' # Before 3.12, available in "structmember.h" + +[function.PyObject_GetTypeData] + added = '3.12' +[function.PyType_GetTypeDataSize] + added = '3.12' +[const.Py_RELATIVE_OFFSET] + added = '3.12' +[const.Py_TPFLAGS_ITEMS_AT_END] + added = '3.12' diff --git a/Modules/Setup.stdlib.in b/Modules/Setup.stdlib.in index fe1b9f8f5380c1..5836281e3da923 100644 --- a/Modules/Setup.stdlib.in +++ b/Modules/Setup.stdlib.in @@ -169,7 +169,7 @@ @MODULE__XXTESTFUZZ_TRUE@_xxtestfuzz _xxtestfuzz/_xxtestfuzz.c _xxtestfuzz/fuzzer.c @MODULE__TESTBUFFER_TRUE@_testbuffer _testbuffer.c @MODULE__TESTINTERNALCAPI_TRUE@_testinternalcapi _testinternalcapi.c -@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/getargs.c _testcapi/pytime.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/pyos.c +@MODULE__TESTCAPI_TRUE@_testcapi _testcapimodule.c _testcapi/vectorcall.c _testcapi/vectorcall_limited.c _testcapi/heaptype.c _testcapi/unicode.c _testcapi/getargs.c _testcapi/pytime.c _testcapi/datetime.c _testcapi/docstring.c _testcapi/mem.c _testcapi/watchers.c _testcapi/long.c _testcapi/float.c _testcapi/structmember.c _testcapi/exceptions.c _testcapi/code.c _testcapi/pyos.c _testcapi/heaptype_relative.c @MODULE__TESTCLINIC_TRUE@_testclinic _testclinic.c # Some testing modules MUST be built as shared libraries. diff --git a/Modules/_testcapi/heaptype.c b/Modules/_testcapi/heaptype.c index 209cc182c0698d..3c74f6c078693f 100644 --- a/Modules/_testcapi/heaptype.c +++ b/Modules/_testcapi/heaptype.c @@ -973,6 +973,110 @@ static PyType_Spec HeapCTypeSetattr_spec = { HeapCTypeSetattr_slots }; +PyDoc_STRVAR(HeapCCollection_doc, +"Tuple-like heap type that uses PyObject_GetItemData for items."); + +static PyObject* +HeapCCollection_new(PyTypeObject *subtype, PyObject *args, PyObject *kwds) +{ + PyObject *self = NULL; + PyObject *result = NULL; + + Py_ssize_t size = PyTuple_GET_SIZE(args); + self = subtype->tp_alloc(subtype, size); + if (!self) { + goto finally; + } + PyObject **data = PyObject_GetItemData(self); + if (!data) { + goto finally; + } + + for (Py_ssize_t i = 0; i < size; i++) { + data[i] = Py_NewRef(PyTuple_GET_ITEM(args, i)); + } + + result = self; + self = NULL; + finally: + Py_XDECREF(self); + return result; +} + +static Py_ssize_t +HeapCCollection_length(PyVarObject *self) +{ + return Py_SIZE(self); +} + +static PyObject* +HeapCCollection_item(PyObject *self, Py_ssize_t i) +{ + if (i < 0 || i >= Py_SIZE(self)) { + return PyErr_Format(PyExc_IndexError, "index %zd out of range", i); + } + PyObject **data = PyObject_GetItemData(self); + if (!data) { + return NULL; + } + return Py_NewRef(data[i]); +} + +static int +HeapCCollection_traverse(PyObject *self, visitproc visit, void *arg) +{ + PyObject **data = PyObject_GetItemData(self); + if (!data) { + return -1; + } + for (Py_ssize_t i = 0; i < Py_SIZE(self); i++) { + Py_VISIT(data[i]); + } + return 0; +} + +static int +HeapCCollection_clear(PyObject *self) { + PyObject **data = PyObject_GetItemData(self); + if (!data) { + return -1; + } + Py_ssize_t size = Py_SIZE(self); + Py_SET_SIZE(self, 0); + for (Py_ssize_t i = 0; i < size; i++) { + Py_CLEAR(data[i]); + } + return 0; +} + +static void HeapCCollection_dealloc(PyObject *self) { + PyTypeObject *tp = Py_TYPE(self); + HeapCCollection_clear(self); + PyObject_GC_UnTrack(self); + tp->tp_free(self); + Py_DECREF(tp); +} + +static PyType_Slot HeapCCollection_slots[] = { + {Py_tp_new, HeapCCollection_new}, + {Py_sq_length, HeapCCollection_length}, + {Py_sq_item, HeapCCollection_item}, + {Py_tp_traverse, HeapCCollection_traverse}, + {Py_tp_clear, HeapCCollection_clear}, + {Py_tp_dealloc, HeapCCollection_dealloc}, + {Py_tp_doc, (char*)HeapCCollection_doc}, + {0, 0}, +}; + +static PyType_Spec HeapCCollection_spec = { + "_testcapi.HeapCCollection", + sizeof(PyVarObject), + sizeof(PyObject*), + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC + | Py_TPFLAGS_ITEMS_AT_END, + HeapCCollection_slots +}; + int _PyTestCapi_Init_Heaptype(PyObject *m) { _testcapimodule = PyModule_GetDef(m); @@ -1096,5 +1200,12 @@ _PyTestCapi_Init_Heaptype(PyObject *m) { } PyModule_AddObject(m, "HeapCTypeMetaclassCustomNew", HeapCTypeMetaclassCustomNew); + PyObject *HeapCCollection = PyType_FromMetaclass( + NULL, m, &HeapCCollection_spec, NULL); + if (HeapCCollection == NULL) { + return -1; + } + PyModule_AddObject(m, "HeapCCollection", HeapCCollection); + return 0; } diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c new file mode 100644 index 00000000000000..4ea41a5405fcd2 --- /dev/null +++ b/Modules/_testcapi/heaptype_relative.c @@ -0,0 +1,270 @@ +#define Py_LIMITED_API 0x030c0000 // 3.12 +#include "parts.h" +#include // alignof +#include // max_align_t +#include // memset + +#ifdef LIMITED_API_AVAILABLE + +static PyType_Slot empty_slots[] = { + {0, NULL}, +}; + +static PyObject * +make_sized_heaptypes(PyObject *module, PyObject *args) +{ + PyObject *base = NULL; + PyObject *sub = NULL; + PyObject *instance = NULL; + PyObject *result = NULL; + + int extra_base_size, basicsize; + + int r = PyArg_ParseTuple(args, "ii", &extra_base_size, &basicsize); + if (!r) { + goto finally; + } + + PyType_Spec base_spec = { + .name = "_testcapi.Base", + .basicsize = sizeof(PyObject) + extra_base_size, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .slots = empty_slots, + }; + PyType_Spec sub_spec = { + .name = "_testcapi.Sub", + .basicsize = basicsize, + .flags = Py_TPFLAGS_DEFAULT, + .slots = empty_slots, + }; + + base = PyType_FromMetaclass(NULL, module, &base_spec, NULL); + if (!base) { + goto finally; + } + sub = PyType_FromMetaclass(NULL, module, &sub_spec, base); + if (!sub) { + goto finally; + } + instance = PyObject_CallNoArgs(sub); + if (!instance) { + goto finally; + } + void *data_ptr = PyObject_GetTypeData(instance, (PyTypeObject *)sub); + if (!data_ptr) { + goto finally; + } + Py_ssize_t data_size = PyType_GetTypeDataSize((PyTypeObject *)sub); + if (data_size < 0) { + goto finally; + } + + result = Py_BuildValue("OOOln", base, sub, instance, (long)data_ptr, + data_size); + finally: + Py_XDECREF(base); + Py_XDECREF(sub); + Py_XDECREF(instance); + return result; +} + +static PyObject * +var_heaptype_set_data_to_3s( + PyObject *self, PyTypeObject *defining_class, + PyObject **args, Py_ssize_t nargs, PyObject *kwnames) +{ + void *data_ptr = PyObject_GetTypeData(self, defining_class); + if (!data_ptr) { + return NULL; + } + Py_ssize_t data_size = PyType_GetTypeDataSize(defining_class); + if (data_size < 0) { + return NULL; + } + memset(data_ptr, 3, data_size); + Py_RETURN_NONE; +} + +static PyObject * +var_heaptype_get_data(PyObject *self, PyTypeObject *defining_class, + PyObject **args, Py_ssize_t nargs, PyObject *kwnames) +{ + void *data_ptr = PyObject_GetTypeData(self, defining_class); + if (!data_ptr) { + return NULL; + } + Py_ssize_t data_size = PyType_GetTypeDataSize(defining_class); + if (data_size < 0) { + return NULL; + } + return PyBytes_FromStringAndSize(data_ptr, data_size); +} + +static PyMethodDef var_heaptype_methods[] = { + {"set_data_to_3s", _PyCFunction_CAST(var_heaptype_set_data_to_3s), + METH_METHOD | METH_FASTCALL | METH_KEYWORDS}, + {"get_data", _PyCFunction_CAST(var_heaptype_get_data), + METH_METHOD | METH_FASTCALL | METH_KEYWORDS}, + {NULL}, +}; + +static PyObject * +subclass_var_heaptype(PyObject *module, PyObject *args) +{ + PyObject *result = NULL; + + PyObject *base; // borrowed from args + int basicsize, itemsize; + long pfunc; + + int r = PyArg_ParseTuple(args, "Oiil", &base, &basicsize, &itemsize, &pfunc); + if (!r) { + goto finally; + } + + PyType_Slot slots[] = { + {Py_tp_methods, var_heaptype_methods}, + {0, NULL}, + }; + + PyType_Spec sub_spec = { + .name = "_testcapi.Sub", + .basicsize = basicsize, + .itemsize = itemsize, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_ITEMS_AT_END, + .slots = slots, + }; + + result = PyType_FromMetaclass(NULL, module, &sub_spec, base); + finally: + return result; +} + +static PyMemberDef * +heaptype_with_member_extract_and_check_memb(PyObject *self) +{ + PyMemberDef *def = PyType_GetSlot(Py_TYPE(self), Py_tp_members); + if (!def) { + if (!PyErr_Occurred()) { + PyErr_SetString(PyExc_ValueError, "tp_members is NULL"); + } + return NULL; + } + if (!def[0].name) { + PyErr_SetString(PyExc_ValueError, "tp_members[0] is NULL"); + return NULL; + } + if (def[1].name) { + PyErr_SetString(PyExc_ValueError, "tp_members[1] is not NULL"); + return NULL; + } + if (strcmp(def[0].name, "memb")) { + PyErr_SetString(PyExc_ValueError, "tp_members[0] is not for `memb`"); + return NULL; + } + if (def[0].flags) { + PyErr_SetString(PyExc_ValueError, "tp_members[0] has flags set"); + return NULL; + } + return def; +} + +static PyObject * +heaptype_with_member_get_memb(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyMemberDef *def = heaptype_with_member_extract_and_check_memb(self); + return PyMember_GetOne((const char *)self, def); +} + +static PyObject * +heaptype_with_member_set_memb(PyObject *self, PyObject *value) +{ + PyMemberDef *def = heaptype_with_member_extract_and_check_memb(self); + int r = PyMember_SetOne((char *)self, def, value); + if (r < 0) { + return NULL; + } + Py_RETURN_NONE; +} + +static PyObject * +get_memb_offset(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyMemberDef *def = heaptype_with_member_extract_and_check_memb(self); + return PyLong_FromSsize_t(def->offset); +} + +static PyMethodDef heaptype_with_member_methods[] = { + {"get_memb", heaptype_with_member_get_memb, METH_NOARGS}, + {"set_memb", heaptype_with_member_set_memb, METH_O}, + {"get_memb_offset", get_memb_offset, METH_NOARGS}, + {NULL}, +}; + +static PyObject * +make_heaptype_with_member(PyObject *module, PyObject *args) +{ + PyObject *base = NULL; + PyObject *result = NULL; + + int extra_base_size, basicsize, offset, add_flag; + + int r = PyArg_ParseTuple(args, "iiip", &extra_base_size, &basicsize, &offset, &add_flag); + if (!r) { + goto finally; + } + + PyType_Spec base_spec = { + .name = "_testcapi.Base", + .basicsize = sizeof(PyObject) + extra_base_size, + .flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, + .slots = empty_slots, + }; + base = PyType_FromMetaclass(NULL, module, &base_spec, NULL); + if (!base) { + goto finally; + } + + PyMemberDef members[] = { + {"memb", Py_T_BYTE, offset, add_flag ? Py_RELATIVE_OFFSET : 0}, + {0}, + }; + PyType_Slot slots[] = { + {Py_tp_members, members}, + {Py_tp_methods, heaptype_with_member_methods}, + {0, NULL}, + }; + + PyType_Spec sub_spec = { + .name = "_testcapi.Sub", + .basicsize = basicsize, + .flags = Py_TPFLAGS_DEFAULT, + .slots = slots, + }; + + result = PyType_FromMetaclass(NULL, module, &sub_spec, base); + finally: + Py_XDECREF(base); + return result; +} + + +static PyMethodDef TestMethods[] = { + {"make_sized_heaptypes", make_sized_heaptypes, METH_VARARGS}, + {"subclass_var_heaptype", subclass_var_heaptype, METH_VARARGS}, + {"make_heaptype_with_member", make_heaptype_with_member, METH_VARARGS}, + {NULL}, +}; + +int +_PyTestCapi_Init_HeaptypeRelative(PyObject *m) { + if (PyModule_AddFunctions(m, TestMethods) < 0) { + return -1; + } + + PyModule_AddIntConstant(m, "alignof_max_align_t", alignof(max_align_t)); + + return 0; +} + +#endif // LIMITED_API_AVAILABLE diff --git a/Modules/_testcapi/parts.h b/Modules/_testcapi/parts.h index 60ec81dad2ba9e..331773737d993f 100644 --- a/Modules/_testcapi/parts.h +++ b/Modules/_testcapi/parts.h @@ -42,6 +42,7 @@ int _PyTestCapi_Init_PyOS(PyObject *module); #ifdef LIMITED_API_AVAILABLE int _PyTestCapi_Init_VectorcallLimited(PyObject *module); +int _PyTestCapi_Init_HeaptypeRelative(PyObject *module); #endif // LIMITED_API_AVAILABLE #endif diff --git a/Modules/_testcapimodule.c b/Modules/_testcapimodule.c index 557a6d46ed4632..255ea8c31a6405 100644 --- a/Modules/_testcapimodule.c +++ b/Modules/_testcapimodule.c @@ -4197,6 +4197,9 @@ PyInit__testcapi(void) if (_PyTestCapi_Init_VectorcallLimited(m) < 0) { return NULL; } + if (_PyTestCapi_Init_HeaptypeRelative(m) < 0) { + return NULL; + } #endif PyState_AddModule(m, &_testcapimodule); diff --git a/Objects/descrobject.c b/Objects/descrobject.c index 334be75e8df9df..17c0c85a06c4b8 100644 --- a/Objects/descrobject.c +++ b/Objects/descrobject.c @@ -978,6 +978,12 @@ PyDescr_NewMember(PyTypeObject *type, PyMemberDef *member) { PyMemberDescrObject *descr; + if (member->flags & Py_RELATIVE_OFFSET) { + PyErr_SetString( + PyExc_SystemError, + "PyDescr_NewMember used with Py_RELATIVE_OFFSET"); + return NULL; + } descr = (PyMemberDescrObject *)descr_new(&PyMemberDescr_Type, type, member->name); if (descr != NULL) diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 9ea458f30394e3..2f5c12d6f103ea 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -18,6 +18,8 @@ #include "structmember.h" // PyMemberDef #include +#include // alignof +#include // ptrdiff_t /*[clinic input] class type "PyTypeObject *" "&PyType_Type" @@ -1368,6 +1370,12 @@ PyType_GenericNew(PyTypeObject *type, PyObject *args, PyObject *kwds) /* Helpers for subtyping */ +static inline PyMemberDef * +_PyHeapType_GET_MEMBERS(PyHeapTypeObject* type) +{ + return PyObject_GetItemData((PyObject *)type); +} + static int traverse_slots(PyTypeObject *type, PyObject *self, visitproc visit, void *arg) { @@ -3542,6 +3550,15 @@ static const PySlot_Offset pyslot_offsets[] = { #include "typeslots.inc" }; +/* Align up to the nearest multiple of alignof(max_align_t) + * (like _Py_ALIGN_UP, but for a size rather than pointer) + */ +static Py_ssize_t +_align_up(Py_ssize_t size) { + const Py_ssize_t alignment = alignof(max_align_t); + return (size + alignment - 1) & ~(alignment - 1); +} + /* Given a PyType_FromMetaclass `bases` argument (NULL, type, or tuple of * types), return a tuple of types. */ @@ -3681,6 +3698,20 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, assert(memb->flags == READONLY); vectorcalloffset = memb->offset; } + if (memb->flags & Py_RELATIVE_OFFSET) { + if(spec->basicsize > 0) { + PyErr_SetString( + PyExc_SystemError, + "With Py_RELATIVE_OFFSET, basicsize must be negative."); + goto finally; + } + if(memb->offset < 0 || memb->offset >= -spec->basicsize) { + PyErr_SetString( + PyExc_SystemError, + "Member offset out of range (0..-basicsize)"); + goto finally; + } + } } break; case Py_tp_doc: @@ -3810,6 +3841,32 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, // here we just check its work assert(_PyType_HasFeature(base, Py_TPFLAGS_BASETYPE)); + /* Calculate sizes */ + + Py_ssize_t basicsize = spec->basicsize; + Py_ssize_t type_data_offset = spec->basicsize; + if (basicsize == 0) { + /* Inherit */ + basicsize = base->tp_basicsize; + } + else if (basicsize < 0) { + /* Extend */ + type_data_offset = _align_up(base->tp_basicsize); + basicsize = type_data_offset + _align_up(-spec->basicsize); + + /* Inheriting variable-sized types is limited */ + if (base->tp_itemsize + && !((base->tp_flags | spec->flags) & Py_TPFLAGS_ITEMS_AT_END)) + { + PyErr_SetString( + PyExc_SystemError, + "Cannot extend variable-size class without Py_TPFLAGS_ITEMS_AT_END."); + goto finally; + } + } + + Py_ssize_t itemsize = spec->itemsize; + /* Allocate the new type * * Between here and PyType_Ready, we should limit: @@ -3857,8 +3914,8 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, /* Copy the sizes */ - type->tp_basicsize = spec->basicsize; - type->tp_itemsize = spec->itemsize; + type->tp_basicsize = basicsize; + type->tp_itemsize = itemsize; /* Copy all the ordinary slots */ @@ -3875,6 +3932,15 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, size_t len = Py_TYPE(type)->tp_itemsize * nmembers; memcpy(_PyHeapType_GET_MEMBERS(res), slot->pfunc, len); type->tp_members = _PyHeapType_GET_MEMBERS(res); + PyMemberDef *memb; + unsigned i; + for (memb = _PyHeapType_GET_MEMBERS(res), i = nmembers; + i > 0; ++memb, --i) { + if (memb->flags & Py_RELATIVE_OFFSET) { + memb->flags &= ~Py_RELATIVE_OFFSET; + memb->offset += type_data_offset; + } + } } break; default: @@ -3883,6 +3949,7 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, PySlot_Offset slotoffsets = pyslot_offsets[slot->slot]; short slot_offset = slotoffsets.slot_offset; if (slotoffsets.subslot_offset == -1) { + /* Set a slot in the main PyTypeObject */ *(void**)((char*)res_start + slot_offset) = slot->pfunc; } else { @@ -4109,6 +4176,29 @@ PyType_GetModuleByDef(PyTypeObject *type, PyModuleDef *def) return NULL; } +void * +PyObject_GetTypeData(PyObject *obj, PyTypeObject *cls) +{ + assert(PyObject_TypeCheck(obj, cls)); + return (char *)obj + _align_up(cls->tp_base->tp_basicsize); +} + +Py_ssize_t +PyType_GetTypeDataSize(PyTypeObject *cls) +{ + ptrdiff_t result = cls->tp_basicsize - _align_up(cls->tp_base->tp_basicsize); + if (result < 0) { + return 0; + } + return result; +} + +void * +PyObject_GetItemData(PyObject *obj) +{ + assert(PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_ITEMS_AT_END)); + return (char *)obj + Py_TYPE(obj)->tp_basicsize; +} /* Internal API to look for a name through the MRO, bypassing the method cache. This returns a borrowed reference, and might set an exception. @@ -4858,7 +4948,8 @@ PyTypeObject PyType_Type = { 0, /* tp_as_buffer */ Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_TYPE_SUBCLASS | - Py_TPFLAGS_HAVE_VECTORCALL, /* tp_flags */ + Py_TPFLAGS_HAVE_VECTORCALL | + Py_TPFLAGS_ITEMS_AT_END, /* tp_flags */ type_doc, /* tp_doc */ (traverseproc)type_traverse, /* tp_traverse */ (inquiry)type_clear, /* tp_clear */ @@ -6270,9 +6361,14 @@ inherit_special(PyTypeObject *type, PyTypeObject *base) else if (PyType_IsSubtype(base, &PyDict_Type)) { type->tp_flags |= Py_TPFLAGS_DICT_SUBCLASS; } + + /* Setup some inheritable flags */ if (PyType_HasFeature(base, _Py_TPFLAGS_MATCH_SELF)) { type->tp_flags |= _Py_TPFLAGS_MATCH_SELF; } + if (PyType_HasFeature(base, Py_TPFLAGS_ITEMS_AT_END)) { + type->tp_flags |= Py_TPFLAGS_ITEMS_AT_END; + } } static int diff --git a/PC/python3dll.c b/PC/python3dll.c index 706affa18351b3..7e848abccfd1fa 100755 --- a/PC/python3dll.c +++ b/PC/python3dll.c @@ -467,6 +467,7 @@ EXPORT_FUNC(PyObject_GetAttrString) EXPORT_FUNC(PyObject_GetBuffer) EXPORT_FUNC(PyObject_GetItem) EXPORT_FUNC(PyObject_GetIter) +EXPORT_FUNC(PyObject_GetTypeData) EXPORT_FUNC(PyObject_HasAttr) EXPORT_FUNC(PyObject_HasAttrString) EXPORT_FUNC(PyObject_Hash) @@ -618,6 +619,7 @@ EXPORT_FUNC(PyType_GetModuleState) EXPORT_FUNC(PyType_GetName) EXPORT_FUNC(PyType_GetQualName) EXPORT_FUNC(PyType_GetSlot) +EXPORT_FUNC(PyType_GetTypeDataSize) EXPORT_FUNC(PyType_IsSubtype) EXPORT_FUNC(PyType_Modified) EXPORT_FUNC(PyType_Ready) diff --git a/PCbuild/_testcapi.vcxproj b/PCbuild/_testcapi.vcxproj index 439cd687fda61d..70079d70aef2b6 100644 --- a/PCbuild/_testcapi.vcxproj +++ b/PCbuild/_testcapi.vcxproj @@ -98,6 +98,7 @@ + diff --git a/PCbuild/_testcapi.vcxproj.filters b/PCbuild/_testcapi.vcxproj.filters index 0e42e4982c21ff..297c9ce799bea1 100644 --- a/PCbuild/_testcapi.vcxproj.filters +++ b/PCbuild/_testcapi.vcxproj.filters @@ -24,6 +24,9 @@ Source Files + + Source Files + Source Files diff --git a/Python/structmember.c b/Python/structmember.c index 1b8be28dcf2eb2..98faea29f8273c 100644 --- a/Python/structmember.c +++ b/Python/structmember.c @@ -8,6 +8,12 @@ PyObject * PyMember_GetOne(const char *obj_addr, PyMemberDef *l) { PyObject *v; + if (l->flags & Py_RELATIVE_OFFSET) { + PyErr_SetString( + PyExc_SystemError, + "PyMember_GetOne used with Py_RELATIVE_OFFSET"); + return NULL; + } const char* addr = obj_addr + l->offset; switch (l->type) { @@ -103,6 +109,12 @@ int PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v) { PyObject *oldv; + if (l->flags & Py_RELATIVE_OFFSET) { + PyErr_SetString( + PyExc_SystemError, + "PyMember_SetOne used with Py_RELATIVE_OFFSET"); + return NULL; + } addr += l->offset; From b8dc072fbac531b6db28d9368c3f61732a6c0cd9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 13 Apr 2023 16:57:08 +0200 Subject: [PATCH 02/24] Add a blurb --- .../C API/2023-04-13-16-54-00.gh-issue-103509.A26Qu8.rst | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 Misc/NEWS.d/next/C API/2023-04-13-16-54-00.gh-issue-103509.A26Qu8.rst diff --git a/Misc/NEWS.d/next/C API/2023-04-13-16-54-00.gh-issue-103509.A26Qu8.rst b/Misc/NEWS.d/next/C API/2023-04-13-16-54-00.gh-issue-103509.A26Qu8.rst new file mode 100644 index 00000000000000..af630c3aafa940 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2023-04-13-16-54-00.gh-issue-103509.A26Qu8.rst @@ -0,0 +1,5 @@ +Added C API for extending types whose instance memory layout is opaque: +:c:member:`PyType_Spec.basicsize` can now be zero or negative, +:c:func:`PyObject_GetTypeData` can be used to get subclass-specific data, +and :c:macro:`Py_TPFLAGS_ITEMS_AT_END` can be used to safely extend +variable-size objects. See :pep:`697` for details. From 1fc5c52d683bc63cd3a19c5974641ec981e72f43 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 17 Apr 2023 14:12:59 +0200 Subject: [PATCH 03/24] Fix PyMember_SetOne error return (thanks MSVC!) --- Python/structmember.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/structmember.c b/Python/structmember.c index 98faea29f8273c..19a75224a0f32e 100644 --- a/Python/structmember.c +++ b/Python/structmember.c @@ -113,7 +113,7 @@ PyMember_SetOne(char *addr, PyMemberDef *l, PyObject *v) PyErr_SetString( PyExc_SystemError, "PyMember_SetOne used with Py_RELATIVE_OFFSET"); - return NULL; + return -1; } addr += l->offset; From 075ca515ef21999f0cd422132e55d7e04f1d9926 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 17 Apr 2023 14:27:12 +0200 Subject: [PATCH 04/24] Add What's New entry --- Doc/whatsnew/3.12.rst | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 4165b16ba76441..58ffb1b6b31bdc 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -1014,6 +1014,21 @@ New Features (Contributed by Petr Viktorin in :gh:`101101`.) +* :pep:`697`: Added API for extending types whose instance memory layout is + opaque: + + - :c:member:`PyType_Spec.basicsize` can be zero or negative to specify + inheriting or extending the base class size. + - :c:func:`PyObject_GetTypeData` and :c:func:`PyType_GetTypeDataSize` + added to allow access to subclass-specific instance data. + - :c:macro:`Py_TPFLAGS_ITEMS_AT_END` and :c:func:`PyObject_GetItemData` + added to allow safely extending certain variable-sized types, including + :c:var:`PyType_Type`. + - :c:macro:`Py_RELATIVE_OFFSET` added to allow defining + :c:type:`members ` in terms of a subclass-specific struct. + + (Contributed by Petr Viktorin in :gh:`103509`.) + * Added the new limited C API function :c:func:`PyType_FromMetaclass`, which generalizes the existing :c:func:`PyType_FromModuleAndSpec` using an additional metaclass argument. From 439de5d3e6ec33e7a8eaf7fbdf24e66000d0f495 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 19 Apr 2023 14:48:52 +0200 Subject: [PATCH 05/24] Fix warning in tests --- Modules/_testcapi/heaptype_relative.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 4ea41a5405fcd2..978543f73611e6 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -59,7 +59,7 @@ make_sized_heaptypes(PyObject *module, PyObject *args) goto finally; } - result = Py_BuildValue("OOOln", base, sub, instance, (long)data_ptr, + result = Py_BuildValue("OOOKn", base, sub, instance, (unsigned long long)data_ptr, data_size); finally: Py_XDECREF(base); From 291731b60fcf6a1a9a94bf6a31df39fce2ed77e6 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 19 Apr 2023 14:49:11 +0200 Subject: [PATCH 06/24] Work around lack of alignof & max_align_t --- Modules/_testcapi/heaptype_relative.c | 6 ++++++ Objects/typeobject.c | 16 ++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 978543f73611e6..44ab66dfd8e680 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -262,7 +262,13 @@ _PyTestCapi_Init_HeaptypeRelative(PyObject *m) { return -1; } +#ifdef __alignof_is_defined + // C11 PyModule_AddIntConstant(m, "alignof_max_align_t", alignof(max_align_t)); +#else + // if alignof and max_align_t is unavailable, skip the alignment tests + PyModule_AddIntConstant(m, "alignof_max_align_t", 1); +#endif return 0; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 2f5c12d6f103ea..3e9878bb467ad5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3553,10 +3553,22 @@ static const PySlot_Offset pyslot_offsets[] = { /* Align up to the nearest multiple of alignof(max_align_t) * (like _Py_ALIGN_UP, but for a size rather than pointer) */ +#if __alignof_is_defined + // C11 + #define MAX_ALIGN alignof(max_align_t) +#else + // workaround for MSVC + typedef union { + intmax_t x; + long double y; + void *z; + void (*f)(); + } _max_align_t_wannabe; + #define MAX_ALIGN _Alignof(_max_align_t_wannabe) +#endif static Py_ssize_t _align_up(Py_ssize_t size) { - const Py_ssize_t alignment = alignof(max_align_t); - return (size + alignment - 1) & ~(alignment - 1); + return (size + MAX_ALIGN - 1) & ~(MAX_ALIGN - 1); } /* Given a PyType_FromMetaclass `bases` argument (NULL, type, or tuple of From 2ba80841f51c1444afb73553b651462f49e07bb7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 19 Apr 2023 15:19:46 +0200 Subject: [PATCH 07/24] Use the same Sphinx role for Py_TPFLAGS_ITEMS_AT_END as for other type flags Now is not the time to fix that. --- Doc/c-api/object.rst | 4 ++-- Doc/c-api/type.rst | 2 +- Doc/c-api/typeobj.rst | 2 +- Doc/whatsnew/3.12.rst | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index ed64f0576ff5a7..1ae8f35c8b8c6a 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -427,10 +427,10 @@ Object Protocol .. c:function:: PyObject* PyObject_GetItemData(PyObject *o) Get a pointer to per-item data for a class with - :c:macro:`Py_TPFLAGS_ITEMS_AT_END`. + :const:`Py_TPFLAGS_ITEMS_AT_END`. On error, set an exception and return ``NULL``. :py:exc:`TypeError` is raised if *o* does not have - :c:macro:`Py_TPFLAGS_ITEMS_AT_END` set. + :const:`Py_TPFLAGS_ITEMS_AT_END` set. .. versionadded:: 3.12 diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index bb133f7f98a84f..7eb64a4245159b 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -363,7 +363,7 @@ The following functions and structs are used to create - The requested :c:member:`PyType_Spec.basicsize` is zero, suggesting that the subclass does not access the instance's memory directly. - - With the :c:macro:`Py_TPFLAGS_ITEMS_AT_END` flag. + - With the :const:`Py_TPFLAGS_ITEMS_AT_END` flag. .. c:member:: int flags diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index 2a6a28c2459ba5..5336dbe01938b1 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -1171,7 +1171,7 @@ and :c:type:`PyType_Type` effectively act as defaults.) :c:member:`~PyTypeObject.tp_weaklistoffset` field is set in a superclass. - .. c:macro:: Py_TPFLAGS_ITEMS_AT_END + .. data:: Py_TPFLAGS_ITEMS_AT_END Only usable with variable-size types, i.e. ones with non-zero :c:member:`~PyObject.tp_itemsize`. diff --git a/Doc/whatsnew/3.12.rst b/Doc/whatsnew/3.12.rst index 58ffb1b6b31bdc..a7341559a1ea98 100644 --- a/Doc/whatsnew/3.12.rst +++ b/Doc/whatsnew/3.12.rst @@ -1021,7 +1021,7 @@ New Features inheriting or extending the base class size. - :c:func:`PyObject_GetTypeData` and :c:func:`PyType_GetTypeDataSize` added to allow access to subclass-specific instance data. - - :c:macro:`Py_TPFLAGS_ITEMS_AT_END` and :c:func:`PyObject_GetItemData` + - :const:`Py_TPFLAGS_ITEMS_AT_END` and :c:func:`PyObject_GetItemData` added to allow safely extending certain variable-sized types, including :c:var:`PyType_Type`. - :c:macro:`Py_RELATIVE_OFFSET` added to allow defining From b03d431b16f5a24914289ee617408c5406aed999 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 19 Apr 2023 17:07:05 +0200 Subject: [PATCH 08/24] Add ALIGNOF_MAX_ALIGN_T to configure & PC/pyconfig.h --- Include/pyport.h | 6 +++++ Modules/_testcapi/heaptype_relative.c | 18 ++++++++++++++ PC/pyconfig.h | 2 ++ configure | 35 +++++++++++++++++++++++++++ configure.ac | 1 + pyconfig.h.in | 3 +++ 6 files changed, 65 insertions(+) diff --git a/Include/pyport.h b/Include/pyport.h index eef0fe1bfd71d8..ebc5f8db99527f 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -745,4 +745,10 @@ extern char * _getpty(int *, int, mode_t, int); #undef __bool__ #endif +// Make sure we have alignof(max_align_t) +// (autoconf report alignment of unknown types to 0) +#if ALIGNOF_MAX_ALIGN_T <= 0 +#error "ALIGNOF_MAX_ALIGN_T must be positive" +#endif + #endif /* Py_PYPORT_H */ diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 44ab66dfd8e680..4e2ac376c0a31f 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -249,10 +249,28 @@ make_heaptype_with_member(PyObject *module, PyObject *args) } +static PyObject * +test_alignof_max_align_t(PyObject *module, PyObject *Py_UNUSED(ignored)) +{ + // We define ALIGNOF_MAX_ALIGN_T even if the compiler doesn't have + // max_afign_t. Double-check that it's correct. + assert(ALIGNOF_MAX_ALIGN_T > 0); + assert(ALIGNOF_MAX_ALIGN_T >= _Alignof(long long)); + assert(ALIGNOF_MAX_ALIGN_T >= _Alignof(long double)); + assert(ALIGNOF_MAX_ALIGN_T >= _Alignof(void*)); + assert(ALIGNOF_MAX_ALIGN_T >= _Alignof(void (*)(void))); + + // Ensure it's a power of two + assert((ALIGNOF_MAX_ALIGN_T & (ALIGNOF_MAX_ALIGN_T - 1)) == 0); + + Py_RETURN_NONE; +} + static PyMethodDef TestMethods[] = { {"make_sized_heaptypes", make_sized_heaptypes, METH_VARARGS}, {"subclass_var_heaptype", subclass_var_heaptype, METH_VARARGS}, {"make_heaptype_with_member", make_heaptype_with_member, METH_VARARGS}, + {"test_alignof_max_align_t", test_alignof_max_align_t, METH_NOARGS}, {NULL}, }; diff --git a/PC/pyconfig.h b/PC/pyconfig.h index 8a3bf8968ce29d..56bc24c4f51568 100644 --- a/PC/pyconfig.h +++ b/PC/pyconfig.h @@ -330,6 +330,7 @@ Py_NO_ENABLE_SHARED to find out. Also support MS_NO_COREDLL for b/w compat */ # define SIZEOF_HKEY 8 # define SIZEOF_SIZE_T 8 # define ALIGNOF_SIZE_T 8 +# define ALIGNOF_MAX_ALIGN_T 8 /* configure.ac defines HAVE_LARGEFILE_SUPPORT iff sizeof(off_t) > sizeof(long), and sizeof(long long) >= sizeof(off_t). On Win64 the second condition is not true, but if fpos_t replaces off_t @@ -351,6 +352,7 @@ Py_NO_ENABLE_SHARED to find out. Also support MS_NO_COREDLL for b/w compat */ # else # define SIZEOF_TIME_T 4 # endif +# define ALIGNOF_MAX_ALIGN_T 4 #endif #ifdef _DEBUG diff --git a/configure b/configure index 4ae8258438e620..61d4cae1d698ba 100755 --- a/configure +++ b/configure @@ -10621,6 +10621,41 @@ cat >>confdefs.h <<_ACEOF _ACEOF +# The cast to long int works around a bug in the HP C Compiler, +# see AC_CHECK_SIZEOF for more information. +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking alignment of max_align_t" >&5 +$as_echo_n "checking alignment of max_align_t... " >&6; } +if ${ac_cv_alignof_max_align_t+:} false; then : + $as_echo_n "(cached) " >&6 +else + if ac_fn_c_compute_int "$LINENO" "(long int) offsetof (ac__type_alignof_, y)" "ac_cv_alignof_max_align_t" "$ac_includes_default +#ifndef offsetof +# define offsetof(type, member) ((char *) &((type *) 0)->member - (char *) 0) +#endif +typedef struct { char x; max_align_t y; } ac__type_alignof_;"; then : + +else + if test "$ac_cv_type_max_align_t" = yes; then + { { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5 +$as_echo "$as_me: error: in \`$ac_pwd':" >&2;} +as_fn_error 77 "cannot compute alignment of max_align_t +See \`config.log' for more details" "$LINENO" 5; } + else + ac_cv_alignof_max_align_t=0 + fi +fi + +fi +{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_alignof_max_align_t" >&5 +$as_echo "$ac_cv_alignof_max_align_t" >&6; } + + + +cat >>confdefs.h <<_ACEOF +#define ALIGNOF_MAX_ALIGN_T $ac_cv_alignof_max_align_t +_ACEOF + + { $as_echo "$as_me:${as_lineno-$LINENO}: checking for long double" >&5 diff --git a/configure.ac b/configure.ac index 4d9eb46f5ce7d8..86798068b781d4 100644 --- a/configure.ac +++ b/configure.ac @@ -2914,6 +2914,7 @@ AC_CHECK_SIZEOF(size_t, 4) AC_CHECK_ALIGNOF(size_t) AC_CHECK_SIZEOF(pid_t, 4) AC_CHECK_SIZEOF(uintptr_t) +AC_CHECK_ALIGNOF(max_align_t) AC_TYPE_LONG_DOUBLE AC_CHECK_SIZEOF(long double, 16) diff --git a/pyconfig.h.in b/pyconfig.h.in index 236cee6588d49b..2c22b27af65ea3 100644 --- a/pyconfig.h.in +++ b/pyconfig.h.in @@ -19,6 +19,9 @@ /* The normal alignment of `long', in bytes. */ #undef ALIGNOF_LONG +/* The normal alignment of `max_align_t', in bytes. */ +#undef ALIGNOF_MAX_ALIGN_T + /* The normal alignment of `size_t', in bytes. */ #undef ALIGNOF_SIZE_T From ec9d5a896188c75af682696368ca5577bc9cf5de Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 19 Apr 2023 17:24:49 +0200 Subject: [PATCH 09/24] Use ALIGNOF_MAX_ALIGN_T --- Modules/_testcapi/heaptype_relative.c | 8 +------- Objects/typeobject.c | 15 +-------------- 2 files changed, 2 insertions(+), 21 deletions(-) diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 4e2ac376c0a31f..ab57ff036b418b 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -280,13 +280,7 @@ _PyTestCapi_Init_HeaptypeRelative(PyObject *m) { return -1; } -#ifdef __alignof_is_defined - // C11 - PyModule_AddIntConstant(m, "alignof_max_align_t", alignof(max_align_t)); -#else - // if alignof and max_align_t is unavailable, skip the alignment tests - PyModule_AddIntConstant(m, "alignof_max_align_t", 1); -#endif + PyModule_AddIntConstant(m, "alignof_max_align_t", ALIGNOF_MAX_ALIGN_T); return 0; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 3e9878bb467ad5..b0e2005f6b0454 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3553,22 +3553,9 @@ static const PySlot_Offset pyslot_offsets[] = { /* Align up to the nearest multiple of alignof(max_align_t) * (like _Py_ALIGN_UP, but for a size rather than pointer) */ -#if __alignof_is_defined - // C11 - #define MAX_ALIGN alignof(max_align_t) -#else - // workaround for MSVC - typedef union { - intmax_t x; - long double y; - void *z; - void (*f)(); - } _max_align_t_wannabe; - #define MAX_ALIGN _Alignof(_max_align_t_wannabe) -#endif static Py_ssize_t _align_up(Py_ssize_t size) { - return (size + MAX_ALIGN - 1) & ~(MAX_ALIGN - 1); + return (size + ALIGNOF_MAX_ALIGN_T - 1) & ~(ALIGNOF_MAX_ALIGN_T - 1); } /* Given a PyType_FromMetaclass `bases` argument (NULL, type, or tuple of From 5cab81498a12e92c96f9fbf857a227d772ee36e9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 20 Apr 2023 08:45:22 +0200 Subject: [PATCH 10/24] Fix typo --- Modules/_testcapi/heaptype_relative.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index ab57ff036b418b..9a75e8222f95a9 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -252,8 +252,8 @@ make_heaptype_with_member(PyObject *module, PyObject *args) static PyObject * test_alignof_max_align_t(PyObject *module, PyObject *Py_UNUSED(ignored)) { - // We define ALIGNOF_MAX_ALIGN_T even if the compiler doesn't have - // max_afign_t. Double-check that it's correct. + // We define ALIGNOF_MAX_ALIGN_T even if the compiler doesn't support + // max_align_t. Double-check that it's correct. assert(ALIGNOF_MAX_ALIGN_T > 0); assert(ALIGNOF_MAX_ALIGN_T >= _Alignof(long long)); assert(ALIGNOF_MAX_ALIGN_T >= _Alignof(long double)); From 3ade5853a76e2d583dfb09fb648eabd6b75e7714 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 20 Apr 2023 15:53:46 +0200 Subject: [PATCH 11/24] Don't include , it's not always available on Windows --- Modules/_testcapi/heaptype_relative.c | 1 - Objects/typeobject.c | 1 - 2 files changed, 2 deletions(-) diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 9a75e8222f95a9..2035cdf274053e 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -1,6 +1,5 @@ #define Py_LIMITED_API 0x030c0000 // 3.12 #include "parts.h" -#include // alignof #include // max_align_t #include // memset diff --git a/Objects/typeobject.c b/Objects/typeobject.c index b0e2005f6b0454..b8bceb7dafa4a1 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -18,7 +18,6 @@ #include "structmember.h" // PyMemberDef #include -#include // alignof #include // ptrdiff_t /*[clinic input] From 986bf26cfa8ba2b5ac0998d3ccbcce3634506619 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 20 Apr 2023 16:11:33 +0200 Subject: [PATCH 12/24] Define ALIGNOF_MAX_ALIGN_T with long double if it's not available --- Include/pyport.h | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Include/pyport.h b/Include/pyport.h index ebc5f8db99527f..65b1547f381a38 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -745,10 +745,15 @@ extern char * _getpty(int *, int, mode_t, int); #undef __bool__ #endif -// Make sure we have alignof(max_align_t) -// (autoconf report alignment of unknown types to 0) -#if ALIGNOF_MAX_ALIGN_T <= 0 -#error "ALIGNOF_MAX_ALIGN_T must be positive" +// Make sure we have maximum alignment, even if the current compiler +// does not support max_align_t. +// - Autoconf report alignment of unknown types to 0. +// - We don't have the Py_MAX macro yet. +// - 'long double' has maximum alignment on *most* platforms, +// looks like the best we can do for pre-C11 compilers. +// - The value is tested, see test_alignof_max_align_t +#if !defined(ALIGNOF_MAX_ALIGN_T) || ALIGNOF_MAX_ALIGN_T == 0 +# define ALIGNOF_MAX_ALIGN_T _Alignof(long double) #endif #endif /* Py_PYPORT_H */ From e7838ee89e86d99f687d047ce9b82b934230699e Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 20 Apr 2023 16:34:31 +0200 Subject: [PATCH 13/24] tests: Compute data_offset in C to avoid overflow issues --- Lib/test/test_capi/test_misc.py | 3 +-- Modules/_testcapi/heaptype_relative.c | 4 +++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index bb15eff76507de..2a8ade109e0523 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -762,7 +762,7 @@ def meth(self): def test_heaptype_relative_sizes(self): # Test subclassing using "relative" basicsize, see PEP 697 def check(extra_base_size, extra_size): - Base, Sub, instance, data_ptr, data_size = ( + Base, Sub, instance, data_ptr, data_offset, data_size = ( _testcapi.make_sized_heaptypes( extra_base_size, -extra_size)) @@ -773,7 +773,6 @@ def check(extra_base_size, extra_size): else: # The following offsets should be in increasing order: - data_offset = data_ptr - id(instance) offsets = [ (0, 'start of object'), (Base.__basicsize__, 'end of base data'), diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 2035cdf274053e..4c232dbbf4e770 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -58,7 +58,9 @@ make_sized_heaptypes(PyObject *module, PyObject *args) goto finally; } - result = Py_BuildValue("OOOKn", base, sub, instance, (unsigned long long)data_ptr, + result = Py_BuildValue("OOOKnn", base, sub, instance, + (unsigned long long)data_ptr, + (Py_ssize_t)(data_ptr - (void*)instance), data_size); finally: Py_XDECREF(base); From 266834d2bd53fb77f5f64a03971764e6e715a753 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 24 Apr 2023 17:25:53 +0200 Subject: [PATCH 14/24] Cast to `char*` for pointer arithmetic Co-authored-by: Oleg Iarygin --- Modules/_testcapi/heaptype_relative.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 4c232dbbf4e770..3699f361b26a84 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -49,7 +49,7 @@ make_sized_heaptypes(PyObject *module, PyObject *args) if (!instance) { goto finally; } - void *data_ptr = PyObject_GetTypeData(instance, (PyTypeObject *)sub); + char *data_ptr = PyObject_GetTypeData(instance, (PyTypeObject *)sub); if (!data_ptr) { goto finally; } @@ -60,7 +60,7 @@ make_sized_heaptypes(PyObject *module, PyObject *args) result = Py_BuildValue("OOOKnn", base, sub, instance, (unsigned long long)data_ptr, - (Py_ssize_t)(data_ptr - (void*)instance), + (Py_ssize_t)(data_ptr - (char*)instance), data_size); finally: Py_XDECREF(base); From f968206d118b10f913b977d51bb41a9b9d638ed9 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 25 Apr 2023 09:54:04 +0200 Subject: [PATCH 15/24] Fix ALIGNOF_MAX_ALIGN_T value for 32-bit Windows (I set it low on purpose, so the CI tests would find the issue.) --- PC/pyconfig.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PC/pyconfig.h b/PC/pyconfig.h index 56bc24c4f51568..3415efe2dea117 100644 --- a/PC/pyconfig.h +++ b/PC/pyconfig.h @@ -352,7 +352,7 @@ Py_NO_ENABLE_SHARED to find out. Also support MS_NO_COREDLL for b/w compat */ # else # define SIZEOF_TIME_T 4 # endif -# define ALIGNOF_MAX_ALIGN_T 4 +# define ALIGNOF_MAX_ALIGN_T 8 #endif #ifdef _DEBUG From 37158af0741fbbb5810ee8a16f7aa8dd21123cdf Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 25 Apr 2023 16:02:03 +0200 Subject: [PATCH 16/24] Fix C++ behaviour and comments for ALIGNOF_MAX_ALIGN_T --- Include/pyport.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Include/pyport.h b/Include/pyport.h index 65b1547f381a38..d0af2a176c21f0 100644 --- a/Include/pyport.h +++ b/Include/pyport.h @@ -746,13 +746,13 @@ extern char * _getpty(int *, int, mode_t, int); #endif // Make sure we have maximum alignment, even if the current compiler -// does not support max_align_t. -// - Autoconf report alignment of unknown types to 0. -// - We don't have the Py_MAX macro yet. +// does not support max_align_t. Note that: +// - Autoconf reports alignment of unknown types to 0. // - 'long double' has maximum alignment on *most* platforms, // looks like the best we can do for pre-C11 compilers. // - The value is tested, see test_alignof_max_align_t #if !defined(ALIGNOF_MAX_ALIGN_T) || ALIGNOF_MAX_ALIGN_T == 0 +# undef ALIGNOF_MAX_ALIGN_T # define ALIGNOF_MAX_ALIGN_T _Alignof(long double) #endif From 1701688e3f96717d916d642ab808a00d38f00e6c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 28 Apr 2023 13:25:56 +0200 Subject: [PATCH 17/24] Don't rely on PyObject* being aligned to ALIGNOF_MAX_ALIGN_T Smaller objects might not be aligned to ALIGNOF_MAX_ALIGN_T. The offsets calculated for PEP 697 should be aligned, though. --- Lib/test/test_capi/test_misc.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 32f0ec1f865a0c..da1fb6b94ca107 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -793,8 +793,8 @@ def check(extra_base_size, extra_size): self.assertLess(data_size - extra_size, _testcapi.alignof_max_align_t) - # Everything should be aligned - self.assertEqual(data_ptr % _testcapi.alignof_max_align_t, 0) + # The offsets/sizes we calculated should be aligned. + self.assertEqual(data_offset % _testcapi.alignof_max_align_t, 0) self.assertEqual(data_size % _testcapi.alignof_max_align_t, 0) sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, From 9d9911d73fd3e5c7ff36f101743fe394d588c3ff Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 2 May 2023 15:15:52 +0200 Subject: [PATCH 18/24] Apply suggestions from code review Co-authored-by: Erlend E. Aasland --- Doc/c-api/object.rst | 6 +++--- Doc/c-api/type.rst | 2 +- Modules/_testcapi/heaptype.c | 27 +++++++++++++++++---------- Objects/typeobject.c | 12 +++++++----- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index 1ae8f35c8b8c6a..d17e4b69154ac9 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -396,11 +396,11 @@ Object Protocol .. versionadded:: 3.10 -.. c:function:: PyObject* PyObject_GetTypeData(PyObject *o, PyTypeObject *cls) +.. c:function:: void *PyObject_GetTypeData(PyObject *o, PyTypeObject *cls) Get a pointer to subclass-specific data reserved for *cls*. - The object *o* **must** be an instance of *cls*, and *cls* must have been + The object *o* must be an instance of *cls*, and *cls* must have been created using negative :c:member:`PyType_Spec.basicsize`. Python does not check this. @@ -424,7 +424,7 @@ Object Protocol .. versionadded:: 3.12 -.. c:function:: PyObject* PyObject_GetItemData(PyObject *o) +.. c:function:: void *PyObject_GetItemData(PyObject *o) Get a pointer to per-item data for a class with :const:`Py_TPFLAGS_ITEMS_AT_END`. diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 133a26d1800abf..5da752db9a856d 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -374,7 +374,7 @@ The following functions and structs are used to create directly. - With the :const:`Py_TPFLAGS_ITEMS_AT_END` flag. - .. c:member:: int flags + .. c:member:: unsigned int flags Type flags, used to set :c:member:`PyTypeObject.tp_flags`. diff --git a/Modules/_testcapi/heaptype.c b/Modules/_testcapi/heaptype.c index 3c74f6c078693f..73de275b80d731 100644 --- a/Modules/_testcapi/heaptype.c +++ b/Modules/_testcapi/heaptype.c @@ -1036,7 +1036,8 @@ HeapCCollection_traverse(PyObject *self, visitproc visit, void *arg) } static int -HeapCCollection_clear(PyObject *self) { +HeapCCollection_clear(PyObject *self) +{ PyObject **data = PyObject_GetItemData(self); if (!data) { return -1; @@ -1049,7 +1050,9 @@ HeapCCollection_clear(PyObject *self) { return 0; } -static void HeapCCollection_dealloc(PyObject *self) { +static void +HeapCCollection_dealloc(PyObject *self) +{ PyTypeObject *tp = Py_TYPE(self); HeapCCollection_clear(self); PyObject_GC_UnTrack(self); @@ -1064,17 +1067,17 @@ static PyType_Slot HeapCCollection_slots[] = { {Py_tp_traverse, HeapCCollection_traverse}, {Py_tp_clear, HeapCCollection_clear}, {Py_tp_dealloc, HeapCCollection_dealloc}, - {Py_tp_doc, (char*)HeapCCollection_doc}, + {Py_tp_doc, (void *)HeapCCollection_doc}, {0, 0}, }; static PyType_Spec HeapCCollection_spec = { - "_testcapi.HeapCCollection", - sizeof(PyVarObject), - sizeof(PyObject*), - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC - | Py_TPFLAGS_ITEMS_AT_END, - HeapCCollection_slots + .name = "_testcapi.HeapCCollection", + .basicsize = sizeof(PyVarObject), + .itemsize = sizeof(PyObject*), + .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | + Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_ITEMS_AT_END), + .slots = HeapCCollection_slots, }; int @@ -1205,7 +1208,11 @@ _PyTestCapi_Init_Heaptype(PyObject *m) { if (HeapCCollection == NULL) { return -1; } - PyModule_AddObject(m, "HeapCCollection", HeapCCollection); + int rc = PyModule_AddType(m, (PyTypeObject *)HeapCCollection); + Py_DECREF(HeapCCollection); + if (rc < 0) { + return -1; + } return 0; } diff --git a/Objects/typeobject.c b/Objects/typeobject.c index 2299e4087c79ae..c222db988ac232 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -3567,7 +3567,8 @@ static const PySlot_Offset pyslot_offsets[] = { * (like _Py_ALIGN_UP, but for a size rather than pointer) */ static Py_ssize_t -_align_up(Py_ssize_t size) { +_align_up(Py_ssize_t size) +{ return (size + ALIGNOF_MAX_ALIGN_T - 1) & ~(ALIGNOF_MAX_ALIGN_T - 1); } @@ -3711,13 +3712,13 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, vectorcalloffset = memb->offset; } if (memb->flags & Py_RELATIVE_OFFSET) { - if(spec->basicsize > 0) { + if (spec->basicsize > 0) { PyErr_SetString( PyExc_SystemError, "With Py_RELATIVE_OFFSET, basicsize must be negative."); goto finally; } - if(memb->offset < 0 || memb->offset >= -spec->basicsize) { + if (memb->offset < 0 || memb->offset >= -spec->basicsize) { PyErr_SetString( PyExc_SystemError, "Member offset out of range (0..-basicsize)"); @@ -3945,9 +3946,10 @@ PyType_FromMetaclass(PyTypeObject *metaclass, PyObject *module, memcpy(_PyHeapType_GET_MEMBERS(res), slot->pfunc, len); type->tp_members = _PyHeapType_GET_MEMBERS(res); PyMemberDef *memb; - unsigned i; + Py_ssize_t i; for (memb = _PyHeapType_GET_MEMBERS(res), i = nmembers; - i > 0; ++memb, --i) { + i > 0; ++memb, --i) + { if (memb->flags & Py_RELATIVE_OFFSET) { memb->flags &= ~Py_RELATIVE_OFFSET; memb->offset += type_data_offset; From 06bcf5b859b20fb74df549064289a3d248139047 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 2 May 2023 14:52:56 +0200 Subject: [PATCH 19/24] Raise TypeError on missing flag --- Lib/test/test_capi/test_misc.py | 10 ++++++++++ Modules/_testcapi/heaptype.c | 10 ++++++++++ Objects/typeobject.c | 7 ++++++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index da1fb6b94ca107..f6cc3c53f771fb 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -870,6 +870,16 @@ def test_heaptype_relative_members(self): Sub = _testcapi.make_heaptype_with_member( extra_base_size, -extra_size, -1, True) + def test_pyobject_getitemdata_error(self): + """Test PyObject_GetItemData fails on unsupported types""" + with self.assertRaises(TypeError): + # None is not variable-length + _testcapi.pyobject_getitemdata(None) + with self.assertRaises(TypeError): + # int is variable-length, but doesn't have the + # Py_TPFLAGS_ITEMS_AT_END layout (and flag) + _testcapi.pyobject_getitemdata(0) + def test_pynumber_tobase(self): from _testcapi import pynumber_tobase small_number = 123 diff --git a/Modules/_testcapi/heaptype.c b/Modules/_testcapi/heaptype.c index 73de275b80d731..213d44bed62d12 100644 --- a/Modules/_testcapi/heaptype.c +++ b/Modules/_testcapi/heaptype.c @@ -385,6 +385,15 @@ make_immutable_type_with_base(PyObject *self, PyObject *base) return PyType_FromSpecWithBases(&ImmutableSubclass_spec, base); } +static PyObject * +pyobject_getitemdata(PyObject *self, PyObject *o) +{ + void *pointer = PyObject_GetItemData(o); + if (pointer == NULL) { + return NULL; + } + return PyLong_FromVoidPtr(pointer); +} static PyMethodDef TestMethods[] = { {"pytype_fromspec_meta", pytype_fromspec_meta, METH_O}, @@ -397,6 +406,7 @@ static PyMethodDef TestMethods[] = { test_from_spec_invalid_metatype_inheritance, METH_NOARGS}, {"make_immutable_type_with_base", make_immutable_type_with_base, METH_O}, + {"pyobject_getitemdata", pyobject_getitemdata, METH_O}, {NULL}, }; diff --git a/Objects/typeobject.c b/Objects/typeobject.c index c222db988ac232..75c0ed955082d5 100644 --- a/Objects/typeobject.c +++ b/Objects/typeobject.c @@ -4210,7 +4210,12 @@ PyType_GetTypeDataSize(PyTypeObject *cls) void * PyObject_GetItemData(PyObject *obj) { - assert(PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_ITEMS_AT_END)); + if (!PyType_HasFeature(Py_TYPE(obj), Py_TPFLAGS_ITEMS_AT_END)) { + PyErr_Format(PyExc_TypeError, + "type '%s' does not have Py_TPFLAGS_ITEMS_AT_END", + Py_TYPE(obj)->tp_name); + return NULL; + } return (char *)obj + Py_TYPE(obj)->tp_basicsize; } From 0b69748a895f96bf3fea056dfe136ad1e68fa098 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 2 May 2023 15:05:23 +0200 Subject: [PATCH 20/24] Wrap new tests in a class --- Lib/test/test_capi/test_misc.py | 246 ++++++++++++++++---------------- 1 file changed, 124 insertions(+), 122 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index f6cc3c53f771fb..aa5f1917014aa4 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -758,128 +758,6 @@ def meth(self): MutableBase.meth = lambda self: 'changed' self.assertEqual(instance.meth(), 'changed') - @requires_limited_api - def test_heaptype_relative_sizes(self): - # Test subclassing using "relative" basicsize, see PEP 697 - def check(extra_base_size, extra_size): - Base, Sub, instance, data_ptr, data_offset, data_size = ( - _testcapi.make_sized_heaptypes( - extra_base_size, -extra_size)) - - # no alignment shenanigans when inheriting directly - if extra_size == 0: - self.assertEqual(Base.__basicsize__, Sub.__basicsize__) - self.assertEqual(data_size, 0) - - else: - # The following offsets should be in increasing order: - offsets = [ - (0, 'start of object'), - (Base.__basicsize__, 'end of base data'), - (data_offset, 'subclass data'), - (data_offset + extra_size, 'end of requested subcls data'), - (data_offset + data_size, 'end of reserved subcls data'), - (Sub.__basicsize__, 'end of object'), - ] - ordered_offsets = sorted(offsets, key=operator.itemgetter(0)) - self.assertEqual( - offsets, ordered_offsets, - msg=f'Offsets not in expected order, got: {ordered_offsets}') - - # end of reserved subcls data == end of object - self.assertEqual(Sub.__basicsize__, data_offset + data_size) - - # we don't reserve (requested + alignment) or more data - self.assertLess(data_size - extra_size, - _testcapi.alignof_max_align_t) - - # The offsets/sizes we calculated should be aligned. - self.assertEqual(data_offset % _testcapi.alignof_max_align_t, 0) - self.assertEqual(data_size % _testcapi.alignof_max_align_t, 0) - - sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, - object.__basicsize__, - object.__basicsize__-1, - object.__basicsize__+1}) - for extra_base_size in sizes: - for extra_size in sizes: - args = dict(extra_base_size=extra_base_size, - extra_size=extra_size) - with self.subTest(**args): - check(**args) - - @requires_limited_api - def test_HeapCCollection(self): - """Make sure HeapCCollection works properly by itself""" - collection = _testcapi.HeapCCollection(1, 2, 3) - self.assertEqual(list(collection), [1, 2, 3]) - - @requires_limited_api - def test_heaptype_inherit_itemsize(self): - """Test HeapCCollection subclasses work properly""" - sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, - object.__basicsize__, - object.__basicsize__-1, - object.__basicsize__+1}) - for extra_size in sizes: - with self.subTest(extra_size=extra_size): - Sub = _testcapi.subclass_var_heaptype( - _testcapi.HeapCCollection, -extra_size, 0, 0) - collection = Sub(1, 2, 3) - collection.set_data_to_3s() - - self.assertEqual(list(collection), [1, 2, 3]) - mem = collection.get_data() - self.assertGreaterEqual(len(mem), extra_size) - self.assertTrue(set(mem) <= {3}, f'got {mem!r}') - - @requires_limited_api - def test_heaptype_relative_members(self): - """Test HeapCCollection subclasses work properly""" - sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, - object.__basicsize__, - object.__basicsize__-1, - object.__basicsize__+1}) - for extra_base_size in sizes: - for extra_size in sizes: - for offset in sizes: - with self.subTest(extra_base_size=extra_base_size, extra_size=extra_size, offset=offset): - if offset < extra_size: - Sub = _testcapi.make_heaptype_with_member( - extra_base_size, -extra_size, offset, True) - Base = Sub.mro()[1] - instance = Sub() - self.assertEqual(instance.memb, instance.get_memb()) - instance.set_memb(13) - self.assertEqual(instance.memb, instance.get_memb()) - self.assertEqual(instance.get_memb(), 13) - instance.memb = 14 - self.assertEqual(instance.memb, instance.get_memb()) - self.assertEqual(instance.get_memb(), 14) - self.assertGreaterEqual(instance.get_memb_offset(), Base.__basicsize__) - self.assertLess(instance.get_memb_offset(), Sub.__basicsize__) - else: - with self.assertRaises(SystemError): - Sub = _testcapi.make_heaptype_with_member( - extra_base_size, -extra_size, offset, True) - with self.assertRaises(SystemError): - Sub = _testcapi.make_heaptype_with_member( - extra_base_size, extra_size, offset, True) - with self.subTest(extra_base_size=extra_base_size, extra_size=extra_size): - with self.assertRaises(SystemError): - Sub = _testcapi.make_heaptype_with_member( - extra_base_size, -extra_size, -1, True) - - def test_pyobject_getitemdata_error(self): - """Test PyObject_GetItemData fails on unsupported types""" - with self.assertRaises(TypeError): - # None is not variable-length - _testcapi.pyobject_getitemdata(None) - with self.assertRaises(TypeError): - # int is variable-length, but doesn't have the - # Py_TPFLAGS_ITEMS_AT_END layout (and flag) - _testcapi.pyobject_getitemdata(0) - def test_pynumber_tobase(self): from _testcapi import pynumber_tobase small_number = 123 @@ -1167,6 +1045,130 @@ class dictsub(dict): ... # dict subclasses must work self.assertEqual(some.__kwdefaults__, None) +@requires_limited_api +class TestHeapTypeRelative(unittest.TestCase): + """Test API for extending opaque types (PEP 697)""" + + @requires_limited_api + def test_heaptype_relative_sizes(self): + # Test subclassing using "relative" basicsize, see PEP 697 + def check(extra_base_size, extra_size): + Base, Sub, instance, data_ptr, data_offset, data_size = ( + _testcapi.make_sized_heaptypes( + extra_base_size, -extra_size)) + + # no alignment shenanigans when inheriting directly + if extra_size == 0: + self.assertEqual(Base.__basicsize__, Sub.__basicsize__) + self.assertEqual(data_size, 0) + + else: + # The following offsets should be in increasing order: + offsets = [ + (0, 'start of object'), + (Base.__basicsize__, 'end of base data'), + (data_offset, 'subclass data'), + (data_offset + extra_size, 'end of requested subcls data'), + (data_offset + data_size, 'end of reserved subcls data'), + (Sub.__basicsize__, 'end of object'), + ] + ordered_offsets = sorted(offsets, key=operator.itemgetter(0)) + self.assertEqual( + offsets, ordered_offsets, + msg=f'Offsets not in expected order, got: {ordered_offsets}') + + # end of reserved subcls data == end of object + self.assertEqual(Sub.__basicsize__, data_offset + data_size) + + # we don't reserve (requested + alignment) or more data + self.assertLess(data_size - extra_size, + _testcapi.alignof_max_align_t) + + # The offsets/sizes we calculated should be aligned. + self.assertEqual(data_offset % _testcapi.alignof_max_align_t, 0) + self.assertEqual(data_size % _testcapi.alignof_max_align_t, 0) + + sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, + object.__basicsize__, + object.__basicsize__-1, + object.__basicsize__+1}) + for extra_base_size in sizes: + for extra_size in sizes: + args = dict(extra_base_size=extra_base_size, + extra_size=extra_size) + with self.subTest(**args): + check(**args) + + def test_HeapCCollection(self): + """Make sure HeapCCollection works properly by itself""" + collection = _testcapi.HeapCCollection(1, 2, 3) + self.assertEqual(list(collection), [1, 2, 3]) + + def test_heaptype_inherit_itemsize(self): + """Test HeapCCollection subclasses work properly""" + sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, + object.__basicsize__, + object.__basicsize__-1, + object.__basicsize__+1}) + for extra_size in sizes: + with self.subTest(extra_size=extra_size): + Sub = _testcapi.subclass_var_heaptype( + _testcapi.HeapCCollection, -extra_size, 0, 0) + collection = Sub(1, 2, 3) + collection.set_data_to_3s() + + self.assertEqual(list(collection), [1, 2, 3]) + mem = collection.get_data() + self.assertGreaterEqual(len(mem), extra_size) + self.assertTrue(set(mem) <= {3}, f'got {mem!r}') + + def test_heaptype_relative_members(self): + """Test HeapCCollection subclasses work properly""" + sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, + object.__basicsize__, + object.__basicsize__-1, + object.__basicsize__+1}) + for extra_base_size in sizes: + for extra_size in sizes: + for offset in sizes: + with self.subTest(extra_base_size=extra_base_size, extra_size=extra_size, offset=offset): + if offset < extra_size: + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, -extra_size, offset, True) + Base = Sub.mro()[1] + instance = Sub() + self.assertEqual(instance.memb, instance.get_memb()) + instance.set_memb(13) + self.assertEqual(instance.memb, instance.get_memb()) + self.assertEqual(instance.get_memb(), 13) + instance.memb = 14 + self.assertEqual(instance.memb, instance.get_memb()) + self.assertEqual(instance.get_memb(), 14) + self.assertGreaterEqual(instance.get_memb_offset(), Base.__basicsize__) + self.assertLess(instance.get_memb_offset(), Sub.__basicsize__) + else: + with self.assertRaises(SystemError): + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, -extra_size, offset, True) + with self.assertRaises(SystemError): + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, extra_size, offset, True) + with self.subTest(extra_base_size=extra_base_size, extra_size=extra_size): + with self.assertRaises(SystemError): + Sub = _testcapi.make_heaptype_with_member( + extra_base_size, -extra_size, -1, True) + + def test_pyobject_getitemdata_error(self): + """Test PyObject_GetItemData fails on unsupported types""" + with self.assertRaises(TypeError): + # None is not variable-length + _testcapi.pyobject_getitemdata(None) + with self.assertRaises(TypeError): + # int is variable-length, but doesn't have the + # Py_TPFLAGS_ITEMS_AT_END layout (and flag) + _testcapi.pyobject_getitemdata(0) + + class TestPendingCalls(unittest.TestCase): def pendingcalls_submit(self, l, n): From 33c5258b9c3fd5d946fb07cf0d4f870917215415 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 2 May 2023 15:13:49 +0200 Subject: [PATCH 21/24] Use PyModule_AddIntMacro in tests --- Lib/test/test_capi/test_misc.py | 6 +++--- Modules/_testcapi/heaptype_relative.c | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index aa5f1917014aa4..bbb04b521186df 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1082,11 +1082,11 @@ def check(extra_base_size, extra_size): # we don't reserve (requested + alignment) or more data self.assertLess(data_size - extra_size, - _testcapi.alignof_max_align_t) + _testcapi.ALIGNOF_MAX_ALIGN_T) # The offsets/sizes we calculated should be aligned. - self.assertEqual(data_offset % _testcapi.alignof_max_align_t, 0) - self.assertEqual(data_size % _testcapi.alignof_max_align_t, 0) + self.assertEqual(data_offset % _testcapi.ALIGNOF_MAX_ALIGN_T, 0) + self.assertEqual(data_size % _testcapi.ALIGNOF_MAX_ALIGN_T, 0) sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, object.__basicsize__, diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 3699f361b26a84..21a965f5f712f1 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -281,7 +281,9 @@ _PyTestCapi_Init_HeaptypeRelative(PyObject *m) { return -1; } - PyModule_AddIntConstant(m, "alignof_max_align_t", ALIGNOF_MAX_ALIGN_T); + if (PyModule_AddIntMacro(m, ALIGNOF_MAX_ALIGN_T) < 0) { + return -1; + } return 0; } From 88dade7f0749efe3648473bfc28b7a747dbb776f Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 2 May 2023 15:59:56 +0200 Subject: [PATCH 22/24] Test failure of extending non-ITEMS_AT_END variable-sized types --- Lib/test/test_capi/test_misc.py | 6 +++++ Modules/_testcapi/heaptype_relative.c | 32 +++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index bbb04b521186df..974d10d490ff0f 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1122,6 +1122,12 @@ def test_heaptype_inherit_itemsize(self): self.assertGreaterEqual(len(mem), extra_size) self.assertTrue(set(mem) <= {3}, f'got {mem!r}') + def test_heaptype_invalid_inheritance(self): + with self.assertRaises(SystemError, + msg="Cannot extend variable-size class without " + + "Py_TPFLAGS_ITEMS_AT_END"): + _testcapi.subclass_heaptype(int, -8, 0) + def test_heaptype_relative_members(self): """Test HeapCCollection subclasses work properly""" sizes = sorted({0, 1, 2, 3, 4, 7, 8, 123, diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 21a965f5f712f1..2f6694401b2200 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -141,6 +141,37 @@ subclass_var_heaptype(PyObject *module, PyObject *args) return result; } +static PyObject * +subclass_heaptype(PyObject *module, PyObject *args) +{ + PyObject *result = NULL; + + PyObject *base; // borrowed from args + int basicsize, itemsize; + + int r = PyArg_ParseTuple(args, "Oii", &base, &basicsize, &itemsize); + if (!r) { + goto finally; + } + + PyType_Slot slots[] = { + {Py_tp_methods, var_heaptype_methods}, + {0, NULL}, + }; + + PyType_Spec sub_spec = { + .name = "_testcapi.Sub", + .basicsize = basicsize, + .itemsize = itemsize, + .flags = Py_TPFLAGS_DEFAULT, + .slots = slots, + }; + + result = PyType_FromMetaclass(NULL, module, &sub_spec, base); + finally: + return result; +} + static PyMemberDef * heaptype_with_member_extract_and_check_memb(PyObject *self) { @@ -270,6 +301,7 @@ test_alignof_max_align_t(PyObject *module, PyObject *Py_UNUSED(ignored)) static PyMethodDef TestMethods[] = { {"make_sized_heaptypes", make_sized_heaptypes, METH_VARARGS}, {"subclass_var_heaptype", subclass_var_heaptype, METH_VARARGS}, + {"subclass_heaptype", subclass_heaptype, METH_VARARGS}, {"make_heaptype_with_member", make_heaptype_with_member, METH_VARARGS}, {"test_alignof_max_align_t", test_alignof_max_align_t, METH_NOARGS}, {NULL}, From 8720b25efa92cd3c265e3cbce3f550a3e7b63f25 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Tue, 2 May 2023 16:07:19 +0200 Subject: [PATCH 23/24] Test error cases around members --- Lib/test/test_capi/test_misc.py | 25 +++++++++++++++++++++++++ Modules/_testcapi/heaptype_relative.c | 20 ++++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/Lib/test/test_capi/test_misc.py b/Lib/test/test_capi/test_misc.py index 974d10d490ff0f..15472022f8164c 100644 --- a/Lib/test/test_capi/test_misc.py +++ b/Lib/test/test_capi/test_misc.py @@ -1152,6 +1152,10 @@ def test_heaptype_relative_members(self): self.assertEqual(instance.get_memb(), 14) self.assertGreaterEqual(instance.get_memb_offset(), Base.__basicsize__) self.assertLess(instance.get_memb_offset(), Sub.__basicsize__) + with self.assertRaises(SystemError): + instance.get_memb_relative() + with self.assertRaises(SystemError): + instance.set_memb_relative(0) else: with self.assertRaises(SystemError): Sub = _testcapi.make_heaptype_with_member( @@ -1164,6 +1168,27 @@ def test_heaptype_relative_members(self): Sub = _testcapi.make_heaptype_with_member( extra_base_size, -extra_size, -1, True) + def test_heaptype_relative_members_errors(self): + with self.assertRaisesRegex( + SystemError, + r"With Py_RELATIVE_OFFSET, basicsize must be negative"): + _testcapi.make_heaptype_with_member(0, 1234, 0, True) + with self.assertRaisesRegex( + SystemError, r"Member offset out of range \(0\.\.-basicsize\)"): + _testcapi.make_heaptype_with_member(0, -8, 1234, True) + with self.assertRaisesRegex( + SystemError, r"Member offset out of range \(0\.\.-basicsize\)"): + _testcapi.make_heaptype_with_member(0, -8, -1, True) + + Sub = _testcapi.make_heaptype_with_member(0, -8, 0, True) + instance = Sub() + with self.assertRaisesRegex( + SystemError, r"PyMember_GetOne used with Py_RELATIVE_OFFSET"): + instance.get_memb_relative() + with self.assertRaisesRegex( + SystemError, r"PyMember_SetOne used with Py_RELATIVE_OFFSET"): + instance.set_memb_relative(0) + def test_pyobject_getitemdata_error(self): """Test PyObject_GetItemData fails on unsupported types""" with self.assertRaises(TypeError): diff --git a/Modules/_testcapi/heaptype_relative.c b/Modules/_testcapi/heaptype_relative.c index 2f6694401b2200..c247ca33b33708 100644 --- a/Modules/_testcapi/heaptype_relative.c +++ b/Modules/_testcapi/heaptype_relative.c @@ -226,10 +226,30 @@ get_memb_offset(PyObject *self, PyObject *Py_UNUSED(ignored)) return PyLong_FromSsize_t(def->offset); } +static PyObject * +heaptype_with_member_get_memb_relative(PyObject *self, PyObject *Py_UNUSED(ignored)) +{ + PyMemberDef def = {"memb", Py_T_BYTE, sizeof(PyObject), Py_RELATIVE_OFFSET}; + return PyMember_GetOne((const char *)self, &def); +} + +static PyObject * +heaptype_with_member_set_memb_relative(PyObject *self, PyObject *value) +{ + PyMemberDef def = {"memb", Py_T_BYTE, sizeof(PyObject), Py_RELATIVE_OFFSET}; + int r = PyMember_SetOne((char *)self, &def, value); + if (r < 0) { + return NULL; + } + Py_RETURN_NONE; +} + static PyMethodDef heaptype_with_member_methods[] = { {"get_memb", heaptype_with_member_get_memb, METH_NOARGS}, {"set_memb", heaptype_with_member_set_memb, METH_O}, {"get_memb_offset", get_memb_offset, METH_NOARGS}, + {"get_memb_relative", heaptype_with_member_get_memb_relative, METH_NOARGS}, + {"set_memb_relative", heaptype_with_member_set_memb_relative, METH_O}, {NULL}, }; From a975de9d1ee11d10b8e3288900b7496e5b8fd0d7 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 3 May 2023 15:29:48 +0200 Subject: [PATCH 24/24] Apply suggestions from code review Co-authored-by: Oleg Iarygin --- Doc/c-api/object.rst | 2 +- Doc/c-api/type.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/c-api/object.rst b/Doc/c-api/object.rst index d17e4b69154ac9..a0c3194ab0fb90 100644 --- a/Doc/c-api/object.rst +++ b/Doc/c-api/object.rst @@ -410,7 +410,7 @@ Object Protocol .. c:function:: Py_ssize_t PyType_GetTypeDataSize(PyTypeObject *cls) - Return the size of the memory reserved for *cls*, i.e. the size of the + Return the size of the instance memory space reserved for *cls*, i.e. the size of the memory :c:func:`PyObject_GetTypeData` returns. This may be larger than requested using :c:member:`-PyType_Spec.basicsize `; diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 5da752db9a856d..aadd9cd955c615 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -354,7 +354,7 @@ The following functions and structs are used to create .. c:member:: int itemsize - Size of one element of a variable-size type, in bytes + Size of one element of a variable-size type, in bytes. Used to set :c:member:`PyTypeObject.tp_itemsize`. See ``tp_itemsize`` documentation for caveats.