classification
Title: python (x64) ctypes incorrectly pass structures parameter
Type: behavior Stage: resolved
Components: ctypes Versions: Python 3.3, Python 3.1, Python 3.2, Python 2.7
process
Status: closed Resolution: duplicate
Dependencies: Superseder: broken ctypes calling convention on MSVC / 64-bit Windows (large structs)
View: 20160
Assigned To: theller Nosy List: Abraham.Soedjito, amaury.forgeotdarc, belopolsky, cgohlke, jwhitecl, mattip, santoso.wijaya, theller, vladris
Priority: normal Keywords: patch

Created on 2011-04-12 15:39 by Abraham.Soedjito, last changed 2015-01-16 03:29 by berker.peksag. This issue is now closed.

Files
File name Uploaded Description Edit
test_issue11835.patch santoso.wijaya, 2011-04-12 18:46 from 2.7 review
issue11835_patch.diff vladris, 2011-08-14 20:46 Patch for this issue review
Messages (7)
msg133583 - (view) Author: Abraham Soedjito (Abraham.Soedjito) Date: 2011-04-12 15:39
void __cdecl foo(unsigned __int32 a,
                 unsigned __int32 b,
                 unsigned __int32 c,
                 unsigned __int32 d,
                 unsigned __int32 e,
                 unsigned __int32 f,
                 unsigned __int32 g);

struct myStruct
{
   unsigned __int32 a;
   unsigned __int32 b;
   unsigned __int32 c;
   unsigned __int32 d;
   unsigned __int32 e;
   unsigned __int32 f;
   unsigned __int32 g;
}
void __cdecl bar(myStruct s);

void __cdecl errorPassingParameter(myStruct s1,
                                   myStruct s2,
                                   unsigned __int32 x);    

Calling foo and bar from python completed successfully, calling errorParsingParameter resulted in stack corruption. It seems that python passed an extra pointer in the stack for s2.
msg133591 - (view) Author: Santoso Wijaya (santoso.wijaya) * Date: 2011-04-12 18:00
I can reproduce this with 2.7 and 3.2 64-bit builds on Windows:

D:\Temp\cdll\x64\Release>C:\Python32\python.exe
Python 3.2 (r32:88445, Feb 20 2011, 21:30:00) [MSC v.1500 64 bit (AMD64)] on win
32
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import *
>>> libcdll = CDLL('cdll')
>>> libcdll.foo(1, 2, 3, 4, 5, 6, 7)
a: 1; b: 2; c: 3; d: 4; e: 5; f: 6; g: 7
0
>>> class MyStruct(Structure):
...     _fields_ = [
... ('a', c_int32),
... ('b', c_int32),
... ('c', c_int32),
... ('d', c_int32),
... ('e', c_int32),
... ('f', c_int32),
... ('g', c_int32),
... ]
...
>>> s1 = MyStruct(1, 2, 3, 4, 5, 6, 7)
>>> s2 = MyStruct(0, 9, 8, 7, 6, 5, 4)
>>> libcdll.bar(s2)
s.a: 0; s.b: 9; s.c: 8; s.d: 7; s.e: 6; s.f: 5; s.g: 4
0
>>> libcdll.errorPassingParameter(s1, s2, 42)
s1.a: 1; s1.b: 2; s1.c: 3; s1.d: 4; s1.e: 5; s1.f: 6; s1.g: 7
s2.a: 28; s2.b: 0; s2.c: 851972; s2.d: 0; s2.e: 31545776; s2.f: 0; s2.g: 0
x: 39805032
0
>>>
msg133593 - (view) Author: Santoso Wijaya (santoso.wijaya) * Date: 2011-04-12 18:06
32-bit (2.6) is fine:

