Issue #20160: broken ctypes calling convention on MSVC / 64-bit Windows (large structs) Patch by mattip

6 files changed, 104 insertions, 15 deletions

diff --git a/Lib/ctypes/test/test_win32.py b/Lib/ctypes/test/test_win32.py
index 93573dd..b90ab71 100644
--- a/Lib/ctypes/test/test_win32.py
+++ b/Lib/ctypes/test/test_win32.py
@@ -111,9 +111,29 @@ class Structures(unittest.TestCase):
 
         dll = CDLL(_ctypes_test.__file__)
 
-        pt = POINT(10, 10)
-        rect = RECT(0, 0, 20, 20)
-        self.assertEqual(1, dll.PointInRect(byref(rect), pt))
+        pt = POINT(15, 25)
+        left = c_long.in_dll(dll, 'left')
+        top = c_long.in_dll(dll, 'top')
+        right = c_long.in_dll(dll, 'right')
+        bottom = c_long.in_dll(dll, 'bottom')
+        rect = RECT(left, top, right, bottom)
+        PointInRect = dll.PointInRect
+        PointInRect.argtypes = [POINTER(RECT), POINT]
+        self.assertEqual(1, PointInRect(byref(rect), pt))
+
+        ReturnRect = dll.ReturnRect
+        ReturnRect.argtypes = [c_int, RECT, POINTER(RECT), POINT, RECT,
+                               POINTER(RECT), POINT, RECT]
+        ReturnRect.restype = RECT
+        for i in range(4):
+            ret = ReturnRect(i, rect, pointer(rect), pt, rect,
+                         byref(rect), pt, rect)
+            # the c function will check and modify ret if something is
+            # passed in improperly
+            self.assertEqual(ret.left, left.value)
+            self.assertEqual(ret.right, right.value)
+            self.assertEqual(ret.top, top.value)
+            self.assertEqual(ret.bottom, bottom.value)
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c
index 3a6845f..0c0d5b6 100644
--- a/Modules/_ctypes/_ctypes_test.c
+++ b/Modules/_ctypes/_ctypes_test.c
@@ -527,6 +527,49 @@ EXPORT(int) PointInRect(RECT *prc, POINT pt)
     return 1;
 }
 
+EXPORT(int left = 10);
+EXPORT(int top = 20);
+EXPORT(int right = 30);
+EXPORT(int bottom = 40);
+
+EXPORT(RECT) ReturnRect(int i, RECT ar, RECT* br, POINT cp, RECT dr,
+                        RECT *er, POINT fp, RECT gr)
+{
+    /*Check input */
+    if (ar.left + br->left + dr.left + er->left + gr.left != left * 5)
+    {
+        ar.left = 100;
+        return ar;
+    }
+    if (ar.right + br->right + dr.right + er->right + gr.right != right * 5)
+    {
+        ar.right = 100;
+        return ar;
+    }
+    if (cp.x != fp.x)
+    {
+        ar.left = -100;
+    }
+    if (cp.y != fp.y)
+    {
+        ar.left = -200;
+    }
+    switch(i)
+    {
+    case 0:
+        return ar;
+        break;
+    case 1:
+        return dr;
+        break;
+    case 2:
+        return gr;
+        break;
+
+    }
+    return ar;
+}
+
 typedef struct {
     short x;
     short y;
diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c
index 6cf358c..8daeffc 100644
--- a/Modules/_ctypes/callproc.c
+++ b/Modules/_ctypes/callproc.c
@@ -1140,9 +1140,6 @@ 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 _WIN64
-            && atypes[i]->size <= sizeof(void *)
-#endif
             )
             avalues[i] = (void *)args[i].value.p;
         else
diff --git a/Modules/_ctypes/libffi_msvc/ffi.c b/Modules/_ctypes/libffi_msvc/ffi.c
index 6e595e9..76cb03e 100644
--- a/Modules/_ctypes/libffi_msvc/ffi.c
+++ b/Modules/_ctypes/libffi_msvc/ffi.c
@@ -102,6 +102,15 @@ void ffi_prep_args(char *stack, extended_cif *ecif)
 	      FFI_ASSERT(0);
 	    }
 	}
