10000 bpo-38368: Added fix for ctypes crash when handling arrays in structs… · python/cpython@d004a5b · GitHub
[go: up one dir, main page]
Skip to content

Commit d004a5b

Browse files
vsajipvsajip
authored
bpo-38368: Added fix for ctypes crash when handling arrays in structs/unions. (GH-16589) (GH-16671)
(cherry picked from commit e8bedbd)
1 parent d8071cb commit d004a5b

File tree

3 files changed

+231
-25
lines changed

3 files changed

+231
-25
lines changed

‎Lib/ctypes/test/test_structures.py

Lines changed: 52 additions & 0 deletions
8000
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import platform
2+
import sys
13
import unittest
24
from ctypes import *
35
from ctypes.test import need_symbol
@@ -491,6 +493,16 @@ class Test3(Structure):
491493
('data', c_double * 2),
492494
]
493495

496+
class Test3A(Structure):
497+
_fields_ = [
498+
('data', c_float * 2),
499+
]
500+
501+
class Test3B(Test3A):
502+
_fields_ = [
503+
('more_data', c_float * 2),
504+
]
505+
494506
s = Test2()
495507
expected = 0
496508
for i in range(16):
@@ -519,6 +531,46 @@ class Test3(Structure):
519531
self.assertEqual(s.data[0], 3.14159)
520532
self.assertEqual(s.data[1], 2.71828)
521533

534+
s = Test3B()
535+
s.data[0] = 3.14159
536+
s.data[1] = 2.71828
537+
s.more_data[0] = -3.0
538+
s.more_data[1] = -2.0
539+
540+
expected = 3.14159 + 2.71828 - 5.0
541+
func = dll._testfunc_array_in_struct2a
542+
func.restype = c_double
543+
func.argtypes = (Test3B,)
544+
result = func(s)
545+
self.assertAlmostEqual(result, expected, places=6)
546+
# check the passed-in struct hasn't changed
547+
self.assertAlmostEqual(s.data[0], 3.14159, places=6)
548+
self.assertAlmostEqual(s.data[1], 2.71828, places=6)
549+
self.assertAlmostEqual(s.more_data[0], -3.0, places=6)
550+
self.assertAlmostEqual(s.more_data[1], -2.0, places=6)
551+
552+
def test_38368(self):
553+
class U(Union):
554+
_fields_ = [
555+
('f1', c_uint8 * 16),
556+
('f2', c_uint16 * 8),
557+
('f3', c_uint32 * 4),
558+
]
559+
u = U()
560+
u.f3[0] = 0x01234567
561+
u.f3[1] = 0x89ABCDEF
562+
u.f3[2] = 0x76543210
563+
u.f3[3] = 0xFEDCBA98
564+
f1 = [u.f1[i] for i in range(16)]
565+
f2 = [u.f2[i] for i in range(8)]
566+
if sys.byteorder == 'little':
567+
self.assertEqual(f1, [0x67, 0x45, 0x23, 0x01,
568+
0xef, 0xcd, 0xab, 0x89,
569+
0x10, 0x32, 0x54, 0x76,
570+
0x98, 0xba, 0xdc, 0xfe])
571+
self.assertEqual(f2, [0x4567, 0x0123, 0xcdef, 0x89ab,
572+
0x3210, 0x7654, 0xba98, 0xfedc])
573+
522574
class PointerMemberTestCase(unittest.TestCase):
523575

524576
def test(self):

‎Modules/_ctypes/_ctypes_test.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ typedef struct {
100100
double data[2];
101101
} Test3;
102102

103+
typedef struct {
104+
float data[2];
105+
float more_data[2];
106+
} Test3B;
107+
103108
EXPORT(double)
104109
_testfunc_array_in_struct2(Test3 in)
105110
{
@@ -114,6 +119,22 @@ _testfunc_array_in_struct2(Test3 in)
114119
return result;
115120
}
116121

