https://github.com/python/cpython/commit/91c15a542cb780377dcde8fc17ba93111bc... commit: 91c15a542cb780377dcde8fc17ba93111bc1d1cf branch: 3.7 author: Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> committer: Vinay Sajip <vinay_sajip@yahoo.co.uk> date: 2019-11-21T21:47:22Z summary: [3.7] bpo-16576: Add checks for bitfields passed by value to functions. (GH-17097) (GH-17224) (cherry picked from commit 106271568c58cfebae58f0c52b640dbe716ba2ce) files: M Lib/ctypes/test/test_structures.py M Modules/_ctypes/_ctypes.c M Modules/_ctypes/_ctypes_test.c diff --git a/Lib/ctypes/test/test_structures.py b/Lib/ctypes/test/test_structures.py index 8e2897ad51230..c129377041f03 100644 --- a/Lib/ctypes/test/test_structures.py +++ b/Lib/ctypes/test/test_structures.py @@ -612,6 +612,87 @@ class Test5(Structure): self.assertEqual(test5.nested.an_int, 0) self.assertEqual(test5.another_int, 0) + #@unittest.skipIf('s390' in MACHINE, 'Test causes segfault on S390') + def test_bitfield_by_value(self): + # See bpo-16576 + + # These should mirror the structures in Modules/_ctypes/_ctypes_test.c + + class Test6(Structure): + _fields_ = [ + ('A', c_int, 1), + ('B', c_int, 2), + ('C', c_int, 3), + ('D', c_int, 2), + ] + + test6 = Test6() + # As these are signed int fields, all are logically -1 due to sign + # extension. + test6.A = 1 + test6.B = 3 + test6.C = 7 + test6.D = 3 + dll = CDLL(_ctypes_test.__file__) + with self.assertRaises(TypeError) as ctx: + func = dll._testfunc_bitfield_by_value1 + func.restype = c_long + func.argtypes = (Test6,) + result = func(test6) + self.assertEqual(ctx.exception.args[0], 'item 1 in _argtypes_ passes ' + 'a struct/union with a bitfield by value, which is ' + 'unsupported.') + # passing by reference should be OK + func = dll._testfunc_bitfield_by_reference1 + func.restype = c_long + func.argtypes = (POINTER(Test6),) + result = func(byref(test6)) + self.assertEqual(result, -4) + self.assertEqual(test6.A, 0) + self.assertEqual(test6.B, 0) + self.assertEqual(test6.C, 0) + self.assertEqual(test6.D, 0) + + class Test7(Structure): + _fields_ = [ + ('A', c_uint, 1), + ('B', c_uint, 2), + ('C', c_uint, 3), + ('D', c_uint, 2), + ] + test7 = Test7() + test7.A = 1 + test7.B = 3 + test7.C = 7 + test7.D = 3 + func = dll._testfunc_bitfield_by_reference2 + func.restype = c_long + func.argtypes = (POINTER(Test7),) + result = func(byref(test7)) + self.assertEqual(result, 14) + self.assertEqual(test7.A, 0) + self.assertEqual(test7.B, 0) + self.assertEqual(test7.C, 0) + self.assertEqual(test7.D, 0) + + # for a union with bitfields, the union check happens first + class Test8(Union): + _fields_ = [ + ('A', c_int, 1), + ('B', c_int, 2), + ('C', c_int, 3), + ('D', c_int, 2), + ] + + test8 = Test8() + with self.assertRaises(TypeError) as ctx: + func = dll._testfunc_bitfield_by_value2 + func.restype = c_long + func.argtypes = (Test8,) + result = func(test8) + self.assertEqual(ctx.exception.args[0], 'item 1 in _argtypes_ passes ' + 'a union by value, which is unsupported.') + class PointerMemberTestCase(unittest.TestCase): def test(self): diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 0e209c33a80af..c8fed44599543 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -2276,16 +2276,13 @@ converters_from_argtypes(PyObject *ob) for (i = 0; i < nArgs; ++i) { PyObject *tp = PyTuple_GET_ITEM(ob, i); - PyObject *cnv = PyObject_GetAttrString(tp, "from_param"); - if (!cnv) - goto argtypes_error_1; + PyObject *cnv; StgDictObject *stgdict = PyType_stgdict(tp); if (stgdict != NULL) { if (stgdict->flags & TYPEFLAG_HASUNION) { Py_DECREF(converters); Py_DECREF(ob); - Py_DECREF(cnv); if (!PyErr_Occurred()) { PyErr_Format(PyExc_TypeError, "item %zd in _argtypes_ passes a union by " @@ -2294,12 +2291,22 @@ converters_from_argtypes(PyObject *ob) } return NULL; } - /* if (stgdict->flags & TYPEFLAG_HASBITFIELD) { - printf("found stgdict with bitfield\n"); + Py_DECREF(converters); + Py_DECREF(ob); + if (!PyErr_Occurred()) { + PyErr_Format(PyExc_TypeError, + "item %zd in _argtypes_ passes a struct/" + "union with a bitfield by value, which is " + "unsupported.", + i + 1); + } + return NULL; } - */ } + cnv = PyObject_GetAttrString(tp, "from_param"); + if (!cnv) + goto argtypes_error_1; PyTuple_SET_ITEM(converters, i, cnv); } Py_DECREF(ob); diff --git a/Modules/_ctypes/_ctypes_test.c b/Modules/_ctypes/_ctypes_test.c index e06a045545578..d08a011e944ad 100644 --- a/Modules/_ctypes/_ctypes_test.c +++ b/Modules/_ctypes/_ctypes_test.c @@ -198,6 +198,56 @@ _testfunc_union_by_reference3(Test5 *in) { return result; } +typedef struct { + signed int A: 1, B:2, C:3, D:2; +} Test6; + +EXPORT(long) +_testfunc_bitfield_by_value1(Test6 in) { + long result = in.A + in.B + in.C + in.D; + + /* As the struct is passed by value, changes to it shouldn't be + * reflected in the caller. + */ + memset(&in, 0, sizeof(in)); + return result; +} + +EXPORT(long) +_testfunc_bitfield_by_reference1(Test6 *in) { + long result = in->A + in->B + in->C + in->D; + + memset(in, 0, sizeof(Test6)); + return result; +} + +typedef struct { + unsigned int A: 1, B:2, C:3, D:2; +} Test7; + +EXPORT(long) +_testfunc_bitfield_by_reference2(Test7 *in) { + long result = in->A + in->B + in->C + in->D; + + memset(in, 0, sizeof(Test7)); + return result; +} + +typedef union { + signed int A: 1, B:2, C:3, D:2; +} Test8; + +EXPORT(long) +_testfunc_bitfield_by_value2(Test8 in) { + long result = in.A + in.B + in.C + in.D; + + /* As the struct is passed by value, changes to it shouldn't be + * reflected in the caller. + */ + memset(&in, 0, sizeof(in)); + return result; +} + EXPORT(void)testfunc_array(int values[4]) { printf("testfunc_array %d %d %d %d\n",