From 36eabff3bb619bd9c99a6e25c60babbfe25d647a Mon Sep 17 00:00:00 2001 From: Jeffrey Kintscher <49998481+websurfer5@users.noreply.github.com> Date: Mon, 1 Jan 2024 08:24:24 -0800 Subject: [PATCH 1/2] [3.12] gh-62260: Fix ctypes.Structure subclassing with multiple layers (GH-13374) The length field of StgDictObject for Structure class contains now the total number of items in ffi_type_pointer.elements (excluding the trailing null). The old behavior of using the number of elements in the parent class can cause the array to be truncated when it is copied, especially when there are multiple layers of subclassing. (cherry picked from commit 5f3cc90a12d6df404fd6f48a0df1334902e271f2) Co-authored-by: Jeffrey Kintscher <49998481+websurfer5@users.noreply.github.com> Co-authored-by: Serhiy Storchaka --- Lib/test/test_ctypes/test_structures.py | 63 +++++++++++++++++++ .../2019-05-17-07-22-33.bpo-18060.5mqTQM.rst | 2 + Modules/_ctypes/_ctypes.c | 10 +-- Modules/_ctypes/stgdict.c | 2 +- 4 files changed, 71 insertions(+), 6 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2019-05-17-07-22-33.bpo-18060.5mqTQM.rst diff --git a/Lib/test/test_ctypes/test_structures.py b/Lib/test/test_ctypes/test_structures.py index 3bffaa322860c9..056c095d77f24b 100644 --- a/Lib/test/test_ctypes/test_structures.py +++ b/Lib/test/test_ctypes/test_structures.py @@ -1,4 +1,6 @@ import platform +from platform import architecture as _architecture +import struct import sys import unittest from test.test_ctypes import need_symbol @@ -7,6 +9,7 @@ c_uint8, c_uint16, c_uint32, c_short, c_ushort, c_int, c_uint, c_long, c_ulong, c_longlong, c_ulonglong, c_float, c_double) +from ctypes.util import find_library from struct import calcsize import _ctypes_test from test import support @@ -479,6 +482,66 @@ class X(Structure): self.assertEqual(s.first, got.first) self.assertEqual(s.second, got.second) + def _test_issue18060(self, Vector): + # The call to atan2() should succeed if the + # class fields were correctly cloned in the + # subclasses. Otherwise, it will segfault. + if sys.platform == 'win32': + libm = CDLL(find_library('msvcrt.dll')) + else: + libm = CDLL(find_library('m')) + + libm.atan2.argtypes = [Vector] + libm.atan2.restype = c_double + + arg = Vector(y=0.0, x=-1.0) + self.assertAlmostEqual(libm.atan2(arg), 3.141592653589793) + + @unittest.skipIf(_architecture() == ('64bit', 'WindowsPE'), "can't test Windows x64 build") + @unittest.skipUnless(sys.byteorder == 'little', "can't test on this platform") + def test_issue18060_a(self): + # This test case calls + # PyCStructUnionType_update_stgdict() for each + # _fields_ assignment, and PyCStgDict_clone() + # for the Mid and Vector class definitions. + class Base(Structure): + _fields_ = [('y', c_double), + ('x', c_double)] + class Mid(Base): + pass + Mid._fields_ = [] + class Vector(Mid): pass + self._test_issue18060(Vector) + + @unittest.skipIf(_architecture() == ('64bit', 'WindowsPE'), "can't test Windows x64 build") + @unittest.skipUnless(sys.byteorder == 'little', "can't test on this platform") + def test_issue18060_b(self): + # This test case calls + # PyCStructUnionType_update_stgdict() for each + # _fields_ assignment. + class Base(Structure): + _fields_ = [('y', c_double), + ('x', c_double)] + class Mid(Base): + _fields_ = [] + class Vector(Mid): + _fields_ = [] + self._test_issue18060(Vector) + + @unittest.skipIf(_architecture() == ('64bit', 'WindowsPE'), "can't test Windows x64 build") + @unittest.skipUnless(sys.byteorder == 'little', "can't test on this platform") + def test_issue18060_c(self): + # This test case calls + # PyCStructUnionType_update_stgdict() for each + # _fields_ assignment. + class Base(Structure): + _fields_ = [('y', c_double)] + class Mid(Base): + _fields_ = [] + class Vector(Mid): + _fields_ = [('x', c_double)] + self._test_issue18060(Vector) + def test_array_in_struct(self): # See bpo-22273 diff --git a/Misc/NEWS.d/next/Library/2019-05-17-07-22-33.bpo-18060.5mqTQM.rst b/Misc/NEWS.d/next/Library/2019-05-17-07-22-33.bpo-18060.5mqTQM.rst new file mode 100644 index 00000000000000..3fefbc3efb63c0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2019-05-17-07-22-33.bpo-18060.5mqTQM.rst @@ -0,0 +1,2 @@ +Fixed a class inheritance issue that can cause segfaults when deriving two or more levels of subclasses from a base class of Structure or Union. + diff --git a/Modules/_ctypes/_ctypes.c b/Modules/_ctypes/_ctypes.c index 534ef8c1d6cf8f..c5157560f6e0d0 100644 --- a/Modules/_ctypes/_ctypes.c +++ b/Modules/_ctypes/_ctypes.c @@ -4352,10 +4352,10 @@ _init_pos_args(PyObject *self, PyTypeObject *type, return index; } - for (i = 0; - i < dict->length && (i+index) < PyTuple_GET_SIZE(args); + for (i = index; + i < dict->length && i < PyTuple_GET_SIZE(args); ++i) { - PyObject *pair = PySequence_GetItem(fields, i); + PyObject *pair = PySequence_GetItem(fields, i - index); PyObject *name, *val; int res; if (!pair) @@ -4365,7 +4365,7 @@ _init_pos_args(PyObject *self, PyTypeObject *type, Py_DECREF(pair); return -1; } - val = PyTuple_GET_ITEM(args, i + index); + val = PyTuple_GET_ITEM(args, i); if (kwds) { res = PyDict_Contains(kwds, name); if (res != 0) { @@ -4386,7 +4386,7 @@ _init_pos_args(PyObject *self, PyTypeObject *type, if (res == -1) return -1; } - return index + dict->length; + return dict->length; } static int diff --git a/Modules/_ctypes/stgdict.c b/Modules/_ctypes/stgdict.c index d088d0cef5ff4b..6364bc077c0cca 100644 --- a/Modules/_ctypes/stgdict.c +++ b/Modules/_ctypes/stgdict.c @@ -694,7 +694,7 @@ PyCStructUnionType_update_stgdict(PyObject *type, PyObject *fields, int isStruct stgdict->size = aligned_size; stgdict->align = total_align; - stgdict->length = len; /* ADD ffi_ofs? */ + stgdict->length = ffi_ofs + len; /* * On Arm platforms, structs with at most 4 elements of any floating point From e9232868e8346f4a875a089c56a0d73a7938dd9c Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 6 Jan 2024 00:21:40 +0200 Subject: [PATCH 2/2] Fix ruff warning. --- Lib/test/test_ctypes/test_structures.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_structures.py b/Lib/test/test_ctypes/test_structures.py index 1e8e219f2d113b..ce2d1b370c402c 100644 --- a/Lib/test/test_ctypes/test_structures.py +++ b/Lib/test/test_ctypes/test_structures.py @@ -192,7 +192,6 @@ class X(Structure): self.assertEqual(sizeof(X), 10) self.assertEqual(X.b.offset, 2) - import struct longlong_size = struct.calcsize("q") longlong_align = struct.calcsize("bq") - longlong_size