From a59f6b3e8a86319413e029aaa419b502cfd6b478 Mon Sep 17 00:00:00 2001 From: "Erik M. Bray" Date: Fri, 12 May 2017 18:17:29 +0200 Subject: [PATCH 1/3] Fix pass by value for structs on 64-bit Cygwin/MinGW --- Modules/_ctypes/callproc.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 0b6faf96c68f6a..14d671611e0e13 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -1123,6 +1123,14 @@ PyObject *_ctypes_callproc(PPROC pProc, goto cleanup; /* leaking ? */ } } +#if defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__)) + void *tmp; + if (pa->ffi_type->type == FFI_TYPE_STRUCT) { + tmp = alloca(pa->ffi_type->size); + memcpy(tmp, pa->value.p, pa->ffi_type->size); + pa->value.p = tmp; + } +#endif } rtype = _ctypes_get_ffi_type(restype); From b60212fae86b664d3b64015c8b9c6e7e093db4eb Mon Sep 17 00:00:00 2001 From: "Erik M. Bray" Date: Fri, 19 May 2017 11:37:50 +0200 Subject: [PATCH 2/3] Apply this workaround in builds for the arm64 architecture, on which libffi has a similar bug; fixes https://bugs.python.org/issue29804 --- Modules/_ctypes/callproc.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 14d671611e0e13..5a07947d3369a9 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -1123,7 +1123,8 @@ PyObject *_ctypes_callproc(PPROC pProc, goto cleanup; /* leaking ? */ } } -#if defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__)) +#if (defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__))) || \ + defined(__aarch64__) void *tmp; if (pa->ffi_type->type == FFI_TYPE_STRUCT) { tmp = alloca(pa->ffi_type->size); From cec12bca51457d08ac25e078cbe5ca4cf2993d1a Mon Sep 17 00:00:00 2001 From: "Erik M. Bray" Date: Tue, 23 May 2017 16:02:02 +0200 Subject: [PATCH 3/3] Reworked after gaining a better understanding of what was going on. In fact, *any* struct (or other type such as long double) that does not fit in a 1/2/4/8 byte register should be passed by reference. Structs that do fit in a register are passed by value and we need to make sure this works properly too (it previously did not). --- Lib/ctypes/test/test_as_parameter.py | 4 ++++ Lib/ctypes/test/test_structures.py | 22 +++++++++++++++++++ Modules/_ctypes/_ctypes_test.c | 18 ++++++++++++++++ Modules/_ctypes/callproc.c | 32 ++++++++++++++++++---------- 4 files changed, 65 insertions(+), 11 deletions(-) diff --git a/Lib/ctypes/test/test_as_parameter.py b/Lib/ctypes/test/test_as_parameter.py index 2a3484bec01b99..a2640575a07452 100644 --- a/Lib/ctypes/test/test_as_parameter.py +++ b/Lib/ctypes/test/test_as_parameter.py @@ -169,6 +169,10 @@ class S2H(Structure): s2h = dll.ret_2h_func(self.wrap(inp)) self.assertEqual((s2h.x, s2h.y), (99*2, 88*3)) + # Test also that the original struct was unmodified (i.e. was passed by + # value) + self.assertEqual((inp.x, inp.y), (99, 88)) + def test_struct_return_8H(self): class S8I(Structure): _fields_ = [("a", c_int), diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index 2e778fb1b43740..d90c71144c9a9b 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -417,6 +417,28 @@ class X(Structure): self.assertEqual(s.second, 0xcafebabe) self.assertEqual(s.third, 0x0bad1dea) + def test_pass_by_value_in_register(self): + class X(Structure): + _fields_ = [ + ('first', c_uint), + ('second', c_uint) + ] + + s = X() + s.first = 0xdeadbeef + s.second = 0xcafebabe + dll = CDLL(_ctypes_test.__file__) + func = dll._testfunc_reg_struct_update_value + func.argtypes = (X,) + func.restype = None + func(s) + self.assertEqual(s.first, 0xdeadbeef) + self.assertEqual(s.second, 0xcafebabe) + got = X.in_dll(dll, "last_tfrsuv_arg") + self.assertEqual(s.first, got.first) + self.assertEqual(s.second, got.second) + + class PointerMemberTestCase(unittest.TestCase): def test(self): diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index fe0015c80136f3..2255e57339301b 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -57,6 +57,24 @@ _testfunc_large_struct_update_value(Test in) ((volatile Test *)&in)->third = 0x0badf00d; } +typedef struct { + unsigned int first; + unsigned int second; +} TestReg; + + +EXPORT(TestReg) last_tfrsuv_arg; + + +EXPORT(void) +_testfunc_reg_struct_update_value(TestReg in) +{ + last_tfrsuv_arg = in; + ((volatile TestReg *)&in)->first = 0x0badf00d; + ((volatile TestReg *)&in)->second = 0x0badf00d; +} + + EXPORT(void)testfunc_array(int values[4]) { printf("testfunc_array %d %d %d %d\n", diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c index 5a07947d3369a9..5439b939dc49c1 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -1039,6 +1039,13 @@ GetComError(HRESULT errcode, GUID *riid, IUnknown *pIunk) } #endif +#if (defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__))) || \ + defined(__aarch64__) +#define CTYPES_PASS_BY_REF_HACK +#define POW2(x) (((x & ~(x - 1)) == x) ? x : 0) +#define IS_PASS_BY_REF(x) (x > 8 || !POW2(x)) +#endif + /* * Requirements, must be ensured by the caller: * - argtuple is tuple of arguments @@ -1123,15 +1130,6 @@ PyObject *_ctypes_callproc(PPROC pProc, goto cleanup; /* leaking ? */ } } -#if (defined(__x86_64__) && (defined(__MINGW64__) || defined(__CYGWIN__))) || \ - defined(__aarch64__) - void *tmp; - if (pa->ffi_type->type == FFI_TYPE_STRUCT) { - tmp = alloca(pa->ffi_type->size); - memcpy(tmp, pa->value.p, pa->ffi_type->size); - pa->value.p = tmp; - } -#endif } rtype = _ctypes_get_ffi_type(restype); @@ -1145,8 +1143,20 @@ PyObject *_ctypes_callproc(PPROC pProc, } for (i = 0; i < argcount; ++i) { atypes[i] = args[i].ffi_type; - if (atypes[i]->type == FFI_TYPE_STRUCT - ) +#ifdef CTYPES_PASS_BY_REF_HACK + size_t size = atypes[i]->size; + if (IS_PASS_BY_REF(size)) { + void *tmp = alloca(size); + if (atypes[i]->type == FFI_TYPE_STRUCT) + memcpy(tmp, args[i].value.p, size); + else + memcpy(tmp, (void*)&args[i].value, size); + + avalues[i] = tmp; + } + else +#endif + if (atypes[i]->type == FFI_TYPE_STRUCT) avalues[i] = (void *)args[i].value.p; else avalues[i] = (void *)&args[i].value;