Skip to content

Commit 0d7ad9f

Browse files
authored
bpo-29753: fix merging packed bitfields in ctypes struct/union (pythonGH-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
1 parent af5fa13 commit 0d7ad9f

File tree

3 files changed

+91
-8
lines changed

3 files changed

+91
-8
lines changed

Lib/ctypes/test/test_bitfields.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,69 @@ class X(Structure):
233233
else:
234234
self.assertEqual(sizeof(X), sizeof(c_int) * 2)
235235

236+
@unittest.skipIf(os.name == 'nt', reason='Posix only')
237+
def test_packed_posix(self):
238+
test_cases = {
239+
(
240+
("a", c_uint8, 4),
241+
("b", c_uint8, 4),
242+
): 1,
243+
(
244+
("a", c_uint8, 1),
245+
("b", c_uint16, 1),
246+
("c", c_uint32, 1),
247+
("d", c_uint64, 1),
248+
): 1,
249+
(
250+
("a", c_uint8, 8),
251+
("b", c_uint16, 1),
252+
("c", c_uint32, 1),
253+
("d", c_uint64, 1),
254+
): 2,
255+
(
256+
("a", c_uint32, 9),
257+
("b", c_uint16, 10),
258+
("c", c_uint32, 25),
259+
("d", c_uint64, 1),
260+
): 6,
261+
(
262+
("a", c_uint32, 9),
263+
("b", c_uint16, 10),
264+
("c", c_uint32, 25),
265+
("d", c_uint64, 5),
266+
): 7,
267+
(
268+
("a", c_uint16),
269+
("b", c_uint16, 9),
270+
("c", c_uint16, 1),
271+
("d", c_uint16, 1),
272+
("e", c_uint16, 1),
273+
("f", c_uint16, 1),
274+
("g", c_uint16, 3),
275+
("h", c_uint32, 10),
276+
("i", c_uint32, 20),
277+
("j", c_uint32, 2),
278+
): 8,
279+
(
280+
("a", c_uint16, 9),
281+
("b", c_uint16, 10),
282+
("d", c_uint16),
283+
("c", c_uint8, 8),
284+
): 6,
285+
(
286+
("a", c_uint32, 9),
287+
("b", c_uint32),
288+
("c", c_uint32, 8),
289+
): 7,
290+
}
291+
292+
for fields, size in test_cases.items():
293+
with self.subTest(fields=fields):
294+
class X(Structure):
295+
_pack_ = 1
296+
_fields_ = list(fields)
297+
self.assertEqual(sizeof(X), size)
298+
236299
def test_anon_bitfields(self):
237300
# anonymous bit-fields gave a strange error message
238301
class X(Structure):
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
In ctypes, now packed bitfields are calculated properly and the first item of packed bitfields is now shrank correctly.

Modules/_ctypes/cfield.c

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index,
7171
Py_DECREF(self);
7272
return NULL;
7373
}
74+
75+
#ifndef MS_WIN32
76+
/* if we have a packed bitfield, calculate the minimum number of bytes we
77+
need to fit it. otherwise use the specified size. */
78+
if (pack && bitsize) {
79+
size = (bitsize - 1) / 8 + 1;
80+
} else
81+
#endif
82+
size = dict->size;
83+
84+
proto = desc;
85+
7486
if (bitsize /* this is a bitfield request */
7587
&& *pfield_size /* we have a bitfield open */
7688
#ifdef MS_WIN32
@@ -87,25 +99,26 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index,
8799
} else if (bitsize /* this is a bitfield request */
88100
&& *pfield_size /* we have a bitfield open */
89101
&& dict->size * 8 >= *pfield_size
90-
&& (*pbitofs + bitsize) <= dict->size * 8) {
102+
/* if this is a packed bitfield, always expand it.
103+
otherwise calculate if we need to expand it. */
104+
&& (((*pbitofs + bitsize) <= dict->size * 8) || pack)) {
91105
/* expand bit field */
92106
fieldtype = EXPAND_BITFIELD;
93107
#endif
94108
} else if (bitsize) {
95109
/* start new bitfield */
96110
fieldtype = NEW_BITFIELD;
97111
*pbitofs = 0;
98-
*pfield_size = dict->size * 8;
112+
/* use our calculated size (size) instead of type size (dict->size),
113+
which can be different for packed bitfields */
114+
*pfield_size = size * 8;
99115
} else {
100116
/* not a bit field */
101117
fieldtype = NO_BITFIELD;
102118
*pbitofs = 0;
103119
*pfield_size = 0;
104120
}
105121

106-
size = dict->size;
107-
proto = desc;
108-
109122
/* Field descriptors for 'c_char * n' are be scpecial cased to
110123
return a Python string instead of an Array object instance...
111124
*/
@@ -170,10 +183,16 @@ PyCField_FromDesc(PyObject *desc, Py_ssize_t index,
170183
break;
171184

172185
case EXPAND_BITFIELD:
173-
*poffset += dict->size - *pfield_size/8;
174-
*psize += dict->size - *pfield_size/8;
186+
/* increase the size if it is a packed bitfield.
187+
EXPAND_BITFIELD should not be selected for non-packed fields if the
188+
current size isn't already enough. */
189+
if (pack)
190+
size = (*pbitofs + bitsize - 1) / 8 + 1;
191+
192+
*poffset += size - *pfield_size/8;
193+
*psize += size - *pfield_size/8;
175194

176-
*pfield_size = dict->size * 8;
195+
*pfield_size = size * 8;
177196

178197
if (big_endian)
179198
self->size = (bitsize << 16) + *pfield_size - *pbitofs - bitsize;

0 commit comments

Comments
 (0)