From 0572043851a31f86720ea80596b5159828707728 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Mon, 19 Sep 2022 14:21:02 +0800 Subject: [PATCH 1/4] gh-96821: Fix undefined behaviour in cfield.c --- Modules/_ctypes/cfield.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index 13ed8b7eda6555..b67dbe2712658c 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -412,21 +412,23 @@ get_ulonglong(PyObject *v, unsigned long long *p) #define NUM_BITS(x) ((x) >> 16) /* Doesn't work if NUM_BITS(size) == 0, but it never happens in SET() call. */ -#define BIT_MASK(type, size) (((((type)1 << (NUM_BITS(size) - 1)) - 1) << 1) + 1) +#define BIT_MASK(type, size) \ + (assert(NUM_BITS(size) > 0), \ + (((((type)1 << (NUM_BITS(size) - 1)) - 1) << 1) + 1)) /* This macro CHANGES the first parameter IN PLACE. For proper sign handling, we must first shift left, then right. */ #define GET_BITFIELD(v, size) \ if (NUM_BITS(size)) { \ - v <<= (sizeof(v)*8 - LOW_BIT(size) - NUM_BITS(size)); \ + v *= 1ULL << (sizeof(v)*8 - LOW_BIT(size) - NUM_BITS(size)); \ v >>= (sizeof(v)*8 - NUM_BITS(size)); \ } /* This macro RETURNS the first parameter with the bit field CHANGED. */ #define SET(type, x, v, size) \ (NUM_BITS(size) ? \ - ( ( (type)x & ~(BIT_MASK(type, size) << LOW_BIT(size)) ) | ( ((type)v & BIT_MASK(type, size)) << LOW_BIT(size) ) ) \ + ( ( (type)x & ~(BIT_MASK(type, size) * (1ULL << LOW_BIT(size))) ) | ( ((type)v & BIT_MASK(type, size)) * (1ULL << LOW_BIT(size) )) ) \ : (type)v) #if SIZEOF_SHORT == 2 From e5b624a69ded40f517969fd3fd28fee9e864ce3c Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Mon, 19 Sep 2022 06:42:21 +0000 Subject: [PATCH 2/4] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2022-09-19-06-42-20.gh-issue-96821.jrmhsw.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2022-09-19-06-42-20.gh-issue-96821.jrmhsw.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-09-19-06-42-20.gh-issue-96821.jrmhsw.rst b/Misc/NEWS.d/next/Core and Builtins/2022-09-19-06-42-20.gh-issue-96821.jrmhsw.rst new file mode 100644 index 00000000000000..e49e73b1a0bb00 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2022-09-19-06-42-20.gh-issue-96821.jrmhsw.rst @@ -0,0 +1 @@ +Fix undefined behaviour in ``_ctypes/cfield.c``. From 31f65095087961896895af598b4344f9ae762024 Mon Sep 17 00:00:00 2001 From: Matthias Goergens Date: Mon, 19 Sep 2022 16:41:18 +0800 Subject: [PATCH 3/4] Explicitly cast --- Modules/_ctypes/cfield.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_ctypes/cfield.c b/Modules/_ctypes/cfield.c index b67dbe2712658c..060ee86fb700fe 100644 --- a/Modules/_ctypes/cfield.c +++ b/Modules/_ctypes/cfield.c @@ -428,7 +428,7 @@ get_ulonglong(PyObject *v, unsigned long long *p) /* This macro RETURNS the first parameter with the bit field CHANGED. */ #define SET(type, x, v, size) \ (NUM_BITS(size) ? \ - ( ( (type)x & ~(BIT_MASK(type, size) * (1ULL << LOW_BIT(size))) ) | ( ((type)v & BIT_MASK(type, size)) * (1ULL << LOW_BIT(size) )) ) \ + (type) ( ( (type)x & ~(BIT_MASK(type, size) * (1ULL << LOW_BIT(size))) ) | ( ((type)v & BIT_MASK(type, size)) * (1ULL << LOW_BIT(size) )) ) \ : (type)v) #if SIZEOF_SHORT == 2 From 45c34913d970d68e6c87863164604f74c30cd0bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matthias=20G=C3=B6rgens?= Date: Mon, 26 Sep 2022 14:31:51 +0800 Subject: [PATCH 4/4] Re-enable test --- Lib/test/test_ctypes/test_bitfields.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_ctypes/test_bitfields.py b/Lib/test/test_ctypes/test_bitfields.py index dad71a0ba7ee4a..af2380e28b8736 100644 --- a/Lib/test/test_ctypes/test_bitfields.py +++ b/Lib/test/test_ctypes/test_bitfields.py @@ -40,8 +40,6 @@ def test_ints(self): setattr(b, name, i) self.assertEqual(getattr(b, name), func(byref(b), name.encode('ascii'))) - # bpo-46913: _ctypes/cfield.c h_get() has an undefined behavior - @support.skip_if_sanitizer(ub=True) def test_shorts(self): b = BITS() name = "M"