gh-110190: Fix ctypes structs with array on Arm (#112604)

Set MAX_STRUCT_SIZE to 32 in stgdict.c when on Arm platforms.
This because 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.
On x86-64 Linux, it's maximum 16 bytes hence we need to differentiate.

Author: diegorusso

Lib/test/test_ctypes/test_structures.py

@@ -2,7 +2,7 @@
 import struct
 import sys
 import unittest
-from ctypes import (CDLL, Structure, Union, POINTER, sizeof, byref, alignment,
+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,
@@ -494,12 +494,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,)
@@ -540,6 +587,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_ = [

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.

Modules/_ctypes/_ctypes_test.c

@@ -150,6 +150,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 {

Modules/_ctypes/stgdict.c

@@ -697,29 +697,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.
@@ -745,6 +759,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