122+
EXPORT(double)
123+
_testfunc_array_in_struct2a(Test3B in)
124+
{
125+
double result = 0;
126+
127+
for (unsigned i = 0; i < 2; i++)
128+
result += in.data[i];
129+
for (unsigned i = 0; i < 2; i++)
130+
result += in.more_data[i];
131+
/* As the structure is passed by value, changes to it shouldn't be
132+
* reflected in the caller.
133+
*/
134+
memset(in.data, 0, sizeof(in.data));
135+
return result;
136+
}
137+
117138
EXPORT(void)testfunc_array(int values[4])
118139
{
119140
printf("testfunc_array %d %d %d %d\n",

‎Modules/_ctypes/stgdict.c

Lines changed: 158 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -644,9 +644,9 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
644644
stgdict->align = total_align;
645645
stgdict->length = len; /* ADD ffi_ofs? */
646646

647-
#define MAX_ELEMENTS 16
647+
#define MAX_STRUCT_SIZE 16
648648

649-
if (arrays_seen && (size <= 16)) {
649+
if (arrays_seen && (size <= MAX_STRUCT_SIZE)) {
650650
/*
651651
* See bpo-22273. Arrays are normally treated as pointers, which is
652652
* fine when an array name is being passed as parameter, but not when
@@ -667,38 +667,175 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
667667
* Although the passing in registers is specific to 64-bit Linux, the
668668
* array-in-struct vs. pointer problem is general. But we restrict the
669669
* type transformation to small structs nonetheless.
670+
*
671+
* Note that although a union may be small in terms of memory usage, it
672+
* could contain many overlapping declarations of arrays, e.g.
673+
*
674+
* union {
675+
* unsigned int_8 foo [16];
676+
* unsigned uint_8 bar [16];
677+
* unsigned int_16 baz[8];
678+
* unsigned uint_16 bozz[8];
679+
* unsigned int_32 fizz[4];
680+
* unsigned uint_32 buzz[4];
681+
* }
682+
*
683+
* which is still only 16 bytes in size. We need to convert this into
684+
* the following equivalent for libffi:
685+
*
686+
* union {
687+
* struct { int_8 e1; int_8 e2; ... int_8 e_16; } f1;
688+
* struct { uint_8 e1; uint_8 e2; ... uint_8 e_16; } f2;
689+
* struct { int_16 e1; int_16 e2; ... int_16 e_8; } f3;
690+
* struct { uint_16 e1; uint_16 e2; ... uint_16 e_8; } f4;
691+
* struct { int_32 e1; int_32 e2; ... int_32 e_4; } f5;
692+
* struct { uint_32 e1; uint_32 e2; ... uint_32 e_4; } f6;
693+
* }
694+
*
695+
* So the struct/union needs setting up as follows: all non-array
696+
* elements copied across as is, and all array elements replaced with
697+
* an equivalent struct which has as many fields as the array has
698+
* elements, plus one NULL pointer.
699+
*/
700+
701+
Py_ssize_t num_ffi_type_pointers = 0; /* for the dummy fields */
702+
Py_ssize_t num_ffi_types = 0; /* for the dummy structures */
703+
size_t alloc_size; /* total bytes to allocate */
704+
void *type_block; /* to hold all the type information needed */
705+
ffi_type **element_types; /* of this struct/union */
706+
ffi_type **dummy_types; /* of the dummy struct elements */
707+
ffi_type *structs; /* point to struct aliases of arrays */
708+
Py_ssize_t element_index; /* index into element_types for this */
709+
Py_ssize_t dummy_index = 0; /* index into dummy field pointers */
710+
Py_ssize_t struct_index = 0; /* index into dummy structs */
711+
712+
/* first pass to see how much memory to allocate */
713+
for (i = 0; i < len; ++i) {
714+
PyObject *name, *desc;
715+
PyObject *pair = PySequence_GetItem(fields, i);
716+
StgDictObject *dict;
717+
int bitsize = 0;
718+
719+
if (pair == NULL) {
720+
retu F987 rn -1;
721+
}
722+
if (!PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {
723+
PyErr_SetString(PyExc_TypeError,
724+
"'_fields_' must be a sequence of (name, C type) pairs");
725+
Py_DECREF(pair);
726+
return -1;
727+
}
728+
dict = PyType_stgdict(desc);
729+
if (dict == NULL) {
730+
Py_DECREF(pair);
731+
PyErr_Format(PyExc_TypeError,
732+
"second item in _fields_ tuple (index %zd) must be a C type",
733+
i);
734+
return -1;
735+
}
736+
if (!PyCArrayTypeObject_Check(desc)) {
737+
/* Not an array. Just need an ffi_type pointer. */
738+
num_ffi_type_pointers++;
739+
}
740+
else {
741+
/* It's an array. */
742+
Py_ssize_t length = dict->length;
743+
StgDictObject *edict;
744+
745+
edict = PyType_stgdict(dict->proto);
746+
if (edict == NULL) {
747+
Py_DECREF(pair);
748+
PyErr_Format(PyExc_TypeError,
749+
"second item in _fields_ tuple (index %zd) must be a C type",
750+
i);
751+
return -1;
752+
}
753+
/*
754+
* We need one extra ffi_type to hold the struct, and one
755+
* ffi_type pointer per array element + one for a NULL to
756+
* mark the end.
757+
*/
758+
num_ffi_types++;
759+
num_ffi_type_pointers += length + 1;
760+
}
761+
Py_DECREF(pair);
762+
}
763+
764+
/*
765+
* At this point, we know we need storage for some ffi_types and some
766+
* ffi_type pointers. We'll allocate these in one block.
767+
* There are three sub-blocks of information: the ffi_type pointers to
768+
* this structure/union's elements, the ffi_type_pointers to the
769+
* dummy fields standing in for array elements, and the
770+
* ffi_types representing the dummy structures.
670771
*/
671-
ffi_type *actual_types[MAX_ELEMENTS + 1];
672-
int actual_type_index = 0;
772+
alloc_size = (ffi_ofs + 1 + len + num_ffi_type_pointers) * sizeof(ffi_type *) +
773+
num_ffi_types * sizeof(ffi_type);
774+
type_block = PyMem_Malloc(alloc_size);
673775

674-
memset(actual_types, 0, sizeof(actual_types));
776+
if (type_block == NULL) {
777+
PyErr_NoMemory();
778+
return -1;
779+
}
780+
/*
781+
* the first block takes up ffi_ofs + len + 1 which is the pointers *
782+
* for this struct/union. The second block takes up
783+
* num_ffi_type_pointers, so the sum of these is ffi_ofs + len + 1 +
784+
* num_ffi_type_pointers as allocated above. The last bit is the
785+
* num_ffi_types structs.
786+
*/
787+
element_types = (ffi_type **) type_block;
788+
dummy_types = &element_types[ffi_ofs + len + 1];
789+
structs = (ffi_type *) &dummy_types[num_ffi_type_pointers];
790+
791+
if (num_ffi_types > 0) {
792+
memset(structs, 0, num_ffi_types * sizeof(ffi_type));
793+
}
794+
if (ffi_ofs && (basedict != NULL)) {
795+
memcpy(element_types,
796+
basedict->ffi_type_pointer.elements,
797+
ffi_ofs * sizeof(ffi_type *));
798+
}
799+
element_index = ffi_ofs;
800+
801+
/* second pass to actually set the type pointers */
675802
for (i = 0; i < len; ++i) {
676803
PyObject *name, *desc;
677804
PyObject *pair = PySequence_GetItem(fields, i);
678805
StgDictObject *dict;
679806
int bitsize = 0;
680807

681808
if (pair == NULL) {
809+
PyMem_Free(type_block);
682810
return -1;
683811
}
812+
/* In theory, we made this call in the first pass, so it *shouldn't*
813+
* fail. However, you never know, and the code above might change
814+
* later - keeping the check in here is a tad defensive but it
815+
* will affect program size only slightly and performance hardly at
816+
* all.
817+
*/
684818
if (!PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {
685819
PyErr_SetString(PyExc_TypeError,
686820
"'_fields_' must be a sequence of (name, C type) pairs");
687-
Py_XDECREF(pair);
821+
Py_DECREF(pair);
822+
PyMem_Free(type_block);
688823
return -1;
689824
}
690825
dict = PyType_stgdict(desc);
826+
/* Possibly this check could be avoided, but see above comment. */
691827
if (dict == NULL) {
692828
Py_DECREF(pair);
829+
PyMem_Free(type_block);
693830
PyErr_Format(PyExc_TypeError,
694831
"second item in _fields_ tuple (index %zd) must be a C type",
695832
i);
696833
return -1;
697834
}
835+
assert(element_index < (ffi_ofs + len)); /* will be used below */
698836
if (!PyCArrayTypeObject_Check(desc)) {
699837
/* Not an array. Just copy over the element ffi_type. */
700-
actual_types[actual_type_index++] = &dict->ffi_type_pointer;
701-
assert(actual_type_index <= MAX_ELEMENTS);
838+
element_types[element_index++] = &dict->ffi_type_pointer;
702839
}
703840
else {
704841
Py_ssize_t length = dict->length;
@@ -707,42 +844,38 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct
707844
edict = PyType_stgdict(dict->proto);
708845
if (edict == NULL) {
709846
Py_DECREF(pair);
847+
PyMem_Free(type_block);
710848
PyErr_Format(PyExc_TypeError,
711849
"second item in _fields_ tuple (index %zd) must be a C type",
712850
i);
713851
return -1;
714852
}
853+
element_types[element_index++] = &structs[struct_index];
854+
structs[struct_index].size = length * edict->ffi_type_pointer.size;
855+
structs[struct_index].alignment = edict->ffi_type_pointer.alignment;
856+
structs[struct_index].type = FFI_TYPE_STRUCT;
857+
structs[struct_index].elements = &dummy_types[dummy_index];
858+
++struct_index;
715859
/* Copy over the element's type, length times. */
716860
while (length > 0) {
717-
actual_types[actual_type_index++] = &edict->ffi_type_pointer;
718-
assert(actual_type_index <= MAX_ELEMENTS);
861+
assert(dummy_index < (num_ffi_type_pointers));
862+
dummy_types[dummy_index++] = &edict->ffi_type_pointer;
719863
length--;
720864
}
865+
assert(dummy_index < (num_ffi_type_pointers));
866+
dummy_types[dummy_index++] = NULL;
721867
}
722868
Py_DECREF(pair);
723869
}
724870

725-
actual_types[actual_type_index++] = NULL;
871+
element_types[element_index] = NULL;
726872
/*
727873
* Replace the old elements with the new, taking into account
728874
* base class elements where necessary.
729875
*/
730876
assert(stgdict->ffi_type_pointer.elements);
731877
PyMem_Free(stgdict->ffi_type_pointer.elements);
732-
stgdict->ffi_type_pointer.elements = PyMem_New(ffi_type *,
733-
ffi_ofs + actual_type_index);
734-
if (stgdict->ffi_type_pointer.elements == NULL) {
735-
PyErr_NoMemory();
736-
return -1;
737-
}
738-
if (ffi_ofs) {
739-
memcpy(stgdict->ffi_type_pointer.elements,
740-
basedict->ffi_type_pointer.elements,
741-
ffi_ofs * sizeof(ffi_type *));
742-
743-
}
744-
memcpy(&stgdict->ffi_type_pointer.elements[ffi_ofs], actual_types,
745-
actual_type_index * sizeof(ffi_type *));
878+
stgdict->ffi_type_pointer.elements = element_types;
746879
}
747880

748881
/* We did check that this flag was NOT set above, it must not

0 commit comments

Comments
 (0)
0