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.
This commit is contained in:
Diego Russo 2023-12-13 16:08:15 +00:00 committed by GitHub
parent 79dad03747
commit 6644ca45cd
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 230 additions and 228 deletions

View File

@ -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):
_fields_ = [
('data', c_double * 2),
]
class Test3A(Structure):
class Test3AParent(Structure):
_fields_ = [
('data', c_float * 2),
]
class Test3B(Test3A):
class Test3A(Test3AParent):
_fields_ = [
('more_data', c_float * 2),
]
# Load the shared library
dll = CDLL(_ctypes_test.__file__)
class Test3B(Structure):
_fields_ = [
('data', c_double * 2),
]
class Test3C(Structure):
_fields_ = [
("data", c_double * 4)
]
class Test3D(Structure):
_fields_ = [
("data", c_double * 8)
]
class Test3E(Structure):
_fields_ = [
("data", c_double * 9)
]
# 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"]
)
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),
]
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
)
for sut in structs_to_test:
s = sut.cls()
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)
# 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))
# 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)
# 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):

View File

@ -0,0 +1 @@
Fix ctypes structs with array on PPC64LE platform by setting ``MAX_STRUCT_SIZE`` to 64 in stgdict. Patch by Diego Russo.

View File

@ -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,79 +108,142 @@ _testfunc_array_in_struct1(Test2 in)
return result;
}
typedef struct {
double data[2];
} Test3;
typedef struct {
float data[2];
float more_data[2];
} Test3B;
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(double)
_testfunc_array_in_struct2a(Test3B in)
{
double result = 0;
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;
}
/*
* See gh-110190. structs containing arrays of up to four floating point types
* (max 32 bytes) are passed in registers on Arm.
* 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];
} Test3A;
EXPORT(double)
_testfunc_array_in_struct3A(Test3A in)
{
double result = 0;
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.
*/
memset(in.data, 0, sizeof(in.data));
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_struct3B(Test3B in)
{
_TESTFUNC_ARRAY_IN_STRUCT_IMPL
}
EXPORT(Test3B)
_testfunc_array_in_struct3B_set_defaults(void)
{
Test3B s;
_TESTFUNC_ARRAY_IN_STRUCT_SET_DEFAULTS_IMPL
}
/*
* 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(Test3C)
_testfunc_array_in_struct_set_defaults_3C(void)
EXPORT(double)
_testfunc_array_in_struct3C(Test3C in)
{
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_IMPL
}
EXPORT(Test3C)
_testfunc_array_in_struct3C_set_defaults(void)
{
Test3C 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 {

View File

@ -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.
*
* 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.
* 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
*
* 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