Skip to content

Commit 16c0f6d

Browse files
miss-islingtonvsajip
authored andcommitted
bpo-22273: Update ctypes to correctly handle arrays in small structur… (GH-15839) (GH-16369)
(cherry picked from commit 12f209e)
1 parent 46f6c56 commit 16c0f6d

File tree

3 files changed

+187
-0
lines changed

3 files changed

+187
-0
lines changed

Lib/ctypes/test/test_structures.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,47 @@ class X(Structure):
438438
self.assertEqual(s.first, got.first)
439439
self.assertEqual(s.second, got.second)
440440

441+
def test_array_in_struct(self):
442+
# See bpo-22273
443+
444+
# These should mirror the structures in Modules/_ctypes/_ctypes_test.c
445+
class Test2(Structure):
446+
_fields_ = [
447+
('data', c_ubyte * 16),
448+
]
449+
450+
class Test3(Structure):
451+
_fields_ = [
452+
('data', c_double * 2),
453+
]
454+
455+
s = Test2()
456+
expected = 0
457+
for i in range(16):
458+
s.data[i] = i
459+
expected += i
460+
dll = CDLL(_ctypes_test.__file__)
461+
func = dll._testfunc_array_in_struct1
462+
func.restype = c_int
463+
func.argtypes = (Test2,)
464+
result = func(s)
465+
self.assertEqual(result, expected)
466+
# check the passed-in struct hasn't changed
467+
for i in range(16):
468+
self.assertEqual(s.data[i], i)
469+
470+
s = Test3()
471+
s.data[0] = 3.14159
472+
s.data[1] = 2.71828
473+
expected = 3.14159 + 2.71828
474+
func = dll._testfunc_array_in_struct2
475+
func.restype = c_double
476+
func.argtypes = (Test3,)
477+
result = func(s)
478+
self.assertEqual(result, expected)
479+
# check the passed-in struct hasn't changed
480+
self.assertEqual(s.data[0], 3.14159)
481+
self.assertEqual(s.data[1], 2.71828)
441482

442483
class PointerMemberTestCase(unittest.TestCase):
443484

Modules/_ctypes/_ctypes_test.c

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,45 @@ _testfunc_reg_struct_update_value(TestReg in)
7474
((volatile TestReg *)&in)->second = 0x0badf00d;
7575
}
7676

77+
/*
78+
* See bpo-22273. Structs containing arrays should work on Linux 64-bit.
79+
*/
80+
81+
typedef struct {
82+
unsigned char data[16];
83+
} Test2;
84+
85+
EXPORT(int)
86+
_testfunc_array_in_struct1(Test2 in)
87+
{
88+
int result = 0;
89+
90+
for (unsigned i = 0; i < 16; i++)
91+
result += in.data[i];
92+
/* As the structure is passed by value, changes to it shouldn't be
93+
* reflected in the caller.
94+
*/
95+
memset(in.data, 0, sizeof(in.data));
96+
return result;
97+
}
98+
99+
typedef struct {
100+
double data[2];
101+
} Test3;
102+
103+
EXPORT(double)
104+
_testfunc_array_in_struct2(Test3 in)
105+
{
106+
double result = 0;
107+
108+
for (unsigned i = 0; i < 2; i++)
109+
result += in.data[i];
110+
/* As the structure is passed by value, changes to it shouldn't be
111+
* reflected in the caller.
112+
*/
113+
memset(in.data, 0, sizeof(in.data));
114+
return result;
115+
}
77116

