https://github.com/python/cpython/commit/0d7ad9fb38c041c46094087b0cf2c8ce449... commit: 0d7ad9fb38c041c46094087b0cf2c8ce44916b11 branch: master author: Filipe Laíns <lains@riseup.net> committer: miss-islington <31488909+miss-islington@users.noreply.github.com> date: 2021-02-28T14:43:19-08:00 summary: bpo-29753: fix merging packed bitfields in ctypes struct/union (GH-19850)

From the commit message:

When the structure is packed we should always expand when needed, otherwise we will add some padding between the fields. This patch makes sure we always merge bitfields together. It also changes the field merging algorithm so that it handles bitfields correctly.

Automerge-Triggered-By: GH:jaraco files: A Misc/NEWS.d/next/Library/2020-05-02-01-01-30.bpo-29753.n2M-AF.rst M Lib/ctypes/test/test_bitfields.py M Modules/_ctypes/cfield.c diff --git a/Lib/ctypes/test/test_bitfields.py b/Lib/ctypes/test/test_bitfields.py index 992b8c4da3a77..68b618a05f436 100644 --- a/Lib/ctypes/test/test_bitfields.py +++ b/Lib/ctypes/test/test_bitfields.py @@ -233,6 +233,69 @@ class X(Structure): else: self.assertEqual(sizeof(X), sizeof(c_int) * 2) + @unittest.skipIf(os.name == 'nt', reason='Posix only') + def test_packed_posix(self): + test_cases = { + ( + ("a", c_uint8, 4), + ("b", c_uint8, 4), + ): 1, + ( + ("a", c_uint8, 1), + ("b", c_uint16, 1), + ("c", c_uint32, 1), + ("d", c_uint64, 1), + ): 1, + ( + ("a", c_uint8, 8), + ("b", c_uint16, 1), + ("c", c_uint32, 1), + ("d", c_uint64, 1), + ): 2, + ( + ("a", c_uint32, 9), + ("b", c_uint16, 10), + ("c", c_uint32, 25), + ("d", c_uint64, 1), + ): 6, + ( + ("a", c_uint32, 9), + ("b", c_uint16, 10), + ("c", c_uint32, 25), + ("d", c_uint64, 5), + ): 7, + ( + ("a", c_uint16), + ("b", c_uint16, 9), + ("c", c_uint16, 1), + ("d", c_uint16, 1), + ("e", c_uint16, 1), + ("f", c_uint16, 1), + ("g", c_uint16, 3), + ("h", c_uint32, 10), + ("i", c_uint32, 20), + ("j", c_uint32, 2), + ): 8, + ( + ("a", c_uint16, 9), + ("b", c_uint16, 10), + ("d", c_uint16), + ("c", c_uint8, 8), + ): 6, + ( + ("a", c_uint32, 9), + ("b", c_uint32), + ("c", c_uint32, 8), + ): 7, + } + + for fields, size in test_cases.items(): + with self.subTest(fields=fields): + class X(Structure): + _pack_ = 1 + _fields_ = list(fields) + self.assertEqual(sizeof(X), size) + def test_anon_bitfields(self): # anonymous bit-fields gave a strange error message class X(Structure): diff --git a/Misc/NEWS.d/next/Library/2020-05-02-01-01-30.bpo-29753.n2M-AF.rst b/Misc/NEWS.d/next/Library/2020-05-02-01-01-30.bpo-29753.n2M-AF.rst new file mode 100644 index 0000000000000..f2a2842239b9c --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-05-02-01-01-30.bpo-29753.n2M-AF.rst @@ -0,0 +1 @@ +In ctypes, now packed bitfields are calculated properly and the first item of packed bitfields is now shrank correctly. diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 5bd96f1eb8c18..d96a1b0651baa 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -71,6 +71,18 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, Py_DECREF(self); return NULL; } + +#ifndef MS_WIN32 + /* if we have a packed bitfield, calculate the minimum number of bytes we + need to fit it. otherwise use the specified size. */ + if (pack && bitsize) { + size = (bitsize - 1) / 8 + 1; + } else +#endif + size = dict->size; + + proto = desc; + if (bitsize /* this is a bitfield request */ && *pfield_size /* we have a bitfield open */ #ifdef MS_WIN32 @@ -87,7 +99,9 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, } else if (bitsize /* this is a bitfield request */ && *pfield_size /* we have a bitfield open */ && dict->size * 8 >= *pfield_size - && (*pbitofs + bitsize) <= dict->size * 8) { + /* if this is a packed bitfield, always expand it. + otherwise calculate if we need to expand it. */ + && (((*pbitofs + bitsize) <= dict->size * 8) || pack)) { /* expand bit field */ fieldtype = EXPAND_BITFIELD; #endif @@ -95,7 +109,9 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, /* start new bitfield */ fieldtype = NEW_BITFIELD; *pbitofs = 0; - *pfield_size = dict->size * 8; + /* use our calculated size (size) instead of type size (dict->size), + which can be different for packed bitfields */ + *pfield_size = size * 8; } else { /* not a bit field */ fieldtype = NO_BITFIELD; @@ -103,9 +119,6 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, *pfield_size = 0; } - size = dict->size; - proto = desc; - /* Field descriptors for 'c_char * n' are be scpecial cased to return a Python string instead of an Array object instance... */ @@ -170,10 +183,16 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index, break; case EXPAND_BITFIELD: - *poffset += dict->size - *pfield_size/8; - *psize += dict->size - *pfield_size/8; + /* increase the size if it is a packed bitfield. + EXPAND_BITFIELD should not be selected for non-packed fields if the + current size isn't already enough. */ + if (pack) + size = (*pbitofs + bitsize - 1) / 8 + 1; + + *poffset += size - *pfield_size/8; + *psize += size - *pfield_size/8; - *pfield_size = dict->size * 8; + *pfield_size = size * 8; if (big_endian) self->size = (bitsize << 16) + *pfield_size - *pbitofs - bitsize;