bpo-34603, ctypes/libffi_msvc: Fix returning structs from functions (GH-9258)

(cherry picked from commit 7843caeb909bd907e903606414e238db4082315a)

Co-authored-by: Vladimir Matveev <v2matveev@outlook.com>

diff --git a/Lib/ctypes/test/test_win32.py b/Lib/ctypes/test/test_win32.py
index 5d85ad6200b31997ef77f3caf7c9fec58b9b58a0..ee722704a35d7f5ae30a90d3f30bed76ac216611 100644 (file)
--- a/Lib/ctypes/test/test_win32.py
+++ b/Lib/ctypes/test/test_win32.py
@@ -54,6 +54,24 @@ class FunctionCallTestCase(unittest.TestCase):
         windll.user32.GetDesktopWindow()
 
 
+@unittest.skipUnless(sys.platform == "win32", 'Windows-specific test')
+class ReturnStructSizesTestCase(unittest.TestCase):
+    def test_sizes(self):
+        dll = CDLL(_ctypes_test.__file__)
+        for i in range(1, 11):
+            fields = [ (f"f{f}", c_char) for f in range(1, i + 1)]
+            class S(Structure):
+                _fields_ = fields
+            f = getattr(dll, f"TestSize{i}")
+            f.restype = S
+            res = f()
+            for i, f in enumerate(fields):
+                value = getattr(res, f[0])
+                expected = bytes([ord('a') + i])
+                self.assertEquals(value, expected)
+
+
+
 @unittest.skipUnless(sys.platform == "win32", 'Windows-specific test')
 class TestWintypes(unittest.TestCase):
     def test_HWND(self):

diff --git a/Misc/NEWS.d/next/Windows/2018-09-13-08-29-04.bpo-34603.2AB7sc.rst b/Misc/NEWS.d/next/Windows/2018-09-13-08-29-04.bpo-34603.2AB7sc.rst
new file mode 100644 (file)
index 0000000..86ae1cd
--- /dev/null
+++ b/Misc/NEWS.d/next/Windows/2018-09-13-08-29-04.bpo-34603.2AB7sc.rst
@@ -0,0 +1 @@
+Fix returning structs from functions produced by MSVC

diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c
index f6af49aea3850b7f7622ef11cd8289ff5617ac9b..6ffbeab8bf6329fb15205bd6dd23b5bb05bf71a5 100644 (file)
--- a/Modules/_ctypes/_ctypes_test.c
+++ b/Modules/_ctypes/_ctypes_test.c
@@ -662,6 +662,200 @@ EXPORT(void) TwoOutArgs(int a, int *pi, int b, int *pj)
     *pj += b;
 }
 
+#ifdef MS_WIN32
+
+typedef struct {
+    char f1;
+} Size1;
+
+typedef struct {
+    char f1;
+    char f2;
+} Size2;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+} Size3;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+} Size4;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+    char f5;
+} Size5;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+    char f5;
+    char f6;
+} Size6;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+    char f5;
+    char f6;
+    char f7;
+} Size7;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+    char f5;
+    char f6;
+    char f7;
+    char f8;
+} Size8;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+    char f5;
+    char f6;
+    char f7;
+    char f8;
+    char f9;
+} Size9;
+
+typedef struct {
+    char f1;
+    char f2;
+    char f3;
+    char f4;
+    char f5;
+    char f6;
+    char f7;
+    char f8;
+    char f9;
+    char f10;
+} Size10;
+
+EXPORT(Size1) TestSize1() {
+    Size1 f;
+    f.f1 = 'a';
+    return f;
+}
+
+EXPORT(Size2) TestSize2() {
+    Size2 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    return f;
+}
+
+EXPORT(Size3) TestSize3() {
+    Size3 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    return f;
+}
+
+EXPORT(Size4) TestSize4() {
+    Size4 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    return f;
+}
+
+EXPORT(Size5) TestSize5() {
+    Size5 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    f.f5 = 'e';
+    return f;
+}
+
+EXPORT(Size6) TestSize6() {
+    Size6 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    f.f5 = 'e';
+    f.f6 = 'f';
+    return f;
+}
+
+EXPORT(Size7) TestSize7() {
+    Size7 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    f.f5 = 'e';
+    f.f6 = 'f';
+    f.f7 = 'g';
+    return f;
+}
+
+EXPORT(Size8) TestSize8() {
+    Size8 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    f.f5 = 'e';
+    f.f6 = 'f';
+    f.f7 = 'g';
+    f.f8 = 'h';
+    return f;
+}
+
+EXPORT(Size9) TestSize9() {
+    Size9 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    f.f5 = 'e';
+    f.f6 = 'f';
+    f.f7 = 'g';
+    f.f8 = 'h';
+    f.f9 = 'i';
+    return f;
+}
+
+EXPORT(Size10) TestSize10() {
+    Size10 f;
+    f.f1 = 'a';
+    f.f2 = 'b';
+    f.f3 = 'c';
+    f.f4 = 'd';
+    f.f5 = 'e';
+    f.f6 = 'f';
+    f.f7 = 'g';
+    f.f8 = 'h';
+    f.f9 = 'i';
+    f.f10 = 'j';
+    return f;
+}
+
+#endif
+
 #ifdef MS_WIN32
 EXPORT(S2H) __stdcall s_ret_2h_func(S2H inp) { return ret_2h_func(inp); }
 EXPORT(S8I) __stdcall s_ret_8i_func(S8I inp) { return ret_8i_func(inp); }

diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c
index 8edffabcdf09ffb368285fa7852b4015f1eeebaa..b8c2c746a0feb7cbe46539c5a6c0181be24d061a 100644 (file)
--- a/Modules/_ctypes/callproc.c
+++ b/Modules/_ctypes/callproc.c
@@ -715,9 +715,9 @@ ffi_type *_ctypes_get_ffi_type(PyObject *obj)
        It returns small structures in registers
     */
     if (dict->ffi_type_pointer.type == FFI_TYPE_STRUCT) {
-        if (dict->ffi_type_pointer.size <= 4)
+        if (can_return_struct_as_int(dict->ffi_type_pointer.size))
             return &ffi_type_sint32;
-        else if (dict->ffi_type_pointer.size <= 8)
+        else if (can_return_struct_as_sint64 (dict->ffi_type_pointer.size))
             return &ffi_type_sint64;
     }
 #endif

diff --git a/Modules/_ctypes/libffi_msvc/ffi.c b/Modules/_ctypes/libffi_msvc/ffi.c
index 91a27dce3f25753676796fd6ae5a9ca9e5f170b3..d202b158b07902abcfa77eec5a21c8f92f3973ba 100644 (file)
--- a/Modules/_ctypes/libffi_msvc/ffi.c
+++ b/Modules/_ctypes/libffi_msvc/ffi.c
@@ -145,6 +145,21 @@ void ffi_prep_args(char *stack, extended_cif *ecif)
   return;
 }
 
+/*
+Per: https://msdn.microsoft.com/en-us/library/7572ztz4.aspx
+To be returned by value in RAX, user-defined types must have a length 
+of 1, 2, 4, 8, 16, 32, or 64 bits
+*/
+int can_return_struct_as_int(size_t s)
+{
+  return s == 1 || s == 2 || s == 4;
+}
+
+int can_return_struct_as_sint64(size_t s)
+{
+  return s == 8;
+}
+
 /* Perform machine dependent cif processing */
 ffi_status ffi_prep_cif_machdep(ffi_cif *cif)
 {
@@ -163,9 +178,9 @@ ffi_status ffi_prep_cif_machdep(ffi_cif *cif)
       /* 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)
+      if (can_return_struct_as_int(cif->rtype->size))
         cif->flags = FFI_TYPE_INT;
-      else if (cif->rtype->size <= 8)
+      else if (can_return_struct_as_sint64(cif->rtype->size))
         cif->flags = FFI_TYPE_SINT64;
       else
         cif->flags = FFI_TYPE_STRUCT;

diff --git a/Modules/_ctypes/libffi_msvc/ffi.h b/Modules/_ctypes/libffi_msvc/ffi.h
index efb14c5f6f3aa11cb28616fe72aa22d979bef0ed..ba74202720a652dd1144cc39c3efa74969dbad45 100644 (file)
--- a/Modules/_ctypes/libffi_msvc/ffi.h
+++ b/Modules/_ctypes/libffi_msvc/ffi.h
@@ -136,6 +136,9 @@ typedef struct _ffi_type
   /*@null@*/ struct _ffi_type **elements;
 } ffi_type;
 
+int can_return_struct_as_int(size_t);
+int can_return_struct_as_sint64(size_t);
+
 /* These are defined in types.c */
 extern ffi_type ffi_type_void;
 extern ffi_type ffi_type_uint8;

diff --git a/Modules/_ctypes/libffi_msvc/prep_cif.c b/Modules/_ctypes/libffi_msvc/prep_cif.c
index b07a2e6db27137b910efaae2ccf3e09ff2c44df6..022435e53fcd118130b5732fa84b15959e459c6c 100644 (file)
--- a/Modules/_ctypes/libffi_msvc/prep_cif.c
+++ b/Modules/_ctypes/libffi_msvc/prep_cif.c
@@ -117,7 +117,8 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@partial@*/ ffi_cif *cif,
   /* Make space for the return structure pointer */
   if (cif->rtype->type == FFI_TYPE_STRUCT
 #ifdef _WIN32
-      && (cif->rtype->size > 8)  /* MSVC returns small structs in registers */
+      && !can_return_struct_as_int(cif->rtype->size)  /* MSVC returns small structs in registers */
+      && !can_return_struct_as_sint64(cif->rtype->size)
 #endif
 #ifdef SPARC
       && (cif->abi != FFI_V9 || cif->rtype->size > 32)
@@ -146,7 +147,9 @@ ffi_status ffi_prep_cif(/*@out@*/ /*@partial@*/ ffi_cif *cif,
        bytes += sizeof(void*);
       else
 #elif defined (_WIN64)
-      if ((*ptr)->type == FFI_TYPE_STRUCT && ((*ptr)->size > 8))
+      if ((*ptr)->type == FFI_TYPE_STRUCT && 
+          !can_return_struct_as_int((*ptr)->size) &&
+          !can_return_struct_as_sint64((*ptr)->size))
            bytes += sizeof(void*);
       else
 #endif