https://github.com/python/cpython/commit/3243f8c1fb16b6de73f1d7a30f5d0904755... commit: 3243f8c1fb16b6de73f1d7a30f5d09047553bce3 branch: 2.7 author: Victor Stinner <vstinner@redhat.com> committer: GitHub <noreply@github.com> date: 2018-08-02T16:47:26+02:00 summary: bpo-29565: Corrected ctypes passing of large structs by value on Windows AMD64 (GH-168) (GH-8625) Fixed bpo-29565: Corrected ctypes passing of large structs by value. Added code and test to check that when a structure passed by value is large enough to need to be passed by reference, a copy of the original structure is passed. The callee updates the passed-in value, and the test verifies that the caller's copy is unchanged. A similar change was also added to the test added for bpo-20160 (that test was passing, but the changes should guard against regressions). (cherry picked from commit a86339b83fbd0932e0529a3c91935e997a234582) files: M Lib/ctypes/test/test_callbacks.py M Lib/ctypes/test/test_structures.py M Modules/_ctypes/_ctypes_test.c M Modules/_ctypes/libffi_msvc/ffi.c diff --git a/Lib/ctypes/test/test_callbacks.py b/Lib/ctypes/test/test_callbacks.py index bf894d481c3e..db3d9e7b198b 100644 --- a/Lib/ctypes/test/test_callbacks.py +++ b/Lib/ctypes/test/test_callbacks.py @@ -250,6 +250,7 @@ def callback(a, b, c, d, e): def test_callback_large_struct(self): class Check: pass + # This should mirror the structure in Modules/_ctypes/_ctypes_test.c class X(Structure): _fields_ = [ ('first', c_ulong), @@ -261,6 +262,11 @@ def callback(check, s): check.first = s.first check.second = s.second check.third = s.third + # See issue #29565. + # The structure should be passed by value, so + # any changes to it should not be reflected in + # the value passed + s.first = s.second = s.third = 0x0badf00d check = Check() s = X() @@ -281,6 +287,11 @@ def callback(check, s): self.assertEqual(check.first, 0xdeadbeef) self.assertEqual(check.second, 0xcafebabe) self.assertEqual(check.third, 0x0bad1dea) + # See issue #29565. + # Ensure that the original struct is unchanged. + self.assertEqual(s.first, check.first) + self.assertEqual(s.second, check.second) + self.assertEqual(s.third, check.third) ################################################################ diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index 5650189f8a89..9a863c94dafc 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -3,6 +3,7 @@ from ctypes.test import need_symbol from struct import calcsize import _testcapi +import _ctypes_test class SubclassesTest(unittest.TestCase): def test_subclass(self): @@ -401,6 +402,28 @@ class Z(Y): (1, 0, 0, 0, 0, 0)) self.assertRaises(TypeError, lambda: Z(1, 2, 3, 4, 5, 6, 7)) + def test_pass_by_value(self): + # This should mirror the structure in Modules/_ctypes/_ctypes_test.c + class X(Structure): + _fields_ = [ + ('first', c_ulong), + ('second', c_ulong), + ('third', c_ulong), + ] + + s = X() + s.first = 0xdeadbeef + s.second = 0xcafebabe + s.third = 0x0bad1dea + dll = CDLL(_ctypes_test.__file__) + func = dll._testfunc_large_struct_update_value + func.argtypes = (X,) + func.restype = None + func(s) + self.assertEqual(s.first, 0xdeadbeef) + self.assertEqual(s.second, 0xcafebabe) + self.assertEqual(s.third, 0x0bad1dea) + class PointerMemberTestCase(unittest.TestCase): def test(self): diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index f295c6fc4c69..94678f3189ce 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -52,6 +52,19 @@ _testfunc_cbk_large_struct(Test in, void (*func)(Test)) func(in); } +/* + * See issue 29565. Update a structure passed by value; + * the caller should not see any change. + */ + +EXPORT(void) +_testfunc_large_struct_update_value(Test in) +{ + in.first = 0x0badf00d; + in.second = 0x0badf00d; + in.third = 0x0badf00d; +} + EXPORT(void)testfunc_array(int values[4]) { printf("testfunc_array %d %d %d %d\n", diff --git a/Modules/_ctypes/libffi_msvc/ffi.c b/Modules/_ctypes/libffi_msvc/ffi.c index 515d802fd894..75dada56133d 100644 --- a/Modules/_ctypes/libffi_msvc/ffi.c +++ b/Modules/_ctypes/libffi_msvc/ffi.c @@ -220,6 +220,16 @@ ffi_call(/*@dependent@*/ ffi_cif *cif, break; #else case FFI_SYSV: + /* If a single argument takes more than 8 bytes, + then a copy is passed by reference. */ + for (unsigned i = 0; i < cif->nargs; i++) { + size_t z = cif->arg_types[i]->size; + if (z > 8) { + void *temp = alloca(z); + memcpy(temp, avalue[i], z); + avalue[i] = temp; + } + } /*@-usedef@*/ return ffi_call_AMD64(ffi_prep_args, &ecif, cif->bytes, cif->flags, ecif.rvalue, fn);