[Python-checkins] [3.6] bpo-30353: Fix pass by value for structs on 64-bit Cygwin/MinGW (GH-1559) (GH-5954)

[Ned Deily]

https://github.com/python/cpython/commit/2f3ba27185a369bcb6b36b13aa3518ffcc970ffa
commit: 2f3ba27185a369bcb6b36b13aa3518ffcc970ffa
branch: 3.6
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Ned Deily <nad at python.org>
date: 2018-03-08T10:28:53-05:00
summary:

[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 <erik.m.bray at gmail.com>

files:
A Misc/NEWS.d/next/Library/2018-03-08-09-54-01.bpo-30353.XdE5aM.rst
M Lib/ctypes/test/test_as_parameter.py
M Lib/ctypes/test/test_structures.py
M Modules/_ctypes/_ctypes_test.c
M Modules/_ctypes/callproc.c

diff --git a/Lib/ctypes/test/test_as_parameter.py b/Lib/ctypes/test/test_as_parameter.py
index 2a3484bec01b..a2640575a074 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 4ad5915ba27a..d1ea43bc7e3b 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/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 000000000000..ddb625c71652
--- /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 6119ecdaf90d..f6af49aea385 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 ff95dffb82c2..8edffabcdf09 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;