Skip to content

Commit 90d85a9

Browse files
authored
gh-76961: Fix the PEP3118 format string for ctypes.Structure (#5561)
The summary of this diff is that it: * adds a `_ctypes_alloc_format_padding` function to append strings like `37x` to a format string to indicate 37 padding bytes * removes the branches that amount to "give up on producing a valid format string if the struct is packed" * combines the resulting adjacent `if (isStruct) {`s now that neither is `if (isStruct && !isPacked) {` * invokes `_ctypes_alloc_format_padding` to add padding between structure fields, and after the last structure field. The computation used for the total size is unchanged from ctypes already used. This patch does not affect any existing aligment computation; all it does is use subtraction to deduce the amount of paddnig introduced by the existing code. --- Without this fix, it would never include padding bytes - an assumption that was only valid in the case when `_pack_` was set - and this case was explicitly not implemented. This should allow conversion from ctypes structs to numpy structs Fixes numpy/numpy#10528
1 parent 0672a6c commit 90d85a9

File tree

4 files changed

+111
-33
lines changed

4 files changed

+111
-33
lines changed

Doc/library/ctypes.rst

+1
Original file line numberDiff line numberDiff line change
@@ -2510,6 +2510,7 @@ fields, or any other data types containing pointer type fields.
25102510
An optional small integer that allows overriding the alignment of
25112511
structure fields in the instance. :attr:`_pack_` must already be defined
25122512
when :attr:`_fields_` is assigned, otherwise it will have no effect.
2513+
Setting this attribute to 0 is the same as not setting it at all.
25132514

25142515

25152516
.. attribute:: _anonymous_

Lib/test/test_ctypes/test_pep3118.py

+20-3
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,20 @@ class PackedPoint(Structure):
8686
_pack_ = 2
8787
_fields_ = [("x", c_long), ("y", c_long)]
8888

89+
class PointMidPad(Structure):
90+
_fields_ = [("x", c_byte), ("y", c_uint64)]
91+
92+
class PackedPointMidPad(Structure):
93+
_pack_ = 2
94+
_fields_ = [("x", c_byte), ("y", c_uint64)]
95+
96+
class PointEndPad(Structure):
97+
_fields_ = [("x", c_uint64), ("y", c_byte)]
98+
99+
class PackedPointEndPad(Structure):
100+
_pack_ = 2
101+
_fields_ = [("x", c_uint64), ("y", c_byte)]
102+
89103
class Point2(Structure):
90104
pass
91105
Point2._fields_ = [("x", c_long), ("y", c_long)]
@@ -185,10 +199,13 @@ class Complete(Structure):
185199

186200
## structures and unions
187201

188-
(Point, "T{<l:x:<l:y:}".replace('l', s_long), (), Point),
189-
# packed structures do not implement the pep
190-
(PackedPoint, "B", (), PackedPoint),
191202
(Point2, "T{<l:x:<l:y:}".replace('l', s_long), (), Point2),
203+
(Point, "T{<l:x:<l:y:}".replace('l', s_long), (), Point),
204+
(PackedPoint, "T{<l:x:<l:y:}".replace('l', s_long), (), PackedPoint),
205+
(PointMidPad, "T{<b:x:7x<Q:y:}", (), PointMidPad),
206+
(PackedPointMidPad, "T{<b:x:x<Q:y:}", (), PackedPointMidPad),
207+
(PointEndPad, "T{<Q:x:<b:y:7x}", (), PointEndPad),
208+
(PackedPointEndPad, "T{<Q:x:<b:y:x}", (), PackedPointEndPad),
192209
(EmptyStruct, "T{}", (), EmptyStruct),
193210
# the pep doesn't support unions
194211
(aUnion, "B", (), aUnion),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Inter-field padding is now inserted into the PEP3118 format strings obtained
2+
from :class:`ctypes.Structure` objects, reflecting their true representation in
3+
memory.

Modules/_ctypes/stgdict.c

+87-30
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,29 @@ MakeAnonFields(PyObject *type)
337337
return 0;
338338
}
339339

340+
/*
341+
Allocate a memory block for a pep3118 format string, copy prefix (if
342+
non-null) into it and append `{padding}x` to the end.
343+
Returns NULL on failure, with the error indicator set.
344+
*/
345+
char *
346+
_ctypes_alloc_format_padding(const char *prefix, Py_ssize_t padding)
347+
{
348+
/* int64 decimal characters + x + null */
349+
char buf[19 + 1 + 1];
350+
351+
assert(padding > 0);
352+
353+
if (padding == 1) {
354+
/* Use x instead of 1x, for brevity */
355+
return _ctypes_alloc_format_string(prefix, "x");
356+
}
357+
358+
int ret = PyOS_snprintf(buf, sizeof(buf), "%zdx", padding);
359+
assert(0 <= ret && ret < sizeof(buf));
360+
return _ctypes_alloc_format_string(prefix, buf);
361+
}
362+
340363
/*
341364
Retrieve the (optional) _pack_ attribute from a type, the _fields_ attribute,
342365
and create an StgDictObject. Used for Structure and Union subclasses.
@@ -346,11 +369,10 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
346369
{
347370
StgDictObject *stgdict, *basedict;
348371
Py_ssize_t len, offset, size, align, i;
349-
Py_ssize_t union_size, total_align;
372+
Py_ssize_t union_size, total_align, aligned_size;
350373
Py_ssize_t field_size = 0;
351374
int bitofs;
352375
PyObject *tmp;
353-
int isPacked;
354376
int pack;
355377
Py_ssize_t ffi_ofs;
356378
int big_endian;
@@ -374,7 +396,6 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
374396
return -1;
375397
}
376398
if (tmp) {
377-
isPacked = 1;
378399
pack = _PyLong_AsInt(tmp);
379400
Py_DECREF(tmp);
380401
if (pack < 0) {
@@ -389,7 +410,7 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
389410
}
390411
}
391412
else {
392-
isPacked = 0;
413+
/* Setting `_pack_ = 0` amounts to using the default alignment */
393414
pack = 0;
394415
}
395416

@@ -470,12 +491,10 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
470491
}
471492

472493
assert(stgdict->format == NULL);
473-
if (isStruct && !isPacked) {
494+
if (isStruct) {
474495
stgdict->format = _ctypes_alloc_format_string(NULL, "T{");
475496
} else {
476-
/* PEP3118 doesn't support union, or packed structures (well,
477-
only standard packing, but we don't support the pep for
478-
that). Use 'B' for bytes. */
497+
/* PEP3118 doesn't support union. Use 'B' for bytes. */
479498
stgdict->format = _ctypes_alloc_format_string(NULL, "B");
480499
}
481500
if (stgdict->format == NULL)
@@ -543,24 +562,53 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
543562
} else
544563
bitsize = 0;
545564

546-
if (isStruct && !isPacked) {
565+
if (isStruct) {
547566
const char *fieldfmt = dict->format ? dict->format : "B";
548567
const char *fieldname = PyUnicode_AsUTF8(name);
549568
char *ptr;
550569
Py_ssize_t len;
551570
char *buf;
571+
Py_ssize_t last_size = size;
572+
Py_ssize_t padding;
552573

553574
if (fieldname == NULL)
554575
{
555576
Py_DECREF(pair);
556577
return -1;
557578
}
558579

580+
/* construct the field now, as `prop->offset` is `offset` with
581+
corrected alignment */
582+
prop = PyCField_FromDesc(desc, i,
583+
&field_size, bitsize, &bitofs,
584+
&size, &offset, &align,
585+
pack, big_endian);
586+
if (prop == NULL) {
587+
Py_DECREF(pair);
588+
return -1;
589+
}
590+
591+
/* number of bytes between the end of the last field and the start
592+
of this one */
593+
padding = ((CFieldObject *)prop)->offset - last_size;
594+
595+
if (padding > 0) {
596+
ptr = stgdict->format;
597+
stgdict->format = _ctypes_alloc_format_padding(ptr, padding);
598+
PyMem_Free(ptr);
599+
if (stgdict->format == NULL) {
600+
Py_DECREF(pair);
601+
Py_DECREF(prop);
602+
return -1;
603+
}
604+
}
605+
559606
len = strlen(fieldname) + strlen(fieldfmt);
560607

561608
buf = PyMem_Malloc(len + 2 + 1);
562609
if (buf == NULL) {
563610
Py_DECREF(pair);
611+
Py_DECREF(prop);
564612
PyErr_NoMemory();
565613
return -1;
566614
}
@@ -578,15 +626,9 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
578626

579627
if (stgdict->format == NULL) {
580628
Py_DECREF(pair);
629+
Py_DECREF(prop);
581630
return -1;
582631
}
583-
}
584-
585-
if (isStruct) {
586-
prop = PyCField_FromDesc(desc, i,
587-
&field_size, bitsize, &bitofs,
588-
&size, &offset, &align,
589-
pack, big_endian);
590632
} else /* union */ {
591633
size = 0;
592634
offset = 0;
@@ -595,14 +637,14 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
595637
&field_size, bitsize, &bitofs,
596638
&size, &offset, &align,
597639
pack, big_endian);
640+
if (prop == NULL) {
641+
Py_DECREF(pair);
642+
return -1;
643+
}
598644
union_size = max(size, union_size);
599645
}
600646
total_align = max(align, total_align);
601647

602-
if (!prop) {
603-
Py_DECREF(pair);
604-
return -1;
605-
}
606648
if (-1 == PyObject_SetAttr(type, name, prop)) {
607649
Py_DECREF(prop);
608650
Py_DECREF(pair);
@@ -612,26 +654,41 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
612654
Py_DECREF(prop);
613655
}
614656

615-
if (isStruct && !isPacked) {
616-
char *ptr = stgdict->format;
657+
if (!isStruct) {
658+
size = union_size;
659+
}
660+
661+
/* Adjust the size according to the alignment requirements */
662+
aligned_size = ((size + total_align - 1) / total_align) * total_align;
663+
664+
if (isStruct) {
665+
char *ptr;
666+
Py_ssize_t padding;
667+
668+
/* Pad up to the full size of the struct */
669+
padding = aligned_size - size;
670+
if (padding > 0) {
671+
ptr = stgdict->format;
672+
stgdict->format = _ctypes_alloc_format_padding(ptr, padding);
673+
PyMem_Free(ptr);
674+
if (stgdict->format == NULL) {
675+
return -1;
676+
}
677+
}
678+
679+
ptr = stgdict->format;
617680
stgdict->format = _ctypes_alloc_format_string(stgdict->format, "}");
618681
PyMem_Free(ptr);
619682
if (stgdict->format == NULL)
620683
return -1;
621684
}
622685

623-
if (!isStruct)
624-
size = union_size;
625-
626-
/* Adjust the size according to the alignment requirements */
627-
size = ((size + total_align - 1) / total_align) * total_align;
628-
629686
stgdict->ffi_type_pointer.alignment = Py_SAFE_DOWNCAST(total_align,
630687
Py_ssize_t,
631688
unsigned short);
632-
stgdict->ffi_type_pointer.size = size;
689+
stgdict->ffi_type_pointer.size = aligned_size;
633690

634-
stgdict->size = size;
691+
stgdict->size = aligned_size;
635692
stgdict->align = total_align;
636693
stgdict->length = len; /* ADD ffi_ofs? */
637694

0 commit comments

Comments
 (0)