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/stgdict.c b/Modules/_ctypes/stgdict.c index 1ccaf2f..8e0a2b9 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -580,6 +580,132 @@ 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 128 + + if (isStruct && (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 128 elements after unrolling arrays, + * even if we allow for 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. + */ + int arrays_seen = 0; + 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; + } + else { + /* It's an array. Get the base type and length. */ + + PyObject *length_attr, *type_attr; + long length; + int overflow; + StgDictObject *edict; + + arrays_seen = 1; /* Remember we saw an array. */ + + /* Get the length */ + length_attr = PyObject_GetAttrString(desc, "_length_"); + if (!length_attr || !PyLong_Check(length_attr)) { + PyErr_SetString(PyExc_AttributeError, + "class must define a '_length_' attribute, " + "which must be a positive integer"); + Py_XDECREF(length_attr); + Py_DECREF(pair); + return -1; + } + length = PyLong_AsLongAndOverflow(length_attr, &overflow); + if (overflow) { + PyErr_SetString(PyExc_OverflowError, + "The '_length_' attribute is too large"); + Py_DECREF(length_attr); + Py_DECREF(pair); + return -1; + } + Py_DECREF(length_attr); + + /* Got the length. Now, the element type. */ + type_attr = PyObject_GetAttrString(desc, "_type_"); + if (!type_attr) { + PyErr_SetString(PyExc_AttributeError, + "class must define a '_type_' attribute"); + Py_XDECREF(type_attr); + Py_DECREF(pair); + return -1; + } + edict = PyType_stgdict(type_attr); + 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(type_attr); + } + Py_DECREF(pair); + } + if (arrays_seen) { + /* Some arrays were seen. The unrolled types are in actual_types + * and actual_type_index. Replace dict->ffi_type_pointer.elements + * with these values. + */ + actual_types[actual_type_index++] = NULL; + assert(stgdict->ffi_type_pointer.elements); + PyMem_Free(stgdict->ffi_type_pointer.elements); + stgdict->ffi_type_pointer.elements = PyMem_New(ffi_type *, actual_type_index); + if (stgdict->ffi_type_pointer.elements == NULL) { + PyErr_NoMemory(); + return -1; + } + memcpy(stgdict->ffi_type_pointer.elements, 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) {