78117
EXPORT(void)testfunc_array(int values[4])
79118
{

Modules/_ctypes/stgdict.c

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,9 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
344344
int pack = 0;
345345
Py_ssize_t ffi_ofs;
346346
int big_endian;
347+
#if defined(X86_64)
348+
int arrays_seen = 0;
349+
#endif
347350

348351
/* HACK Alert: I cannot be bothered to fix ctypes.com, so there has to
349352
be a way to use the old, broken sematics: _fields_ are not extended
@@ -468,6 +471,10 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
468471
Py_XDECREF(pair);
469472
return -1;
470473
}
474+
#if defined(X86_64)
475+
if (PyCArrayTypeObject_Check(desc))
476+
arrays_seen = 1;
477+
#endif
471478
dict = PyType_stgdict(desc);
472479
if (dict == NULL) {
473480
Py_DECREF(pair);
@@ -608,6 +615,106 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
608615
stgdict->align = total_align;
609616
stgdict->length = len; /* ADD ffi_ofs? */
610617

618+
#if defined(X86_64)
619+
620+
#define MAX_ELEMENTS 16
621+
622+
if (arrays_seen && (size <= 16)) {
623+
/*
624+
* See bpo-22273. Arrays are normally treated as pointers, which is
625+
* fine when an array name is being passed as parameter, but not when
626+
* passing structures by value that contain arrays. On 64-bit Linux,
627+
* small structures passed by value are passed in registers, and in
628+
* order to do this, libffi needs to know the true type of the array
629+
* members of structs. Treating them as pointers breaks things.
630+
*
631+
* By small structures, we mean ones that are 16 bytes or less. In that
632+
* case, there can't be more than 16 elements after unrolling arrays,
633+
* as we (will) disallow bitfields. So we can collect the true ffi_type
634+
* values in a fixed-size local array on the stack and, if any arrays
635+
* were seen, replace the ffi_type_pointer.elements with a more
636+
* accurate set, to allow libffi to marshal them into registers
637+
* correctly. It means one more loop over the fields, but if we got
638+
* here, the structure is small, so there aren't too many of those.
639+
*/
640+
ffi_type *actual_types[MAX_ELEMENTS + 1];
641+
int actual_type_index = 0;
642+
643+
memset(actual_types, 0, sizeof(actual_types));
644+
for (i = 0; i < len; ++i) {
645+
PyObject *name, *desc;
646+
PyObject *pair = PySequence_GetItem(fields, i);
647+
StgDictObject *dict;
648+
int bitsize = 0;
649+
650+
if (pair == NULL) {
651+
return -1;
652+
}
653+
if (!PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {
654+
PyErr_SetString(PyExc_TypeError,
655+
"'_fields_' must be a sequence of (name, C type) pairs");
656+
Py_XDECREF(pair);
657+
return -1;
658+
}
659+
dict = PyType_stgdict(desc);
660+
if (dict == NULL) {
661+
Py_DECREF(pair);
662+
PyErr_Format(PyExc_TypeError,
663+
"second item in _fields_ tuple (index %zd) must be a C type",
664+
i);
665+
return -1;
666+
}
667+
if (!PyCArrayTypeObject_Check(desc)) {
668+
/* Not an array. Just copy over the element ffi_type. */
669+
actual_types[actual_type_index++] = &dict->ffi_type_pointer;
670+
assert(actual_type_index <= MAX_ELEMENTS);
671+
}
672+
else {
673+
int length = dict->length;
674+
StgDictObject *edict;
675+
676+
edict = PyType_stgdict(dict->proto);
677+
if (edict == NULL) {
678+
Py_DECREF(pair);
679+
PyErr_Format(PyExc_TypeError,
680+
"second item in _fields_ tuple (index %zd) must be a C type",
681+
i);
682+
return -1;
683+
}
684+
/* Copy over the element's type, length times. */
685+
while (length > 0) {
686+
actual_types[actual_type_index++] = &edict->ffi_type_pointer;
687+
assert(actual_type_index <= MAX_ELEMENTS);
688+
length--;
689+
}
690+
}
691+
Py_DECREF(pair);
692+
}
693+
694+
actual_types[actual_type_index++] = NULL;
695+
/*
696+
* Replace the old elements with the new, taking into account
697+
* base class elements where necessary.
698+
*/
699+
assert(stgdict->ffi_type_pointer.elements);
700+
PyMem_Free(stgdict->ffi_type_pointer.elements);
701+
stgdict->ffi_type_pointer.elements = PyMem_New(ffi_type *,
702+
ffi_ofs + actual_type_index);
703+
if (stgdict->ffi_type_pointer.elements == NULL) {
704+
PyErr_NoMemory();
705+
return -1;
706+
}
707+
if (ffi_ofs) {
708+
memcpy(stgdict->ffi_type_pointer.elements,
709+
basedict->ffi_type_pointer.elements,
710+
ffi_ofs * sizeof(ffi_type *));
711+
712+
}
713+
memcpy(&stgdict->ffi_type_pointer.elements[ffi_ofs], actual_types,
714+
actual_type_index * sizeof(ffi_type *));
715+
}
716+
#endif
717+
611718
/* We did check that this flag was NOT set above, it must not
612719
have been set until now. */
613720
if (stgdict->flags & DICTFLAG_FINAL) {

0 commit comments

Comments
 (0)