Merge branch 'master' into ctypes-padding

mdickinson · Feb 5, 2023 · Feb 6, 2018 · Nov 28, 2019 · Jul 7, 2022 · Jul 8, 2022
commit e6021e1add19b308d7326e92e77d9745786a4cc3
diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c
@@ -729,6 +729,239 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
     stgdict->align = total_align;
     stgdict->length = len;      /* ADD ffi_ofs? */
 
+#define MAX_STRUCT_SIZE 16
+
+    if (arrays_seen && (size <= MAX_STRUCT_SIZE)) {
+        /*
+         * See bpo-22273. Arrays are normally treated as pointers, which is
+         * fine when an array name is being passed as parameter, but not when
+         * passing structures by value that contain arrays. On 64-bit Linux,
+         * small structures passed by value are passed in registers, and in
+         * order to do this, libffi needs to know the true type of the array
+         * members of structs. Treating them as pointers breaks things.
+         *
+         * By small structures, we mean ones that are 16 bytes or less. In that
+         * case, there can't be more than 16 elements after unrolling arrays,
+         * as we (will) disallow bitfields. So we can collect the true ffi_type
+         * values in a fixed-size local array on the stack and, if any arrays
+         * were seen, replace the ffi_type_pointer.elements with a more
+         * accurate set, to allow libffi to marshal them into registers
+         * correctly. It means one more loop over the fields, but if we got
+         * here, the structure is small, so there aren't too many of those.
+         *
+         * Although the passing in registers is specific to 64-bit Linux, the
+         * array-in-struct vs. pointer problem is general. But we restrict the
+         * type transformation to small structs nonetheless.
+         *
+         * Note that although a union may be small in terms of memory usage, it
+         * could contain many overlapping declarations of arrays, e.g.
+         *
+         * union {
+         *     unsigned int_8 foo [16];
+         *     unsigned uint_8 bar [16];
+         *     unsigned int_16 baz[8];
+         *     unsigned uint_16 bozz[8];
+         *     unsigned int_32 fizz[4];
+         *     unsigned uint_32 buzz[4];
+         * }
+         *
+         * which is still only 16 bytes in size. We need to convert this into
+         * the following equivalent for libffi:
+         *
+         * union {
+         *     struct { int_8 e1; int_8 e2; ... int_8 e_16; } f1;
+         *     struct { uint_8 e1; uint_8 e2; ... uint_8 e_16; } f2;
+         *     struct { int_16 e1; int_16 e2; ... int_16 e_8; } f3;
+         *     struct { uint_16 e1; uint_16 e2; ... uint_16 e_8; } f4;
+         *     struct { int_32 e1; int_32 e2; ... int_32 e_4; } f5;
+         *     struct { uint_32 e1; uint_32 e2; ... uint_32 e_4; } f6;
+         * }
+         *
+         * So the struct/union needs setting up as follows: all non-array
+         * elements copied across as is, and all array elements replaced with
+         * an equivalent struct which has as many fields as the array has
+         * elements, plus one NULL pointer.
+         */
+
+        Py_ssize_t num_ffi_type_pointers = 0;  /* for the dummy fields */
+        Py_ssize_t num_ffi_types = 0;  /* for the dummy structures */
+        size_t alloc_size;  /* total bytes to allocate */
+        void *type_block;  /* to hold all the type information needed */
+        ffi_type **element_types;  /* of this struct/union */
+        ffi_type **dummy_types;  /* of the dummy struct elements */
+        ffi_type *structs;  /* point to struct aliases of arrays */
+        Py_ssize_t element_index;  /* index into element_types for this */
+        Py_ssize_t dummy_index = 0; /* index into dummy field pointers */
+        Py_ssize_t struct_index = 0; /* index into dummy structs */
+
+        /* first pass to see how much memory to allocate */
+        for (i = 0; i < len; ++i) {
+            PyObject *name, *desc;
+            PyObject *pair = PySequence_GetItem(fields, i);
+            StgDictObject *dict;
+            int bitsize = 0;
+
+            if (pair == NULL) {
+                return -1;
+            }
+            if (!PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {
+                PyErr_SetString(PyExc_TypeError,
+                    "'_fields_' must be a sequence of (name, C type) pairs");
+                Py_DECREF(pair);
+                return -1;
+            }
+            dict = PyType_stgdict(desc);
+            if (dict == NULL) {
+                Py_DECREF(pair);
+                PyErr_Format(PyExc_TypeError,
+                    "second item in _fields_ tuple (index %zd) must be a C type",
+                    i);
+                return -1;
+            }
+            if (!PyCArrayTypeObject_Check(desc)) {
+                /* Not an array. Just need an ffi_type pointer. */
+                num_ffi_type_pointers++;
+            }
+            else {
+                /* It's an array. */
+                Py_ssize_t length = dict->length;
+                StgDictObject *edict;
+
+                edict = PyType_stgdict(dict->proto);
+                if (edict == NULL) {
+                    Py_DECREF(pair);
+                    PyErr_Format(PyExc_TypeError,
+                        "second item in _fields_ tuple (index %zd) must be a C type",
+                        i);
+                    return -1;
+                }
+                /*
+                 * We need one extra ffi_type to hold the struct, and one
+                 * ffi_type pointer per array element + one for a NULL to
+                 * mark the end.
+                 */
+                num_ffi_types++;
+                num_ffi_type_pointers += length + 1;
+            }
+            Py_DECREF(pair);
+        }
+
+        /*
+         * At this point, we know we need storage for some ffi_types and some
+         * ffi_type pointers. We'll allocate these in one block.
+         * There are three sub-blocks of information: the ffi_type pointers to
+         * this structure/union's elements, the ffi_type_pointers to the
+         * dummy fields standing in for array elements, and the
+         * ffi_types representing the dummy structures.
+         */
+        alloc_size = (ffi_ofs + 1 + len + num_ffi_type_pointers) * sizeof(ffi_type *) +
+                        num_ffi_types * sizeof(ffi_type);
+        type_block = PyMem_Malloc(alloc_size);
+
+        if (type_block == NULL) {
+            PyErr_NoMemory();
+            return -1;
+        }
+        /*
+         * the first block takes up ffi_ofs + len + 1 which is the pointers *
+         * for this struct/union. The second block takes up
+         * num_ffi_type_pointers, so the sum of these is ffi_ofs + len + 1 +
+         * num_ffi_type_pointers as allocated above. The last bit is the
+         * num_ffi_types structs.
+         */
+        element_types = (ffi_type **) type_block;
+        dummy_types = &element_types[ffi_ofs + len + 1];
+        structs = (ffi_type *) &dummy_types[num_ffi_type_pointers];
+
+        if (num_ffi_types > 0) {
+            memset(structs, 0, num_ffi_types * sizeof(ffi_type));
+        }
+        if (ffi_ofs && (basedict != NULL)) {
+            memcpy(element_types,
+                basedict->ffi_type_pointer.elements,
+                ffi_ofs * sizeof(ffi_type *));
+        }
+        element_index = ffi_ofs;
+
+        /* second pass to actually set the type pointers */
+        for (i = 0; i < len; ++i) {
+            PyObject *name, *desc;
+            PyObject *pair = PySequence_GetItem(fields, i);
+            StgDictObject *dict;
+            int bitsize = 0;
+
+            if (pair == NULL) {
+                PyMem_Free(type_block);
+                return -1;
+            }
+            /* In theory, we made this call in the first pass, so it *shouldn't*
+             * fail. However, you never know, and the code above might change
+             * later - keeping the check in here is a tad defensive but it
+             * will affect program size only slightly and performance hardly at
+             * all.
+             */
+            if (!PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {
+                PyErr_SetString(PyExc_TypeError,
+                                "'_fields_' must be a sequence of (name, C type) pairs");
+                Py_DECREF(pair);
+                PyMem_Free(type_block);
+                return -1;
+            }
+            dict = PyType_stgdict(desc);
+            /* Possibly this check could be avoided, but see above comment. */
+            if (dict == NULL) {
+                Py_DECREF(pair);
+                PyMem_Free(type_block);
+                PyErr_Format(PyExc_TypeError,
+                             "second item in _fields_ tuple (index %zd) must be a C type",
+                             i);
+                return -1;
+            }
+            assert(element_index < (ffi_ofs + len)); /* will be used below */
+            if (!PyCArrayTypeObject_Check(desc)) {
+                /* Not an array. Just copy over the element ffi_type. */
+                element_types[element_index++] = &dict->ffi_type_pointer;
+            }
+            else {
+                Py_ssize_t length = dict->length;
+                StgDictObject *edict;
+
+                edict = PyType_stgdict(dict->proto);
+                if (edict == NULL) {
+                    Py_DECREF(pair);
+                    PyMem_Free(type_block);
+                    PyErr_Format(PyExc_TypeError,
+                                 "second item in _fields_ tuple (index %zd) must be a C type",
+                                 i);
+                    return -1;
+                }
+                element_types[element_index++] = &structs[struct_index];
+                structs[struct_index].size = length * edict->ffi_type_pointer.size;
+                structs[struct_index].alignment = edict->ffi_type_pointer.alignment;
+                structs[struct_index].type = FFI_TYPE_STRUCT;
+                structs[struct_index].elements = &dummy_types[dummy_index];
+                ++struct_index;
+                /* Copy over the element's type, length times. */
+                while (length > 0) {
+                    assert(dummy_index < (num_ffi_type_pointers));
+                    dummy_types[dummy_index++] = &edict->ffi_type_pointer;
+                    length--;
+                }
+                assert(dummy_index < (num_ffi_type_pointers));
+                dummy_types[dummy_index++] = NULL;
+            }
+            Py_DECREF(pair);
+        }
+
+        element_types[element_index] = NULL;
+        /*
+         * Replace the old elements with the new, taking into account
+         * base class elements where necessary.
+         */
+        assert(stgdict->ffi_type_pointer.elements);
+        PyMem_Free(stgdict->ffi_type_pointer.elements);
+        stgdict->ffi_type_pointer.elements = element_types;
+    }
 
     /* We did check that this flag was NOT set above, it must not
        have been set until now. */