summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDiego Russo <diego.russo@arm.com>2023-12-06 15:57:42 (GMT)
committerGitHub <noreply@github.com>2023-12-06 15:57:42 (GMT)
commitb49c963e85f5a82822983c20be93790d71cbc408 (patch)
treec88e778cea8b2a4b4f8dd4501bc679703220ee76
parent010819a29552eca7020e497e86c7a0bc28bba46a (diff)
downloadcpython-b49c963e85f5a82822983c20be93790d71cbc408.zip
cpython-b49c963e85f5a82822983c20be93790d71cbc408.tar.gz
cpython-b49c963e85f5a82822983c20be93790d71cbc408.tar.bz2
[3.11] gh-110190: Fix ctypes structs with array on Arm (#112604) (#112766)
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. (cherry picked from commit bc68f4a4abcfbea60bb1db1ccadb07613561931c)
-rw-r--r--Lib/ctypes/test/test_structures.py127
-rw-r--r--Misc/NEWS.d/next/Library/2023-12-01-18-05-09.gh-issue-110190.5bf-c9.rst1
-rw-r--r--Modules/_ctypes/_ctypes_test.c36
-rw-r--r--Modules/_ctypes/stgdict.c53
4 files changed, 197 insertions, 20 deletions
diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py
index 2168aa7..972ff87 100644
--- a/Lib/ctypes/test/test_structures.py
+++ b/Lib/ctypes/test/test_structures.py
@@ -1,8 +1,12 @@
import platform
import sys
import unittest
-from ctypes import *
from ctypes.test 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 StructureTestCase(unittest.TestCase):
('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 StructureTestCase(unittest.TestCase):
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 0000000..730b9d4
--- /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 8a6a116..5f9ba0c 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 b7134ce..7746f95 100644
--- a/Modules/_ctypes/stgdict.c
+++ b/Modules/_ctypes/stgdict.c
@@ -662,29 +662,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.
@@ -710,6 +724,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