From 7843caeb909bd907e903606414e238db4082315a Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Sat, 15 Sep 2018 22:36:29 -0700 Subject: [PATCH] bpo-34603, ctypes/libffi_msvc: Fix returning structs from functions (GH-9258) --- Lib/ctypes/test/test_win32.py | 18 ++ .../2018-09-13-08-29-04.bpo-34603.2AB7sc.rst | 1 + Modules/_ctypes/_ctypes_test.c | 194 ++++++++++++++++++ Modules/_ctypes/callproc.c | 4 +- Modules/_ctypes/libffi_msvc/ffi.c | 19 +- Modules/_ctypes/libffi_msvc/ffi.h | 3 + Modules/_ctypes/libffi_msvc/prep_cif.c | 7 +- 7 files changed, 240 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Windows/2018-09-13-08-29-04.bpo-34603.2AB7sc.rst diff --git a/Lib/ctypes/test/test_win32.py b/Lib/ctypes/test/test_win32.py index 5d85ad6200b..ee722704a35 100644 --- 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 index 00000000000..86ae1cd0617 --- /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 620a3c6aea6..0152945ca1a 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -660,6 +660,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 bdc37281159..ba154fe61b6 100644 --- 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 91a27dce3f2..d202b158b07 100644 --- 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 efb14c5f6f3..ba74202720a 100644 --- 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 b07a2e6db27..022435e53fc 100644 --- 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