+#ifdef _WIN64
+      else if (z > 8)
+        {
+          /* On Win64, if a single argument takes more than 8 bytes,
+             then it is always passed by reference. */
+          *(void **)argp = *p_argv;
+          z = 8;
+        }
+#endif
       else
 	{
 	  memcpy(argp, *p_argv, z);
@@ -124,7 +133,6 @@ ffi_status ffi_prep_cif_machdep(ffi_cif *cif)
   switch (cif->rtype->type)
     {
     case FFI_TYPE_VOID:
-    case FFI_TYPE_STRUCT:
     case FFI_TYPE_SINT64:
     case FFI_TYPE_FLOAT:
     case FFI_TYPE_DOUBLE:
@@ -132,6 +140,18 @@ ffi_status ffi_prep_cif_machdep(ffi_cif *cif)
       cif->flags = (unsigned) cif->rtype->type;
       break;
 
+    case FFI_TYPE_STRUCT:
+      /* MSVC returns small structures in registers.  Put in cif->flags
+         the value FFI_TYPE_STRUCT only if the structure is big enough;
+         otherwise, put the 4- or 8-bytes integer type. */
+      if (cif->rtype->size <= 4)
+        cif->flags = FFI_TYPE_INT;
+      else if (cif->rtype->size <= 8)
+        cif->flags = FFI_TYPE_SINT64;
+      else
+        cif->flags = FFI_TYPE_STRUCT;
+      break;
+
     case FFI_TYPE_UINT64:
 #ifdef _WIN64
     case FFI_TYPE_POINTER:
@@ -201,8 +221,7 @@ ffi_call(/*@dependent@*/ ffi_cif *cif,
 #else
     case FFI_SYSV:
       /*@-usedef@*/
-      /* Function call needs at least 40 bytes stack size, on win64 AMD64 */
-      return ffi_call_AMD64(ffi_prep_args, &ecif, cif->bytes ? cif->bytes : 40,
+      return ffi_call_AMD64(ffi_prep_args, &ecif, cif->bytes,
 			   cif->flags, ecif.rvalue, fn);
       /*@=usedef@*/
       break;
@@ -227,7 +246,7 @@ void *
 #else
 static void __fastcall
 #endif
-ffi_closure_SYSV (ffi_closure *closure, int *argp)
+ffi_closure_SYSV (ffi_closure *closure, char *argp)
 {
   // this is our return value storage
   long double    res;
@@ -237,7 +256,7 @@ ffi_closure_SYSV (ffi_closure *closure, int *argp)
   void         **arg_area;
   unsigned short rtype;
   void          *resp = (void*)&res;
-  void *args = &argp[1];
+  void *args = argp + sizeof(void*);
 
   cif         = closure->cif;
   arg_area    = (void**) alloca (cif->nargs * sizeof (void*));  
diff --git a/Modules/_ctypes/libffi_msvc/prep_cif.c b/Modules/_ctypes/libffi_msvc/prep_cif.c
index 2650fa0..b07a2e6 100644
--- a/Modules/_ctypes/libffi_msvc/prep_cif.c
+++ b/Modules/_ctypes/libffi_msvc/prep_cif.c
@@ -116,9 +116,9 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@partial@*/ ffi_cif *cif,
 #if !defined M68K && !defined __x86_64__ && !defined S390
   /* Make space for the return structure pointer */
   if (cif->rtype->type == FFI_TYPE_STRUCT
-      /* MSVC returns small structures in registers.  But we have a different
-      workaround: pretend int32 or int64 return type, and converting to
-      structure afterwards. */
+#ifdef _WIN32
+      && (cif->rtype->size > 8)  /* MSVC returns small structs in registers */
+#endif
 #ifdef SPARC
       && (cif->abi != FFI_V9 || cif->rtype->size > 32)
 #endif
@@ -145,6 +145,10 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@partial@*/ ffi_cif *cif,
 	      && cif->abi != FFI_V9))
 	bytes += sizeof(void*);
       else
+#elif defined (_WIN64)
+      if ((*ptr)->type == FFI_TYPE_STRUCT && ((*ptr)->size > 8))
+	    bytes += sizeof(void*);
+      else
 #endif
 	{
 #if !defined(_MSC_VER) && !defined(__MINGW32__)
@@ -168,6 +172,12 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@partial@*/ ffi_cif *cif,
 #endif
     }
 
+#ifdef _WIN64
+  /* Function call needs at least 40 bytes stack size, on win64 AMD64 */
+  if (bytes < 40)
+      bytes = 40;
+#endif
+
   cif->bytes = bytes;
 
   /* Perform machine dependent cif processing */
diff --git a/Modules/_ctypes/libffi_msvc/types.c b/Modules/_ctypes/libffi_msvc/types.c
index df32190..4433ac2 100644
--- a/Modules/_ctypes/libffi_msvc/types.c
+++ b/Modules/_ctypes/libffi_msvc/types.c
@@ -43,7 +43,7 @@ FFI_INTEGRAL_TYPEDEF(sint32, 4, 4, FFI_TYPE_SINT32);
 FFI_INTEGRAL_TYPEDEF(float, 4, 4, FFI_TYPE_FLOAT);
 
 #if defined ALPHA || defined SPARC64 || defined X86_64 || defined S390X \
-    || defined IA64
+    || defined IA64 || defined _WIN64
 
 FFI_INTEGRAL_TYPEDEF(pointer, 8, 8, FFI_TYPE_POINTER);