From 9d4be49d18ab03bba2e8e329d4d68cac23a389f3 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Wed, 18 Dec 2024 15:12:31 +0100 Subject: [PATCH 01/29] Restore max field size to sys.maxsize, as in Python 3.13 & below --- Lib/test/test_ctypes/test_struct_fields.py | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index 1b3e64efd410b8..cdcbe372125ba3 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -79,23 +79,22 @@ class Subclass(BrokenStructure): ... def test_max_field_size_gh126937(self): # Classes for big structs should be created successfully. # (But they most likely can't be instantiated.) - # Here we test the exact limit: the number of *bits* must fit - # in Py_ssize_t. + # The size must fit in Py_ssize_t. - class X(self.cls): + class X(Structure): _fields_ = [('char', c_char),] - max_field_size = sys.maxsize // 8 + max_field_size = sys.maxsize - class Y(self.cls): + class Y(Structure): _fields_ = [('largeField', X * max_field_size)] - class Z(self.cls): + class Z(Structure): _fields_ = [('largeField', c_char * max_field_size)] - with self.assertRaises(ValueError): - class TooBig(self.cls): + with self.assertRaises(OverflowError): + class TooBig(Structure): _fields_ = [('largeField', X * (max_field_size + 1))] - with self.assertRaises(ValueError): - class TooBig(self.cls): + with self.assertRaises(OverflowError): + class TooBig(Structure): _fields_ = [('largeField', c_char * (max_field_size + 1))] # __set__ and __get__ should raise a TypeError in case their self From 09c81a827507ba728a8855bc7574e361d21a2c49 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 9 Jan 2025 18:11:28 +0100 Subject: [PATCH 02/29] PyCField: Split out bit/byte sizes/offsets. --- Doc/library/ctypes.rst | 62 +++++++- Lib/ctypes/_layout.py | 10 +- Modules/_ctypes/cfield.c | 255 ++++++++++++++++++++++-------- Modules/_ctypes/clinic/cfield.c.h | 47 ++++-- Modules/_ctypes/ctypes.h | 31 +++- Modules/_ctypes/stgdict.c | 10 +- 6 files changed, 322 insertions(+), 93 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 164d706e7738e2..c78a16fb40c943 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -657,7 +657,8 @@ Nested structures can also be initialized in the constructor in several ways:: >>> r = RECT((1, 2), (3, 4)) Field :term:`descriptor`\s can be retrieved from the *class*, they are useful -for debugging because they can provide useful information:: +for debugging because they can provide useful information. +See :class:`_Field`:: >>> print(POINT.x) @@ -2776,6 +2777,65 @@ fields, or any other data types containing pointer type fields. present in :attr:`_fields_`. +.. class:: _Field(*args, **kw) + + Field descriptors are an internal class with the following attributes. + (Note that the name ``_Field`` is not available.) + + .. attribute:: name + + Name of the field, as a string. + + .. attribute:: type + + Type of the field, as a :ref:`ctypes class `. + + .. attribute:: offset + byte_offset + + Offset of the field, in bytes. + + For bitfields, this excludes any :attr:`~_Field.bit_offset`. + + .. attribute:: byte_size + + Size of the field, in bytes. + + For bitfields, this is the size of the underlying type; it may be + much larger than the field itself. + + .. attribute:: size + + For non-bitfields, equivalent to :attr:`~_Field.byte_size`. + + For bitfields, this contains a backwards-compatible bit-packed + value that combines :attr:`~_Field.bitfield_size` and + :attr:`~_Field.bitfield_offset`. + + Prefer using the explicit attributes instead. + + .. attribute:: is_bitfield + + True iff this is a bitfield. + + .. attribute:: bit_offset + + Additional offset, in bits, of a bitfield. + + Zero for non-bitfields. + + .. attribute:: bit_size + + Size of the field, in bits. + + For non-bitfields, this is equal to ``byte_size * 8``. + + .. attribute:: is_anonymous + + True iff this field is anonymous, that is, it contains nested sub-fields + that should be be merged into a containing structure or union. + + .. _ctypes-arrays-pointers: Arrays and pointers diff --git a/Lib/ctypes/_layout.py b/Lib/ctypes/_layout.py index e30db598ab22e1..852d19536abece 100644 --- a/Lib/ctypes/_layout.py +++ b/Lib/ctypes/_layout.py @@ -216,6 +216,7 @@ def get_layout(cls, input_fields, is_struct, base): offset = round_down(next_bit_offset, type_bit_align) // 8 if is_bitfield: effective_bit_offset = next_bit_offset - 8 * offset + bit_offset = effective_bit_offset size = build_size(bit_size, effective_bit_offset, big_endian, type_size) assert effective_bit_offset <= type_bit_size @@ -255,8 +256,9 @@ def get_layout(cls, input_fields, is_struct, base): offset = next_byte_offset - last_field_bit_size // 8 if is_bitfield: assert 0 <= (last_field_bit_size + next_bit_offset) + bit_offset = last_field_bit_size + next_bit_offset size = build_size(bit_size, - last_field_bit_size + next_bit_offset, + bit_offset, big_endian, type_size) else: size = type_size @@ -294,10 +296,12 @@ def get_layout(cls, input_fields, is_struct, base): result_fields.append(CField( name=name, type=ctype, - size=size, - offset=offset, + byte_size=type_size, + byte_offset=offset, bit_size=bit_size if is_bitfield else None, + bit_offset=bit_offset if is_bitfield else None, index=i, + for_big_endian=big_endian, )) if is_bitfield and not gcc_layout: assert type_bit_size > 0 diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index dcac9da75360a4..cbda9ad84afe92 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -56,51 +56,34 @@ Py_ssize_t LOW_BIT(Py_ssize_t offset); @classmethod _ctypes.CField.__new__ as PyCField_new + * name: object(subclass_of='&PyUnicode_Type') type as proto: object - size: Py_ssize_t - offset: Py_ssize_t + byte_size: Py_ssize_t + byte_offset: Py_ssize_t index: Py_ssize_t + for_big_endian: bool bit_size as bit_size_obj: object = None + bit_offset as bit_offset_obj: object = None [clinic start generated code]*/ static PyObject * PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, - Py_ssize_t size, Py_ssize_t offset, Py_ssize_t index, - PyObject *bit_size_obj) -/*[clinic end generated code: output=43649ef9157c5f58 input=3d813f56373c4caa]*/ + Py_ssize_t byte_size, Py_ssize_t byte_offset, + Py_ssize_t index, int for_big_endian, + PyObject *bit_size_obj, PyObject *bit_offset_obj) +/*[clinic end generated code: output=7c45e9b31f9ba07b input=d13e02ff15b0d2fe]*/ { CFieldObject* self = NULL; - if (size < 0) { + if (byte_size < 0) { PyErr_Format(PyExc_ValueError, - "size of field %R must not be negative, got %zd", - name, size); - goto error; - } - // assert: no overflow; - if ((unsigned long long int) size - >= (1ULL << (8*sizeof(Py_ssize_t)-1)) / 8) { - PyErr_Format(PyExc_ValueError, - "size of field %R is too big: %zd", name, size); + "byte size of field %R must not be negative, got %zd", + name, byte_size); goto error; } - PyTypeObject *tp = type; - ctypes_state *st = get_module_state_by_class(tp); - self = (CFieldObject *)tp->tp_alloc(tp, 0); - if (!self) { - return NULL; - } - if (PyUnicode_CheckExact(name)) { - self->name = Py_NewRef(name); - } else { - self->name = PyObject_Str(name); - if (!self->name) { - goto error; - } - } - + ctypes_state *st = get_module_state_by_class(type); StgInfo *info; if (PyStgInfo_FromType(st, proto, &info) < 0) { goto error; @@ -110,17 +93,12 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, "type of field %R must be a C type", self->name); goto error; } + assert(byte_size == info->size); + Py_ssize_t bitfield_size = 0; + Py_ssize_t bit_offset = 0; if (bit_size_obj != Py_None) { -#ifdef Py_DEBUG - Py_ssize_t bit_size = NUM_BITS(size); - assert(bit_size > 0); - assert(bit_size <= info->size * 8); - // Currently, the bit size is specified redundantly - // in NUM_BITS(size) and bit_size_obj. - // Verify that they match. - assert(PyLong_AsSsize_t(bit_size_obj) == bit_size); -#endif + // It's a bit field! switch(info->ffi_type_pointer.type) { case FFI_TYPE_UINT8: case FFI_TYPE_UINT16: @@ -144,11 +122,69 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, ((PyTypeObject*)proto)->tp_name); goto error; } + + if (byte_size > 100) { + // Bitfields must "live" in a field defined by a ffi type, + // so they're limited to about 8 bytes. + // This check is here to avoid overflow in later checks. + PyErr_Format(PyExc_ValueError, + "bit field %R size too large, got %zd", + name, byte_size); + } + bitfield_size = PyLong_AsSsize_t(bit_size_obj); + if ((bitfield_size <= 0) || (bitfield_size > 255)) { + if (!PyErr_Occurred()) { + PyErr_Format(PyExc_ValueError, + "bit size of field %R out of range, got %zd", + name, bitfield_size); + } + goto error; + } + bit_offset = PyLong_AsSsize_t(bit_offset_obj); + if ((bit_offset < 0) || (bit_offset > 255)) { + if (!PyErr_Occurred()) { + PyErr_Format(PyExc_ValueError, + "bit offset of field %R out of range, got %zd", + name, bit_offset); + } + goto error; + } + if ((bitfield_size + bit_offset) > byte_size * 8) { + PyErr_Format( + PyExc_ValueError, + "bit field %R overflows its type (%zd + %zd >= %zd)", + name, bit_offset, byte_size*8); + goto error; + } + } + else { + if (bit_offset_obj != Py_None) { + PyErr_Format( + PyExc_ValueError, + "field %R: bit_offset must be specified if bit_size is", + name); + } + } + + self = (CFieldObject *)type->tp_alloc(type, 0); + if (!self) { + return NULL; + } + if (PyUnicode_CheckExact(name)) { + self->name = Py_NewRef(name); + } else { + self->name = PyObject_Str(name); + if (!self->name) { + goto error; + } } self->proto = Py_NewRef(proto); - self->size = size; - self->offset = offset; + self->byte_size = byte_size; + self->byte_offset = byte_offset; + self->bitfield_size = bitfield_size; + self->bit_offset = bit_offset; + self->_for_big_endian = for_big_endian; self->index = index; @@ -192,6 +228,19 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, return NULL; } +static inline Py_ssize_t +_pack_legacy_size(CFieldObject *field) +{ + if (field->bitfield_size) { + assert(field->bit_offset < (1 << 16)); + Py_ssize_t bit_offset = field->bit_offset; + if (field->_for_big_endian) { + bit_offset = 8 * field->byte_size - bit_offset - field->bitfield_size; + } + return (field->bitfield_size << 16) | bit_offset; + } + return field->byte_size; +} static int PyCField_set(CFieldObject *self, PyObject *inst, PyObject *value) @@ -205,14 +254,14 @@ PyCField_set(CFieldObject *self, PyObject *inst, PyObject *value) return -1; } dst = (CDataObject *)inst; - ptr = dst->b_ptr + self->offset; + ptr = dst->b_ptr + self->byte_offset; if (value == NULL) { PyErr_SetString(PyExc_TypeError, "can't delete attribute"); return -1; } return PyCData_set(st, inst, self->proto, self->setfunc, value, - self->index, self->size, ptr); + self->index, _pack_legacy_size(self), ptr); } static PyObject * @@ -230,25 +279,102 @@ PyCField_get(CFieldObject *self, PyObject *inst, PyTypeObject *type) } src = (CDataObject *)inst; return PyCData_get(st, self->proto, self->getfunc, inst, - self->index, self->size, src->b_ptr + self->offset); + self->index, _pack_legacy_size(self), + src->b_ptr + self->byte_offset); } static PyObject * -PyCField_get_offset(PyObject *self, void *data) +PyCField_get_legacy_size(PyObject *self, void *data) { - return PyLong_FromSsize_t(((CFieldObject *)self)->offset); + CFieldObject *field = (CFieldObject *)self; + return PyLong_FromSsize_t(_pack_legacy_size(field)); } static PyObject * -PyCField_get_size(PyObject *self, void *data) +PyCField_get_bit_size(PyObject *self, void *data) { - return PyLong_FromSsize_t(((CFieldObject *)self)->size); + CFieldObject *field = (CFieldObject *)self; + if (field->bitfield_size) { + return PyLong_FromSsize_t(field->bitfield_size); + } + if (field->byte_size < PY_SSIZE_T_MAX / 8) { + return PyLong_FromSsize_t(field->byte_size * 8); + } + // XXX test this! + PyObject *byte_size_obj = PyLong_FromSsize_t(field->byte_size); + if (!byte_size_obj) { + return NULL; + } + PyObject *eight = PyLong_FromLong(8); + if (!eight) { + return NULL; + } + return PyNumber_Multiply(byte_size_obj, eight); + +} + +static PyObject * +PyCField_is_bitfield(PyObject *self, void *data) +{ + CFieldObject *field = (CFieldObject *)self; + return PyBool_FromLong(field->bitfield_size); +} + +static PyObject * +PyCField_is_anonymous(PyObject *self, void *data) +{ + CFieldObject *field = (CFieldObject *)self; + return PyBool_FromLong(field->anonymous); } static PyGetSetDef PyCField_getset[] = { - { "offset", PyCField_get_offset, NULL, PyDoc_STR("offset in bytes of this field") }, - { "size", PyCField_get_size, NULL, PyDoc_STR("size in bytes of this field") }, - { NULL, NULL, NULL, NULL }, + { "size", PyCField_get_legacy_size, NULL, + PyDoc_STR("size in bytes of this field. For bitfields, this is a " + "legacy packed value; use byte_size instead") }, + + { "bit_size", PyCField_get_bit_size, NULL, + PyDoc_STR("size of this field in bits") }, + { "is_bitfield", PyCField_is_bitfield, NULL, + PyDoc_STR("true if this is a bitfield") }, + { "is_anonymous", PyCField_is_anonymous, NULL, + PyDoc_STR("true if this field is anonymous") }, + { NULL }, +}; + +static PyMemberDef PyCField_members[] = { + { "name", + .type=Py_T_OBJECT_EX, + .offset=offsetof(CFieldObject, name), + .flags=Py_READONLY, + .doc=PyDoc_STR("name of this field") }, + { "type", + .type=Py_T_OBJECT_EX, + .offset=offsetof(CFieldObject, proto), + .flags=Py_READONLY, + .doc=PyDoc_STR("type of this field") }, + { "offset", + .type=Py_T_PYSSIZET, + .offset=offsetof(CFieldObject, byte_offset), + .flags=Py_READONLY, + .doc=PyDoc_STR("offset in bytes of this field (same as byte_offset)") }, + { "byte_offset", + .type=Py_T_PYSSIZET, + .offset=offsetof(CFieldObject, byte_offset), + .flags=Py_READONLY, + .doc=PyDoc_STR("offset in bytes of this field. " + "For bitfields: excludes bit_offset.") }, + { "byte_size", + .type=Py_T_PYSSIZET, + .offset=offsetof(CFieldObject, byte_size), + .flags=Py_READONLY, + .doc=PyDoc_STR("size of this field in bytes") }, + { "bit_offset", + .type=Py_T_PYSSIZET, + .offset=offsetof(CFieldObject, bit_offset), + .flags=Py_READONLY, + .doc=PyDoc_STR("additional offset in bits (relative to byte_offset); " + "zero for non-bitfields") }, + { NULL }, }; static int @@ -279,23 +405,25 @@ PyCField_dealloc(PyObject *self) } static PyObject * -PyCField_repr(CFieldObject *self) +PyCField_repr(PyObject *self) { + CFieldObject *field = (CFieldObject *)self; PyObject *result; - Py_ssize_t bits = NUM_BITS(self->size); - Py_ssize_t size = LOW_BIT(self->size); - const char *name; + const char *tp_name = ((PyTypeObject *)field->proto)->tp_name; - name = ((PyTypeObject *)self->proto)->tp_name; - - if (bits) + if (field->bitfield_size) { result = PyUnicode_FromFormat( - "", - name, self->offset, size, bits); - else + "", + field->name, tp_name, field->byte_offset, + (Py_ssize_t)field->bitfield_size, + (Py_ssize_t)field->bit_offset); + } + else { result = PyUnicode_FromFormat( - "", - name, self->offset, size); + "", + field->name, tp_name, field->byte_offset, + field->byte_size); + } return result; } @@ -307,6 +435,7 @@ static PyType_Slot cfield_slots[] = { {Py_tp_traverse, PyCField_traverse}, {Py_tp_clear, PyCField_clear}, {Py_tp_getset, PyCField_getset}, + {Py_tp_members, PyCField_members}, {Py_tp_descr_get, PyCField_get}, {Py_tp_descr_set, PyCField_set}, {0, NULL}, diff --git a/Modules/_ctypes/clinic/cfield.c.h b/Modules/_ctypes/clinic/cfield.c.h index bbf108a1a07b67..99eb289f35d617 100644 --- a/Modules/_ctypes/clinic/cfield.c.h +++ b/Modules/_ctypes/clinic/cfield.c.h @@ -11,8 +11,9 @@ preserve static PyObject * PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, - Py_ssize_t size, Py_ssize_t offset, Py_ssize_t index, - PyObject *bit_size_obj); + Py_ssize_t byte_size, Py_ssize_t byte_offset, + Py_ssize_t index, int for_big_endian, + PyObject *bit_size_obj, PyObject *bit_offset_obj); static PyObject * PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) @@ -20,14 +21,14 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) - #define NUM_KEYWORDS 6 + #define NUM_KEYWORDS 8 static struct { PyGC_Head _this_is_not_used; PyObject_VAR_HEAD PyObject *ob_item[NUM_KEYWORDS]; } _kwtuple = { .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) - .ob_item = { &_Py_ID(name), &_Py_ID(type), &_Py_ID(size), &_Py_ID(offset), &_Py_ID(index), &_Py_ID(bit_size), }, + .ob_item = { &_Py_ID(name), &_Py_ID(type), &_Py_ID(byte_size), &_Py_ID(byte_offset), &_Py_ID(index), &_Py_ID(for_big_endian), &_Py_ID(bit_size), &_Py_ID(bit_offset), }, }; #undef NUM_KEYWORDS #define KWTUPLE (&_kwtuple.ob_base.ob_base) @@ -36,26 +37,28 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) # define KWTUPLE NULL #endif // !Py_BUILD_CORE - static const char * const _keywords[] = {"name", "type", "size", "offset", "index", "bit_size", NULL}; + static const char * const _keywords[] = {"name", "type", "byte_size", "byte_offset", "index", "for_big_endian", "bit_size", "bit_offset", NULL}; static _PyArg_Parser _parser = { .keywords = _keywords, .fname = "CField", .kwtuple = KWTUPLE, }; #undef KWTUPLE - PyObject *argsbuf[6]; + PyObject *argsbuf[8]; PyObject * const *fastargs; Py_ssize_t nargs = PyTuple_GET_SIZE(args); - Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 5; + Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 6; PyObject *name; PyObject *proto; - Py_ssize_t size; - Py_ssize_t offset; + Py_ssize_t byte_size; + Py_ssize_t byte_offset; Py_ssize_t index; + int for_big_endian; PyObject *bit_size_obj = Py_None; + PyObject *bit_offset_obj = Py_None; fastargs = _PyArg_UnpackKeywords(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, - /*minpos*/ 5, /*maxpos*/ 6, /*minkw*/ 0, /*varpos*/ 0, argsbuf); + /*minpos*/ 0, /*maxpos*/ 0, /*minkw*/ 6, /*varpos*/ 0, argsbuf); if (!fastargs) { goto exit; } @@ -75,7 +78,7 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) if (ival == -1 && PyErr_Occurred()) { goto exit; } - size = ival; + byte_size = ival; } { Py_ssize_t ival = -1; @@ -87,7 +90,7 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) if (ival == -1 && PyErr_Occurred()) { goto exit; } - offset = ival; + byte_offset = ival; } { Py_ssize_t ival = -1; @@ -101,14 +104,24 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) } index = ival; } + for_big_endian = PyObject_IsTrue(fastargs[5]); + if (for_big_endian < 0) { + goto exit; + } if (!noptargs) { - goto skip_optional_pos; + goto skip_optional_kwonly; + } + if (fastargs[6]) { + bit_size_obj = fastargs[6]; + if (!--noptargs) { + goto skip_optional_kwonly; + } } - bit_size_obj = fastargs[5]; -skip_optional_pos: - return_value = PyCField_new_impl(type, name, proto, size, offset, index, bit_size_obj); + bit_offset_obj = fastargs[7]; +skip_optional_kwonly: + return_value = PyCField_new_impl(type, name, proto, byte_size, byte_offset, index, for_big_endian, bit_size_obj, bit_offset_obj); exit: return return_value; } -/*[clinic end generated code: output=6b450bdd861571e7 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=cc75c5e88f661cec input=a9049054013a1b77]*/ diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index cc09639e21f7c2..b945f816b0223f 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -257,14 +257,35 @@ struct fielddesc { typedef struct CFieldObject { PyObject_HEAD - Py_ssize_t offset; - Py_ssize_t size; - Py_ssize_t index; /* Index into CDataObject's - object array */ + + /* byte size & offset + * For bit fields, this identifies a chunk of memory that the bits are + * extracted from; the entire chunk needs to be readable/writable. + * byte_size is the same as the underlying ctype size. + * Note that byte_offset might not be aligned to proto's alignment. + */ + Py_ssize_t byte_offset; + Py_ssize_t byte_size; + + Py_ssize_t index; /* Index into CDataObject's object array */ PyObject *proto; /* underlying ctype; must have StgInfo */ GETFUNC getfunc; /* getter function if proto is NULL */ SETFUNC setfunc; /* setter function if proto is NULL */ - int anonymous; + bool anonymous: 1; + bool _for_big_endian: 1; /* true if the class is big-endian */ + + /* If this is a bit field, bitfield_size must be positive. + * bitfield_size and bit_offset specify the field inside the chunk of + * memory identified by byte_offset & byte_size. + * Otherwise, these are both zero. + * + * Note that for NON-bitfields: + * - `bit_size` (user-facing Python attribute) `is byte_size*8` + * - `bitfield_size` (this) is zero + * Hence the different name. + */ + uint8_t bitfield_size; + uint8_t bit_offset; PyObject *name; /* exact PyUnicode */ } CFieldObject; diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index 5ca5b62427600d..667aa70677eb15 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -120,7 +120,7 @@ MakeFields(PyObject *type, CFieldObject *descr, if (fdescr->anonymous) { int rc = MakeFields(type, fdescr, index + fdescr->index, - offset + fdescr->offset); + offset + fdescr->byte_offset); Py_DECREF(fdescr); if (rc == -1) { Py_DECREF(fieldlist); @@ -135,8 +135,10 @@ MakeFields(PyObject *type, CFieldObject *descr, return -1; } assert(Py_IS_TYPE(new_descr, cfield_tp)); - new_descr->size = fdescr->size; - new_descr->offset = fdescr->offset + offset; + new_descr->byte_size = fdescr->byte_size; + new_descr->byte_offset = fdescr->byte_offset + offset; + new_descr->bitfield_size = fdescr->bitfield_size; + new_descr->bit_offset = fdescr->bit_offset; new_descr->index = fdescr->index + index; new_descr->proto = Py_XNewRef(fdescr->proto); new_descr->getfunc = fdescr->getfunc; @@ -198,7 +200,7 @@ MakeAnonFields(PyObject *type) /* descr is in the field descriptor. */ if (-1 == MakeFields(type, (CFieldObject *)descr, ((CFieldObject *)descr)->index, - ((CFieldObject *)descr)->offset)) { + ((CFieldObject *)descr)->byte_offset)) { Py_DECREF(descr); Py_DECREF(anon_names); return -1; From c397cf44c9a156ebf83f863bdde06fa9c327556d Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 10 Jan 2025 18:20:26 +0100 Subject: [PATCH 03/29] Expose CField --- Doc/library/ctypes.rst | 62 +++++++++++++++++++++++-------- Doc/whatsnew/3.14.rst | 5 +++ Lib/ctypes/__init__.py | 1 + Lib/ctypes/_layout.py | 4 ++ Modules/_ctypes/cfield.c | 22 ++++++++--- Modules/_ctypes/clinic/cfield.c.h | 29 +++++++++------ Modules/_ctypes/ctypes.h | 6 ++- 7 files changed, 93 insertions(+), 36 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index c78a16fb40c943..0351f8ab88f987 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -658,12 +658,12 @@ Nested structures can also be initialized in the constructor in several ways:: Field :term:`descriptor`\s can be retrieved from the *class*, they are useful for debugging because they can provide useful information. -See :class:`_Field`:: +See :class:`CField`:: - >>> print(POINT.x) - - >>> print(POINT.y) - + >>> POINT.x + + >>> POINT.y + >>> @@ -2777,10 +2777,41 @@ fields, or any other data types containing pointer type fields. present in :attr:`_fields_`. -.. class:: _Field(*args, **kw) +.. class:: CField(*args, **kw) - Field descriptors are an internal class with the following attributes. - (Note that the name ``_Field`` is not available.) + Descriptor for fields of a :class:`Structure` and :class:`Union`. + For example:: + + >>> class Color(Structure): + ... _fields_ = ( + ... ('red', c_uint8), + ... ('green', c_uint8), + ... ('blue', c_uint8), + ... ('intense', c_bool, 1), + ... ('blinking', c_bool, 1), + ... ) + ... + >>> Color.red + + >>> Color.green.type + + >>> Color.blue.byte_offset + 2 + >>> Color.intense + + >>> Color.blinking.bit_offset + 1 + + All attributes are read-only. + + :class:`!CField` objects are created via :attr:`~Structure._fields_`; + do not instantiate the class directly. + + .. versionadded:: next + + Previously, descriptors only had ``offset`` and ``size`` attributes + and a readable string representation; the :class:`!CField` class was not + available directly. .. attribute:: name @@ -2795,28 +2826,27 @@ fields, or any other data types containing pointer type fields. Offset of the field, in bytes. - For bitfields, this excludes any :attr:`~_Field.bit_offset`. + For bitfields, this excludes any :attr:`~CField.bit_offset`. .. attribute:: byte_size Size of the field, in bytes. - For bitfields, this is the size of the underlying type; it may be + For bitfields, this is the size of the underlying type, which may be much larger than the field itself. .. attribute:: size - For non-bitfields, equivalent to :attr:`~_Field.byte_size`. + For non-bitfields, equivalent to :attr:`~CField.byte_size`. For bitfields, this contains a backwards-compatible bit-packed - value that combines :attr:`~_Field.bitfield_size` and - :attr:`~_Field.bitfield_offset`. - + value that combines :attr:`~CField.bit_size` and + :attr:`~CField.bit_offset`. Prefer using the explicit attributes instead. .. attribute:: is_bitfield - True iff this is a bitfield. + True if this is a bitfield. .. attribute:: bit_offset @@ -2832,7 +2862,7 @@ fields, or any other data types containing pointer type fields. .. attribute:: is_anonymous - True iff this field is anonymous, that is, it contains nested sub-fields + True if this field is anonymous, that is, it contains nested sub-fields that should be be merged into a containing structure or union. diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index 9f7ef101e56478..6c5273ba7e13f7 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -338,6 +338,11 @@ ctypes to help match a non-default ABI. (Contributed by Petr Viktorin in :gh:`97702`.) +* The class of :class:`~ctypes.Structure`/:class:`~ctypes.Union` + field descriptors is now available as :class:`~ctypes.CField`, + and has new attributes to aid debugging and introspection. + (Contributed by Petr Viktorin in :gh:`128715`.) + * On Windows, the :exc:`~ctypes.COMError` exception is now public. (Contributed by Jun Komoda in :gh:`126686`.) diff --git a/Lib/ctypes/__init__.py b/Lib/ctypes/__init__.py index 8e2a2926f7a853..d9e55816211737 100644 --- a/Lib/ctypes/__init__.py +++ b/Lib/ctypes/__init__.py @@ -12,6 +12,7 @@ from _ctypes import RTLD_LOCAL, RTLD_GLOBAL from _ctypes import ArgumentError from _ctypes import SIZEOF_TIME_T +from _ctypes import CField from struct import calcsize as _calcsize diff --git a/Lib/ctypes/_layout.py b/Lib/ctypes/_layout.py index 852d19536abece..25af2ff4e0af0d 100644 --- a/Lib/ctypes/_layout.py +++ b/Lib/ctypes/_layout.py @@ -302,6 +302,10 @@ def get_layout(cls, input_fields, is_struct, base): bit_offset=bit_offset if is_bitfield else None, index=i, for_big_endian=big_endian, + + # Do not use CField outside ctypes, yet. + # The constructor is internal API and may change without warning. + _internal_use=True, )) if is_bitfield and not gcc_layout: assert type_bit_size > 0 diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index cbda9ad84afe92..0079189ea50921 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -63,6 +63,7 @@ _ctypes.CField.__new__ as PyCField_new byte_offset: Py_ssize_t index: Py_ssize_t for_big_endian: bool + _internal_use: bool bit_size as bit_size_obj: object = None bit_offset as bit_offset_obj: object = None @@ -71,11 +72,18 @@ _ctypes.CField.__new__ as PyCField_new static PyObject * PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, Py_ssize_t byte_size, Py_ssize_t byte_offset, - Py_ssize_t index, int for_big_endian, + Py_ssize_t index, int for_big_endian, int _internal_use, PyObject *bit_size_obj, PyObject *bit_offset_obj) -/*[clinic end generated code: output=7c45e9b31f9ba07b input=d13e02ff15b0d2fe]*/ +/*[clinic end generated code: output=79505dee1dad9b8e input=a6376bdec96976b8]*/ { CFieldObject* self = NULL; + + if (!_internal_use) { + // Do not instantiate outside ctypes, yet. + // The constructor is internal API and may change without warning. + PyErr_Format(PyExc_TypeError, "Cannot create %T object.", type); + goto error; + } if (byte_size < 0) { PyErr_Format(PyExc_ValueError, "byte size of field %R must not be negative, got %zd", @@ -369,7 +377,7 @@ static PyMemberDef PyCField_members[] = { .flags=Py_READONLY, .doc=PyDoc_STR("size of this field in bytes") }, { "bit_offset", - .type=Py_T_PYSSIZET, + .type=Py_T_UBYTE, .offset=offsetof(CFieldObject, bit_offset), .flags=Py_READONLY, .doc=PyDoc_STR("additional offset in bits (relative to byte_offset); " @@ -413,14 +421,16 @@ PyCField_repr(PyObject *self) if (field->bitfield_size) { result = PyUnicode_FromFormat( - "", + "<%T %R type=%s, ofs=%zd, bit_size=%zd, bit_offset=%zd>", + self, field->name, tp_name, field->byte_offset, (Py_ssize_t)field->bitfield_size, (Py_ssize_t)field->bit_offset); } else { result = PyUnicode_FromFormat( - "", + "<%T %R type=%s, ofs=%zd, size=%zd>", + self, field->name, tp_name, field->byte_offset, field->byte_size); } @@ -442,7 +452,7 @@ static PyType_Slot cfield_slots[] = { }; PyType_Spec cfield_spec = { - .name = "_ctypes.CField", + .name = "ctypes.CField", .basicsize = sizeof(CFieldObject), .flags = (Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | Py_TPFLAGS_IMMUTABLETYPE), diff --git a/Modules/_ctypes/clinic/cfield.c.h b/Modules/_ctypes/clinic/cfield.c.h index 99eb289f35d617..f06ab46b8ea83b 100644 --- a/Modules/_ctypes/clinic/cfield.c.h +++ b/Modules/_ctypes/clinic/cfield.c.h @@ -12,7 +12,7 @@ preserve static PyObject * PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, Py_ssize_t byte_size, Py_ssize_t byte_offset, - Py_ssize_t index, int for_big_endian, + Py_ssize_t index, int for_big_endian, int _internal_use, PyObject *bit_size_obj, PyObject *bit_offset_obj); static PyObject * @@ -21,14 +21,14 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) - #define NUM_KEYWORDS 8 + #define NUM_KEYWORDS 9 static struct { PyGC_Head _this_is_not_used; PyObject_VAR_HEAD PyObject *ob_item[NUM_KEYWORDS]; } _kwtuple = { .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) - .ob_item = { &_Py_ID(name), &_Py_ID(type), &_Py_ID(byte_size), &_Py_ID(byte_offset), &_Py_ID(index), &_Py_ID(for_big_endian), &_Py_ID(bit_size), &_Py_ID(bit_offset), }, + .ob_item = { &_Py_ID(name), &_Py_ID(type), &_Py_ID(byte_size), &_Py_ID(byte_offset), &_Py_ID(index), &_Py_ID(for_big_endian), &_Py_ID(_internal_use), &_Py_ID(bit_size), &_Py_ID(bit_offset), }, }; #undef NUM_KEYWORDS #define KWTUPLE (&_kwtuple.ob_base.ob_base) @@ -37,28 +37,29 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) # define KWTUPLE NULL #endif // !Py_BUILD_CORE - static const char * const _keywords[] = {"name", "type", "byte_size", "byte_offset", "index", "for_big_endian", "bit_size", "bit_offset", NULL}; + static const char * const _keywords[] = {"name", "type", "byte_size", "byte_offset", "index", "for_big_endian", "_internal_use", "bit_size", "bit_offset", NULL}; static _PyArg_Parser _parser = { .keywords = _keywords, .fname = "CField", .kwtuple = KWTUPLE, }; #undef KWTUPLE - PyObject *argsbuf[8]; + PyObject *argsbuf[9]; PyObject * const *fastargs; Py_ssize_t nargs = PyTuple_GET_SIZE(args); - Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 6; + Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 7; PyObject *name; PyObject *proto; Py_ssize_t byte_size; Py_ssize_t byte_offset; Py_ssize_t index; int for_big_endian; + int _internal_use; PyObject *bit_size_obj = Py_None; PyObject *bit_offset_obj = Py_None; fastargs = _PyArg_UnpackKeywords(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, - /*minpos*/ 0, /*maxpos*/ 0, /*minkw*/ 6, /*varpos*/ 0, argsbuf); + /*minpos*/ 0, /*maxpos*/ 0, /*minkw*/ 7, /*varpos*/ 0, argsbuf); if (!fastargs) { goto exit; } @@ -108,20 +109,24 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) if (for_big_endian < 0) { goto exit; } + _internal_use = PyObject_IsTrue(fastargs[6]); + if (_internal_use < 0) { + goto exit; + } if (!noptargs) { goto skip_optional_kwonly; } - if (fastargs[6]) { - bit_size_obj = fastargs[6]; + if (fastargs[7]) { + bit_size_obj = fastargs[7]; if (!--noptargs) { goto skip_optional_kwonly; } } - bit_offset_obj = fastargs[7]; + bit_offset_obj = fastargs[8]; skip_optional_kwonly: - return_value = PyCField_new_impl(type, name, proto, byte_size, byte_offset, index, for_big_endian, bit_size_obj, bit_offset_obj); + return_value = PyCField_new_impl(type, name, proto, byte_size, byte_offset, index, for_big_endian, _internal_use, bit_size_obj, bit_offset_obj); exit: return return_value; } -/*[clinic end generated code: output=cc75c5e88f661cec input=a9049054013a1b77]*/ +/*[clinic end generated code: output=42a8a8fedff88230 input=a9049054013a1b77]*/ diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index b945f816b0223f..2387f87366c387 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -260,8 +260,10 @@ typedef struct CFieldObject { /* byte size & offset * For bit fields, this identifies a chunk of memory that the bits are - * extracted from; the entire chunk needs to be readable/writable. - * byte_size is the same as the underlying ctype size. + * extracted from. The entire chunk needs to be contained in the enclosing + * struct/union. + * byte_size is the same as the underlying ctype size (and thus it is + * redundant and could be eliminated). * Note that byte_offset might not be aligned to proto's alignment. */ Py_ssize_t byte_offset; From 20ecd844248a41ac547ca98bc98b2b67e4c45e59 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 14:31:00 +0100 Subject: [PATCH 04/29] Add generic checks for all the test structs/unions --- Lib/test/test_ctypes/_support.py | 95 ++++++++++++++++ .../test_ctypes/test_aligned_structures.py | 25 ++++- Lib/test/test_ctypes/test_anon.py | 8 +- Lib/test/test_ctypes/test_bitfields.py | 106 ++++++++++++------ Lib/test/test_ctypes/test_bytes.py | 5 +- Lib/test/test_ctypes/test_byteswap.py | 17 ++- Lib/test/test_ctypes/test_funcptr.py | 5 +- .../test_ctypes/test_generated_structs.py | 5 +- Lib/test/test_ctypes/test_structures.py | 41 ++++++- 9 files changed, 263 insertions(+), 44 deletions(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index e4c2b33825ae8f..804b34b9d4b7d5 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -22,3 +22,98 @@ class _X(Structure): # type flags Py_TPFLAGS_DISALLOW_INSTANTIATION = 1 << 7 Py_TPFLAGS_IMMUTABLETYPE = 1 << 8 + + +class StructCheckMixin: + def check_struct(self, structure): + """Assert that a structure is well-formed""" + self._check_struct_or_union(structure, is_struct=True) + + def check_union(self, union): + """Assert that a union is well-formed""" + self._check_struct_or_union(union, is_struct=False) + + def check_struct_or_union(self, cls): + if issubclass(cls, Structure): + self._check_struct_or_union(cls, is_struct=True) + elif issubclass(cls, Union): + self._check_struct_or_union(cls, is_struct=False) + else: + raise TypeError(cls) + + def _check_struct_or_union(self, cls, is_struct): + + # Check that fields are not overlapping (for structs), + # and that their metadata is consistent. + + # offset of the last checked bit, from start of struct + # stays 0 for unions + next_bit = 0 + + anon_names = getattr(cls, '_anonymous_', ()) + cls_size = ctypes.sizeof(cls) + for name, field_type, *rest in cls._fields_: + field = getattr(cls, name) + try: + is_bitfield = bool(rest) + + # name + self.assertEqual(field.name, name) + # type + self.assertEqual(field.type, field_type) + # offset == byte_offset + self.assertEqual(field.byte_offset, field.offset) + if not is_struct: + self.assertEqual(field.byte_offset, 0) + # byte_size + self.assertEqual(field.byte_size, ctypes.sizeof(field_type)) + self.assertGreaterEqual(field.byte_size, 0) + self.assertLessEqual(field.byte_offset + field.byte_size, + cls_size) + # size + self.assertGreaterEqual(field.size, 0) + if is_bitfield: + if not hasattr(cls, '_swappedbytes_'): + self.assertEqual( + field.size, + (field.bit_size << 16) + field.bit_offset) + else: + self.assertEqual(field.size, field.byte_size) + # is_bitfield + self.assertEqual(field.is_bitfield, is_bitfield) + # bit_offset + if is_bitfield: + self.assertGreaterEqual(field.bit_offset, 0) + self.assertLessEqual(field.bit_offset + field.bit_size, + field.byte_size * 8) + else: + self.assertEqual(field.bit_offset, 0) + if not is_struct: + self.assertEqual(field.bit_offset, 0) + # bit_size + if is_bitfield: + self.assertGreaterEqual(field.bit_size, 0) + else: + self.assertEqual(field.bit_size, field.byte_size * 8) + + # is_anonymous + self.assertEqual(field.is_anonymous, name in anon_names) + + # and for struct, not overlapping earlier members. + # (this assumes fields are laid out in order) + self.assertGreaterEqual( + field.byte_offset * 8 + field.bit_offset, + next_bit) + next_bit = (field.byte_offset * 8 + + field.bit_offset + + field.bit_size) + # field is inside cls + self.assertLessEqual(next_bit, cls_size * 8) + + if not is_struct: + # union fields may overlap + next_bit = 0 + except Exception as e: + # Similar to `self.subTest`, but subTest doesn't nest well. + e.add_note(f'while checking field {name!r}: {field}') + raise diff --git a/Lib/test/test_ctypes/test_aligned_structures.py b/Lib/test/test_ctypes/test_aligned_structures.py index a208fb9a00966a..26d24f31b29f7b 100644 --- a/Lib/test/test_ctypes/test_aligned_structures.py +++ b/Lib/test/test_ctypes/test_aligned_structures.py @@ -5,9 +5,9 @@ ) import struct import unittest +from ._support import StructCheckMixin - -class TestAlignedStructures(unittest.TestCase): +class TestAlignedStructures(unittest.TestCase, StructCheckMixin): def test_aligned_string(self): for base, e in ( (LittleEndianStructure, "<"), @@ -19,12 +19,14 @@ class Aligned(base): _fields_ = [ ('value', c_char * 12) ] + self.check_struct(Aligned) class Main(base): _fields_ = [ ('first', c_uint32), ('string', Aligned), ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(main.first, 7) @@ -46,12 +48,14 @@ class SomeBools(base): ("bool1", c_ubyte), ("bool2", c_ubyte), ] + self.check_struct(SomeBools) class Main(base): _fields_ = [ ("x", c_ubyte), ("y", SomeBools), ("z", c_ubyte), ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(alignment(SomeBools), 4) @@ -75,11 +79,13 @@ class SomeBoolsTooBig(base): ("bool2", c_ubyte), ("bool3", c_ubyte), ] + self.check_struct(SomeBoolsTooBig) class Main(base): _fields_ = [ ("y", SomeBoolsTooBig), ("z", c_uint32), ] + self.check_struct(Main) with self.assertRaises(ValueError) as ctx: Main.from_buffer(data) self.assertEqual( @@ -98,18 +104,21 @@ class UnalignedSub(base): _fields_ = [ ("x", c_uint32), ] + self.check_struct(UnalignedSub) class AlignedStruct(UnalignedSub): _align_ = 8 _fields_ = [ ("y", c_uint32), ] + self.check_struct(AlignedStruct) class Main(base): _fields_ = [ ("a", c_uint32), ("b", AlignedStruct) ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(alignment(main.b), 8) @@ -134,12 +143,14 @@ class AlignedUnion(ubase): ("a", c_uint32), ("b", c_ubyte * 7), ] + self.check_union(AlignedUnion) class Main(sbase): _fields_ = [ ("first", c_uint32), ("union", AlignedUnion), ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(main.first, 1) @@ -162,18 +173,21 @@ class Sub(sbase): ("x", c_uint32), ("y", c_uint32), ] + self.check_struct(Sub) class MainUnion(ubase): _fields_ = [ ("a", c_uint32), ("b", Sub), ] + self.check_union(MainUnion) class Main(sbase): _fields_ = [ ("first", c_uint32), ("union", MainUnion), ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(Main.first.size, 4) @@ -198,17 +212,20 @@ class SubUnion(ubase): ("unsigned", c_ubyte), ("signed", c_byte), ] + self.check_union(SubUnion) class MainUnion(SubUnion): _fields_ = [ ("num", c_uint32) ] + self.check_union(SubUnion) class Main(sbase): _fields_ = [ ("first", c_uint16), ("union", MainUnion), ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(main.union.num, 0xD60102D7) @@ -232,11 +249,13 @@ class SubUnion(ubase): ("unsigned", c_ubyte), ("signed", c_byte), ] + self.check_union(SubUnion) class Main(SubUnion): _fields_ = [ ("num", c_uint32) ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(alignment(main), 8) @@ -258,6 +277,7 @@ class Inner(sbase): ("x", c_uint16), ("y", c_uint16), ] + self.check_struct(Inner) class Main(sbase): _pack_ = 1 @@ -266,6 +286,7 @@ class Main(sbase): ("b", Inner), ("c", c_ubyte), ] + self.check_struct(Main) main = Main.from_buffer(data) self.assertEqual(sizeof(main), 10) diff --git a/Lib/test/test_ctypes/test_anon.py b/Lib/test/test_ctypes/test_anon.py index b36397b510fefe..2e16e708635989 100644 --- a/Lib/test/test_ctypes/test_anon.py +++ b/Lib/test/test_ctypes/test_anon.py @@ -1,20 +1,23 @@ import unittest import test.support from ctypes import c_int, Union, Structure, sizeof +from ._support import StructCheckMixin -class AnonTest(unittest.TestCase): +class AnonTest(unittest.TestCase, StructCheckMixin): def test_anon(self): class ANON(Union): _fields_ = [("a", c_int), ("b", c_int)] + self.check_union(ANON) class Y(Structure): _fields_ = [("x", c_int), ("_", ANON), ("y", c_int)] _anonymous_ = ["_"] + self.check_struct(Y) self.assertEqual(Y.a.offset, sizeof(c_int)) self.assertEqual(Y.b.offset, sizeof(c_int)) @@ -52,17 +55,20 @@ class Name(Structure): def test_nested(self): class ANON_S(Structure): _fields_ = [("a", c_int)] + self.check_struct(ANON_S) class ANON_U(Union): _fields_ = [("_", ANON_S), ("b", c_int)] _anonymous_ = ["_"] + self.check_union(ANON_U) class Y(Structure): _fields_ = [("x", c_int), ("_", ANON_U), ("y", c_int)] _anonymous_ = ["_"] + self.check_struct(Y) self.assertEqual(Y.x.offset, 0) self.assertEqual(Y.a.offset, sizeof(c_int)) diff --git a/Lib/test/test_ctypes/test_bitfields.py b/Lib/test/test_ctypes/test_bitfields.py index 19ba2f4484e7da..dc81e752567c42 100644 --- a/Lib/test/test_ctypes/test_bitfields.py +++ b/Lib/test/test_ctypes/test_bitfields.py @@ -10,27 +10,33 @@ Union) from test import support from test.support import import_helper +from ._support import StructCheckMixin _ctypes_test = import_helper.import_module("_ctypes_test") +TEST_FIELDS = ( + ("A", c_int, 1), + ("B", c_int, 2), + ("C", c_int, 3), + ("D", c_int, 4), + ("E", c_int, 5), + ("F", c_int, 6), + ("G", c_int, 7), + ("H", c_int, 8), + ("I", c_int, 9), + + ("M", c_short, 1), + ("N", c_short, 2), + ("O", c_short, 3), + ("P", c_short, 4), + ("Q", c_short, 5), + ("R", c_short, 6), + ("S", c_short, 7), +) + + class BITS(Structure): - _fields_ = [("A", c_int, 1), - ("B", c_int, 2), - ("C", c_int, 3), - ("D", c_int, 4), - ("E", c_int, 5), - ("F", c_int, 6), - ("G", c_int, 7), - ("H", c_int, 8), - ("I", c_int, 9), - - ("M", c_short, 1), - ("N", c_short, 2), - ("O", c_short, 3), - ("P", c_short, 4), - ("Q", c_short, 5), - ("R", c_short, 6), - ("S", c_short, 7)] + _fields_ = TEST_FIELDS func = CDLL(_ctypes_test.__file__).unpack_bitfields func.argtypes = POINTER(BITS), c_char @@ -38,23 +44,12 @@ class BITS(Structure): class BITS_msvc(Structure): _layout_ = "ms" - _fields_ = [("A", c_int, 1), - ("B", c_int, 2), - ("C", c_int, 3), - ("D", c_int, 4), - ("E", c_int, 5), - ("F", c_int, 6), - ("G", c_int, 7), - ("H", c_int, 8), - ("I", c_int, 9), - - ("M", c_short, 1), - ("N", c_short, 2), - ("O", c_short, 3), - ("P", c_short, 4), - ("Q", c_short, 5), - ("R", c_short, 6), - ("S", c_short, 7)] + _fields_ = TEST_FIELDS + + +class BITS_gcc(Structure): + _layout_ = "gcc-sysv" + _fields_ = TEST_FIELDS try: @@ -124,13 +119,19 @@ def test_shorts_msvc_mode(self): unsigned_int_types = (c_ubyte, c_ushort, c_uint, c_ulong, c_ulonglong) int_types = unsigned_int_types + signed_int_types -class BitFieldTest(unittest.TestCase): +class BitFieldTest(unittest.TestCase, StructCheckMixin): + + def test_generic_checks(self): + self.check_struct(BITS) + self.check_struct(BITS_msvc) + self.check_struct(BITS_gcc) def test_longlong(self): class X(Structure): _fields_ = [("a", c_longlong, 1), ("b", c_longlong, 62), ("c", c_longlong, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_longlong)) x = X() @@ -142,6 +143,7 @@ class X(Structure): _fields_ = [("a", c_ulonglong, 1), ("b", c_ulonglong, 62), ("c", c_ulonglong, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_longlong)) x = X() @@ -159,6 +161,7 @@ class X(Structure): ("a", c_typ, 3), ("b", c_typ, 3), ("c", c_typ, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_typ)*2) x = X() @@ -178,6 +181,7 @@ class X(Structure): _fields_ = [("a", c_typ, 3), ("b", c_typ, 3), ("c", c_typ, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_typ)) x = X() @@ -210,12 +214,14 @@ def test_nonint_types(self): class Empty(Structure): _fields_ = [] + self.check_struct(Empty) result = self.fail_fields(("a", Empty, 1)) self.assertEqual(result, (ValueError, "number of bits invalid for bit field 'a'")) class Dummy(Structure): _fields_ = [("x", c_int)] + self.check_struct(Dummy) result = self.fail_fields(("a", Dummy, 1)) self.assertEqual(result, (TypeError, 'bit fields not allowed for type Dummy')) @@ -240,10 +246,12 @@ def test_single_bitfield_size(self): class X(Structure): _fields_ = [("a", c_typ, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_typ)) class X(Structure): _fields_ = [("a", c_typ, sizeof(c_typ)*8)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_typ)) result = self.fail_fields(("a", c_typ, sizeof(c_typ)*8 + 1)) @@ -255,6 +263,7 @@ class X(Structure): _fields_ = [("a", c_short, 1), ("b", c_short, 14), ("c", c_short, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_short)) class X(Structure): @@ -262,6 +271,7 @@ class X(Structure): ("a1", c_short), ("b", c_short, 14), ("c", c_short, 1)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_short)*3) self.assertEqual(X.a.offset, 0) self.assertEqual(X.a1.offset, sizeof(c_short)) @@ -272,6 +282,7 @@ class X(Structure): _fields_ = [("a", c_short, 3), ("b", c_short, 14), ("c", c_short, 14)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_short)*3) self.assertEqual(X.a.offset, sizeof(c_short)*0) self.assertEqual(X.b.offset, sizeof(c_short)*1) @@ -287,6 +298,7 @@ def test_mixed_1(self): class X(Structure): _fields_ = [("a", c_byte, 4), ("b", c_int, 4)] + self.check_struct(X) if os.name == "nt": self.assertEqual(sizeof(X), sizeof(c_int)*2) else: @@ -296,12 +308,14 @@ def test_mixed_2(self): class X(Structure): _fields_ = [("a", c_byte, 4), ("b", c_int, 32)] + self.check_struct(X) self.assertEqual(sizeof(X), alignment(c_int)+sizeof(c_int)) def test_mixed_3(self): class X(Structure): _fields_ = [("a", c_byte, 4), ("b", c_ubyte, 4)] + self.check_struct(X) self.assertEqual(sizeof(X), sizeof(c_byte)) def test_mixed_4(self): @@ -312,6 +326,7 @@ class X(Structure): ("d", c_short, 4), ("e", c_short, 4), ("f", c_int, 24)] + self.check_struct(X) # MSVC does NOT combine c_short and c_int into one field, GCC # does (unless GCC is run with '-mms-bitfields' which # produces code compatible with MSVC). @@ -325,6 +340,7 @@ class X(Structure): _fields_ = [ ('A', c_uint, 1), ('B', c_ushort, 16)] + self.check_struct(X) a = X() a.A = 0 a.B = 1 @@ -335,6 +351,7 @@ class X(Structure): _fields_ = [ ('A', c_ulonglong, 1), ('B', c_uint, 32)] + self.check_struct(X) a = X() a.A = 0 a.B = 1 @@ -348,6 +365,7 @@ class X(Structure): ("A", c_uint32), ('B', c_uint32, 20), ('C', c_uint64, 24)] + self.check_struct(X) self.assertEqual(16, sizeof(X)) def test_mixed_8(self): @@ -357,6 +375,7 @@ class Foo(Structure): ("B", c_uint32, 32), ("C", c_ulonglong, 1), ] + self.check_struct(Foo) class Bar(Structure): _fields_ = [ @@ -364,6 +383,7 @@ class Bar(Structure): ("B", c_uint32), ("C", c_ulonglong, 1), ] + self.check_struct(Bar) self.assertEqual(sizeof(Foo), sizeof(Bar)) def test_mixed_9(self): @@ -372,6 +392,7 @@ class X(Structure): ("A", c_uint8), ("B", c_uint32, 1), ] + self.check_struct(X) if sys.platform == 'win32': self.assertEqual(8, sizeof(X)) else: @@ -385,6 +406,7 @@ class X(Structure): ("A", c_uint32, 1), ("B", c_uint64, 1), ] + self.check_struct(X) if sys.platform == 'win32': self.assertEqual(8, alignment(X)) self.assertEqual(16, sizeof(X)) @@ -399,6 +421,7 @@ class TestStruct(Structure): ("Field1", c_uint32, field_width), ("Field2", c_uint8, 8) ] + self.check_struct(TestStruct) cmd = TestStruct() cmd.Field2 = 1 @@ -442,6 +465,9 @@ class Good(Structure): ("b0", c_uint16, 4), ("b1", c_uint16, 12), ] + self.check_struct(Bad) + self.check_struct(GoodA) + self.check_struct(Good) self.assertEqual(3, sizeof(Bad)) self.assertEqual(3, sizeof(Good)) @@ -461,6 +487,7 @@ class MyStructure(Structure): ("C", c_uint32, 20), ("R2", c_uint32, 2) ] + self.check_struct(MyStructure) self.assertEqual(8, sizeof(MyStructure)) def test_gh_86098(self): @@ -470,6 +497,7 @@ class X(Structure): ("b", c_uint8, 8), ("c", c_uint32, 16) ] + self.check_struct(X) if sys.platform == 'win32': self.assertEqual(8, sizeof(X)) else: @@ -484,9 +512,13 @@ class Y(Structure): _anonymous_ = ["_"] _fields_ = [("_", X)] + self.check_struct(X) + self.check_struct(Y) + def test_uint32(self): class X(Structure): _fields_ = [("a", c_uint32, 32)] + self.check_struct(X) x = X() x.a = 10 self.assertEqual(x.a, 10) @@ -496,6 +528,7 @@ class X(Structure): def test_uint64(self): class X(Structure): _fields_ = [("a", c_uint64, 64)] + self.check_struct(X) x = X() x.a = 10 self.assertEqual(x.a, 10) @@ -508,6 +541,7 @@ class Little(LittleEndianStructure): _fields_ = [("a", c_uint32, 24), ("b", c_uint32, 4), ("c", c_uint32, 4)] + self.check_struct(Little) b = bytearray(4) x = Little.from_buffer(b) x.a = 0xabcdef @@ -521,6 +555,7 @@ class Big(BigEndianStructure): _fields_ = [("a", c_uint32, 24), ("b", c_uint32, 4), ("c", c_uint32, 4)] + self.check_struct(Big) b = bytearray(4) x = Big.from_buffer(b) x.a = 0xabcdef @@ -533,6 +568,7 @@ class BitfieldUnion(Union): _fields_ = [("a", c_uint32, 1), ("b", c_uint32, 2), ("c", c_uint32, 3)] + self.check_union(BitfieldUnion) self.assertEqual(sizeof(BitfieldUnion), 4) b = bytearray(4) x = BitfieldUnion.from_buffer(b) diff --git a/Lib/test/test_ctypes/test_bytes.py b/Lib/test/test_ctypes/test_bytes.py index fa11e1bbd49faf..0e7f81b9482e06 100644 --- a/Lib/test/test_ctypes/test_bytes.py +++ b/Lib/test/test_ctypes/test_bytes.py @@ -3,9 +3,10 @@ import unittest from _ctypes import _SimpleCData from ctypes import Structure, c_char, c_char_p, c_wchar, c_wchar_p +from ._support import StructCheckMixin -class BytesTest(unittest.TestCase): +class BytesTest(unittest.TestCase, StructCheckMixin): def test_c_char(self): x = c_char(b"x") self.assertRaises(TypeError, c_char, "x") @@ -40,6 +41,7 @@ def test_c_wchar_p(self): def test_struct(self): class X(Structure): _fields_ = [("a", c_char * 3)] + self.check_struct(X) x = X(b"abc") self.assertRaises(TypeError, X, "abc") @@ -49,6 +51,7 @@ class X(Structure): def test_struct_W(self): class X(Structure): _fields_ = [("a", c_wchar * 3)] + self.check_struct(X) x = X("abc") self.assertRaises(TypeError, X, b"abc") diff --git a/Lib/test/test_ctypes/test_byteswap.py b/Lib/test/test_ctypes/test_byteswap.py index 78eff0392c4548..072c60d53dd8cb 100644 --- a/Lib/test/test_ctypes/test_byteswap.py +++ b/Lib/test/test_ctypes/test_byteswap.py @@ -11,6 +11,7 @@ c_short, c_ushort, c_int, c_uint, c_long, c_ulong, c_longlong, c_ulonglong, c_uint32, c_float, c_double) +from ._support import StructCheckMixin def bin(s): @@ -24,15 +25,17 @@ def bin(s): # # For Structures and Unions, these types are created on demand. -class Test(unittest.TestCase): +class Test(unittest.TestCase, StructCheckMixin): def test_slots(self): class BigPoint(BigEndianStructure): __slots__ = () _fields_ = [("x", c_int), ("y", c_int)] + self.check_struct(BigPoint) class LowPoint(LittleEndianStructure): __slots__ = () _fields_ = [("x", c_int), ("y", c_int)] + self.check_struct(LowPoint) big = BigPoint() little = LowPoint() @@ -200,6 +203,7 @@ def test_struct_fields_unsupported_byte_order(self): with self.assertRaises(TypeError): class T(BigEndianStructure if sys.byteorder == "little" else LittleEndianStructure): _fields_ = fields + [("x", typ)] + self.check_struct(T) def test_struct_struct(self): @@ -219,9 +223,11 @@ def test_struct_struct(self): class NestedStructure(nested): _fields_ = [("x", c_uint32), ("y", c_uint32)] + self.check_struct(NestedStructure) class TestStructure(parent): _fields_ = [("point", NestedStructure)] + self.check_struct(TestStructure) self.assertEqual(len(data), sizeof(TestStructure)) ptr = POINTER(TestStructure) @@ -248,6 +254,7 @@ class S(base): ("h", c_short), ("i", c_int), ("d", c_double)] + self.check_struct(S) s1 = S(0x12, 0x1234, 0x12345678, 3.14) s2 = struct.pack(fmt, 0x12, 0x1234, 0x12345678, 3.14) @@ -271,6 +278,7 @@ class S(base): ("_2", c_byte), ("d", c_double)] + self.check_struct(S) s1 = S() s1.b = 0x12 @@ -298,6 +306,7 @@ class S(Structure): ("_2", c_byte), ("d", c_double)] + self.check_struct(S) s1 = S() s1.b = 0x12 @@ -334,6 +343,7 @@ def test_union_fields_unsupported_byte_order(self): with self.assertRaises(TypeError): class T(BigEndianUnion if sys.byteorder == "little" else LittleEndianUnion): _fields_ = fields + [("x", typ)] + self.check_union(T) def test_union_struct(self): # nested structures in unions with different byteorders @@ -352,9 +362,11 @@ def test_union_struct(self): class NestedStructure(nested): _fields_ = [("x", c_uint32), ("y", c_uint32)] + self.check_struct(NestedStructure) class TestUnion(parent): _fields_ = [("point", NestedStructure)] + self.check_union(TestUnion) self.assertEqual(len(data), sizeof(TestUnion)) ptr = POINTER(TestUnion) @@ -374,12 +386,15 @@ def test_build_struct_union_opposite_system_byteorder(self): class S1(_Structure): _fields_ = [("a", c_byte), ("b", c_byte)] + self.check_struct(S1) class U1(_Union): _fields_ = [("s1", S1), ("ab", c_short)] + self.check_union(U1) class S2(_Structure): _fields_ = [("u1", U1), ("c", c_byte)] + self.check_struct(S2) if __name__ == "__main__": diff --git a/Lib/test/test_ctypes/test_funcptr.py b/Lib/test/test_ctypes/test_funcptr.py index 8362fb16d94dcd..be641da30eadae 100644 --- a/Lib/test/test_ctypes/test_funcptr.py +++ b/Lib/test/test_ctypes/test_funcptr.py @@ -5,7 +5,7 @@ from test.support import import_helper _ctypes_test = import_helper.import_module("_ctypes_test") from ._support import (_CData, PyCFuncPtrType, Py_TPFLAGS_DISALLOW_INSTANTIATION, - Py_TPFLAGS_IMMUTABLETYPE) + Py_TPFLAGS_IMMUTABLETYPE, StructCheckMixin) try: @@ -17,7 +17,7 @@ lib = CDLL(_ctypes_test.__file__) -class CFuncPtrTestCase(unittest.TestCase): +class CFuncPtrTestCase(unittest.TestCase, StructCheckMixin): def test_inheritance_hierarchy(self): self.assertEqual(_CFuncPtr.mro(), [_CFuncPtr, _CData, object]) @@ -88,6 +88,7 @@ class WNDCLASS(Structure): ("hCursor", HCURSOR), ("lpszMenuName", LPCTSTR), ("lpszClassName", LPCTSTR)] + self.check_struct(WNDCLASS) wndclass = WNDCLASS() wndclass.lpfnWndProc = WNDPROC(wndproc) diff --git a/Lib/test/test_ctypes/test_generated_structs.py b/Lib/test/test_ctypes/test_generated_structs.py index d61754d6d49e70..1309a8575db1b2 100644 --- a/Lib/test/test_ctypes/test_generated_structs.py +++ b/Lib/test/test_ctypes/test_generated_structs.py @@ -20,6 +20,8 @@ from ctypes import sizeof, alignment, pointer, string_at _ctypes_test = import_helper.import_module("_ctypes_test") +from test.test_ctypes._support import StructCheckMixin + # ctypes erases the difference between `c_int` and e.g.`c_int16`. # To keep it, we'll use custom subclasses with the C name stashed in `_c_name`: @@ -426,7 +428,7 @@ class X(Structure): _fields_ = [("_", X), ('y', c_byte)] -class GeneratedTest(unittest.TestCase): +class GeneratedTest(unittest.TestCase, StructCheckMixin): def test_generated_data(self): """Check that a ctypes struct/union matches its C equivalent. @@ -448,6 +450,7 @@ def test_generated_data(self): """ for name, cls in TESTCASES.items(): with self.subTest(name=name): + self.check_struct_or_union(cls) if _maybe_skip := getattr(cls, '_maybe_skip', None): _maybe_skip() expected = iter(_ctypes_test.get_generated_test_data(name)) diff --git a/Lib/test/test_ctypes/test_structures.py b/Lib/test/test_ctypes/test_structures.py index 0ec238e04b74cd..67616086b1907c 100644 --- a/Lib/test/test_ctypes/test_structures.py +++ b/Lib/test/test_ctypes/test_structures.py @@ -15,15 +15,17 @@ from collections import namedtuple from test import support from test.support import import_helper +from ._support import StructCheckMixin _ctypes_test = import_helper.import_module("_ctypes_test") -class StructureTestCase(unittest.TestCase): +class StructureTestCase(unittest.TestCase, StructCheckMixin): def test_packed(self): class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 1 + self.check_struct(X) self.assertEqual(sizeof(X), 9) self.assertEqual(X.b.offset, 1) @@ -32,6 +34,7 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 2 + self.check_struct(X) self.assertEqual(sizeof(X), 10) self.assertEqual(X.b.offset, 2) @@ -42,6 +45,7 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 4 + self.check_struct(X) self.assertEqual(sizeof(X), min(4, longlong_align) + longlong_size) self.assertEqual(X.b.offset, min(4, longlong_align)) @@ -49,6 +53,7 @@ class X(Structure): _fields_ = [("a", c_byte), ("b", c_longlong)] _pack_ = 8 + self.check_struct(X) self.assertEqual(sizeof(X), min(8, longlong_align) + longlong_size) self.assertEqual(X.b.offset, min(8, longlong_align)) @@ -89,6 +94,7 @@ class Person(Structure): def test_conflicting_initializers(self): class POINT(Structure): _fields_ = [("phi", c_float), ("rho", c_float)] + self.check_struct(POINT) # conflicting positional and keyword args self.assertRaisesRegex(TypeError, "phi", POINT, 2, 3, phi=4) self.assertRaisesRegex(TypeError, "rho", POINT, 2, 3, rho=4) @@ -99,6 +105,7 @@ class POINT(Structure): def test_keyword_initializers(self): class POINT(Structure): _fields_ = [("x", c_int), ("y", c_int)] + self.check_struct(POINT) pt = POINT(1, 2) self.assertEqual((pt.x, pt.y), (1, 2)) @@ -110,11 +117,13 @@ def test_nested_initializers(self): class Phone(Structure): _fields_ = [("areacode", c_char*6), ("number", c_char*12)] + self.check_struct(Phone) class Person(Structure): _fields_ = [("name", c_char * 12), ("phone", Phone), ("age", c_int)] + self.check_struct(Person) p = Person(b"Someone", (b"1234", b"5678"), 5) @@ -127,6 +136,7 @@ def test_structures_with_wchar(self): class PersonW(Structure): _fields_ = [("name", c_wchar * 12), ("age", c_int)] + self.check_struct(PersonW) p = PersonW("Someone \xe9") self.assertEqual(p.name, "Someone \xe9") @@ -142,11 +152,13 @@ def test_init_errors(self): class Phone(Structure): _fields_ = [("areacode", c_char*6), ("number", c_char*12)] + self.check_struct(Phone) class Person(Structure): _fields_ = [("name", c_char * 12), ("phone", Phone), ("age", c_int)] + self.check_struct(Person) cls, msg = self.get_except(Person, b"Someone", (1, 2)) self.assertEqual(cls, RuntimeError) @@ -169,12 +181,19 @@ def test_positional_args(self): # see also http://bugs.python.org/issue5042 class W(Structure): _fields_ = [("a", c_int), ("b", c_int)] + self.check_struct(W) + class X(W): _fields_ = [("c", c_int)] + self.check_struct(X) + class Y(X): pass + self.check_struct(Y) + class Z(Y): _fields_ = [("d", c_int), ("e", c_int), ("f", c_int)] + self.check_struct(Z) z = Z(1, 2, 3, 4, 5, 6) self.assertEqual((z.a, z.b, z.c, z.d, z.e, z.f), @@ -193,6 +212,7 @@ class Test(Structure): ('second', c_ulong), ('third', c_ulong), ] + self.check_struct(Test) s = Test() s.first = 0xdeadbeef @@ -222,6 +242,7 @@ class Test(Structure): ] def __del__(self): finalizer_calls.append("called") + self.check_struct(Test) s = Test(1, 2, 3) # Test the StructUnionType_paramfunc() code path which copies the @@ -251,6 +272,7 @@ class X(Structure): ('first', c_uint), ('second', c_uint) ] + self.check_struct(X) s = X() s.first = 0xdeadbeef @@ -339,36 +361,43 @@ class Test2(Structure): _fields_ = [ ('data', c_ubyte * 16), ] + self.check_struct(Test2) class Test3AParent(Structure): _fields_ = [ ('data', c_float * 2), ] + self.check_struct(Test3AParent) class Test3A(Test3AParent): _fields_ = [ ('more_data', c_float * 2), ] + self.check_struct(Test3A) class Test3B(Structure): _fields_ = [ ('data', c_double * 2), ] + self.check_struct(Test3B) class Test3C(Structure): _fields_ = [ ("data", c_double * 4) ] + self.check_struct(Test3C) class Test3D(Structure): _fields_ = [ ("data", c_double * 8) ] + self.check_struct(Test3D) class Test3E(Structure): _fields_ = [ ("data", c_double * 9) ] + self.check_struct(Test3E) # Tests for struct Test2 @@ -467,6 +496,8 @@ class U(Union): ('f2', c_uint16 * 8), ('f3', c_uint32 * 4), ] + self.check_union(U) + u = U() u.f3[0] = 0x01234567 u.f3[1] = 0x89ABCDEF @@ -493,18 +524,21 @@ class Nested1(Structure): ('an_int', c_int), ('another_int', c_int), ] + self.check_struct(Nested1) class Test4(Union): _fields_ = [ ('a_long', c_long), ('a_struct', Nested1), ] + self.check_struct(Test4) class Nested2(Structure): _fields_ = [ ('an_int', c_int), ('a_union', Test4), ] + self.check_struct(Nested2) class Test5(Structure): _fields_ = [ @@ -512,6 +546,7 @@ class Test5(Structure): ('nested', Nested2), ('another_int', c_int), ] + self.check_struct(Test5) test4 = Test4() dll = CDLL(_ctypes_test.__file__) @@ -576,6 +611,7 @@ class Test6(Structure): ('C', c_int, 3), ('D', c_int, 2), ] + self.check_struct(Test6) test6 = Test6() # As these are signed int fields, all are logically -1 due to sign @@ -611,6 +647,8 @@ class Test7(Structure): ('C', c_uint, 3), ('D', c_uint, 2), ] + self.check_struct(Test7) + test7 = Test7() test7.A = 1 test7.B = 3 @@ -634,6 +672,7 @@ class Test8(Union): ('C', c_int, 3), ('D', c_int, 2), ] + self.check_union(Test8) test8 = Test8() with self.assertRaises(TypeError) as ctx: From 60e7b3291b9c381f39e6c39401217e9c9c6f3f75 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 15:46:33 +0100 Subject: [PATCH 05/29] More testing --- Lib/test/test_ctypes/_support.py | 47 +++++++++++++++------- Lib/test/test_ctypes/test_struct_fields.py | 30 ++++++++++++-- Modules/_ctypes/cfield.c | 3 +- 3 files changed, 61 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index 804b34b9d4b7d5..0d937c1ad0f0c6 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -52,35 +52,47 @@ def _check_struct_or_union(self, cls, is_struct): anon_names = getattr(cls, '_anonymous_', ()) cls_size = ctypes.sizeof(cls) - for name, field_type, *rest in cls._fields_: + for name, requested_type, *rest_of_tuple in cls._fields_: field = getattr(cls, name) try: - is_bitfield = bool(rest) + is_bitfield = len(rest_of_tuple) > 0 # name self.assertEqual(field.name, name) + # type - self.assertEqual(field.type, field_type) - # offset == byte_offset + self.assertEqual(field.type, requested_type) + + # offset === byte_offset self.assertEqual(field.byte_offset, field.offset) if not is_struct: self.assertEqual(field.byte_offset, 0) + # byte_size - self.assertEqual(field.byte_size, ctypes.sizeof(field_type)) + self.assertEqual(field.byte_size, ctypes.sizeof(field.type)) self.assertGreaterEqual(field.byte_size, 0) self.assertLessEqual(field.byte_offset + field.byte_size, cls_size) + # size self.assertGreaterEqual(field.size, 0) if is_bitfield: - if not hasattr(cls, '_swappedbytes_'): - self.assertEqual( - field.size, - (field.bit_size << 16) + field.bit_offset) + # size has backwards-compatible bit-packed info + if hasattr(cls, '_swappedbytes_'): + offset_for_size = (8 * field.byte_size + - field.bit_offset + - field.bit_size) + else: + offset_for_size = field.bit_offset + expected_size = (field.bit_size << 16) + offset_for_size + self.assertEqual(field.size, expected_size) else: + # size == byte_size self.assertEqual(field.size, field.byte_size) - # is_bitfield - self.assertEqual(field.is_bitfield, is_bitfield) + + # is_bitfield (bool) + self.assertIs(field.is_bitfield, is_bitfield) + # bit_offset if is_bitfield: self.assertGreaterEqual(field.bit_offset, 0) @@ -90,16 +102,20 @@ def _check_struct_or_union(self, cls, is_struct): self.assertEqual(field.bit_offset, 0) if not is_struct: self.assertEqual(field.bit_offset, 0) + # bit_size if is_bitfield: self.assertGreaterEqual(field.bit_size, 0) + self.assertLessEqual(field.bit_size, field.byte_size * 8) + [requested_bit_size] = rest_of_tuple + self.assertEqual(field.bit_size, requested_bit_size) else: self.assertEqual(field.bit_size, field.byte_size * 8) - # is_anonymous - self.assertEqual(field.is_anonymous, name in anon_names) + # is_anonymous (bool) + self.assertIs(field.is_anonymous, name in anon_names) - # and for struct, not overlapping earlier members. + # field is not overlapping earlier members in a struct. # (this assumes fields are laid out in order) self.assertGreaterEqual( field.byte_offset * 8 + field.bit_offset, @@ -114,6 +130,7 @@ def _check_struct_or_union(self, cls, is_struct): # union fields may overlap next_bit = 0 except Exception as e: - # Similar to `self.subTest`, but subTest doesn't nest well. + # Mention the failing field in the traceback + # (`self.subTest` would be nicer, but it doesn't nest well) e.add_note(f'while checking field {name!r}: {field}') raise diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index cdcbe372125ba3..b77a451588fb76 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -2,12 +2,12 @@ import sys from ctypes import Structure, Union, sizeof, c_char, c_int from ._support import (CField, Py_TPFLAGS_DISALLOW_INSTANTIATION, - Py_TPFLAGS_IMMUTABLETYPE) + Py_TPFLAGS_IMMUTABLETYPE, StructCheckMixin) NOTHING = object() -class FieldsTestBase: +class FieldsTestBase(StructCheckMixin): # Structure/Union classes must get 'finalized' sooner or # later, when one of these things happen: # @@ -81,14 +81,28 @@ def test_max_field_size_gh126937(self): # (But they most likely can't be instantiated.) # The size must fit in Py_ssize_t. + max_field_size = sys.maxsize + class X(Structure): _fields_ = [('char', c_char),] - max_field_size = sys.maxsize + self.check_struct(X) class Y(Structure): _fields_ = [('largeField', X * max_field_size)] + self.check_struct(Y) + class Z(Structure): _fields_ = [('largeField', c_char * max_field_size)] + self.check_struct(Z) + + # The *bit* size overflows Py_ssize_t. + self.assertEqual(Y.largeField.bit_size, max_field_size * 8) + self.assertEqual(Z.largeField.bit_size, max_field_size * 8) + + self.assertEqual(Y.largeField.byte_size, max_field_size) + self.assertEqual(Z.largeField.byte_size, max_field_size) + self.assertEqual(sizeof(Y), max_field_size) + self.assertEqual(sizeof(Z), max_field_size) with self.assertRaises(OverflowError): class TooBig(Structure): @@ -97,6 +111,16 @@ class TooBig(Structure): class TooBig(Structure): _fields_ = [('largeField', c_char * (max_field_size + 1))] + # Also test around edge case for the bit_size calculation + for size in (max_field_size // 8 - 1, + max_field_size // 8, + max_field_size // 8 + 1): + class S(Structure): + _fields_ = [('largeField', c_char * size),] + self.check_struct(S) + self.assertEqual(S.largeField.bit_size, size * 8) + + # __set__ and __get__ should raise a TypeError in case their self # argument is not a ctype instance. def test___set__(self): diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 0079189ea50921..b1665fecb6b3d1 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -308,7 +308,8 @@ PyCField_get_bit_size(PyObject *self, void *data) if (field->byte_size < PY_SSIZE_T_MAX / 8) { return PyLong_FromSsize_t(field->byte_size * 8); } - // XXX test this! + // If the bit size overflows Py_ssize_t, we don't try fitting it in + // a bigger C type. Use Python ints. PyObject *byte_size_obj = PyLong_FromSsize_t(field->byte_size); if (!byte_size_obj) { return NULL; From 18334d826da1b4af68380f826d964d46bc08a8ad Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 15:57:24 +0100 Subject: [PATCH 06/29] Tests: import CField from ctypes --- Lib/test/test_ctypes/_support.py | 4 ---- Lib/test/test_ctypes/test_struct_fields.py | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index 0d937c1ad0f0c6..ce29b2b33c9a0d 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -7,10 +7,6 @@ _CData = Structure.__base__ assert _CData.__name__ == "_CData" -class _X(Structure): - _fields_ = [("x", ctypes.c_int)] -CField = type(_X.x) - # metaclasses PyCStructType = type(Structure) UnionType = type(Union) diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index b77a451588fb76..6cfdf50f160578 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -1,7 +1,7 @@ import unittest import sys -from ctypes import Structure, Union, sizeof, c_char, c_int -from ._support import (CField, Py_TPFLAGS_DISALLOW_INSTANTIATION, +from ctypes import Structure, Union, sizeof, c_char, c_int, CField +from ._support import (Py_TPFLAGS_DISALLOW_INSTANTIATION, Py_TPFLAGS_IMMUTABLETYPE, StructCheckMixin) From 294b1b82f03c2b0b38f7050ae35af0d58bf34580 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 16:09:06 +0100 Subject: [PATCH 07/29] Clarify bit_offset --- Doc/library/ctypes.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index 0351f8ab88f987..7846558969a6ef 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -2850,7 +2850,10 @@ fields, or any other data types containing pointer type fields. .. attribute:: bit_offset - Additional offset, in bits, of a bitfield. + Additional offset of a bitfield, in bits. + The value is relative to :attr:`byte_offset`. That is, the *total* bit + offset, from the start of the structure, + is ``byte_offset * 8 + bit_offset``. Zero for non-bitfields. From 52114fa5b6577238b7b3c1ef06a6404fb7060b17 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 17:35:32 +0100 Subject: [PATCH 08/29] Add a blurb --- .../Library/2025-01-17-17-35-16.gh-issue-128715.tQjo89.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-01-17-17-35-16.gh-issue-128715.tQjo89.rst diff --git a/Misc/NEWS.d/next/Library/2025-01-17-17-35-16.gh-issue-128715.tQjo89.rst b/Misc/NEWS.d/next/Library/2025-01-17-17-35-16.gh-issue-128715.tQjo89.rst new file mode 100644 index 00000000000000..5ca6250795a44f --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-01-17-17-35-16.gh-issue-128715.tQjo89.rst @@ -0,0 +1,3 @@ +The class of :class:`~ctypes.Structure`/:class:`~ctypes.Union` field +descriptors is now available as :class:`~ctypes.CField`, and has new +attributes to aid debugging and introspection. From 32d41f304f1dadb3bd2161d0090af3cf52799175 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 18:16:08 +0100 Subject: [PATCH 09/29] Regen --- .../pycore_global_objects_fini_generated.h | 5 +++++ Include/internal/pycore_global_strings.h | 5 +++++ .../internal/pycore_runtime_init_generated.h | 5 +++++ .../internal/pycore_unicodeobject_generated.h | 20 +++++++++++++++++++ 4 files changed, 35 insertions(+) diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 90214a314031d1..4bbaefa124d1e0 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -756,6 +756,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_get_sourcefile)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_handle_fromlist)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_initializing)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_internal_use)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_io)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_is_text_encoding)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_isatty_open_only)); @@ -806,6 +807,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(before)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(big)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(binary_form)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(bit_offset)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(bit_size)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(block)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(bound)); @@ -816,6 +818,8 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(buffers)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(bufsize)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(builtins)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(byte_offset)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(byte_size)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(byteorder)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(bytes)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(bytes_per_sep)); @@ -951,6 +955,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(flush)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(fold)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(follow_symlinks)); + _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(for_big_endian)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(format)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(format_spec)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(from_param)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 97a75d0c46c867..240eb3d3bdca58 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -245,6 +245,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(_get_sourcefile) STRUCT_FOR_ID(_handle_fromlist) STRUCT_FOR_ID(_initializing) + STRUCT_FOR_ID(_internal_use) STRUCT_FOR_ID(_io) STRUCT_FOR_ID(_is_text_encoding) STRUCT_FOR_ID(_isatty_open_only) @@ -295,6 +296,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(before) STRUCT_FOR_ID(big) STRUCT_FOR_ID(binary_form) + STRUCT_FOR_ID(bit_offset) STRUCT_FOR_ID(bit_size) STRUCT_FOR_ID(block) STRUCT_FOR_ID(bound) @@ -305,6 +307,8 @@ struct _Py_global_strings { STRUCT_FOR_ID(buffers) STRUCT_FOR_ID(bufsize) STRUCT_FOR_ID(builtins) + STRUCT_FOR_ID(byte_offset) + STRUCT_FOR_ID(byte_size) STRUCT_FOR_ID(byteorder) STRUCT_FOR_ID(bytes) STRUCT_FOR_ID(bytes_per_sep) @@ -440,6 +444,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(flush) STRUCT_FOR_ID(fold) STRUCT_FOR_ID(follow_symlinks) + STRUCT_FOR_ID(for_big_endian) STRUCT_FOR_ID(format) STRUCT_FOR_ID(format_spec) STRUCT_FOR_ID(from_param) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index 4f928cc050bf8e..f2cfda1036ad41 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -754,6 +754,7 @@ extern "C" { INIT_ID(_get_sourcefile), \ INIT_ID(_handle_fromlist), \ INIT_ID(_initializing), \ + INIT_ID(_internal_use), \ INIT_ID(_io), \ INIT_ID(_is_text_encoding), \ INIT_ID(_isatty_open_only), \ @@ -804,6 +805,7 @@ extern "C" { INIT_ID(before), \ INIT_ID(big), \ INIT_ID(binary_form), \ + INIT_ID(bit_offset), \ INIT_ID(bit_size), \ INIT_ID(block), \ INIT_ID(bound), \ @@ -814,6 +816,8 @@ extern "C" { INIT_ID(buffers), \ INIT_ID(bufsize), \ INIT_ID(builtins), \ + INIT_ID(byte_offset), \ + INIT_ID(byte_size), \ INIT_ID(byteorder), \ INIT_ID(bytes), \ INIT_ID(bytes_per_sep), \ @@ -949,6 +953,7 @@ extern "C" { INIT_ID(flush), \ INIT_ID(fold), \ INIT_ID(follow_symlinks), \ + INIT_ID(for_big_endian), \ INIT_ID(format), \ INIT_ID(format_spec), \ INIT_ID(from_param), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index 5b78d038fc1192..fb9cc67da574bb 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -776,6 +776,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(_internal_use); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(_io); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); @@ -976,6 +980,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(bit_offset); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(bit_size); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); @@ -1016,6 +1024,14 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(byte_offset); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(byte_size); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(byteorder); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); @@ -1556,6 +1572,10 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); + string = &_Py_ID(for_big_endian); + _PyUnicode_InternStatic(interp, &string); + assert(_PyUnicode_CheckConsistency(string, 1)); + assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(format); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); From 9b0e7ebd90b32b888081defd0f06ad7db4d1f611 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 19:20:27 +0100 Subject: [PATCH 10/29] include in the common header --- Modules/_ctypes/callbacks.c | 2 -- Modules/_ctypes/callproc.c | 2 -- Modules/_ctypes/cfield.c | 1 - Modules/_ctypes/ctypes.h | 2 ++ 4 files changed, 2 insertions(+), 5 deletions(-) diff --git a/Modules/_ctypes/callbacks.c b/Modules/_ctypes/callbacks.c index 89c0749a093765..21b0e0e9efdb32 100644 --- a/Modules/_ctypes/callbacks.c +++ b/Modules/_ctypes/callbacks.c @@ -11,8 +11,6 @@ #include "pycore_call.h" // _PyObject_CallNoArgs() #include "pycore_runtime.h" // _Py_ID() -#include - #ifdef MS_WIN32 # include #endif diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 92eedff5ec94f1..6a6753fc994f42 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -66,8 +66,6 @@ module _ctypes #include "Python.h" -#include - #ifdef MS_WIN32 #include #include diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index b1665fecb6b3d1..814767fb7bb07c 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -10,7 +10,6 @@ #include "pycore_bitutils.h" // _Py_bswap32() #include "pycore_call.h" // _PyObject_CallNoArgs() -#include // bool #include #include "ctypes.h" diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 2387f87366c387..ce915403eebbdb 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -2,6 +2,8 @@ # include #endif +#include + #include "pycore_moduleobject.h" // _PyModule_GetState() #include "pycore_typeobject.h" // _PyType_GetModuleState() From d7bd835cdc1706039b286d5afdc006c983547c19 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 19:22:31 +0100 Subject: [PATCH 11/29] Explicit casts --- Modules/_ctypes/cfield.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 814767fb7bb07c..0e353d75dc3a1a 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -189,8 +189,8 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, self->proto = Py_NewRef(proto); self->byte_size = byte_size; self->byte_offset = byte_offset; - self->bitfield_size = bitfield_size; - self->bit_offset = bit_offset; + self->bitfield_size = (uint8_t)bitfield_size; + self->bit_offset = (uint8_t)bit_offset; self->_for_big_endian = for_big_endian; self->index = index; From 06458f6baed982a37e5ec0c38b7a97761743cad5 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 17 Jan 2025 19:27:17 +0100 Subject: [PATCH 12/29] Remove problematic assert --- Modules/_ctypes/cfield.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 0e353d75dc3a1a..5672de6bc7c60e 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -239,7 +239,6 @@ static inline Py_ssize_t _pack_legacy_size(CFieldObject *field) { if (field->bitfield_size) { - assert(field->bit_offset < (1 << 16)); Py_ssize_t bit_offset = field->bit_offset; if (field->_for_big_endian) { bit_offset = 8 * field->byte_size - bit_offset - field->bitfield_size; From 91365b06319bcd2d8bd6275c3e77d7653760a3fe Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 24 Jan 2025 15:35:27 +0100 Subject: [PATCH 13/29] Add a test for the new info, and fix 'name' for nested anonymous structs --- Lib/test/test_ctypes/test_structunion.py | 98 +++++++++++++++++++++++- Modules/_ctypes/stgdict.c | 1 + 2 files changed, 98 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_structunion.py b/Lib/test/test_ctypes/test_structunion.py index 973ac3b2f1919d..ac2cbe8364b542 100644 --- a/Lib/test/test_ctypes/test_structunion.py +++ b/Lib/test/test_ctypes/test_structunion.py @@ -4,7 +4,8 @@ from ctypes import (Structure, Union, POINTER, sizeof, alignment, c_char, c_byte, c_ubyte, c_short, c_ushort, c_int, c_uint, - c_long, c_ulong, c_longlong, c_ulonglong, c_float, c_double) + c_long, c_ulong, c_longlong, c_ulonglong, c_float, c_double, + c_int8, c_int16, c_int32) from ._support import (_CData, PyCStructType, UnionType, Py_TPFLAGS_DISALLOW_INSTANTIATION, Py_TPFLAGS_IMMUTABLETYPE) @@ -175,6 +176,101 @@ class X(self.cls): # XXX Should we check nested data types also? # offset is always relative to the class... + def test_field_descriptor_attributes(self): + """Test information provided by the descriptors""" + class Inner(Structure): + _fields_ = [ + ("a", c_int16), + ("b", c_int8, 1), + ("c", c_int8, 2), + ] + class X(self.cls): + _fields_ = [ + ("x", c_int32), + ("y", c_int16, 1), + ("_", Inner), + ] + _anonymous_ = ["_"] + + field_names = "xy_abc" + + # name + + for name in field_names: + with self.subTest(name=name): + self.assertEqual(getattr(X, name).name, name) + + # type + + expected_types = dict( + x=c_int32, + y=c_int16, + _=Inner, + a=c_int16, + b=c_int8, + c=c_int8, + ) + assert set(expected_types) == set(field_names) + for name, tp in expected_types.items(): + with self.subTest(name=name): + self.assertEqual(getattr(X, name).type, tp) + self.assertEqual(getattr(X, name).byte_size, sizeof(tp)) + + # offset, byte_offset + + expected_offsets = dict( + x=(0, 0), + y=(0, 4), + _=(0, 6), + a=(0, 6), + b=(2, 8), + c=(2, 8), + ) + assert set(expected_offsets) == set(field_names) + for name, (union_offset, struct_offset) in expected_offsets.items(): + with self.subTest(name=name): + self.assertEqual(getattr(X, name).offset, + getattr(X, name).byte_offset) + if self.cls == Structure: + self.assertEqual(getattr(X, name).offset, struct_offset) + else: + self.assertEqual(getattr(X, name).offset, union_offset) + + # is_bitfield, bit_size, bit_offset + # size + + expected_bitfield_info = dict( + # (bit_size, bit_offset) + b=(1, 0), + c=(2, 1), + y=(1, 0), + ) + for name in field_names: + with self.subTest(name=name): + if info := expected_bitfield_info.get(name): + self.assertEqual(getattr(X, name).is_bitfield, True) + expected_bit_size, expected_bit_offset = info + self.assertEqual(getattr(X, name).bit_size, + expected_bit_size) + self.assertEqual(getattr(X, name).bit_offset, + expected_bit_offset) + self.assertEqual(getattr(X, name).size, + (expected_bit_size << 16) + | expected_bit_offset) + else: + self.assertEqual(getattr(X, name).is_bitfield, False) + type_size = sizeof(expected_types[name]) + self.assertEqual(getattr(X, name).bit_size, type_size * 8) + self.assertEqual(getattr(X, name).bit_offset, 0) + self.assertEqual(getattr(X, name).size, type_size) + + # is_anonymous + + for name in field_names: + with self.subTest(name=name): + self.assertEqual(getattr(X, name).is_anonymous, (name == '_')) + + def test_invalid_field_types(self): class POINT(self.cls): pass diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index 667aa70677eb15..980a7f65a7edec 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -143,6 +143,7 @@ MakeFields(PyObject *type, CFieldObject *descr, new_descr->proto = Py_XNewRef(fdescr->proto); new_descr->getfunc = fdescr->getfunc; new_descr->setfunc = fdescr->setfunc; + new_descr->name = Py_NewRef(fdescr->name); Py_DECREF(fdescr); From b6d0510f6982a8a8dea20d7f77a13bb28d562706 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 24 Jan 2025 18:30:57 +0100 Subject: [PATCH 14/29] Use subTest --- Lib/test/test_ctypes/_support.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index ce29b2b33c9a0d..ca1a5120341db5 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -50,7 +50,7 @@ def _check_struct_or_union(self, cls, is_struct): cls_size = ctypes.sizeof(cls) for name, requested_type, *rest_of_tuple in cls._fields_: field = getattr(cls, name) - try: + with self.subTest(field=field): is_bitfield = len(rest_of_tuple) > 0 # name @@ -125,8 +125,3 @@ def _check_struct_or_union(self, cls, is_struct): if not is_struct: # union fields may overlap next_bit = 0 - except Exception as e: - # Mention the failing field in the traceback - # (`self.subTest` would be nicer, but it doesn't nest well) - e.add_note(f'while checking field {name!r}: {field}') - raise From 6e279e2fbf9b0ab426d6f6ca72d799f4590760db Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 27 Jan 2025 13:23:49 +0100 Subject: [PATCH 15/29] Use PyUnicode_FromObject to get an exact PyUnicode --- Lib/ctypes/_layout.py | 2 +- Lib/test/test_ctypes/test_structunion.py | 18 +++++++++++++----- Modules/_ctypes/cfield.c | 11 ++++------- 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Lib/ctypes/_layout.py b/Lib/ctypes/_layout.py index 25af2ff4e0af0d..83598331856676 100644 --- a/Lib/ctypes/_layout.py +++ b/Lib/ctypes/_layout.py @@ -290,7 +290,7 @@ def get_layout(cls, input_fields, is_struct, base): # a bytes name would be rejected later, but we check early # to avoid a BytesWarning with `python -bb` raise TypeError( - "field {name!r}: name must be a string, not bytes") + f"field {name!r}: name must be a string, not bytes") format_spec_parts.append(f"{fieldfmt}:{name}:") result_fields.append(CField( diff --git a/Lib/test/test_ctypes/test_structunion.py b/Lib/test/test_ctypes/test_structunion.py index ac2cbe8364b542..af332537d7dfc6 100644 --- a/Lib/test/test_ctypes/test_structunion.py +++ b/Lib/test/test_ctypes/test_structunion.py @@ -278,11 +278,19 @@ class POINT(self.cls): def test_invalid_name(self): # field name must be string - def declare_with_name(name): - class S(self.cls): - _fields_ = [(name, c_int)] - - self.assertRaises(TypeError, declare_with_name, b"x") + for name in b"x", 3, None: + with self.subTest(name=name): + with self.assertRaises(TypeError): + class S(self.cls): + _fields_ = [(name, c_int)] + + def test_str_name(self): + class WeirdString(str): + def __str__(self): + return "unwanted value" + class S(self.cls): + _fields_ = [(WeirdString("f"), c_int)] + self.assertEqual(S.f.name, "f") def test_intarray_fields(self): class SomeInts(self.cls): diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index b67acd406f73af..d31db5424e1755 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -177,14 +177,11 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, if (!self) { return NULL; } - if (PyUnicode_CheckExact(name)) { - self->name = Py_NewRef(name); - } else { - self->name = PyObject_Str(name); - if (!self->name) { - goto error; - } + self->name = PyUnicode_FromObject(name); + if (!self->name) { + goto error; } + assert (PyUnicode_CheckExact(self->name)); self->proto = Py_NewRef(proto); self->byte_size = byte_size; From 176de87ae95fe20cca3a13dfaebdd51334bce170 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 31 Jan 2025 16:04:20 +0100 Subject: [PATCH 16/29] Normalize exception message --- Modules/_ctypes/cfield.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index d31db5424e1755..758da82b4d4607 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -80,7 +80,7 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, if (!_internal_use) { // Do not instantiate outside ctypes, yet. // The constructor is internal API and may change without warning. - PyErr_Format(PyExc_TypeError, "Cannot create %T object.", type); + PyErr_Format(PyExc_TypeError, "cannot create %T object", type); goto error; } if (byte_size < 0) { From 085720eacab7a46f73daf54b08f6f493c4560101 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 31 Jan 2025 16:08:23 +0100 Subject: [PATCH 17/29] Fix refcounting --- Modules/_ctypes/cfield.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 758da82b4d4607..c97d4c2aa38edf 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -305,18 +305,26 @@ PyCField_get_bit_size(PyObject *self, void *data) if (field->byte_size < PY_SSIZE_T_MAX / 8) { return PyLong_FromSsize_t(field->byte_size * 8); } + // If the bit size overflows Py_ssize_t, we don't try fitting it in // a bigger C type. Use Python ints. - PyObject *byte_size_obj = PyLong_FromSsize_t(field->byte_size); + PyObject *byte_size_obj = NULL; + PyObject *eight = NULL; + PyObject *result = NULL; + + byte_size_obj = PyLong_FromSsize_t(field->byte_size); if (!byte_size_obj) { - return NULL; + goto finally; } - PyObject *eight = PyLong_FromLong(8); + eight = PyLong_FromLong(8); if (!eight) { - return NULL; + goto finally; } - return PyNumber_Multiply(byte_size_obj, eight); - + result = PyNumber_Multiply(byte_size_obj, eight); +finally: + Py_XDECREF(byte_size_obj); + Py_XDECREF(eight); + return result; } static PyObject * From 05c9591d613c6fc302ccd82a71dedd603deda57b Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 31 Jan 2025 16:10:26 +0100 Subject: [PATCH 18/29] Add pretty spaces --- Modules/_ctypes/cfield.c | 53 ++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 26 deletions(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index c97d4c2aa38edf..28a5eda32f0843 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -355,37 +355,38 @@ static PyGetSetDef PyCField_getset[] = { static PyMemberDef PyCField_members[] = { { "name", - .type=Py_T_OBJECT_EX, - .offset=offsetof(CFieldObject, name), - .flags=Py_READONLY, - .doc=PyDoc_STR("name of this field") }, + .type = Py_T_OBJECT_EX, + .offset = offsetof(CFieldObject, name), + .flags = Py_READONLY, + .doc = PyDoc_STR("name of this field") }, { "type", - .type=Py_T_OBJECT_EX, - .offset=offsetof(CFieldObject, proto), - .flags=Py_READONLY, - .doc=PyDoc_STR("type of this field") }, + .type = Py_T_OBJECT_EX, + .offset = offsetof(CFieldObject, proto), + .flags = Py_READONLY, + .doc = PyDoc_STR("type of this field") }, { "offset", - .type=Py_T_PYSSIZET, - .offset=offsetof(CFieldObject, byte_offset), - .flags=Py_READONLY, - .doc=PyDoc_STR("offset in bytes of this field (same as byte_offset)") }, + .type = Py_T_PYSSIZET, + .offset = offsetof(CFieldObject, byte_offset), + .flags = Py_READONLY, + .doc = PyDoc_STR( + "offset in bytes of this field (same as byte_offset)") }, { "byte_offset", - .type=Py_T_PYSSIZET, - .offset=offsetof(CFieldObject, byte_offset), - .flags=Py_READONLY, - .doc=PyDoc_STR("offset in bytes of this field. " - "For bitfields: excludes bit_offset.") }, + .type = Py_T_PYSSIZET, + .offset = offsetof(CFieldObject, byte_offset), + .flags = Py_READONLY, + .doc = PyDoc_STR("offset in bytes of this field. " + "For bitfields: excludes bit_offset.") }, { "byte_size", - .type=Py_T_PYSSIZET, - .offset=offsetof(CFieldObject, byte_size), - .flags=Py_READONLY, - .doc=PyDoc_STR("size of this field in bytes") }, + .type = Py_T_PYSSIZET, + .offset = offsetof(CFieldObject, byte_size), + .flags = Py_READONLY, + .doc = PyDoc_STR("size of this field in bytes") }, { "bit_offset", - .type=Py_T_UBYTE, - .offset=offsetof(CFieldObject, bit_offset), - .flags=Py_READONLY, - .doc=PyDoc_STR("additional offset in bits (relative to byte_offset); " - "zero for non-bitfields") }, + .type = Py_T_UBYTE, + .offset = offsetof(CFieldObject, bit_offset), + .flags = Py_READONLY, + .doc = PyDoc_STR("additional offset in bits (relative to byte_offset);" + " zero for non-bitfields") }, { NULL }, }; From 4e95755f2c8c3b9b271d90ba43fc19532084d827 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 31 Jan 2025 17:43:16 +0100 Subject: [PATCH 19/29] Fix bit-packed size test for big-endian machines --- Lib/test/test_ctypes/_support.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index ca1a5120341db5..02427473c956ac 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -2,6 +2,7 @@ import ctypes from _ctypes import Structure, Union, _Pointer, Array, _SimpleCData, CFuncPtr +import sys _CData = Structure.__base__ @@ -74,7 +75,11 @@ def _check_struct_or_union(self, cls, is_struct): self.assertGreaterEqual(field.size, 0) if is_bitfield: # size has backwards-compatible bit-packed info - if hasattr(cls, '_swappedbytes_'): + is_big_endian = ( + hasattr(cls, '_swappedbytes_') + ^ (sys.byteorder == 'big') + ) + if is_big_endian: offset_for_size = (8 * field.byte_size - field.bit_offset - field.bit_size) From 9cde20e5aa0f2949d9f6c8ed4e2a1ab90878aa96 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 31 Jan 2025 17:53:35 +0100 Subject: [PATCH 20/29] Remove `size` from _layout.py --- Lib/ctypes/_layout.py | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/Lib/ctypes/_layout.py b/Lib/ctypes/_layout.py index 83598331856676..0045d719081ee8 100644 --- a/Lib/ctypes/_layout.py +++ b/Lib/ctypes/_layout.py @@ -21,29 +21,6 @@ def round_up(n, multiple): assert multiple > 0 return ((n + multiple - 1) // multiple) * multiple -def LOW_BIT(offset): - return offset & 0xFFFF - -def NUM_BITS(bitsize): - return bitsize >> 16 - -def BUILD_SIZE(bitsize, offset): - assert 0 <= offset, offset - assert offset <= 0xFFFF, offset - # We don't support zero length bitfields. - # And GET_BITFIELD uses NUM_BITS(size) == 0, - # to figure out whether we are handling a bitfield. - assert bitsize > 0, bitsize - result = (bitsize << 16) + offset - assert bitsize == NUM_BITS(result), (bitsize, result) - assert offset == LOW_BIT(result), (offset, result) - return result - -def build_size(bit_size, bit_offset, big_endian, type_size): - if big_endian: - return BUILD_SIZE(bit_size, 8 * type_size - bit_offset - bit_size) - return BUILD_SIZE(bit_size, bit_offset) - _INT_MAX = (1 << (ctypes.sizeof(ctypes.c_int) * 8) - 1) - 1 @@ -217,12 +194,9 @@ def get_layout(cls, input_fields, is_struct, base): if is_bitfield: effective_bit_offset = next_bit_offset - 8 * offset bit_offset = effective_bit_offset - size = build_size(bit_size, effective_bit_offset, - big_endian, type_size) assert effective_bit_offset <= type_bit_size else: assert offset == next_bit_offset / 8 - size = type_size next_bit_offset += bit_size struct_size = round_up(next_bit_offset, 8) // 8 @@ -257,19 +231,12 @@ def get_layout(cls, input_fields, is_struct, base): if is_bitfield: assert 0 <= (last_field_bit_size + next_bit_offset) bit_offset = last_field_bit_size + next_bit_offset - size = build_size(bit_size, - bit_offset, - big_endian, type_size) - else: - size = type_size if type_bit_size: assert (last_field_bit_size + next_bit_offset) < type_bit_size next_bit_offset += bit_size struct_size = next_byte_offset - assert (not is_bitfield) or (LOW_BIT(size) <= size * 8) - # Add the format spec parts if is_struct: padding = offset - last_size From 34865e8efc953b59fd3e7650bd893829246d7acc Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 7 Feb 2025 15:23:54 +0100 Subject: [PATCH 21/29] Fix bit_offset for big-endian structs (where bitfields are laid out from the big end) --- Doc/library/ctypes.rst | 26 ++++---- .../pycore_global_objects_fini_generated.h | 1 - Include/internal/pycore_global_strings.h | 1 - .../internal/pycore_runtime_init_generated.h | 1 - .../internal/pycore_unicodeobject_generated.h | 4 -- Lib/ctypes/_layout.py | 11 ++-- Lib/test/test_ctypes/_support.py | 60 ++++++++++--------- .../test_ctypes/test_generated_structs.py | 41 +++++++++---- Lib/test/test_ctypes/test_structunion.py | 8 ++- Modules/_ctypes/_ctypes_test_generated.c.h | 11 ++-- Modules/_ctypes/cfield.c | 9 +-- Modules/_ctypes/clinic/cfield.c.h | 31 ++++------ Modules/_ctypes/ctypes.h | 1 - Modules/_ctypes/stgdict.c | 1 + 14 files changed, 109 insertions(+), 97 deletions(-) diff --git a/Doc/library/ctypes.rst b/Doc/library/ctypes.rst index c54f68740d606a..499b35c8485b5e 100644 --- a/Doc/library/ctypes.rst +++ b/Doc/library/ctypes.rst @@ -2826,14 +2826,15 @@ fields, or any other data types containing pointer type fields. Offset of the field, in bytes. - For bitfields, this excludes any :attr:`~CField.bit_offset`. + For bitfields, this is the offset of the underlying byte-aligned + *storage unit*; see :attr:`~CField.bit_offset`. .. attribute:: byte_size Size of the field, in bytes. - For bitfields, this is the size of the underlying type, which may be - much larger than the field itself. + For bitfields, this is the size of the underlying *storage unit*. + Typically, it has the same size as the bitfield's type. .. attribute:: size @@ -2849,19 +2850,18 @@ fields, or any other data types containing pointer type fields. True if this is a bitfield. .. attribute:: bit_offset + bit_size - Additional offset of a bitfield, in bits. - The value is relative to :attr:`byte_offset`. That is, the *total* bit - offset, from the start of the structure, - is ``byte_offset * 8 + bit_offset``. + The location of a bitfield within its *storage unit*, that is, within + :attr:`~CField.byte_size` bytes of memory starting at + :attr:`~CField.byte_offset`. - Zero for non-bitfields. + To get the field's value, read the storage unit as an integer, + :ref:`shift left ` by :attr:`!bit_offset` and + take the :attr:`!bit_size` least significant bits. - .. attribute:: bit_size - - Size of the field, in bits. - - For non-bitfields, this is equal to ``byte_size * 8``. + For non-bitfields, :attr:`!bit_offset` is zero + and :attr:`!bit_size` is equal to ``byte_size * 8``. .. attribute:: is_anonymous diff --git a/Include/internal/pycore_global_objects_fini_generated.h b/Include/internal/pycore_global_objects_fini_generated.h index 4bbaefa124d1e0..0612184e10c70c 100644 --- a/Include/internal/pycore_global_objects_fini_generated.h +++ b/Include/internal/pycore_global_objects_fini_generated.h @@ -955,7 +955,6 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) { _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(flush)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(fold)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(follow_symlinks)); - _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(for_big_endian)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(format)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(format_spec)); _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(from_param)); diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 240eb3d3bdca58..22ffe80d9a5b11 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -444,7 +444,6 @@ struct _Py_global_strings { STRUCT_FOR_ID(flush) STRUCT_FOR_ID(fold) STRUCT_FOR_ID(follow_symlinks) - STRUCT_FOR_ID(for_big_endian) STRUCT_FOR_ID(format) STRUCT_FOR_ID(format_spec) STRUCT_FOR_ID(from_param) diff --git a/Include/internal/pycore_runtime_init_generated.h b/Include/internal/pycore_runtime_init_generated.h index f2cfda1036ad41..4a7111a01bf00c 100644 --- a/Include/internal/pycore_runtime_init_generated.h +++ b/Include/internal/pycore_runtime_init_generated.h @@ -953,7 +953,6 @@ extern "C" { INIT_ID(flush), \ INIT_ID(fold), \ INIT_ID(follow_symlinks), \ - INIT_ID(for_big_endian), \ INIT_ID(format), \ INIT_ID(format_spec), \ INIT_ID(from_param), \ diff --git a/Include/internal/pycore_unicodeobject_generated.h b/Include/internal/pycore_unicodeobject_generated.h index fb9cc67da574bb..1ec99a1b5b3a5c 100644 --- a/Include/internal/pycore_unicodeobject_generated.h +++ b/Include/internal/pycore_unicodeobject_generated.h @@ -1572,10 +1572,6 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) { _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); assert(PyUnicode_GET_LENGTH(string) != 1); - string = &_Py_ID(for_big_endian); - _PyUnicode_InternStatic(interp, &string); - assert(_PyUnicode_CheckConsistency(string, 1)); - assert(PyUnicode_GET_LENGTH(string) != 1); string = &_Py_ID(format); _PyUnicode_InternStatic(interp, &string); assert(_PyUnicode_CheckConsistency(string, 1)); diff --git a/Lib/ctypes/_layout.py b/Lib/ctypes/_layout.py index 0045d719081ee8..11a97ddb4c72a6 100644 --- a/Lib/ctypes/_layout.py +++ b/Lib/ctypes/_layout.py @@ -192,9 +192,8 @@ def get_layout(cls, input_fields, is_struct, base): offset = round_down(next_bit_offset, type_bit_align) // 8 if is_bitfield: - effective_bit_offset = next_bit_offset - 8 * offset - bit_offset = effective_bit_offset - assert effective_bit_offset <= type_bit_size + bit_offset = next_bit_offset - 8 * offset + assert bit_offset <= type_bit_size else: assert offset == next_bit_offset / 8 @@ -237,6 +236,11 @@ def get_layout(cls, input_fields, is_struct, base): next_bit_offset += bit_size struct_size = next_byte_offset + if is_bitfield and big_endian: + # On big-endian architectures, bit fields are also laid out + # starting with the big end. + bit_offset = type_bit_size - bit_size - bit_offset + # Add the format spec parts if is_struct: padding = offset - last_size @@ -268,7 +272,6 @@ def get_layout(cls, input_fields, is_struct, base): bit_size=bit_size if is_bitfield else None, bit_offset=bit_offset if is_bitfield else None, index=i, - for_big_endian=big_endian, # Do not use CField outside ctypes, yet. # The constructor is internal API and may change without warning. diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index 02427473c956ac..b72721d9bf6b20 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -3,6 +3,7 @@ import ctypes from _ctypes import Structure, Union, _Pointer, Array, _SimpleCData, CFuncPtr import sys +from test import support _CData = Structure.__base__ @@ -43,9 +44,10 @@ def _check_struct_or_union(self, cls, is_struct): # Check that fields are not overlapping (for structs), # and that their metadata is consistent. - # offset of the last checked bit, from start of struct - # stays 0 for unions - next_bit = 0 + used_bits = 0 + + is_little_endian = ( + hasattr(cls, '_swappedbytes_') ^ (sys.byteorder == 'little')) anon_names = getattr(cls, '_anonymous_', ()) cls_size = ctypes.sizeof(cls) @@ -75,17 +77,7 @@ def _check_struct_or_union(self, cls, is_struct): self.assertGreaterEqual(field.size, 0) if is_bitfield: # size has backwards-compatible bit-packed info - is_big_endian = ( - hasattr(cls, '_swappedbytes_') - ^ (sys.byteorder == 'big') - ) - if is_big_endian: - offset_for_size = (8 * field.byte_size - - field.bit_offset - - field.bit_size) - else: - offset_for_size = field.bit_offset - expected_size = (field.bit_size << 16) + offset_for_size + expected_size = (field.bit_size << 16) + field.bit_offset self.assertEqual(field.size, expected_size) else: # size == byte_size @@ -102,7 +94,11 @@ def _check_struct_or_union(self, cls, is_struct): else: self.assertEqual(field.bit_offset, 0) if not is_struct: - self.assertEqual(field.bit_offset, 0) + if is_little_endian: + self.assertEqual(field.bit_offset, 0) + else: + self.assertEqual(field.bit_offset, + field.byte_size * 8 - field.bit_size) # bit_size if is_bitfield: @@ -116,17 +112,27 @@ def _check_struct_or_union(self, cls, is_struct): # is_anonymous (bool) self.assertIs(field.is_anonymous, name in anon_names) - # field is not overlapping earlier members in a struct. - # (this assumes fields are laid out in order) - self.assertGreaterEqual( - field.byte_offset * 8 + field.bit_offset, - next_bit) - next_bit = (field.byte_offset * 8 - + field.bit_offset - + field.bit_size) + # In a struct, field should not overlap. + # (Test skipped if the structs is enormous.) + if is_struct and cls_size < 10_000: + # Get a mask indicating where the field is within the struct + if is_little_endian: + tp_shift = field.byte_offset * 8 + else: + tp_shift = (cls_size + - field.byte_offset + - field.byte_size) * 8 + mask = (1 << field.bit_size) - 1 + mask <<= (tp_shift + field.bit_offset) + assert mask.bit_count() == field.bit_size + # Check that these bits aren't shared with previous fields + self.assertEqual(used_bits & mask, 0) + # Mark the bits for future checks + used_bits |= mask + # field is inside cls - self.assertLessEqual(next_bit, cls_size * 8) + bit_end = (field.byte_offset * 8 + + field.bit_offset + + field.bit_size) + self.assertLessEqual(bit_end, cls_size * 8) - if not is_struct: - # union fields may overlap - next_bit = 0 diff --git a/Lib/test/test_ctypes/test_generated_structs.py b/Lib/test/test_ctypes/test_generated_structs.py index 7c1b49e3d80e06..9ec470f0954766 100644 --- a/Lib/test/test_ctypes/test_generated_structs.py +++ b/Lib/test/test_ctypes/test_generated_structs.py @@ -10,10 +10,11 @@ """ import unittest -from test.support import import_helper +from test.support import import_helper, verbose import re from dataclasses import dataclass from functools import cached_property +import sys import ctypes from ctypes import Structure, Union @@ -449,6 +450,8 @@ def test_generated_data(self): Common compilers seem to do so. """ for name, cls in TESTCASES.items(): + is_little_endian = ( + hasattr(cls, '_swappedbytes_') ^ (sys.byteorder == 'little')) with self.subTest(name=name): self.check_struct_or_union(cls) if _maybe_skip := getattr(cls, '_maybe_skip', None): @@ -464,7 +467,7 @@ def test_generated_data(self): obj = cls() ptr = pointer(obj) for field in iterfields(cls): - for value in -1, 1, 0: + for value in -1, 1, 2926941915, 0: with self.subTest(field=field.full_name, value=value): field.set_to(obj, value) py_mem = string_at(ptr, sizeof(obj)) @@ -475,6 +478,17 @@ def test_generated_data(self): m = "\n".join([str(field), 'in:', *lines]) self.assertEqual(py_mem.hex(), c_mem.hex(), m) + descriptor = field.descriptor + field_mem = py_mem[ + field.byte_offset + : field.byte_offset + descriptor.byte_size] + field_int = int.from_bytes(field_mem, sys.byteorder) + mask = (1 << descriptor.bit_size) - 1 + self.assertEqual( + (field_int >> descriptor.bit_offset) & mask, + value & mask) + + # The rest of this file is generating C code from a ctypes type. # This is only meant for (and tested with) the known inputs in this file! @@ -572,6 +586,8 @@ class FieldInfo: bits: int | None # number if this is a bit field parent_type: type parent: 'FieldInfo' #| None + descriptor: object + byte_offset: int @cached_property def attr_path(self): @@ -603,10 +619,6 @@ def root(self): else: return self.parent - @cached_property - def descriptor(self): - return getattr(self.parent_type, self.name) - def __repr__(self): qname = f'{self.root.parent_type.__name__}.{self.full_name}' try: @@ -624,7 +636,11 @@ def iterfields(tp, parent=None): else: for fielddesc in fields: f_name, f_tp, f_bits = unpack_field_desc(*fielddesc) - sub = FieldInfo(f_name, f_tp, f_bits, tp, parent) + descriptor = getattr(tp, f_name) + byte_offset = descriptor.byte_offset + if parent: + byte_offset += parent.byte_offset + sub = FieldInfo(f_name, f_tp, f_bits, tp, parent, descriptor, byte_offset) yield from iterfields(f_tp, sub) @@ -660,12 +676,13 @@ def output(string): (char*)&value, sizeof(value))); \\ } - // Set a field to -1, 1 and 0; append a snapshot of the memory + // Set a field to test values; append a snapshot of the memory // after each of the operations. - #define TEST_FIELD(TYPE, TARGET) { \\ - SET_AND_APPEND(TYPE, TARGET, -1) \\ - SET_AND_APPEND(TYPE, TARGET, 1) \\ - SET_AND_APPEND(TYPE, TARGET, 0) \\ + #define TEST_FIELD(TYPE, TARGET) { \\ + SET_AND_APPEND(TYPE, TARGET, -1) \\ + SET_AND_APPEND(TYPE, TARGET, 1) \\ + SET_AND_APPEND(TYPE, TARGET, (TYPE)2926941915) \\ + SET_AND_APPEND(TYPE, TARGET, 0) \\ } #if defined(__GNUC__) || defined(__clang__) diff --git a/Lib/test/test_ctypes/test_structunion.py b/Lib/test/test_ctypes/test_structunion.py index af332537d7dfc6..8d8b7e5e995132 100644 --- a/Lib/test/test_ctypes/test_structunion.py +++ b/Lib/test/test_ctypes/test_structunion.py @@ -1,6 +1,7 @@ """Common tests for ctypes.Structure and ctypes.Union""" import unittest +import sys from ctypes import (Structure, Union, POINTER, sizeof, alignment, c_char, c_byte, c_ubyte, c_short, c_ushort, c_int, c_uint, @@ -239,11 +240,12 @@ class X(self.cls): # is_bitfield, bit_size, bit_offset # size + little_endian = (sys.byteorder == 'little') expected_bitfield_info = dict( # (bit_size, bit_offset) - b=(1, 0), - c=(2, 1), - y=(1, 0), + b=(1, 0 if little_endian else 7), + c=(2, 1 if little_endian else 5), + y=(1, 0 if little_endian else 15), ) for name in field_names: with self.subTest(name=name): diff --git a/Modules/_ctypes/_ctypes_test_generated.c.h b/Modules/_ctypes/_ctypes_test_generated.c.h index d70b33eaa8b515..10f7332fdc620d 100644 --- a/Modules/_ctypes/_ctypes_test_generated.c.h +++ b/Modules/_ctypes/_ctypes_test_generated.c.h @@ -25,12 +25,13 @@ (char*)&value, sizeof(value))); \ } - // Set a field to -1, 1 and 0; append a snapshot of the memory + // Set a field to test values; append a snapshot of the memory // after each of the operations. - #define TEST_FIELD(TYPE, TARGET) { \ - SET_AND_APPEND(TYPE, TARGET, -1) \ - SET_AND_APPEND(TYPE, TARGET, 1) \ - SET_AND_APPEND(TYPE, TARGET, 0) \ + #define TEST_FIELD(TYPE, TARGET) { \ + SET_AND_APPEND(TYPE, TARGET, -1) \ + SET_AND_APPEND(TYPE, TARGET, 1) \ + SET_AND_APPEND(TYPE, TARGET, (TYPE)2926941915) \ + SET_AND_APPEND(TYPE, TARGET, 0) \ } #if defined(__GNUC__) || defined(__clang__) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 28a5eda32f0843..eb92f70c0c7b0c 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -61,7 +61,6 @@ _ctypes.CField.__new__ as PyCField_new byte_size: Py_ssize_t byte_offset: Py_ssize_t index: Py_ssize_t - for_big_endian: bool _internal_use: bool bit_size as bit_size_obj: object = None bit_offset as bit_offset_obj: object = None @@ -71,9 +70,9 @@ _ctypes.CField.__new__ as PyCField_new static PyObject * PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, Py_ssize_t byte_size, Py_ssize_t byte_offset, - Py_ssize_t index, int for_big_endian, int _internal_use, + Py_ssize_t index, int _internal_use, PyObject *bit_size_obj, PyObject *bit_offset_obj) -/*[clinic end generated code: output=79505dee1dad9b8e input=a6376bdec96976b8]*/ +/*[clinic end generated code: output=3f2885ee4108b6e2 input=b343436e33c0d782]*/ { CFieldObject* self = NULL; @@ -188,7 +187,6 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, self->byte_offset = byte_offset; self->bitfield_size = (uint8_t)bitfield_size; self->bit_offset = (uint8_t)bit_offset; - self->_for_big_endian = for_big_endian; self->index = index; @@ -237,9 +235,6 @@ _pack_legacy_size(CFieldObject *field) { if (field->bitfield_size) { Py_ssize_t bit_offset = field->bit_offset; - if (field->_for_big_endian) { - bit_offset = 8 * field->byte_size - bit_offset - field->bitfield_size; - } return (field->bitfield_size << 16) | bit_offset; } return field->byte_size; diff --git a/Modules/_ctypes/clinic/cfield.c.h b/Modules/_ctypes/clinic/cfield.c.h index f06ab46b8ea83b..3d16139f2d614a 100644 --- a/Modules/_ctypes/clinic/cfield.c.h +++ b/Modules/_ctypes/clinic/cfield.c.h @@ -12,7 +12,7 @@ preserve static PyObject * PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, Py_ssize_t byte_size, Py_ssize_t byte_offset, - Py_ssize_t index, int for_big_endian, int _internal_use, + Py_ssize_t index, int _internal_use, PyObject *bit_size_obj, PyObject *bit_offset_obj); static PyObject * @@ -21,14 +21,14 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) PyObject *return_value = NULL; #if defined(Py_BUILD_CORE) && !defined(Py_BUILD_CORE_MODULE) - #define NUM_KEYWORDS 9 + #define NUM_KEYWORDS 8 static struct { PyGC_Head _this_is_not_used; PyObject_VAR_HEAD PyObject *ob_item[NUM_KEYWORDS]; } _kwtuple = { .ob_base = PyVarObject_HEAD_INIT(&PyTuple_Type, NUM_KEYWORDS) - .ob_item = { &_Py_ID(name), &_Py_ID(type), &_Py_ID(byte_size), &_Py_ID(byte_offset), &_Py_ID(index), &_Py_ID(for_big_endian), &_Py_ID(_internal_use), &_Py_ID(bit_size), &_Py_ID(bit_offset), }, + .ob_item = { &_Py_ID(name), &_Py_ID(type), &_Py_ID(byte_size), &_Py_ID(byte_offset), &_Py_ID(index), &_Py_ID(_internal_use), &_Py_ID(bit_size), &_Py_ID(bit_offset), }, }; #undef NUM_KEYWORDS #define KWTUPLE (&_kwtuple.ob_base.ob_base) @@ -37,29 +37,28 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) # define KWTUPLE NULL #endif // !Py_BUILD_CORE - static const char * const _keywords[] = {"name", "type", "byte_size", "byte_offset", "index", "for_big_endian", "_internal_use", "bit_size", "bit_offset", NULL}; + static const char * const _keywords[] = {"name", "type", "byte_size", "byte_offset", "index", "_internal_use", "bit_size", "bit_offset", NULL}; static _PyArg_Parser _parser = { .keywords = _keywords, .fname = "CField", .kwtuple = KWTUPLE, }; #undef KWTUPLE - PyObject *argsbuf[9]; + PyObject *argsbuf[8]; PyObject * const *fastargs; Py_ssize_t nargs = PyTuple_GET_SIZE(args); - Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 7; + Py_ssize_t noptargs = nargs + (kwargs ? PyDict_GET_SIZE(kwargs) : 0) - 6; PyObject *name; PyObject *proto; Py_ssize_t byte_size; Py_ssize_t byte_offset; Py_ssize_t index; - int for_big_endian; int _internal_use; PyObject *bit_size_obj = Py_None; PyObject *bit_offset_obj = Py_None; fastargs = _PyArg_UnpackKeywords(_PyTuple_CAST(args)->ob_item, nargs, kwargs, NULL, &_parser, - /*minpos*/ 0, /*maxpos*/ 0, /*minkw*/ 7, /*varpos*/ 0, argsbuf); + /*minpos*/ 0, /*maxpos*/ 0, /*minkw*/ 6, /*varpos*/ 0, argsbuf); if (!fastargs) { goto exit; } @@ -105,28 +104,24 @@ PyCField_new(PyTypeObject *type, PyObject *args, PyObject *kwargs) } index = ival; } - for_big_endian = PyObject_IsTrue(fastargs[5]); - if (for_big_endian < 0) { - goto exit; - } - _internal_use = PyObject_IsTrue(fastargs[6]); + _internal_use = PyObject_IsTrue(fastargs[5]); if (_internal_use < 0) { goto exit; } if (!noptargs) { goto skip_optional_kwonly; } - if (fastargs[7]) { - bit_size_obj = fastargs[7]; + if (fastargs[6]) { + bit_size_obj = fastargs[6]; if (!--noptargs) { goto skip_optional_kwonly; } } - bit_offset_obj = fastargs[8]; + bit_offset_obj = fastargs[7]; skip_optional_kwonly: - return_value = PyCField_new_impl(type, name, proto, byte_size, byte_offset, index, for_big_endian, _internal_use, bit_size_obj, bit_offset_obj); + return_value = PyCField_new_impl(type, name, proto, byte_size, byte_offset, index, _internal_use, bit_size_obj, bit_offset_obj); exit: return return_value; } -/*[clinic end generated code: output=42a8a8fedff88230 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=7160ded221fb00ff input=a9049054013a1b77]*/ diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index b35f5d534b85a7..2b8192059a0dc2 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -299,7 +299,6 @@ typedef struct CFieldObject { GETFUNC getfunc; /* getter function if proto is NULL */ SETFUNC setfunc; /* setter function if proto is NULL */ bool anonymous: 1; - bool _for_big_endian: 1; /* true if the class is big-endian */ /* If this is a bit field, bitfield_size must be positive. * bitfield_size and bit_offset specify the field inside the chunk of diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index 6dba4e8dff7f40..60b30ddc3927a5 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -144,6 +144,7 @@ MakeFields(PyObject *type, CFieldObject *descr, new_descr->getfunc = fdescr->getfunc; new_descr->setfunc = fdescr->setfunc; new_descr->name = Py_NewRef(fdescr->name); + new_descr->anonymous = fdescr->anonymous; Py_DECREF(fdescr); From f327ffbdfc3abdb83ae4a5fbba2ce3890a9be19a Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 7 Feb 2025 19:05:58 +0100 Subject: [PATCH 22/29] Name the magic constant --- .../test_ctypes/test_generated_structs.py | 20 ++++++++++--------- Modules/_ctypes/_ctypes_test_generated.c.h | 15 +++++++------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/Lib/test/test_ctypes/test_generated_structs.py b/Lib/test/test_ctypes/test_generated_structs.py index 9ec470f0954766..38bc050e1a44eb 100644 --- a/Lib/test/test_ctypes/test_generated_structs.py +++ b/Lib/test/test_ctypes/test_generated_structs.py @@ -23,6 +23,9 @@ from test.test_ctypes._support import StructCheckMixin +# A 64-bit number where each nibble (hex digit) is different and +# has 2-3 bits set. +TEST_PATTERN = 0xae7596db # ctypes erases the difference between `c_int` and e.g.`c_int16`. # To keep it, we'll use custom subclasses with the C name stashed in `_c_name`: @@ -467,7 +470,7 @@ def test_generated_data(self): obj = cls() ptr = pointer(obj) for field in iterfields(cls): - for value in -1, 1, 2926941915, 0: + for value in -1, 1, TEST_PATTERN, 0: with self.subTest(field=field.full_name, value=value): field.set_to(obj, value) py_mem = string_at(ptr, sizeof(obj)) @@ -648,10 +651,9 @@ def iterfields(tp, parent=None): # Dump C source to stdout def output(string): print(re.compile(r'^ +$', re.MULTILINE).sub('', string).lstrip('\n')) + output("/* Generated by Lib/test/test_ctypes/test_generated_structs.py */") + output(f"#define TEST_PATTERN {TEST_PATTERN}") output(""" - /* Generated by Lib/test/test_ctypes/test_generated_structs.py */ - - // Append VALUE to the result. #define APPEND(ITEM) { \\ PyObject *item = ITEM; \\ @@ -678,11 +680,11 @@ def output(string): // Set a field to test values; append a snapshot of the memory // after each of the operations. - #define TEST_FIELD(TYPE, TARGET) { \\ - SET_AND_APPEND(TYPE, TARGET, -1) \\ - SET_AND_APPEND(TYPE, TARGET, 1) \\ - SET_AND_APPEND(TYPE, TARGET, (TYPE)2926941915) \\ - SET_AND_APPEND(TYPE, TARGET, 0) \\ + #define TEST_FIELD(TYPE, TARGET) { \\ + SET_AND_APPEND(TYPE, TARGET, -1) \\ + SET_AND_APPEND(TYPE, TARGET, 1) \\ + SET_AND_APPEND(TYPE, TARGET, (TYPE)TEST_PATTERN) \\ + SET_AND_APPEND(TYPE, TARGET, 0) \\ } #if defined(__GNUC__) || defined(__clang__) diff --git a/Modules/_ctypes/_ctypes_test_generated.c.h b/Modules/_ctypes/_ctypes_test_generated.c.h index 10f7332fdc620d..8bcbc770f21cdf 100644 --- a/Modules/_ctypes/_ctypes_test_generated.c.h +++ b/Modules/_ctypes/_ctypes_test_generated.c.h @@ -1,6 +1,5 @@ - /* Generated by Lib/test/test_ctypes/test_generated_structs.py */ - - +/* Generated by Lib/test/test_ctypes/test_generated_structs.py */ +#define TEST_PATTERN 2926941915 // Append VALUE to the result. #define APPEND(ITEM) { \ PyObject *item = ITEM; \ @@ -27,11 +26,11 @@ // Set a field to test values; append a snapshot of the memory // after each of the operations. - #define TEST_FIELD(TYPE, TARGET) { \ - SET_AND_APPEND(TYPE, TARGET, -1) \ - SET_AND_APPEND(TYPE, TARGET, 1) \ - SET_AND_APPEND(TYPE, TARGET, (TYPE)2926941915) \ - SET_AND_APPEND(TYPE, TARGET, 0) \ + #define TEST_FIELD(TYPE, TARGET) { \ + SET_AND_APPEND(TYPE, TARGET, -1) \ + SET_AND_APPEND(TYPE, TARGET, 1) \ + SET_AND_APPEND(TYPE, TARGET, (TYPE)TEST_PATTERN) \ + SET_AND_APPEND(TYPE, TARGET, 0) \ } #if defined(__GNUC__) || defined(__clang__) From 14270ad1d7431a82618f00ce61655c93fb514df6 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 7 Feb 2025 19:06:58 +0100 Subject: [PATCH 23/29] Remove unused variable --- Lib/test/test_ctypes/test_generated_structs.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_ctypes/test_generated_structs.py b/Lib/test/test_ctypes/test_generated_structs.py index 38bc050e1a44eb..9a8102219d8769 100644 --- a/Lib/test/test_ctypes/test_generated_structs.py +++ b/Lib/test/test_ctypes/test_generated_structs.py @@ -453,8 +453,6 @@ def test_generated_data(self): Common compilers seem to do so. """ for name, cls in TESTCASES.items(): - is_little_endian = ( - hasattr(cls, '_swappedbytes_') ^ (sys.byteorder == 'little')) with self.subTest(name=name): self.check_struct_or_union(cls) if _maybe_skip := getattr(cls, '_maybe_skip', None): From 7ce3cb96c5649a7d0b8ec9af0e4f2bef7d1055bd Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 7 Feb 2025 19:32:59 +0100 Subject: [PATCH 24/29] Remove an unacceptable blank line --- Lib/test/test_ctypes/_support.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index b72721d9bf6b20..b328c1261cd7f0 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -135,4 +135,3 @@ def _check_struct_or_union(self, cls, is_struct): + field.bit_offset + field.bit_size) self.assertLessEqual(bit_end, cls_size * 8) - From d9d593c2594d43a7be551a93425369c180493a7a Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Sat, 8 Feb 2025 12:28:41 +0100 Subject: [PATCH 25/29] Update documentation for tp_basicsize & tp_itemsize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add alignment requirement - Mention that ob_size is unreliable if you don't control it - Add some links for context - basicsize should include the base type in generaly not just PyObject This adds a “by-the-way” link to `PyObject_New`, which shouldn't be used for GC types. In order to be comfortable linking to it, I also add a link to `PyObject_GC_New` from its docs. And the same for `*Var` variants, while I'm here. --- Doc/c-api/allocation.rst | 8 ++++ Doc/c-api/type.rst | 3 ++ Doc/c-api/typeobj.rst | 96 +++++++++++++++++++++++++++------------- 3 files changed, 76 insertions(+), 31 deletions(-) diff --git a/Doc/c-api/allocation.rst b/Doc/c-api/allocation.rst index b7e0f22b52c57e..7cbc99ad145ad4 100644 --- a/Doc/c-api/allocation.rst +++ b/Doc/c-api/allocation.rst @@ -35,6 +35,10 @@ Allocating Objects on the Heap The size of the memory allocation is determined from the :c:member:`~PyTypeObject.tp_basicsize` field of the type object. + Note that this function is unsuitable if *typeobj* has + :c:macro:`Py_TPFLAGS_HAVE_GC` set. For such objects, + use :c:func:`PyObject_GC_New` instead. + .. c:macro:: PyObject_NewVar(TYPE, typeobj, size) @@ -49,6 +53,10 @@ Allocating Objects on the Heap fields into the same allocation decreases the number of allocations, improving the memory management efficiency. + Note that this function is unsuitable if *typeobj* has + :c:macro:`Py_TPFLAGS_HAVE_GC` set. For such objects, + use :c:func:`PyObject_GC_NewVar` instead. + .. c:function:: void PyObject_Del(void *op) diff --git a/Doc/c-api/type.rst b/Doc/c-api/type.rst index 444b3456f051d8..356cf253cfca57 100644 --- a/Doc/c-api/type.rst +++ b/Doc/c-api/type.rst @@ -456,6 +456,9 @@ The following functions and structs are used to create class need *in addition* to the superclass. Use :c:func:`PyObject_GetTypeData` to get a pointer to subclass-specific memory reserved this way. + For negative :c:member:`!basicsize`, Python will insert padding when + needed to meet :c:member:`~PyTypeObject.tp_basicsize`'s alignment + requirements. .. versionchanged:: 3.12 diff --git a/Doc/c-api/typeobj.rst b/Doc/c-api/typeobj.rst index 69c518d8e64cbc..6067886c878b25 100644 --- a/Doc/c-api/typeobj.rst +++ b/Doc/c-api/typeobj.rst @@ -587,47 +587,81 @@ and :c:data:`PyType_Type` effectively act as defaults.) .. c:member:: Py_ssize_t PyTypeObject.tp_basicsize - Py_ssize_t PyTypeObject.tp_itemsize + Py_ssize_t PyTypeObject.tp_itemsize These fields allow calculating the size in bytes of instances of the type. There are two kinds of types: types with fixed-length instances have a zero - :c:member:`~PyTypeObject.tp_itemsize` field, types with variable-length instances have a non-zero - :c:member:`~PyTypeObject.tp_itemsize` field. For a type with fixed-length instances, all - instances have the same size, given in :c:member:`~PyTypeObject.tp_basicsize`. + :c:member:`!tp_itemsize` field, types with variable-length instances have a non-zero + :c:member:`!tp_itemsize` field. For a type with fixed-length instances, all + instances have the same size, given in :c:member:`!tp_basicsize`. + (Exceptions to this rule can be made using + :c:func:`PyUnstable_Object_GC_NewWithExtraData`.) For a type with variable-length instances, the instances must have an - :c:member:`~PyVarObject.ob_size` field, and the instance size is :c:member:`~PyTypeObject.tp_basicsize` plus N - times :c:member:`~PyTypeObject.tp_itemsize`, where N is the "length" of the object. The value of - N is typically stored in the instance's :c:member:`~PyVarObject.ob_size` field. There are - exceptions: for example, ints use a negative :c:member:`~PyVarObject.ob_size` to indicate a - negative number, and N is ``abs(ob_size)`` there. Also, the presence of an - :c:member:`~PyVarObject.ob_size` field in the instance layout doesn't mean that the instance - structure is variable-length (for example, the structure for the list type has - fixed-length instances, yet those instances have a meaningful :c:member:`~PyVarObject.ob_size` - field). - - The basic size includes the fields in the instance declared by the macro - :c:macro:`PyObject_HEAD` or :c:macro:`PyObject_VAR_HEAD` (whichever is used to - declare the instance struct) and this in turn includes the :c:member:`~PyObject._ob_prev` and - :c:member:`~PyObject._ob_next` fields if they are present. This means that the only correct - way to get an initializer for the :c:member:`~PyTypeObject.tp_basicsize` is to use the + :c:member:`~PyVarObject.ob_size` field, and the instance size is + :c:member:`!tp_basicsize` plus N times :c:member:`!tp_itemsize`, + where N is the "length" of the object. + + Functions like :c:func:`PyObject_NewVar` will take the value of N as an + argument, and store in the instance's :c:member:`~PyVarObject.ob_size` field. + Note that the :c:member:`~PyVarObject.ob_size` field may later be used for + other purposes. For example, :py:type:`int` instances use the bits of + :c:member:`~PyVarObject.ob_size` in an implementation-defined + way; the underlying storage and its size should be acessed using + :c:func:`PyLong_Export`. + + Also, the presence of an :c:member:`~PyVarObject.ob_size` field in the + instance layout doesn't mean that the instance structure is variable-length. + For example, the :py:type:`list` type has fixed-length instances, yet those + instances have a :c:member:`~PyVarObject.ob_size` field. + (As with :py:type:`int`, avoid reading lists' :c:member:`!ob_size` directly. + Call :c:func:`PyList_Size` instead.) + + The :c:member:`!tp_basicsize` includes size needed for data of the type's + :c:member:`~PyTypeObject.tp_base`, plus any extra data needed + by each instance. + + The correct way to set :c:member:`!tp_basicsize` is to use the ``sizeof`` operator on the struct used to declare the instance layout. - The basic size does not include the GC header size. - - A note about alignment: if the variable items require a particular alignment, - this should be taken care of by the value of :c:member:`~PyTypeObject.tp_basicsize`. Example: - suppose a type implements an array of ``double``. :c:member:`~PyTypeObject.tp_itemsize` is - ``sizeof(double)``. It is the programmer's responsibility that - :c:member:`~PyTypeObject.tp_basicsize` is a multiple of ``sizeof(double)`` (assuming this is the - alignment requirement for ``double``). - - For any type with variable-length instances, this field must not be ``NULL``. + This struct must include the struct used to declare the base type. + In other words, :c:member:`!tp_basicsize` must be greater than or equal + to the base's :c:member:`!tp_basicsize`. + + Since every type is a subtype of :py:type:`object`, this struct must + include :c:type:`PyObject` or :c:type:`PyVarObject` (depending on + whether :c:member:`~PyVarObject.ob_size` should be included). These are + usually defined by the macro :c:macro:`PyObject_HEAD` or + :c:macro:`PyObject_VAR_HEAD`, respectively. + + The basic size does not include the GC header size, as that header is not + part of :c:macro:`PyObject_HEAD`. + + For cases where struct used to declare the base type is unknown, + see :c:member:`PyType_Spec.basicsize` and :c:func:`PyType_FromMetaclass`. + + Notes about alignment: + + - :c:member:`!tp_basicsize` must be a multiple of ``_Alignof(PyObject)``. + When using ``sizeof`` on a ``struct`` that includes + :c:macro:`PyObject_HEAD`, as recommended, the compiler ensures this. + When not using a C ``struct``, or when using compiler + extensions like ``__attribute__((packed))``, it is up to you. + - If the variable items require a particular alignment, + :c:member:`!tp_basicsize` and :c:member:`!tp_itemsize` must each be a + multiple of that alignment. + For example, if a type's variable part stores a ``double``, it is + your responsibility that both fields are a multiple of + ``_Alignof(double)``. **Inheritance:** - These fields are inherited separately by subtypes. If the base type has a - non-zero :c:member:`~PyTypeObject.tp_itemsize`, it is generally not safe to set + These fields are inherited separately by subtypes. + (That is, if the field is set to zero, :c:func:`PyType_Ready` will copy + the value from the base type, indicating that the instances do not + need additional storage.) + + If the base type has a non-zero :c:member:`~PyTypeObject.tp_itemsize`, it is generally not safe to set :c:member:`~PyTypeObject.tp_itemsize` to a different non-zero value in a subtype (though this depends on the implementation of the base type). From 2cdcb5bab02b87f6d1323f0b27012972f1b70e0c Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 21 Feb 2025 16:12:28 +0100 Subject: [PATCH 26/29] Skip "is in" test for bitfields of underaligned types (bug filed) --- Lib/test/test_ctypes/_support.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index b328c1261cd7f0..950e6297340d1b 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -22,6 +22,14 @@ Py_TPFLAGS_IMMUTABLETYPE = 1 << 8 +def is_underaligned(ctype): + """Return true when type's alignment is less than its size. + + A famous example is 64-bit int on 32-bit x86. + """ + return ctypes.alignment(ctype) < ctypes.sizeof(ctype) + + class StructCheckMixin: def check_struct(self, structure): """Assert that a structure is well-formed""" @@ -70,8 +78,14 @@ def _check_struct_or_union(self, cls, is_struct): # byte_size self.assertEqual(field.byte_size, ctypes.sizeof(field.type)) self.assertGreaterEqual(field.byte_size, 0) - self.assertLessEqual(field.byte_offset + field.byte_size, - cls_size) + + # Check that the field is inside the struct. + # See gh-130410 for why this is skipped for bitfields of + # underaligned types. Later in this function (see `bit_end`) + # we assert that the value *bits* are inside the struct. + if not (field.is_bitfield and is_underaligned(field.type)): + self.assertLessEqual(field.byte_offset + field.byte_size, + cls_size) # size self.assertGreaterEqual(field.size, 0) From fde4204ec782e6f94fbc28db5254982391524290 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 14 Mar 2025 18:08:10 +0100 Subject: [PATCH 27/29] Address review --- Lib/test/test_ctypes/_support.py | 2 +- Lib/test/test_ctypes/test_struct_fields.py | 2 +- Modules/_ctypes/cfield.c | 22 ++++++++++++---------- 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/Lib/test/test_ctypes/_support.py b/Lib/test/test_ctypes/_support.py index 950e6297340d1b..946d654a19aff8 100644 --- a/Lib/test/test_ctypes/_support.py +++ b/Lib/test/test_ctypes/_support.py @@ -61,7 +61,7 @@ def _check_struct_or_union(self, cls, is_struct): cls_size = ctypes.sizeof(cls) for name, requested_type, *rest_of_tuple in cls._fields_: field = getattr(cls, name) - with self.subTest(field=field): + with self.subTest(name=name, field=field): is_bitfield = len(rest_of_tuple) > 0 # name diff --git a/Lib/test/test_ctypes/test_struct_fields.py b/Lib/test/test_ctypes/test_struct_fields.py index 7bff11c8988dc2..5c713247a0f418 100644 --- a/Lib/test/test_ctypes/test_struct_fields.py +++ b/Lib/test/test_ctypes/test_struct_fields.py @@ -1,7 +1,7 @@ import unittest import sys from ctypes import Structure, Union, sizeof, c_char, c_int, CField -from ._support import Py_TPFLAGS_IMMUTABLETYPE, StructCheckMixin) +from ._support import Py_TPFLAGS_IMMUTABLETYPE, StructCheckMixin NOTHING = object() diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index eb92f70c0c7b0c..5431760e1ae1ad 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -134,8 +134,9 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, // so they're limited to about 8 bytes. // This check is here to avoid overflow in later checks. PyErr_Format(PyExc_ValueError, - "bit field %R size too large, got %zd", - name, byte_size); + "bit field %R size too large, got %zd", + name, byte_size); + goto error; } bitfield_size = PyLong_AsSsize_t(bit_size_obj); if ((bitfield_size <= 0) || (bitfield_size > 255)) { @@ -150,8 +151,8 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, if ((bit_offset < 0) || (bit_offset > 255)) { if (!PyErr_Occurred()) { PyErr_Format(PyExc_ValueError, - "bit offset of field %R out of range, got %zd", - name, bit_offset); + "bit offset of field %R out of range, got %zd", + name, bit_offset); } goto error; } @@ -160,7 +161,7 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, PyExc_ValueError, "bit field %R overflows its type (%zd + %zd >= %zd)", name, bit_offset, byte_size*8); - goto error; + goto error; } } else { @@ -169,6 +170,7 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, PyExc_ValueError, "field %R: bit_offset must be specified if bit_size is", name); + goto error; } } @@ -180,7 +182,7 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, if (!self->name) { goto error; } - assert (PyUnicode_CheckExact(self->name)); + assert(PyUnicode_CheckExact(self->name)); self->proto = Py_NewRef(proto); self->byte_size = byte_size; @@ -284,14 +286,14 @@ PyCField_get(PyObject *op, PyObject *inst, PyTypeObject *type) } static PyObject * -PyCField_get_legacy_size(PyObject *self, void *data) +PyCField_get_legacy_size(PyObject *self, void *Py_UNUSED(closure)) { CFieldObject *field = _CFieldObject_CAST(self); return PyLong_FromSsize_t(_pack_legacy_size(field)); } static PyObject * -PyCField_get_bit_size(PyObject *self, void *data) +PyCField_get_bit_size(PyObject *self, void *Py_UNUSED(closure)) { CFieldObject *field = _CFieldObject_CAST(self); if (field->bitfield_size) { @@ -323,13 +325,13 @@ PyCField_get_bit_size(PyObject *self, void *data) } static PyObject * -PyCField_is_bitfield(PyObject *self, void *data) +PyCField_is_bitfield(PyObject *self, void *Py_UNUSED(closure)) { return PyBool_FromLong(_CFieldObject_CAST(self)->bitfield_size); } static PyObject * -PyCField_is_anonymous(PyObject *self, void *data) +PyCField_is_anonymous(PyObject *self, void *Py_UNUSED(closure)) { return PyBool_FromLong(_CFieldObject_CAST(self)->anonymous); } From b2fddd8d75878a3af7aeb3568d654d4844ffb68b Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Fri, 14 Mar 2025 18:18:56 +0100 Subject: [PATCH 28/29] One more alignment --- Modules/_ctypes/cfield.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 5431760e1ae1ad..f81e364477bf54 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -142,8 +142,8 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, if ((bitfield_size <= 0) || (bitfield_size > 255)) { if (!PyErr_Occurred()) { PyErr_Format(PyExc_ValueError, - "bit size of field %R out of range, got %zd", - name, bitfield_size); + "bit size of field %R out of range, got %zd", + name, bitfield_size); } goto error; } From 5ce595c8e4f474ee1c9e7e88582de7250fdede07 Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Mon, 17 Mar 2025 15:21:45 +0100 Subject: [PATCH 29/29] Don't use `self` while it's NULL --- Modules/_ctypes/cfield.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index f81e364477bf54..7086ba2010607d 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -96,7 +96,7 @@ PyCField_new_impl(PyTypeObject *type, PyObject *name, PyObject *proto, } if (info == NULL) { PyErr_Format(PyExc_TypeError, - "type of field %R must be a C type", self->name); + "type of field %R must be a C type", name); goto error; } assert(byte_size == info->size);