Commit a86339b8 authored by Vinay Sajip's avatar Vinay Sajip Committed by GitHub

Fixed bpo-29565: Corrected ctypes passing of large structs by value on Windows AMD64. (#168)

* Fixed bpo-29565: Corrected ctypes passing of large structs by value.

Added code and test to check that when a structure passed by value
is large enough to need to be passed by reference, a copy of the
original structure is passed. The callee updates the passed-in value,
and the test verifies that the caller's copy is unchanged. A similar
change was also added to the test added for bpo-20160 (that test was
passing, but the changes should guard against regressions).

* Reverted unintended whitespace changes.
parent 3eea8c67
...@@ -244,6 +244,7 @@ class SampleCallbacksTestCase(unittest.TestCase): ...@@ -244,6 +244,7 @@ class SampleCallbacksTestCase(unittest.TestCase):
def test_callback_large_struct(self): def test_callback_large_struct(self):
class Check: pass class Check: pass
# This should mirror the structure in Modules/_ctypes/_ctypes_test.c
class X(Structure): class X(Structure):
_fields_ = [ _fields_ = [
('first', c_ulong), ('first', c_ulong),
...@@ -255,6 +256,11 @@ class SampleCallbacksTestCase(unittest.TestCase): ...@@ -255,6 +256,11 @@ class SampleCallbacksTestCase(unittest.TestCase):
check.first = s.first check.first = s.first
check.second = s.second check.second = s.second
check.third = s.third check.third = s.third
# See issue #29565.
# The structure should be passed by value, so
# any changes to it should not be reflected in
# the value passed
s.first = s.second = s.third = 0x0badf00d
check = Check() check = Check()
s = X() s = X()
...@@ -275,6 +281,11 @@ class SampleCallbacksTestCase(unittest.TestCase): ...@@ -275,6 +281,11 @@ class SampleCallbacksTestCase(unittest.TestCase):
self.assertEqual(check.first, 0xdeadbeef) self.assertEqual(check.first, 0xdeadbeef)
self.assertEqual(check.second, 0xcafebabe) self.assertEqual(check.second, 0xcafebabe)
self.assertEqual(check.third, 0x0bad1dea) self.assertEqual(check.third, 0x0bad1dea)
# See issue #29565.
# Ensure that the original struct is unchanged.
self.assertEqual(s.first, check.first)
self.assertEqual(s.second, check.second)
self.assertEqual(s.third, check.third)
################################################################ ################################################################
......
...@@ -3,6 +3,7 @@ from ctypes import * ...@@ -3,6 +3,7 @@ from ctypes import *
from ctypes.test import need_symbol from ctypes.test import need_symbol
from struct import calcsize from struct import calcsize
import _testcapi import _testcapi
import _ctypes_test
class SubclassesTest(unittest.TestCase): class SubclassesTest(unittest.TestCase):
def test_subclass(self): def test_subclass(self):
...@@ -391,6 +392,28 @@ class StructureTestCase(unittest.TestCase): ...@@ -391,6 +392,28 @@ class StructureTestCase(unittest.TestCase):
(1, 0, 0, 0, 0, 0)) (1, 0, 0, 0, 0, 0))
self.assertRaises(TypeError, lambda: Z(1, 2, 3, 4, 5, 6, 7)) self.assertRaises(TypeError, lambda: Z(1, 2, 3, 4, 5, 6, 7))
def test_pass_by_value(self):
# This should mirror the structure in Modules/_ctypes/_ctypes_test.c
class X(Structure):
_fields_ = [
('first', c_ulong),
('second', c_ulong),
('third', c_ulong),
]
s = X()
s.first = 0xdeadbeef
s.second = 0xcafebabe
s.third = 0x0bad1dea
dll = CDLL(_ctypes_test.__file__)
func = dll._testfunc_large_struct_update_value
func.argtypes = (X,)
func.restype = None
func(s)
self.assertEqual(s.first, 0xdeadbeef)
self.assertEqual(s.second, 0xcafebabe)
self.assertEqual(s.third, 0x0bad1dea)
class PointerMemberTestCase(unittest.TestCase): class PointerMemberTestCase(unittest.TestCase):
def test(self): def test(self):
......
...@@ -44,6 +44,19 @@ _testfunc_cbk_large_struct(Test in, void (*func)(Test)) ...@@ -44,6 +44,19 @@ _testfunc_cbk_large_struct(Test in, void (*func)(Test))
func(in); func(in);
} }
/*
* See issue 29565. Update a structure passed by value;
* the caller should not see any change.
*/
EXPORT(void)
_testfunc_large_struct_update_value(Test in)
{
in.first = 0x0badf00d;
in.second = 0x0badf00d;
in.third = 0x0badf00d;
}
EXPORT(void)testfunc_array(int values[4]) EXPORT(void)testfunc_array(int values[4])
{ {
printf("testfunc_array %d %d %d %d\n", printf("testfunc_array %d %d %d %d\n",
......
...@@ -239,6 +239,16 @@ ffi_call(/*@dependent@*/ ffi_cif *cif, ...@@ -239,6 +239,16 @@ ffi_call(/*@dependent@*/ ffi_cif *cif,
break; break;
#else #else
case FFI_SYSV: case FFI_SYSV:
/* If a single argument takes more than 8 bytes,
then a copy is passed by reference. */
for (unsigned i = 0; i < cif->nargs; i++) {
size_t z = cif->arg_types[i]->size;
if (z > 8) {
void *temp = alloca(z);
memcpy(temp, avalue[i], z);
avalue[i] = temp;
}
}
/*@-usedef@*/ /*@-usedef@*/
return ffi_call_AMD64(ffi_prep_args, &ecif, cif->bytes, return ffi_call_AMD64(ffi_prep_args, &ecif, cif->bytes,
cif->flags, ecif.rvalue, fn); cif->flags, ecif.rvalue, fn);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment