[Python-checkins] [3.7] bpo-16576: Add checks for bitfields passed by value to functions. (GH-17097) (GH-17224)

Vinay Sajip webhook-mailer at python.org
Thu Nov 21 16:47:26 EST 2019


https://github.com/python/cpython/commit/91c15a542cb780377dcde8fc17ba93111bc1d1cf
commit: 91c15a542cb780377dcde8fc17ba93111bc1d1cf
branch: 3.7
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Vinay Sajip <vinay_sajip at 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",



More information about the Python-checkins mailing list