diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index 3eded77..85f1b56 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -414,6 +414,46 @@ class StructureTestCase(unittest.TestCase): self.assertEqual(s.second, 0xcafebabe) self.assertEqual(s.third, 0x0bad1dea) + def test_array_in_struct(self): + # These should mirror the structures in Modules/_ctypes/_ctypes_test.c + class Test2(Structure): + _fields_ = [ + ('data', c_ubyte * 16), + ] + + class Test3(Structure): + _fields_ = [ + ('data', c_double * 2), + ] + + s = Test2() + expected = 0 + for i in range(16): + s.data[i] = i + expected += i + dll = CDLL(_ctypes_test.__file__) + func = dll._testfunc_array_in_struct1 + func.restype = c_int + func.argtypes = (Test2,) + result = func(s) + self.assertEqual(result, expected) + # check the passed-in struct hasn't changed + for i in range(16): + self.assertEqual(s.data[i], i) + + s = Test3() + s.data[0] = 3.14159 + s.data[1] = 2.71828 + expected = 3.14159 + 2.71828 + func = dll._testfunc_array_in_struct2 + func.restype = c_double + func.argtypes = (Test3,) + result = func(s) + self.assertEqual(result, expected) + # check the passed-in struct hasn't changed + self.assertEqual(s.data[0], 3.14159) + self.assertEqual(s.data[1], 2.71828) + class PointerMemberTestCase(unittest.TestCase): def test(self): diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index 9410c7f..728a713 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -57,6 +57,46 @@ _testfunc_large_struct_update_value(Test in) in.third = 0x0badf00d; } +/* + * See issue 22273. Structs containing arrays should work on Linux 64-bit. + */ + +typedef struct { + unsigned char data[16]; +} Test2; + +EXPORT(int) +_testfunc_array_in_struct1(Test2 in) +{ + int result = 0; + + for (unsigned i = 0; i < 16; i++) + result += in.data[i]; + /* As the structure is passed by value, changes to it shouldn't be + * reflected in the caller. + */ + memset(in.data, 0, sizeof(in.data)); + return result; +} + +typedef struct { + double data[2]; +} Test3; + +EXPORT(double) +_testfunc_array_in_struct2(Test3 in) +{ + double result = 0; + + for (unsigned i = 0; i < 2; i++) + result += in.data[i]; + /* As the structure is passed by value, changes to it shouldn't be + * reflected in the caller. + */ + memset(in.data, 0, sizeof(in.data)); + return result; +} + EXPORT(void)testfunc_array(int values[4]) { printf("testfunc_array %d %d %d %d\n", diff --git a/Modules/_ctypes/ctypes.h b/Modules/_ctypes/ctypes.h index 5d3b966..2b27c2f 100644 --- a/Modules/_ctypes/ctypes.h +++ b/Modules/_ctypes/ctypes.h @@ -288,6 +288,7 @@ PyObject *_ctypes_callproc(PPROC pProc, #define TYPEFLAG_ISPOINTER 0x100 #define TYPEFLAG_HASPOINTER 0x200 +#define TYPEFLAG_NONARGTYPE 0x400 #define DICTFLAG_FINAL 0x1000 diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index 1ccaf2f..23a0d62 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -316,6 +316,11 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct int pack = 0; Py_ssize_t ffi_ofs; int big_endian; +#if defined(X86_64) + int arrays_seen = 0; + int bitfields_seen = 0; + int isArgType; +#endif /* HACK Alert: I cannot be bothered to fix ctypes.com, so there has to be a way to use the old, broken sematics: _fields_ are not extended @@ -440,6 +445,10 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct Py_XDECREF(pair); return -1; } +#if defined(X86_64) + if (PyCArrayTypeObject_Check(desc)) + arrays_seen = 1; +#endif dict = PyType_stgdict(desc); if (dict == NULL) { Py_DECREF(pair); @@ -453,6 +462,9 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct stgdict->flags |= TYPEFLAG_HASPOINTER; dict->flags |= DICTFLAG_FINAL; /* mark field type final */ if (PyTuple_Size(pair) == 3) { /* bits specified */ +#if defined(X86_64) + bitfields_seen = 1; +#endif switch(dict->ffi_type_pointer.type) { case FFI_TYPE_UINT8: case FFI_TYPE_UINT16: @@ -580,6 +592,106 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct stgdict->align = total_align; stgdict->length = len; /* ADD ffi_ofs? */ +#if defined(X86_64) + +#define MAX_ELEMENTS 16 + + isArgType = (!(stgdict->flags & TYPEFLAG_NONARGTYPE) && + isStruct && !bitfields_seen); + if (!isArgType) { + stgdict->flags |= TYPEFLAG_NONARGTYPE; + } else if (arrays_seen && (size <= 16)) { + /* + * See issue #22273. Arrays are normally treated as pointers, which + * is fine when an array name is being passed as parameter, but not + * when passing structures by value that contain arrays. On 64-bit + * Linux, small structures passed by value are passed in registers, + * and in order to do this, libffi needs to know the true type of the + * array members of structs. Treating them as pointers breaks things. + * + * By small structures, we mean ones that are 16 bytes or less. In that + * case, there can't be more than 16 elements after unrolling arrays, + * as we disallow bitfields. So we can collect the true ffi_type values + * in a fixed-size local array on the stack and, if any arrays were + * seen, replace the ffi_type_pointer.elements with a more accurate set, + * to allow libffi to marshal them into registers correctly. It means + * one more loop over the fields, but if we got here, the structure is + * small, so there aren't too many of those. + */ + ffi_type * actual_types[MAX_ELEMENTS + 1]; + int actual_type_index = 0; + + memset(actual_types, 0, sizeof(actual_types)); + for (i = 0; i < len; ++i) { + PyObject *name = NULL, *desc = NULL; + PyObject *pair = PySequence_GetItem(fields, i); + StgDictObject *dict; + int bitsize = 0; + + if (!pair || !PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) { + PyErr_SetString(PyExc_TypeError, + "'_fields_' must be a sequence of (name, C type) pairs"); + Py_XDECREF(pair); + return -1; + } + dict = PyType_stgdict(desc); + if (dict == NULL) { + Py_DECREF(pair); + PyErr_Format(PyExc_TypeError, + "second item in _fields_ tuple (index %zd) must be a C type", + i); + return -1; + } + if (!PyCArrayTypeObject_Check(desc)) { + /* Not an array. Just copy over the element ffi_type. */ + actual_types[actual_type_index++] = &dict->ffi_type_pointer; + assert(actual_type_index <= MAX_ELEMENTS); + } + else { + int length = dict->length; + StgDictObject *edict; + + edict = PyType_stgdict(dict->proto); + if (edict == NULL) { + Py_DECREF(pair); + PyErr_Format(PyExc_TypeError, + "second item in _fields_ tuple (index %zd) must be a C type", + i); + return -1; + } + /* Copy over the element's type, length times. */ + while (length > 0) { + actual_types[actual_type_index++] = &edict->ffi_type_pointer; + assert(actual_type_index <= MAX_ELEMENTS); + length--; + } + } + Py_DECREF(pair); + } + + actual_types[actual_type_index++] = NULL; + /* + * Replace the old elements with the new, taking into account + * base class elements where necessary. + */ + assert(stgdict->ffi_type_pointer.elements); + PyMem_Free(stgdict->ffi_type_pointer.elements); + stgdict->ffi_type_pointer.elements = PyMem_New(ffi_type *, + ffi_ofs + actual_type_index); + if (stgdict->ffi_type_pointer.elements == NULL) { + PyErr_NoMemory(); + return -1; + } + if (ffi_ofs) { + memcpy(stgdict->ffi_type_pointer.elements, + basedict->ffi_type_pointer.elements, + ffi_ofs * sizeof(ffi_type *)); + + } + memcpy(&stgdict->ffi_type_pointer.elements[ffi_ofs], actual_types, + actual_type_index * sizeof(ffi_type *)); + } +#endif /* We did check that this flag was NOT set above, it must not have been set until now. */ if (stgdict->flags & DICTFLAG_FINAL) {