D:\Temp\cdll\Release>C:\Python26\python.exe
Python 2.6.6 (r266:84297, Aug 24 2010, 18:46:32) [MSC v.1500 32 bit (Intel)] on
win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from ctypes import *
>>> libcdll = CDLL('cdll')
>>> class MyStruct(Structure):
...     _fields_ = []
...     for name in ('a', 'b', 'c', 'd', 'e', 'f', 'g'):
...         _fields_.append((name, c_int32))
...
>>> MyStruct._fields_
[('a', <class 'ctypes.c_long'>), ('b', <class 'ctypes.c_long'>), ('c', <class 'c
types.c_long'>), ('d', <class 'ctypes.c_long'>), ('e', <class 'ctypes.c_long'>),
 ('f', <class 'ctypes.c_long'>), ('g', <class 'ctypes.c_long'>)]
>>> s1 = MyStruct(1, 2, 3, 4, 5, 6, 7)
>>> s2 = MyStruct(0, 9, 8, 7, 6, 5, 4)
>>> libcdll.errorPassingParameter(s1, s2, 42)
s1.a: 1; s1.b: 2; s1.c: 3; s1.d: 4; s1.e: 5; s1.f: 6; s1.g: 7
s2.a: 0; s2.b: 9; s2.c: 8; s2.d: 7; s2.e: 6; s2.f: 5; s2.g: 4
x: 42
0
>>>
msg133596 - (view) Author: Santoso Wijaya (santoso.wijaya) * Date: 2011-04-12 18:46
Attaching a ctypes unittest (against 2.7 branch) that will expose this bug in regrtest.
msg142083 - (view) Author: Vlad Riscutia (vladris) Date: 2011-08-14 20:46
Attached patch for this issue.

This only happens on MSVC x64 (I actually tired to repro on Arch Linux x64 before starting work on it and it didn't repro).

What happens is that MSVC on x64 always passes structures larger than 8 bytes by reference. See here: http://msdn.microsoft.com/en-us/library/ms235286(v=vs.90).aspx

Now this was accounted for in callproc.c, line 1143 in development branch with this:

        if (atypes[i]->type == FFI_TYPE_STRUCT
#ifdef _WIN64
            && atypes[i]->size <= sizeof(void *)
#endif
            )
            avalues[i] = (void *)args[i].value.p;
        else
            avalues[i] = (void *)&args[i].value;

This fix wasn't made in libffi_msvc/ffi.c though. Here, regardless of whether we have x64 or x86 build, if z >= sizeof(int) we will hit else branch in libffi_msvc/ffi.c at line 114 and do:

      else
	{
	  memcpy(argp, *p_argv, z);
	}
      p_argv++;
      argp += z;

In our case, we copy 28 bytes as arguments (size of our structure) but in fact for x64 we only need 8 as structure is passed by reference so argument is just a pointer. My patch will adjust z before hitting if statement on x64 and it will cause correct copy as pointer.
msg142092 - (view) Author: Vlad Riscutia (vladris) Date: 2011-08-15 01:58
Changing type to behavior as it doesn't crash on 3.3. I believe issue was opened against 2.6 and Santoso changed it to 2.7 and up where there is no crash.

Another data point: there is similar fix in current version of libffi here: https://github.com/atgreen/libffi/blob/master/.pc/win64-struct-args/src/x86/ffi.c

Since at the moment we are not integrating new libffi, I believe my fix should do (libffi fix is slightly different but I'm matching what we have in callproc.c which is not part of libffi).
msg234094 - (view) Author: mattip (mattip) * Date: 2015-01-15 18:40
This is a duplicate of issue 20160, and has been fixed. Can someone mark it as a duplicate and close it?
History
Date User Action Args
2015-01-16 03:29:48berker.peksagsetstatus: open -> closed
superseder: broken ctypes calling convention on MSVC / 64-bit Windows (large structs)
resolution: duplicate
stage: resolved
2015-01-15 18:40:38mattipsetnosy: + mattip
messages: + msg234094
2011-08-15 01:58:25vladrissettype: crash -> behavior
messages: + msg142092
2011-08-14 21:42:57skrahsetnosy: + amaury.forgeotdarc, belopolsky
2011-08-14 20:46:20vladrissetfiles: + issue11835_patch.diff
nosy: + vladris
messages: + msg142083

2011-04-18 08:23:51cgohlkesetnosy: + cgohlke
2011-04-12 18:46:34santoso.wijayasetfiles: + test_issue11835.patch
keywords: + patch
messages: + msg133596
2011-04-12 18:19:37jwhiteclsetnosy: + jwhitecl
2011-04-12 18:06:30santoso.wijayasetmessages: + msg133593
2011-04-12 18:00:32santoso.wijayasetmessages: + msg133591
versions: + Python 3.1, Python 2.7, Python 3.2, Python 3.3, - Python 2.6
2011-04-12 16:20:08santoso.wijayasetnosy: + santoso.wijaya
2011-04-12 15:39:25Abraham.Soedjitocreate