summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDiego Russo <diego.russo@arm.com>2023-12-13 16:08:15 (GMT)
committerGitHub <noreply@github.com>2023-12-13 16:08:15 (GMT)
commit6644ca45cde9ca1b80513a90dacccfeea2d98620 (patch)
tree4029519873aff47a763094ea153a519bfeb27d59
parent79dad03747fe17634136209f1bcaf346a8c10617 (diff)
downloadcpython-6644ca45cde9ca1b80513a90dacccfeea2d98620.zip
cpython-6644ca45cde9ca1b80513a90dacccfeea2d98620.tar.gz
cpython-6644ca45cde9ca1b80513a90dacccfeea2d98620.tar.bz2
gh-110190: Fix ctypes structs with array on PPCLE64 (GH-112959)
Fix the same issue of PR #112604 on PPC64LE platform Refactor tests to make easier to add more platfroms if needed.
-rw-r--r--Lib/test/test_ctypes/test_structures.py232
-rw-r--r--Misc/NEWS.d/next/Library/2023-12-11-14-12-46.gh-issue-110190.e0iEUa.rst1
-rw-r--r--Modules/_ctypes/_ctypes_test.c138
-rw-r--r--Modules/_ctypes/stgdict.c45
4 files changed, 209 insertions, 207 deletions
diff --git a/Lib/test/test_ctypes/test_structures.py b/Lib/test/test_ctypes/test_structures.py
index 57ae924..21039f0 100644
--- a/Lib/test/test_ctypes/test_structures.py
+++ b/Lib/test/test_ctypes/test_structures.py
@@ -9,6 +9,7 @@ from ctypes import (CDLL, Array, Structure, Union, POINTER, sizeof, byref, align
c_short, c_ushort, c_int, c_uint,
c_long, c_ulong, c_longlong, c_ulonglong, c_float, c_double)
from struct import calcsize
+from collections import namedtuple
from test import support
@@ -474,36 +475,53 @@ class StructureTestCase(unittest.TestCase):
def test_array_in_struct(self):
# See bpo-22273
+ # Load the shared library
+ dll = CDLL(_ctypes_test.__file__)
+
# These should mirror the structures in Modules/_ctypes/_ctypes_test.c
class Test2(Structure):
_fields_ = [
('data', c_ubyte * 16),
]
- class Test3(Structure):
+ class Test3AParent(Structure):
+ _fields_ = [
+ ('data', c_float * 2),
+ ]
+
+ class Test3A(Test3AParent):
+ _fields_ = [
+ ('more_data', c_float * 2),
+ ]
+
+ class Test3B(Structure):
_fields_ = [
('data', c_double * 2),
]
- class Test3A(Structure):
+ class Test3C(Structure):
_fields_ = [
- ('data', c_float * 2),
+ ("data", c_double * 4)
]
- class Test3B(Test3A):
+ class Test3D(Structure):
_fields_ = [
- ('more_data', c_float * 2),
+ ("data", c_double * 8)
+ ]
+
+ class Test3E(Structure):
+ _fields_ = [
+ ("data", c_double * 9)
]
- # Load the shared library
- dll = CDLL(_ctypes_test.__file__)
+ # Tests for struct Test2
s = Test2()
expected = 0
for i in range(16):
s.data[i] = i
expected += i
- func = dll._testfunc_array_in_struct1
+ func = dll._testfunc_array_in_struct2
func.restype = c_int
func.argtypes = (Test2,)
result = func(s)
@@ -512,29 +530,16 @@ class StructureTestCase(unittest.TestCase):
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)
-
- s = Test3B()
+ # Tests for struct Test3A
+ s = Test3A()
s.data[0] = 3.14159
s.data[1] = 2.71828
s.more_data[0] = -3.0
s.more_data[1] = -2.0
-
- expected = 3.14159 + 2.71828 - 5.0
- func = dll._testfunc_array_in_struct2a
+ expected = 3.14159 + 2.71828 - 3.0 - 2.0
+ func = dll._testfunc_array_in_struct3A
func.restype = c_double
- func.argtypes = (Test3B,)
+ func.argtypes = (Test3A,)
result = func(s)
self.assertAlmostEqual(result, expected, places=6)
# check the passed-in struct hasn't changed
@@ -543,129 +548,60 @@ class StructureTestCase(unittest.TestCase):
self.assertAlmostEqual(s.more_data[0], -3.0, places=6)
self.assertAlmostEqual(s.more_data[1], -2.0, places=6)
- @unittest.skipIf(
- 'ppc64le' in platform.uname().machine,
- "gh-110190: currently fails on ppc64le",
- )
- def test_array_in_struct_registers(self):
- dll = CDLL(_ctypes_test.__file__)
-
- 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)
- ]
-
- # 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
+ # Test3B, Test3C, Test3D, Test3E have the same logic with different
+ # sizes hence putting them in a loop.
+ StructCtype = namedtuple(
+ "StructCtype",
+ ["cls", "cfunc1", "cfunc2", "items"]
)
-
- 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)
+ structs_to_test = [
+ StructCtype(
+ Test3B,
+ dll._testfunc_array_in_struct3B,
+ dll._testfunc_array_in_struct3B_set_defaults,
+ 2),
+ StructCtype(
+ Test3C,
+ dll._testfunc_array_in_struct3C,
+ dll._testfunc_array_in_struct3C_set_defaults,
+ 4),
+ StructCtype(
+ Test3D,
+ dll._testfunc_array_in_struct3D,
+ dll._testfunc_array_in_struct3D_set_defaults,
+ 8),
+ StructCtype(
+ Test3E,
+ dll._testfunc_array_in_struct3E,
+ dll._testfunc_array_in_struct3E_set_defaults,
+ 9),
+ ]
+
+ for sut in structs_to_test:
+ s = sut.cls()
+
+ # Test for cfunc1
+ expected = 0
+ for i in range(sut.items):
+ float_i = float(i)
+ s.data[i] = float_i
+ expected += float_i
+ func = sut.cfunc1
+ func.restype = c_double
+ func.argtypes = (sut.cls,)
+ result = func(s)
+ self.assertEqual(result, expected)
+ # check the passed-in struct hasn't changed
+ for i in range(sut.items):
+ self.assertEqual(s.data[i], float(i))
+
+ # Test for cfunc2
+ func = sut.cfunc2
+ func.restype = sut.cls
+ result = func()
+ # check if the default values have been set correctly
+ for i in range(sut.items):
+ self.assertEqual(result.data[i], float(i+1))
def test_38368(self):
class U(Union):
diff --git a/Misc/NEWS.d/next/Library/2023-12-11-14-12-46.gh-issue-110190.e0iEUa.rst b/Misc/NEWS.d/next/Library/2023-12-11-14-12-46.gh-issue-110190.e0iEUa.rst
new file mode 100644
index 0000000..3bfed1e
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-12-11-14-12-46.gh-issue-110190.e0iEUa.rst
@@ -0,0 +1 @@
+Fix ctypes structs with array on PPC64LE platform by setting ``MAX_STRUCT_SIZE`` to 64 in stgdict. Patch by Diego Russo.
diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c
index 2681b9c..ecc6041 100644
--- a/Modules/_ctypes/_ctypes_test.c
+++ b/Modules/_ctypes/_ctypes_test.c
@@ -96,11 +96,10 @@ typedef struct {
} Test2;
EXPORT(int)
-_testfunc_array_in_struct1(Test2 in)
+_testfunc_array_in_struct2(Test2 in)
{
int result = 0;
-
- for (unsigned i = 0; i < 16; i++)
+ for (unsigned i = 0; i < sizeof(in.data)/sizeof(in.data[0]); i++)
result += in.data[i];
/* As the structure is passed by value, changes to it shouldn't be
* reflected in the caller.
@@ -109,22 +108,25 @@ _testfunc_array_in_struct1(Test2 in)
return result;
}
-typedef struct {
- double data[2];
-} Test3;
-
+/*
+ * Test3A struct test the MAX_STRUCT_SIZE 16 with single precision floats.
+ * These structs should be passed via registers on all platforms and they are
+ * used for within bounds tests.
+ */
typedef struct {
float data[2];
float more_data[2];
-} Test3B;
+} Test3A;
EXPORT(double)
-_testfunc_array_in_struct2(Test3 in)
+_testfunc_array_in_struct3A(Test3A in)
{
double result = 0;
- for (unsigned i = 0; i < 2; i++)
+ for (unsigned i = 0; i < sizeof(in.data)/sizeof(in.data[0]); i++)
result += in.data[i];
+ for (unsigned i = 0; i < sizeof(in.more_data)/sizeof(in.more_data[0]); i++)
+ result += in.more_data[i];
/* As the structure is passed by value, changes to it shouldn't be
* reflected in the caller.
*/
@@ -132,56 +134,116 @@ _testfunc_array_in_struct2(Test3 in)
return result;
}
+/* The structs Test3B..Test3E use the same functions hence using the MACRO
+ * to define their implementation.
+ */
+
+#define _TESTFUNC_ARRAY_IN_STRUCT_IMPL \
+ double result = 0; \
+ \
+ for (unsigned i = 0; i < sizeof(in.data)/sizeof(in.data[0]); 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; \
+
+#define _TESTFUNC_ARRAY_IN_STRUCT_SET_DEFAULTS_IMPL \
+ for (unsigned i = 0; i < sizeof(s.data)/sizeof(s.data[0]); i++) \
+ s.data[i] = (double)i+1; \
+ return s; \
+
+
+/*
+ * Test3B struct test the MAX_STRUCT_SIZE 16 with double precision floats.
+ * These structs should be passed via registers on all platforms and they are
+ * used for within bounds tests.
+ */
+typedef struct {
+ double data[2];
+} Test3B;
+
EXPORT(double)
-_testfunc_array_in_struct2a(Test3B in)
+_testfunc_array_in_struct3B(Test3B in)
{
- double result = 0;
+ _TESTFUNC_ARRAY_IN_STRUCT_IMPL
+}
- for (unsigned i = 0; i < 2; i++)
- result += in.data[i];
- for (unsigned i = 0; i < 2; i++)
- result += in.more_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(Test3B)
+_testfunc_array_in_struct3B_set_defaults(void)
+{
+ Test3B s;
+ _TESTFUNC_ARRAY_IN_STRUCT_SET_DEFAULTS_IMPL
}
/*
- * See gh-110190. structs containing arrays of up to four floating point types
- * (max 32 bytes) are passed in registers on Arm.
+ * Test3C struct tests the MAX_STRUCT_SIZE 32. Structs containing arrays of up
+ * to four floating point types are passed in registers on Arm platforms.
+ * This struct is used for within bounds test on Arm platfroms and for an
+ * out-of-bounds tests for platfroms where MAX_STRUCT_SIZE is less than 32.
+ * See gh-110190.
*/
-
typedef struct {
double data[4];
} Test3C;
+EXPORT(double)
+_testfunc_array_in_struct3C(Test3C in)
+{
+ _TESTFUNC_ARRAY_IN_STRUCT_IMPL
+}
+
EXPORT(Test3C)
-_testfunc_array_in_struct_set_defaults_3C(void)
+_testfunc_array_in_struct3C_set_defaults(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;
+ _TESTFUNC_ARRAY_IN_STRUCT_SET_DEFAULTS_IMPL
}
+/*
+ * Test3D struct tests the MAX_STRUCT_SIZE 64. Structs containing arrays of up
+ * to eight floating point types are passed in registers on PPC64LE platforms.
+ * This struct is used for within bounds test on PPC64LE platfroms and for an
+ * out-of-bounds tests for platfroms where MAX_STRUCT_SIZE is less than 64.
+ * See gh-110190.
+ */
typedef struct {
- double data[5];
+ double data[8];
} Test3D;
+EXPORT(double)
+_testfunc_array_in_struct3D(Test3D in)
+{
+ _TESTFUNC_ARRAY_IN_STRUCT_IMPL
+}
+
EXPORT(Test3D)
-_testfunc_array_in_struct_set_defaults_3D(void)
+_testfunc_array_in_struct3D_set_defaults(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;
+ _TESTFUNC_ARRAY_IN_STRUCT_SET_DEFAULTS_IMPL
+}
+
+/*
+ * Test3E struct tests the out-of-bounds for all architectures.
+ * See gh-110190.
+ */
+typedef struct {
+ double data[9];
+} Test3E;
+
+EXPORT(double)
+_testfunc_array_in_struct3E(Test3E in)
+{
+ _TESTFUNC_ARRAY_IN_STRUCT_IMPL
+}
+
+EXPORT(Test3E)
+_testfunc_array_in_struct3E_set_defaults(void)
+{
+ Test3E s;
+ _TESTFUNC_ARRAY_IN_STRUCT_SET_DEFAULTS_IMPL
}
typedef union {
diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c
index 04dd9ba..dfdb96b 100644
--- a/Modules/_ctypes/stgdict.c
+++ b/Modules/_ctypes/stgdict.c
@@ -698,13 +698,12 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
stgdict->length = len; /* ADD ffi_ofs? */
/*
- * 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.
-*/
+ * The value of MAX_STRUCT_SIZE depends on the platform Python is running on.
+ */
#if defined(__aarch64__) || defined(__arm__)
# define MAX_STRUCT_SIZE 32
+#elif defined(__powerpc64__)
+# define MAX_STRUCT_SIZE 64
#else
# define MAX_STRUCT_SIZE 16
#endif
@@ -715,24 +714,29 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
* 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.
+ * 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.
+ *
+ * Small structures have different sizes depending on the platform
+ * where Python is running on:
+ *
+ * * x86-64: 16 bytes or less
+ * * Arm platforms (both 32 and 64 bit): 32 bytes or less
+ * * PowerPC 64 Little Endian: 64 bytes or less
*
- * 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.
+ * In that case, there can't be more than 16, 32 or 64 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 x86_64 Linux
- * and Arm platforms, the array-in-struct vs. pointer problem is
- * general. But we restrict the type transformation to small structs
+ * Although the passing in registers is specific to the above
+ * 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
@@ -759,8 +763,7 @@ 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.
+ * The same principle applies for a struct 32 or 64 bytes in size.
*
* So the struct/union needs setting up as follows: all non-array
* elements copied across as is, and all array elements replaced with