From 2f3ba27185a369bcb6b36b13aa3518ffcc970ffa Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Thu, 8 Mar 2018 07:28:53 -0800 Subject: [PATCH] [3.6] bpo-30353: Fix pass by value for structs on 64-bit Cygwin/MinGW (GH-1559) (GH-5954) (cherry picked from commit 9ba3aa4d02a110d1a1ea464a8aff3be7dd9c63c3) Co-authored-by: Erik Bray --- Lib/ctypes/test/test_as_parameter.py | 4 ++++ Lib/ctypes/test/test_structures.py | 22 ++++++++++++++++++ .../2018-03-08-09-54-01.bpo-30353.XdE5aM.rst | 1 + Modules/_ctypes/_ctypes_test.c | 18 +++++++++++++++ Modules/_ctypes/callproc.c | 23 +++++++++++++++++-- 5 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-03-08-09-54-01.bpo-30353.XdE5aM.rst diff --git a/Lib/ctypes/test/test_as_parameter.py b/Lib/ctypes/test/test_as_parameter.py index 2a3484bec01..a2640575a07 100644 --- a/Lib/ctypes/test/test_as_parameter.py +++ b/Lib/ctypes/test/test_as_parameter.py @@ -169,6 +169,10 @@ class BasicWrapTestCase(unittest.TestCase): 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 4ad5915ba27..d1ea43bc7e3 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -417,6 +417,28 @@ class StructureTestCase(unittest.TestCase): 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/Misc/NEWS.d/next/Library/2018-03-08-09-54-01.bpo-30353.XdE5aM.rst b/Misc/NEWS.d/next/Library/2018-03-08-09-54-01.bpo-30353.XdE5aM.rst new file mode 100644 index 00000000000..ddb625c7165 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-03-08-09-54-01.bpo-30353.XdE5aM.rst @@ -0,0 +1 @@ +Fix ctypes pass-by-value for structs on 64-bit Cygwin/MinGW. diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index 6119ecdaf90..f6af49aea38 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 ff95dffb82c..8edffabcdf0 100644 --- a/Modules/_ctypes/callproc.c +++ b/Modules/_ctypes/callproc.c @@ -1040,6 +1040,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 @@ -1137,8 +1144,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;