diff --git a/Lib/test/test_ctypes/test_structures.py b/Lib/test/test_ctypes/test_structures.py index a40ce3e86695a1..978bfed1a24476 100644 --- a/Lib/test/test_ctypes/test_structures.py +++ b/Lib/test/test_ctypes/test_structures.py @@ -1,8 +1,12 @@ import platform import sys import unittest -from ctypes import * from test.test_ctypes import need_symbol +from ctypes import (CDLL, Array, Structure, Union, POINTER, sizeof, byref, alignment, + c_void_p, c_char, c_wchar, c_byte, c_ubyte, + c_uint8, c_uint16, c_uint32, + c_short, c_ushort, c_int, c_uint, + c_long, c_ulong, c_longlong, c_ulonglong, c_float, c_double) from struct import calcsize import _ctypes_test from test import support @@ -499,12 +503,59 @@ class Test3B(Test3A): ('more_data', c_float * 2), ] + class Test3C1(Structure): + _fields_ = [ + ("data", c_double * 4) + ] + + class DataType4(Array): + _type_ = c_double + _length_ = 4 + + class Test3C2(Structure): + _fields_ = [ + ("data", DataType4) + ] + + class Test3C3(Structure): + _fields_ = [ + ("x", c_double), + ("y", c_double), + ("z", c_double), + ("t", c_double) + ] + + class Test3D1(Structure): + _fields_ = [ + ("data", c_double * 5) + ] + + class DataType5(Array): + _type_ = c_double + _length_ = 5 + + class Test3D2(Structure): + _fields_ = [ + ("data", DataType5) + ] + + class Test3D3(Structure): + _fields_ = [ + ("x", c_double), + ("y", c_double), + ("z", c_double), + ("t", c_double), + ("u", c_double) + ] + + # Load the shared library + dll = CDLL(_ctypes_test.__file__) + 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,) @@ -545,6 +596,78 @@ class Test3B(Test3A): self.assertAlmostEqual(s.more_data[0], -3.0, places=6) self.assertAlmostEqual(s.more_data[1], -2.0, places=6) + # Tests for struct Test3C + expected = (1.0, 2.0, 3.0, 4.0) + func = dll._testfunc_array_in_struct_set_defaults_3C + func.restype = Test3C1 + result = func() + # check the default values have been set properly + self.assertEqual( + (result.data[0], + result.data[1], + result.data[2], + result.data[3]), + expected + ) + + func = dll._testfunc_array_in_struct_set_defaults_3C + func.restype = Test3C2 + result = func() + # check the default values have been set properly + self.assertEqual( + (result.data[0], + result.data[1], + result.data[2], + result.data[3]), + expected + ) + + func = dll._testfunc_array_in_struct_set_defaults_3C + func.restype = Test3C3 + result = func() + # check the default values have been set properly + self.assertEqual((result.x, result.y, result.z, result.t), expected) + + # Tests for struct Test3D + expected = (1.0, 2.0, 3.0, 4.0, 5.0) + func = dll._testfunc_array_in_struct_set_defaults_3D + func.restype = Test3D1 + result = func() + # check the default values have been set properly + self.assertEqual( + (result.data[0], + result.data[1], + result.data[2], + result.data[3], + result.data[4]), + expected + ) + + func = dll._testfunc_array_in_struct_set_defaults_3D + func.restype = Test3D2 + result = func() + # check the default values have been set properly + self.assertEqual( + (result.data[0], + result.data[1], + result.data[2], + result.data[3], + result.data[4]), + expected + ) + + func = dll._testfunc_array_in_struct_set_defaults_3D + func.restype = Test3D3 + result = func() + # check the default values have been set properly + self.assertEqual( + (result.x, + result.y, + result.z, + result.t, + result.u), + expected) + def test_38368(self): class U(Union): _fields_ = [ diff --git a/Misc/NEWS.d/next/Library/2023-12-01-18-05-09.gh-issue-110190.5bf-c9.rst b/Misc/NEWS.d/next/Library/2023-12-01-18-05-09.gh-issue-110190.5bf-c9.rst new file mode 100644 index 00000000000000..730b9d49119805 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-12-01-18-05-09.gh-issue-110190.5bf-c9.rst @@ -0,0 +1 @@ +Fix ctypes structs with array on Arm platform by setting ``MAX_STRUCT_SIZE`` to 32 in stgdict. Patch by Diego Russo. diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index ddfb2c8a332a9e..15a9efa60ef545 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -133,6 +133,42 @@ _testfunc_array_in_struct2a(Test3B in) return result; } +/* + * See gh-110190. structs containing arrays of up to four floating point types + * (max 32 bytes) are passed in registers on Arm. + */ + +typedef struct { + double data[4]; +} Test3C; + +EXPORT(Test3C) +_testfunc_array_in_struct_set_defaults_3C(void) +{ + Test3C s; + s.data[0] = 1.0; + s.data[1] = 2.0; + s.data[2] = 3.0; + s.data[3] = 4.0; + return s; +} + +typedef struct { + double data[5]; +} Test3D; + +EXPORT(Test3D) +_testfunc_array_in_struct_set_defaults_3D(void) +{ + Test3D s; + s.data[0] = 1.0; + s.data[1] = 2.0; + s.data[2] = 3.0; + s.data[3] = 4.0; + s.data[4] = 5.0; + return s; +} + typedef union { long a_long; struct { diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index b1b2bac1455e67..d088d0cef5ff4b 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -696,29 +696,43 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct stgdict->align = total_align; stgdict->length = len; /* ADD ffi_ofs? */ -#define MAX_STRUCT_SIZE 16 +/* + * On Arm platforms, structs with at most 4 elements of any floating point + * type values can be passed through registers. If the type is double the + * maximum size of the struct is 32 bytes. + * By Arm platforms it is meant both 32 and 64-bit. +*/ +#if defined(__aarch64__) || defined(__arm__) +# define MAX_STRUCT_SIZE 32 +#else +# define MAX_STRUCT_SIZE 16 +#endif if (arrays_seen && (size <= MAX_STRUCT_SIZE)) { /* - * See bpo-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. + * See bpo-22273 and gh-110190. 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 x86_64 Linux and Arm platforms, 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 (will) 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. + * By small structures, we mean ones that are 16 bytes or less on + * x86-64 and 32 bytes or less on Arm. In that case, there can't be + * more than 16 or 32 elements after unrolling arrays, as we (will) + * 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. * - * Although the passing in registers is specific to 64-bit Linux, the - * array-in-struct vs. pointer problem is general. But we restrict the - * type transformation to small structs nonetheless. + * Although the passing in registers is specific to x86_64 Linux + * and Arm platforms, the array-in-struct vs. pointer problem is + * general. But we restrict the type transformation to small structs + * nonetheless. * * Note that although a union may be small in terms of memory usage, it * could contain many overlapping declarations of arrays, e.g. @@ -744,6 +758,9 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct * struct { uint_32 e1; uint_32 e2; ... uint_32 e_4; } f6; * } * + * The same principle applies for a struct 32 bytes in size like in + * the case of Arm platforms. + * * So the struct/union needs setting up as follows: all non-array * elements copied across as is, and all array elements replaced with * an equivalent struct which has as many fields as the array has