From a5ff5e7fc841f6ff5568b50608c141abd4b6f7af Mon Sep 17 00:00:00 2001 From: Paul Monson Date: Mon, 3 Jun 2019 16:56:31 -0700 Subject: [PATCH 1/4] bpo-37140 restore union parameter passing code Linux --- Modules/_ctypes/_ctypes.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index f7513a3d74c4bc..199bc606aa8827 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -403,8 +403,9 @@ static PyCArgObject * StructUnionType_paramfunc(CDataObject *self) { PyCArgObject *parg; - CDataObject *copied_self; StgDictObject *stgdict; +#ifdef MS_WINDOWS + CDataObject *copied_self; if ((size_t)self->b_size > sizeof(void*)) { void *new_ptr = PyMem_Malloc(self->b_size); @@ -432,6 +433,24 @@ StructUnionType_paramfunc(CDataObject *self) parg->value.p = copied_self->b_ptr; parg->size = copied_self->b_size; parg->obj = (PyObject *)copied_self; +#else + parg = PyCArgObject_new(); + if (parg == NULL) + return NULL; + + parg->tag = 'V'; + stgdict = PyObject_stgdict((PyObject *)self); + assert(stgdict); /* Cannot be NULL for structure/union instances */ + parg->pffi_type = &stgdict->ffi_type_pointer; + /* For structure parameters (by value), parg->value doesn't contain the structure + data itself, instead parg->value.p *points* to the structure's data + See also _ctypes.c, function _call_function_pointer(). + */ + parg->value.p = self->b_ptr; + parg->size = self->b_size; + Py_INCREF(self); + parg->obj = (PyObject *)self; +#endif return parg; } From 533bc5a4e8ad39fb944eb2b51eaa60d3829431f6 Mon Sep 17 00:00:00 2001 From: Paul Monson Date: Tue, 4 Jun 2019 12:02:27 -0700 Subject: [PATCH 2/4] test for struct-by-value pass with unmanaged resource in struct --- Lib/ctypes/test/test_structures.py | 31 ++++++++++++++++++++++++++++++ Modules/_ctypes/_ctypes_test.c | 20 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index d1ea43bc7e3b6f..696e027c60142c 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -438,6 +438,37 @@ class X(Structure): self.assertEqual(s.first, got.first) self.assertEqual(s.second, got.second) + def test_pass_by_value_string_buffer(self): + dll = CDLL(_ctypes_test.__file__) + dll.my_strdup.restype = POINTER(c_char) + dll.my_free.restype = None + + # This should mirror the structure in Modules/_ctypes/_ctypes_test.c + class X(Structure): + _fields_ = [ + ('first', c_ulong), + ('second', c_ulong), + ('third', POINTER(c_char)), + ] + def __del__(self): + # Windows throws 'Windows fatal exception: access violation' + # on second call to __del__ + # causing this test to fail + dll.my_free(self.third) + + hello = b"Hello" + s = X() + s.first = 0xdeadbeef + s.second = 0xcafebabe + s.third = dll.my_strdup(hello) + func = dll._testfunc_large_struct_update_value_string_buffer + func.argtypes = (X,) + func.restype = None + func(s) + self.assertEqual(s.first, 0xdeadbeef) + self.assertEqual(s.second, 0xcafebabe) + # string is already freed by pass-by-value copy of struct 's' + #self.assertEqual(s.third[0], b'J') class PointerMemberTestCase(unittest.TestCase): diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index bae4976a08d31b..4316a0632535cf 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -57,6 +57,26 @@ _testfunc_large_struct_update_value(Test in) ((volatile Test *)&in)->third = 0x0badf00d; } +typedef struct { + unsigned long first; + unsigned long second; + char *third; +} TestStringBuffer; + +/* + * See issue 37140. Update a structure passed by value + * that contains a pointer to a string and a destructor; + * the pointer to the string should not be double-freed. + */ + +EXPORT(void) +_testfunc_large_struct_update_value_string_buffer(TestStringBuffer in) +{ + ((volatile TestStringBuffer *)&in)->first = 0x0badf00d; + ((volatile TestStringBuffer *)&in)->second = 0x0badf00d; + ((volatile TestStringBuffer *)&in)->third[0] = 'J'; +} + typedef struct { unsigned int first; unsigned int second; From 79390e14add77c4b593b472bc6bb3cf6c2bcf2f0 Mon Sep 17 00:00:00 2001 From: Paul Monson Date: Wed, 5 Jun 2019 11:18:58 -0700 Subject: [PATCH 3/4] skip test on Windows --- Lib/ctypes/test/test_structures.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index 696e027c60142c..cd9d1042e07def 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -4,6 +4,7 @@ from struct import calcsize import _ctypes_test import test.support +import sys class SubclassesTest(unittest.TestCase): def test_subclass(self): @@ -438,6 +439,7 @@ class X(Structure): self.assertEqual(s.first, got.first) self.assertEqual(s.second, got.second) + @unittest.skipIf(sys.platform == "win32", "pass-by-value copy causes exception") def test_pass_by_value_string_buffer(self): dll = CDLL(_ctypes_test.__file__) dll.my_strdup.restype = POINTER(c_char) From ba52bf9c7b567e3a96dacbcdb7a9ae8bc1e8280c Mon Sep 17 00:00:00 2001 From: Paul Monson Date: Thu, 11 Jul 2019 15:06:24 -0700 Subject: [PATCH 4/4] use PyTuple_Pack --- Lib/ctypes/test/test_structures.py | 2 +- Modules/_ctypes/_ctypes.c | 23 ++--------------------- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index cd9d1042e07def..2e0802e1fc3eaa 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -439,7 +439,7 @@ class X(Structure): self.assertEqual(s.first, got.first) self.assertEqual(s.second, got.second) - @unittest.skipIf(sys.platform == "win32", "pass-by-value copy causes exception") + # bpo-37140: ctypes change made clang fail to build def test_pass_by_value_string_buffer(self): dll = CDLL(_ctypes_test.__file__) dll.my_strdup.restype = POINTER(c_char) diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 199bc606aa8827..d9497f6557a3e2 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -403,9 +403,8 @@ static PyCArgObject * StructUnionType_paramfunc(CDataObject *self) { PyCArgObject *parg; - StgDictObject *stgdict; -#ifdef MS_WINDOWS CDataObject *copied_self; + StgDictObject *stgdict; if ((size_t)self->b_size > sizeof(void*)) { void *new_ptr = PyMem_Malloc(self->b_size); @@ -432,25 +431,7 @@ StructUnionType_paramfunc(CDataObject *self) parg->pffi_type = &stgdict->ffi_type_pointer; parg->value.p = copied_self->b_ptr; parg->size = copied_self->b_size; - parg->obj = (PyObject *)copied_self; -#else - parg = PyCArgObject_new(); - if (parg == NULL) - return NULL; - - parg->tag = 'V'; - stgdict = PyObject_stgdict((PyObject *)self); - assert(stgdict); /* Cannot be NULL for structure/union instances */ - parg->pffi_type = &stgdict->ffi_type_pointer; - /* For structure parameters (by value), parg->value doesn't contain the structure - data itself, instead parg->value.p *points* to the structure's data - See also _ctypes.c, function _call_function_pointer(). - */ - parg->value.p = self->b_ptr; - parg->size = self->b_size; - Py_INCREF(self); - parg->obj = (PyObject *)self; -#endif + parg->obj = PyTuple_Pack(2, self, copied_self); return parg; }