gh-97588: Align ctypes struct layout to GCC/MSVC (GH-97702)

Structure layout, and especially bitfields, sometimes resulted in clearly
wrong behaviour like overlapping fields. This fixes

Co-authored-by: Gregory P. Smith <gps@python.org>
Co-authored-by: Petr Viktorin <encukou@gmail.com>

Doc/library/ctypes.rst

@@ -661,14 +661,18 @@ for debugging because they can provide useful information::
    guaranteed by the library to work in the general case.  Unions and
    structures with bit-fields should always be passed to functions by pointer.
 
-Structure/union alignment and byte order
+Structure/union layout, alignment and byte order
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+By default, Structure and Union fields are laid out in the same way the C
+compiler does it.  It is possible to override this behavior entirely by specifying a
+:attr:`~Structure._layout_` class attribute in the subclass definition; see
+the attribute documentation for details.
+
+It is possible to specify the maximum alignment for the fields by setting
+the :attr:`~Structure._pack_` class attribute to a positive integer.
+This matches what ``#pragma pack(n)`` does in MSVC.
 
-By default, Structure and Union fields are aligned in the same way the C
-compiler does it. It is possible to override this behavior by specifying a
-:attr:`~Structure._pack_` class attribute in the subclass definition.
-This must be set to a positive integer and specifies the maximum alignment for the fields.
-This is what ``#pragma pack(n)`` also does in MSVC.
 It is also possible to set a minimum alignment for how the subclass itself is packed in the
 same way ``#pragma align(n)`` works in MSVC.
 This can be achieved by specifying a ::attr:`~Structure._align_` class attribute
@@ -2540,6 +2544,31 @@ fields, or any other data types containing pointer type fields.
       the structure when being packed or unpacked to/from memory.
       Setting this attribute to 0 is the same as not setting it at all.
 
+   .. attribute:: _layout_
+
+      An optional string naming the struct/union layout. It can currently
+      be set to:
+
+      - ``"ms"``: the layout used by the Microsoft compiler (MSVC).
+        On GCC and Clang, this layout can be selected with
+        ``__attribute__((ms_struct))``.
+      - ``"gcc-sysv"``: the layout used by GCC with the System V or “SysV-like”
+        data model, as used on Linux and macOS.
+        With this layout, :attr:`~Structure._pack_` must be unset or zero.
+
+      If not set explicitly, ``ctypes`` will use a default that
+      matches the platform conventions. This default may change in future
+      Python releases (for example, when a new platform gains official support,
+      or when a difference between similar platforms is found).
+      Currently the default will be:
+
+      - On Windows: ``"ms"``
+      - When :attr:`~Structure._pack_` is specified: ``"ms"``
+      - Otherwise: ``"gcc-sysv"``
+
+      :attr:`!_layout_` must already be defined when
+      :attr:`~Structure._fields_` is assigned, otherwise it will have no effect.
+
    .. attribute:: _anonymous_
 
       An optional sequence that lists the names of unnamed (anonymous) fields.

Doc/whatsnew/3.13.rst

@@ -656,6 +656,18 @@ copy
   any user classes which define the :meth:`!__replace__` method.
   (Contributed by Serhiy Storchaka in :gh:`108751`.)
 
+ctypes
+------
+
+* The layout of :ref:`bit fields <ctypes-bit-fields-in-structures-unions>` in
+  :class:`~ctypes.Structure` and :class:`~ctypes.Union` was improved to better
+  match platform defaults (GCC/Clang or MSC). In particular, fields no longer
+  overlap.
+  (Contributed by Matthias Görgens in :gh:`97702`.)
+* A :attr:`ctypes.Structure._layout_` class attribute can be set
+  to help match a non-default ABI.
+  (Contributed by Petr Viktorin in :gh:`97702`.)
+
 dbm
 ---
 

Include/internal/pycore_global_objects_fini_generated.h

@@ -767,6 +767,7 @@ _PyStaticObjects_CheckRefcnt(PyInterpreterState *interp) {
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_initializing));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_io));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_is_text_encoding));
+    _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_layout_));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_length_));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_limbo));
     _PyStaticObject_CheckRefcnt((PyObject *)&_Py_ID(_lock_unlock_module));

Include/internal/pycore_global_strings.h

@@ -256,6 +256,7 @@ struct _Py_global_strings {
         STRUCT_FOR_ID(_initializing)
         STRUCT_FOR_ID(_io)
         STRUCT_FOR_ID(_is_text_encoding)
+        STRUCT_FOR_ID(_layout_)
         STRUCT_FOR_ID(_length_)
         STRUCT_FOR_ID(_limbo)
         STRUCT_FOR_ID(_lock_unlock_module)

Include/internal/pycore_runtime_init_generated.h

@@ -765,6 +765,7 @@ extern "C" {
     INIT_ID(_initializing), \
     INIT_ID(_io), \
     INIT_ID(_is_text_encoding), \
+    INIT_ID(_layout_), \
     INIT_ID(_length_), \
     INIT_ID(_limbo), \
     INIT_ID(_lock_unlock_module), \

Include/internal/pycore_unicodeobject_generated.h

@@ -609,6 +609,9 @@ _PyUnicode_InitStaticStrings(PyInterpreterState *interp) {
     string = &_Py_ID(_is_text_encoding);
     assert(_PyUnicode_CheckConsistency(string, 1));
     _PyUnicode_InternInPlace(interp, &string);
+    string = &_Py_ID(_layout_);
+    assert(_PyUnicode_CheckConsistency(string, 1));
+    _PyUnicode_InternInPlace(interp, &string);
     string = &_Py_ID(_length_);
     assert(_PyUnicode_CheckConsistency(string, 1));
     _PyUnicode_InternInPlace(interp, &string);

Lib/test/test_ctypes/test_bitfields.py

@@ -1,9 +1,10 @@
 import os
+import sys
 import unittest
 from ctypes import (CDLL, Structure, sizeof, POINTER, byref, alignment,
                     LittleEndianStructure, BigEndianStructure,
                     c_byte, c_ubyte, c_char, c_char_p, c_void_p, c_wchar,
-                    c_uint32, c_uint64,
+                    c_uint8, c_uint16, c_uint32, c_uint64,
                     c_short, c_ushort, c_int, c_uint, c_long, c_ulong, c_longlong, c_ulonglong)
 from test import support
 from test.support import import_helper
@@ -33,27 +34,88 @@ func = CDLL(_ctypes_test.__file__).unpack_bitfields
 func.argtypes = POINTER(BITS), c_char
 
 
+class BITS_msvc(Structure):
+    _layout_ = "ms"
+    _fields_ = [("A", c_int, 1),
+                ("B", c_int, 2),
+                ("C", c_int, 3),
+                ("D", c_int, 4),
+                ("E", c_int, 5),
+                ("F", c_int, 6),
+                ("G", c_int, 7),
+                ("H", c_int, 8),
+                ("I", c_int, 9),
+
+                ("M", c_short, 1),
+                ("N", c_short, 2),
+                ("O", c_short, 3),
+                ("P", c_short, 4),
+                ("Q", c_short, 5),
+                ("R", c_short, 6),
+                ("S", c_short, 7)]
+
+
+try:
+    func_msvc = CDLL(_ctypes_test.__file__).unpack_bitfields_msvc
+except AttributeError as err:
+    # The MSVC struct must be available on Windows; it's optional elsewhere
+    if support.MS_WINDOWS:
+        raise err
+    func_msvc = None
+else:
+    func_msvc.argtypes = POINTER(BITS_msvc), c_char
+
+
 class C_Test(unittest.TestCase):
 
     def test_ints(self):
         for i in range(512):
             for name in "ABCDEFGHI":
+                with self.subTest(i=i, name=name):
                     b = BITS()
                     setattr(b, name, i)
-                self.assertEqual(getattr(b, name), func(byref(b), name.encode('ascii')))
+                    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"
+        # See Modules/_ctypes/_ctypes_test.c for where the magic 999 comes from.
         if func(byref(b), name.encode('ascii')) == 999:
+            # unpack_bitfields and unpack_bitfields_msvc in
+            # Modules/_ctypes/_ctypes_test.c return 999 to indicate
+            # an invalid name. 'M' is only valid, if signed short bitfields
+            # are supported by the C compiler.
             self.skipTest("Compiler does not support signed short bitfields")
         for i in range(256):
             for name in "MNOPQRS":
+                with self.subTest(i=i, name=name):
                     b = BITS()
                     setattr(b, name, i)
-                self.assertEqual(getattr(b, name), func(byref(b), name.encode('ascii')))
+                    self.assertEqual(
+                        getattr(b, name),
+                        func(byref(b), (name.encode('ascii'))))
+
+    @unittest.skipUnless(func_msvc, "need MSVC or __attribute__((ms_struct))")
+    def test_shorts_msvc_mode(self):
+        b = BITS_msvc()
+        name = "M"
+        # See Modules/_ctypes/_ctypes_test.c for where the magic 999 comes from.
+        if func_msvc(byref(b), name.encode('ascii')) == 999:
+            # unpack_bitfields and unpack_bitfields_msvc in
+            # Modules/_ctypes/_ctypes_test.c return 999 to indicate
+            # an invalid name. 'M' is only valid, if signed short bitfields
+            # are supported by the C compiler.
+            self.skipTest("Compiler does not support signed short bitfields")
+        for i in range(256):
+            for name in "MNOPQRS":
+                with self.subTest(i=i, name=name):
+                    b = BITS_msvc()
+                    setattr(b, name, i)
+                    self.assertEqual(
+                        getattr(b, name),
+                        func_msvc(byref(b), name.encode('ascii')))
 
 
 signed_int_types = (c_byte, c_short, c_int, c_long, c_longlong)
@@ -87,6 +149,9 @@ class BitFieldTest(unittest.TestCase):
 
     def test_signed(self):
         for c_typ in signed_int_types:
+            with self.subTest(c_typ):
+                if sizeof(c_typ) != alignment(c_typ):
+                     self.skipTest('assumes size=alignment')
                 class X(Structure):
                     _fields_ = [("dummy", c_typ),
                                 ("a", c_typ, 3),
@@ -104,6 +169,9 @@ class BitFieldTest(unittest.TestCase):
 
     def test_unsigned(self):
         for c_typ in unsigned_int_types:
+            with self.subTest(c_typ):
+                if sizeof(c_typ) != alignment(c_typ):
+                     self.skipTest('assumes size=alignment')
                 class X(Structure):
                     _fields_ = [("a", c_typ, 3),
                                 ("b", c_typ, 3),
@@ -149,11 +217,16 @@ class BitFieldTest(unittest.TestCase):
 
     def test_single_bitfield_size(self):
         for c_typ in int_types:
+            with self.subTest(c_typ):
+                if sizeof(c_typ) != alignment(c_typ):
+                     self.skipTest('assumes size=alignment')
                 result = self.fail_fields(("a", c_typ, -1))
-            self.assertEqual(result, (ValueError, 'number of bits invalid for bit field'))
+                self.assertEqual(result, (ValueError,
+                    "number of bits invalid for bit field 'a'"))
 
                 result = self.fail_fields(("a", c_typ, 0))
-            self.assertEqual(result, (ValueError, 'number of bits invalid for bit field'))
+                self.assertEqual(result, (ValueError,
+                    "number of bits invalid for bit field 'a'"))
 
                 class X(Structure):
                     _fields_ = [("a", c_typ, 1)]
@@ -164,7 +237,8 @@ class BitFieldTest(unittest.TestCase):
                 self.assertEqual(sizeof(X), sizeof(c_typ))
 
                 result = self.fail_fields(("a", c_typ, sizeof(c_typ)*8 + 1))
-            self.assertEqual(result, (ValueError, 'number of bits invalid for bit field'))
+                self.assertEqual(result, (ValueError,
+                    "number of bits invalid for bit field 'a'"))
 
     def test_multi_bitfields_size(self):
         class X(Structure):
@@ -236,6 +310,161 @@ class BitFieldTest(unittest.TestCase):
         else:
             self.assertEqual(sizeof(X), sizeof(c_int) * 2)
 
+    def test_mixed_5(self):
+        class X(Structure):
+            _fields_ = [
+                ('A', c_uint, 1),
+                ('B', c_ushort, 16)]
+        a = X()
+        a.A = 0
+        a.B = 1
+        self.assertEqual(1, a.B)
+
+    def test_mixed_6(self):
+        class X(Structure):
+            _fields_ = [
+                ('A', c_ulonglong, 1),
+                ('B', c_uint, 32)]
+        a = X()
+        a.A = 0
+        a.B = 1
+        self.assertEqual(1, a.B)
+
+    @unittest.skipIf(sizeof(c_uint64) != alignment(c_uint64),
+                     'assumes size=alignment')
+    def test_mixed_7(self):
+        class X(Structure):
+            _fields_ = [
+                ("A", c_uint32),
+                ('B', c_uint32, 20),
+                ('C', c_uint64, 24)]
+        self.assertEqual(16, sizeof(X))
+
+    def test_mixed_8(self):
+        class Foo(Structure):
+            _fields_ = [
+                ("A", c_uint32),
+                ("B", c_uint32, 32),
+                ("C", c_ulonglong, 1),
+                ]
+
+        class Bar(Structure):
+            _fields_ = [
+                ("A", c_uint32),
+                ("B", c_uint32),
+                ("C", c_ulonglong, 1),
+                ]
+        self.assertEqual(sizeof(Foo), sizeof(Bar))
+
+    def test_mixed_9(self):
+        class X(Structure):
+            _fields_ = [
+                ("A", c_uint8),
+                ("B", c_uint32, 1),
+                ]
+        if sys.platform == 'win32':
+            self.assertEqual(8, sizeof(X))
+        else:
+            self.assertEqual(4, sizeof(X))
+
+    @unittest.skipIf(sizeof(c_uint64) != alignment(c_uint64),
+                     'assumes size=alignment')
+    def test_mixed_10(self):
+        class X(Structure):
+            _fields_ = [
+                ("A", c_uint32, 1),
+                ("B", c_uint64, 1),
+                ]
+        if sys.platform == 'win32':
+            self.assertEqual(8, alignment(X))
+            self.assertEqual(16, sizeof(X))
+        else:
+            self.assertEqual(8, alignment(X))
+            self.assertEqual(8, sizeof(X))
+
+    def test_gh_95496(self):
+        for field_width in range(1, 33):
+            class TestStruct(Structure):
+                _fields_ = [
+                    ("Field1", c_uint32, field_width),
+                    ("Field2", c_uint8, 8)
+                ]
+
+            cmd = TestStruct()
+            cmd.Field2 = 1
+            self.assertEqual(1, cmd.Field2)
+
+    def test_gh_84039(self):
+        class Bad(Structure):
+            _pack_ = 1
+            _fields_ = [
+                ("a0", c_uint8, 1),
+                ("a1", c_uint8, 1),
+                ("a2", c_uint8, 1),
+                ("a3", c_uint8, 1),
+                ("a4", c_uint8, 1),
+                ("a5", c_uint8, 1),
+                ("a6", c_uint8, 1),
+                ("a7", c_uint8, 1),
+                ("b0", c_uint16, 4),
+                ("b1", c_uint16, 12),
+            ]
+
+
+        class GoodA(Structure):
+            _pack_ = 1
+            _fields_ = [
+                ("a0", c_uint8, 1),
+                ("a1", c_uint8, 1),
+                ("a2", c_uint8, 1),
+                ("a3", c_uint8, 1),
+                ("a4", c_uint8, 1),
+                ("a5", c_uint8, 1),
+                ("a6", c_uint8, 1),
+                ("a7", c_uint8, 1),
+            ]
+
+
+        class Good(Structure):
+            _pack_ = 1
+            _fields_ = [
+                ("a", GoodA),
+                ("b0", c_uint16, 4),
+                ("b1", c_uint16, 12),
+            ]
+
+        self.assertEqual(3, sizeof(Bad))
+        self.assertEqual(3, sizeof(Good))
+
+    def test_gh_73939(self):
+        class MyStructure(Structure):
+            _pack_      = 1
+            _fields_    = [
+                            ("P",       c_uint16),
+                            ("L",       c_uint16, 9),
+                            ("Pro",     c_uint16, 1),
+                            ("G",       c_uint16, 1),
+                            ("IB",      c_uint16, 1),
+                            ("IR",      c_uint16, 1),
+                            ("R",       c_uint16, 3),
+                            ("T",       c_uint32, 10),
+                            ("C",       c_uint32, 20),
+                            ("R2",      c_uint32, 2)
+                        ]
+        self.assertEqual(8, sizeof(MyStructure))
+
+    def test_gh_86098(self):
+        class X(Structure):
+            _fields_ = [
+                ("a", c_uint8, 8),
+                ("b", c_uint8, 8),
+                ("c", c_uint32, 16)
+            ]
+        if sys.platform == 'win32':
+            self.assertEqual(8, sizeof(X))
+        else:
+            self.assertEqual(4, sizeof(X))
+
     def test_anon_bitfields(self):
         # anonymous bit-fields gave a strange error message
         class X(Structure):

Lib/test/test_ctypes/test_generated_structs.py

@@ -0,0 +1,718 @@
+"""Test CTypes structs, unions, bitfields against C equivalents.
+
+The types here are auto-converted to C source at
+`Modules/_ctypes/_ctypes_test_generated.c.h`, which is compiled into
+_ctypes_test.
+
+Run this module to regenerate the files:
+
+./python Lib/test/test_ctypes/test_generated_structs.py > Modules/_ctypes/_ctypes_test_generated.c.h
+"""
+
+import unittest
+from test.support import import_helper
+import re
+from dataclasses import dataclass
+from functools import cached_property
+import sys
+
+import ctypes
+from ctypes import Structure, Union, _SimpleCData
+from ctypes import sizeof, alignment, pointer, string_at
+_ctypes_test = import_helper.import_module("_ctypes_test")
+
+
+# ctypes erases the difference between `c_int` and e.g.`c_int16`.
+# To keep it, we'll use custom subclasses with the C name stashed in `_c_name`:
+class c_bool(ctypes.c_bool):
+    _c_name = '_Bool'
+
+# To do it for all the other types, use some metaprogramming:
+for c_name, ctypes_name in {
+    'signed char': 'c_byte',
+    'short': 'c_short',
+    'int': 'c_int',
+    'long': 'c_long',
+    'long long': 'c_longlong',
+    'unsigned char': 'c_ubyte',
+    'unsigned short': 'c_ushort',
+    'unsigned int': 'c_uint',
+    'unsigned long': 'c_ulong',
+    'unsigned long long': 'c_ulonglong',
+    **{f'{u}int{n}_t': f'c_{u}int{n}'
+       for u in ('', 'u')
+       for n in (8, 16, 32, 64)}
+}.items():
+    ctype = getattr(ctypes, ctypes_name)
+    newtype = type(ctypes_name, (ctype,), {'_c_name': c_name})
+    globals()[ctypes_name] = newtype
+
+
+# Register structs and unions to test
+
+TESTCASES = {}
+def register(name=None, set_name=False):
+    def decorator(cls, name=name):
+        if name is None:
+            name = cls.__name__
+        assert name.isascii()  # will be used in _PyUnicode_EqualToASCIIString
+        assert name.isidentifier()  # will be used as a C identifier
+        assert name not in TESTCASES
+        TESTCASES[name] = cls
+        if set_name:
+            cls.__name__ = name
+        return cls
+    return decorator
+
+@register()
+class SingleInt(Structure):
+    _fields_ = [('a', c_int)]
+
+@register()
+class SingleInt_Union(Union):
+    _fields_ = [('a', c_int)]
+
+
+@register()
+class SingleU32(Structure):
+    _fields_ = [('a', c_uint32)]
+
+
+@register()
+class SimpleStruct(Structure):
+    _fields_ = [('x', c_int32), ('y', c_int8), ('z', c_uint16)]
+
+
+@register()
+class SimpleUnion(Union):
+    _fields_ = [('x', c_int32), ('y', c_int8), ('z', c_uint16)]
+
+
+@register()
+class ManyTypes(Structure):
+    _fields_ = [
+        ('i8', c_int8), ('u8', c_uint8),
+        ('i16', c_int16), ('u16', c_uint16),
+        ('i32', c_int32), ('u32', c_uint32),
+        ('i64', c_int64), ('u64', c_uint64),
+    ]
+
+
+@register()
+class ManyTypesU(Union):
+    _fields_ = [
+        ('i8', c_int8), ('u8', c_uint8),
+        ('i16', c_int16), ('u16', c_uint16),
+        ('i32', c_int32), ('u32', c_uint32),
+        ('i64', c_int64), ('u64', c_uint64),
+    ]
+
+
+@register()
+class Nested(Structure):
+    _fields_ = [
+        ('a', SimpleStruct), ('b', SimpleUnion), ('anon', SimpleStruct),
+    ]
+    _anonymous_ = ['anon']
+
+
+@register()
+class Packed1(Structure):
+    _fields_ = [('a', c_int8), ('b', c_int64)]
+    _pack_ = 1
+
+
+@register()
+class Packed2(Structure):
+    _fields_ = [('a', c_int8), ('b', c_int64)]
+    _pack_ = 2
+
+
+@register()
+class Packed3(Structure):
+    _fields_ = [('a', c_int8), ('b', c_int64)]
+    _pack_ = 4
+
+
+@register()
+class Packed4(Structure):
+    _fields_ = [('a', c_int8), ('b', c_int64)]
+    _pack_ = 8
+
+@register()
+class X86_32EdgeCase(Structure):
+    # On a Pentium, long long (int64) is 32-bit aligned,
+    # so these are packed tightly.
+    _fields_ = [('a', c_int32), ('b', c_int64), ('c', c_int32)]
+
+@register()
+class MSBitFieldExample(Structure):
+    # From https://learn.microsoft.com/en-us/cpp/c-language/c-bit-fields
+    _fields_ = [
+        ('a', c_uint, 4),
+        ('b', c_uint, 5),
+        ('c', c_uint, 7)]
+
+@register()
+class MSStraddlingExample(Structure):
+    # From https://learn.microsoft.com/en-us/cpp/c-language/c-bit-fields
+    _fields_ = [
+        ('first', c_uint, 9),
+        ('second', c_uint, 7),
+        ('may_straddle', c_uint, 30),
+        ('last', c_uint, 18)]
+
+@register()
+class IntBits(Structure):
+    _fields_ = [("A", c_int, 1),
+                ("B", c_int, 2),
+                ("C", c_int, 3),
+                ("D", c_int, 4),
+                ("E", c_int, 5),
+                ("F", c_int, 6),
+                ("G", c_int, 7),
+                ("H", c_int, 8),
+                ("I", c_int, 9)]
+
+@register()
+class Bits(Structure):
+    _fields_ = [*IntBits._fields_,
+
+                ("M", c_short, 1),
+                ("N", c_short, 2),
+                ("O", c_short, 3),
+                ("P", c_short, 4),
+                ("Q", c_short, 5),
+                ("R", c_short, 6),
+                ("S", c_short, 7)]
+
+@register()
+class IntBits_MSVC(Structure):
+    _layout_ = "ms"
+    _fields_ = [("A", c_int, 1),
+                ("B", c_int, 2),
+                ("C", c_int, 3),
+                ("D", c_int, 4),
+                ("E", c_int, 5),
+                ("F", c_int, 6),
+                ("G", c_int, 7),
+                ("H", c_int, 8),
+                ("I", c_int, 9)]
+
+@register()
+class Bits_MSVC(Structure):
+    _layout_ = "ms"
+    _fields_ = [*IntBits_MSVC._fields_,
+
+                ("M", c_short, 1),
+                ("N", c_short, 2),
+                ("O", c_short, 3),
+                ("P", c_short, 4),
+                ("Q", c_short, 5),
+                ("R", c_short, 6),
+                ("S", c_short, 7)]
+
+# Skipped for now -- we don't always match the alignment
+#@register()
+class IntBits_Union(Union):
+    _fields_ = [("A", c_int, 1),
+                ("B", c_int, 2),
+                ("C", c_int, 3),
+                ("D", c_int, 4),
+                ("E", c_int, 5),
+                ("F", c_int, 6),
+                ("G", c_int, 7),
+                ("H", c_int, 8),
+                ("I", c_int, 9)]
+
+# Skipped for now -- we don't always match the alignment
+#@register()
+class BitsUnion(Union):
+    _fields_ = [*IntBits_Union._fields_,
+
+                ("M", c_short, 1),
+                ("N", c_short, 2),
+                ("O", c_short, 3),
+                ("P", c_short, 4),
+                ("Q", c_short, 5),
+                ("R", c_short, 6),
+                ("S", c_short, 7)]
+
+@register()
+class I64Bits(Structure):
+    _fields_ = [("a", c_int64, 1),
+                ("b", c_int64, 62),
+                ("c", c_int64, 1)]
+
+@register()
+class U64Bits(Structure):
+    _fields_ = [("a", c_uint64, 1),
+                ("b", c_uint64, 62),
+                ("c", c_uint64, 1)]
+
+for n in 8, 16, 32, 64:
+    for signedness in '', 'u':
+        ctype = globals()[f'c_{signedness}int{n}']
+
+        @register(f'Struct331_{signedness}{n}', set_name=True)
+        class _cls(Structure):
+            _fields_ = [("a", ctype, 3),
+                        ("b", ctype, 3),
+                        ("c", ctype, 1)]
+
+        @register(f'Struct1x1_{signedness}{n}', set_name=True)
+        class _cls(Structure):
+            _fields_ = [("a", ctype, 1),
+                        ("b", ctype, n-2),
+                        ("c", ctype, 1)]
+
+        @register(f'Struct1nx1_{signedness}{n}', set_name=True)
+        class _cls(Structure):
+            _fields_ = [("a", ctype, 1),
+                        ("full", ctype),
+                        ("b", ctype, n-2),
+                        ("c", ctype, 1)]
+
+        @register(f'Struct3xx_{signedness}{n}', set_name=True)
+        class _cls(Structure):
+            _fields_ = [("a", ctype, 3),
+                        ("b", ctype, n-2),
+                        ("c", ctype, n-2)]
+
+@register()
+class Mixed1(Structure):
+    _fields_ = [("a", c_byte, 4),
+                ("b", c_int, 4)]
+
+@register()
+class Mixed2(Structure):
+    _fields_ = [("a", c_byte, 4),
+                ("b", c_int32, 32)]
+
+@register()
+class Mixed3(Structure):
+    _fields_ = [("a", c_byte, 4),
+                ("b", c_ubyte, 4)]
+
+@register()
+class Mixed4(Structure):
+    _fields_ = [("a", c_short, 4),
+                ("b", c_short, 4),
+                ("c", c_int, 24),
+                ("d", c_short, 4),
+                ("e", c_short, 4),
+                ("f", c_int, 24)]
+
+@register()
+class Mixed5(Structure):
+    _fields_ = [('A', c_uint, 1),
+                ('B', c_ushort, 16)]
+
+@register()
+class Mixed6(Structure):
+    _fields_ = [('A', c_ulonglong, 1),
+                ('B', c_uint, 32)]
+
+@register()
+class Mixed7(Structure):
+    _fields_ = [("A", c_uint32),
+                ('B', c_uint32, 20),
+                ('C', c_uint64, 24)]
+
+@register()
+class Mixed8_a(Structure):
+    _fields_ = [("A", c_uint32),
+                ("B", c_uint32, 32),
+                ("C", c_ulonglong, 1)]
+
+@register()
+class Mixed8_b(Structure):
+    _fields_ = [("A", c_uint32),
+                ("B", c_uint32),
+                ("C", c_ulonglong, 1)]
+
+@register()
+class Mixed9(Structure):
+    _fields_ = [("A", c_uint8),
+                ("B", c_uint32, 1)]
+
+@register()
+class Mixed10(Structure):
+    _fields_ = [("A", c_uint32, 1),
+                ("B", c_uint64, 1)]
+
+@register()
+class Example_gh_95496(Structure):
+    _fields_ = [("A", c_uint32, 1),
+                ("B", c_uint64, 1)]
+
+@register()
+class Example_gh_84039_bad(Structure):
+    _pack_ = 1
+    _fields_ = [("a0", c_uint8, 1),
+                ("a1", c_uint8, 1),
+                ("a2", c_uint8, 1),
+                ("a3", c_uint8, 1),
+                ("a4", c_uint8, 1),
+                ("a5", c_uint8, 1),
+                ("a6", c_uint8, 1),
+                ("a7", c_uint8, 1),
+                ("b0", c_uint16, 4),
+                ("b1", c_uint16, 12)]
+
+@register()
+class Example_gh_84039_good_a(Structure):
+    _pack_ = 1
+    _fields_ = [("a0", c_uint8, 1),
+                ("a1", c_uint8, 1),
+                ("a2", c_uint8, 1),
+                ("a3", c_uint8, 1),
+                ("a4", c_uint8, 1),
+                ("a5", c_uint8, 1),
+                ("a6", c_uint8, 1),
+                ("a7", c_uint8, 1)]
+
+@register()
+class Example_gh_84039_good(Structure):
+    _pack_ = 1
+    _fields_ = [("a", Example_gh_84039_good_a),
+                ("b0", c_uint16, 4),
+                ("b1", c_uint16, 12)]
+
+@register()
+class Example_gh_73939(Structure):
+    _pack_ = 1
+    _fields_ = [("P", c_uint16),
+                ("L", c_uint16, 9),
+                ("Pro", c_uint16, 1),
+                ("G", c_uint16, 1),
+                ("IB", c_uint16, 1),
+                ("IR", c_uint16, 1),
+                ("R", c_uint16, 3),
+                ("T", c_uint32, 10),
+                ("C", c_uint32, 20),
+                ("R2", c_uint32, 2)]
+
+@register()
+class Example_gh_86098(Structure):
+    _fields_ = [("a", c_uint8, 8),
+                ("b", c_uint8, 8),
+                ("c", c_uint32, 16)]
+
+@register()
+class Example_gh_86098_pack(Structure):
+    _pack_ = 1
+    _fields_ = [("a", c_uint8, 8),
+                ("b", c_uint8, 8),
+                ("c", c_uint32, 16)]
+
+@register()
+class AnonBitfields(Structure):
+    class X(Structure):
+        _fields_ = [("a", c_byte, 4),
+                    ("b", c_ubyte, 4)]
+    _anonymous_ = ["_"]
+    _fields_ = [("_", X), ('y', c_byte)]
+
+
+class GeneratedTest(unittest.TestCase):
+    def test_generated_data(self):
+        """Check that a ctypes struct/union matches its C equivalent.
+
+        This compares with data from get_generated_test_data(), a list of:
+        - name (str)
+        - size (int)
+        - alignment (int)
+        - for each field, three snapshots of memory, as bytes:
+            - memory after the field is set to -1
+            - memory after the field is set to 1
+            - memory after the field is set to 0
+
+        or:
+        - None
+        - reason to skip the test (str)
+
+        This does depend on the C compiler keeping padding bits zero.
+        Common compilers seem to do so.
+        """
+        for name, cls in TESTCASES.items():
+            with self.subTest(name=name):
+                expected = iter(_ctypes_test.get_generated_test_data(name))
+                expected_name = next(expected)
+                if expected_name is None:
+                    self.skipTest(next(expected))
+                self.assertEqual(name, expected_name)
+                self.assertEqual(sizeof(cls), next(expected))
+                with self.subTest('alignment'):
+                    self.assertEqual(alignment(cls), next(expected))
+                obj = cls()
+                ptr = pointer(obj)
+                for field in iterfields(cls):
+                    for value in -1, 1, 0:
+                        with self.subTest(field=field.full_name, value=value):
+                            field.set_to(obj, value)
+                            py_mem = string_at(ptr, sizeof(obj))
+                            c_mem = next(expected)
+                            if py_mem != c_mem:
+                                # Generate a helpful failure message
+                                lines, requires = dump_ctype(cls)
+                                m = "\n".join([str(field), 'in:', *lines])
+                                self.assertEqual(py_mem.hex(), c_mem.hex(), m)
+
+
+# The rest of this file is generating C code from a ctypes type.
+# This is only meant for (and tested with) the known inputs in this file!
+
+def c_str_repr(string):
+    """Return a string as a C literal"""
+    return '"' + re.sub('([\"\'\\\\\n])', r'\\\1', string) + '"'
+
+def dump_simple_ctype(tp, variable_name='', semi=''):
+    """Get C type name or declaration of a scalar type
+
+    variable_name: if given, declare the given variable
+    semi: a semicolon, and/or bitfield specification to tack on to the end
+    """
+    length = getattr(tp, '_length_', None)
+    if length is not None:
+        return f'{dump_simple_ctype(tp._type_, variable_name)}[{length}]{semi}'
+    assert not issubclass(tp, (Structure, Union))
+    return f'{tp._c_name}{maybe_space(variable_name)}{semi}'
+
+
+def dump_ctype(tp, struct_or_union_tag='', variable_name='', semi=''):
+    """Get C type name or declaration of a ctype
+
+    struct_or_union_tag: name of the struct or union
+    variable_name: if given, declare the given variable
+    semi: a semicolon, and/or bitfield specification to tack on to the end
+    """
+    requires = set()
+    if issubclass(tp, (Structure, Union)):
+        attributes = []
+        pushes = []
+        pops = []
+        pack = getattr(tp, '_pack_', None)
+        if pack is not None:
+            pushes.append(f'#pragma pack(push, {pack})')
+            pops.append(f'#pragma pack(pop)')
+        layout = getattr(tp, '_layout_', None)
+        if layout == 'ms' or pack:
+            # The 'ms_struct' attribute only works on x86 and PowerPC
+            requires.add(
+                'defined(MS_WIN32) || ('
+                    '(defined(__x86_64__) || defined(__i386__) || defined(__ppc64__)) && ('
+                    'defined(__GNUC__) || defined(__clang__)))'
+                )
+            attributes.append('ms_struct')
+        if attributes:
+            a = f' GCC_ATTR({", ".join(attributes)})'
+        else:
+            a = ''
+        lines = [f'{struct_or_union(tp)}{a}{maybe_space(struct_or_union_tag)} ' +'{']
+        for fielddesc in tp._fields_:
+            f_name, f_tp, f_bits = unpack_field_desc(*fielddesc)
+            if f_name in getattr(tp, '_anonymous_', ()):
+                f_name = ''
+            if f_bits is None:
+                subsemi = ';'
+            else:
+                if f_tp not in (c_int, c_uint):
+                    # XLC can reportedly only handle int & unsigned int
+                    # bitfields (the only types required by C spec)
+                    requires.add('!defined(__xlc__)')
+                subsemi = f' :{f_bits};'
+            sub_lines, sub_requires = dump_ctype(
+                f_tp, variable_name=f_name, semi=subsemi)
+            requires.update(sub_requires)
+            for line in sub_lines:
+                lines.append('    ' + line)
+        lines.append(f'}}{maybe_space(variable_name)}{semi}')
+        return [*pushes, *lines, *reversed(pops)], requires
+    else:
+        return [dump_simple_ctype(tp, variable_name, semi)], requires
+
+def struct_or_union(cls):
+    if issubclass(cls, Structure):
+         return 'struct'
+    if issubclass(cls, Union):
+        return 'union'
+    raise TypeError(cls)
+
+def maybe_space(string):
+    if string:
+        return ' ' + string
+    return string
+
+def unpack_field_desc(f_name, f_tp, f_bits=None):
+    """Unpack a _fields_ entry into a (name, type, bits) triple"""
+    return f_name, f_tp, f_bits
+
+@dataclass
+class FieldInfo:
+    """Information about a (possibly nested) struct/union field"""
+    name: str
+    tp: type
+    bits: int | None  # number if this is a bit field
+    parent_type: type
+    parent: 'FieldInfo' #| None
+
+    @cached_property
+    def attr_path(self):
+        """Attribute names to get at the value of this field"""
+        if self.name in getattr(self.parent_type, '_anonymous_', ()):
+            selfpath = ()
+        else:
+            selfpath = (self.name,)
+        if self.parent:
+            return (*self.parent.attr_path, *selfpath)
+        else:
+            return selfpath
+
+    @cached_property
+    def full_name(self):
+        """Attribute names to get at the value of this field"""
+        return '.'.join(self.attr_path)
+
+    def set_to(self, obj, new):
+        """Set the field on a given Structure/Union instance"""
+        for attr_name in self.attr_path[:-1]:
+            obj = getattr(obj, attr_name)
+        setattr(obj, self.attr_path[-1], new)
+
+    @cached_property
+    def root(self):
+        if self.parent is None:
+            return self
+        else:
+            return self.parent
+
+    @cached_property
+    def descriptor(self):
+        return getattr(self.parent_type, self.name)
+
+    def __repr__(self):
+        qname = f'{self.root.parent_type.__name__}.{self.full_name}'
+        try:
+            desc = self.descriptor
+        except AttributeError:
+            desc = '???'
+        return f'<{type(self).__name__} for {qname}: {desc}>'
+
+def iterfields(tp, parent=None):
+    """Get *leaf* fields of a structure or union, as FieldInfo"""
+    try:
+        fields = tp._fields_
+    except AttributeError:
+        yield parent
+    else:
+        for fielddesc in fields:
+            f_name, f_tp, f_bits = unpack_field_desc(*fielddesc)
+            sub = FieldInfo(f_name, f_tp, f_bits, tp, parent)
+            yield from iterfields(f_tp, sub)
+
+
+if __name__ == '__main__':
+    # Dump C source to stdout
+    def output(string):
+        print(re.compile(r'^ +$', re.MULTILINE).sub('', string).lstrip('\n'))
+    output("""
+        /* Generated by Lib/test/test_ctypes/test_generated_structs.py */
+
+
+        // Append VALUE to the result.
+        #define APPEND(ITEM) {                          \\
+            PyObject *item = ITEM;                      \\
+            if (!item) {                                \\
+                Py_DECREF(result);                      \\
+                return NULL;                            \\
+            }                                           \\
+            int rv = PyList_Append(result, item);       \\
+            Py_DECREF(item);                            \\
+            if (rv < 0) {                               \\
+                Py_DECREF(result);                      \\
+                return NULL;                            \\
+            }                                           \\
+        }
+
+        // Set TARGET, and append a snapshot of `value`'s
+        // memory to the result.
+        #define SET_AND_APPEND(TYPE, TARGET, VAL) {     \\
+            TYPE v = VAL;                               \\
+            TARGET = v;                                 \\
+            APPEND(PyBytes_FromStringAndSize(           \\
+                (char*)&value, sizeof(value)));         \\
+        }
+
+        // Set a field to -1, 1 and 0; append a snapshot of the memory
+        // after each of the operations.
+        #define TEST_FIELD(TYPE, TARGET) {              \\
+            SET_AND_APPEND(TYPE, TARGET, -1)            \\
+            SET_AND_APPEND(TYPE, TARGET, 1)             \\
+            SET_AND_APPEND(TYPE, TARGET, 0)             \\
+        }
+
+        #if defined(__GNUC__) || defined(__clang__)
+        #define GCC_ATTR(X) __attribute__((X))
+        #else
+        #define GCC_ATTR(X) /* */
+        #endif
+
+        static PyObject *
+        get_generated_test_data(PyObject *self, PyObject *name)
+        {
+            if (!PyUnicode_Check(name)) {
+                PyErr_SetString(PyExc_TypeError, "need a string");
+                return NULL;
+            }
+            PyObject *result = PyList_New(0);
+            if (!result) {
+                return NULL;
+            }
+    """)
+    for name, cls in TESTCASES.items():
+        output("""
+            if (PyUnicode_CompareWithASCIIString(name, %s) == 0) {
+            """ % c_str_repr(name))
+        lines, requires = dump_ctype(cls, struct_or_union_tag=name, semi=';')
+        if requires:
+            output(f"""
+            #if {" && ".join(f'({r})' for r in sorted(requires))}
+            """)
+        for line in lines:
+            output('                ' + line)
+        typename = f'{struct_or_union(cls)} {name}'
+        output(f"""
+                {typename} value = {{0}};
+                APPEND(PyUnicode_FromString({c_str_repr(name)}));
+                APPEND(PyLong_FromLong(sizeof({typename})));
+                APPEND(PyLong_FromLong(_Alignof({typename})));
+        """.rstrip())
+        for field in iterfields(cls):
+            f_tp = dump_simple_ctype(field.tp)
+            output(f"""\
+                TEST_FIELD({f_tp}, value.{field.full_name});
+            """.rstrip())
+        if requires:
+            output(f"""
+            #else
+                APPEND(Py_NewRef(Py_None));
+                APPEND(PyUnicode_FromString("skipped on this compiler"));
+            #endif
+            """)
+        output("""
+                return result;
+            }
+        """)
+
+    output("""
+            Py_DECREF(result);
+            PyErr_Format(PyExc_ValueError, "unknown testcase %R", name);
+            return NULL;
+        }
+
+        #undef GCC_ATTR
+        #undef TEST_FIELD
+        #undef SET_AND_APPEND
+        #undef APPEND
+    """)

Makefile.pre.in

@@ -3106,6 +3106,7 @@ MODULE_PYEXPAT_DEPS=@LIBEXPAT_INTERNAL@
 MODULE_UNICODEDATA_DEPS=$(srcdir)/Modules/unicodedata_db.h $(srcdir)/Modules/unicodename_db.h
 MODULE__BLAKE2_DEPS=$(srcdir)/Modules/_blake2/impl/blake2-config.h $(srcdir)/Modules/_blake2/impl/blake2-impl.h $(srcdir)/Modules/_blake2/impl/blake2.h $(srcdir)/Modules/_blake2/impl/blake2b-load-sse2.h $(srcdir)/Modules/_blake2/impl/blake2b-load-sse41.h $(srcdir)/Modules/_blake2/impl/blake2b-ref.c $(srcdir)/Modules/_blake2/impl/blake2b-round.h $(srcdir)/Modules/_blake2/impl/blake2b.c $(srcdir)/Modules/_blake2/impl/blake2s-load-sse2.h $(srcdir)/Modules/_blake2/impl/blake2s-load-sse41.h $(srcdir)/Modules/_blake2/impl/blake2s-load-xop.h $(srcdir)/Modules/_blake2/impl/blake2s-ref.c $(srcdir)/Modules/_blake2/impl/blake2s-round.h $(srcdir)/Modules/_blake2/impl/blake2s.c $(srcdir)/Modules/_blake2/blake2module.h $(srcdir)/Modules/hashlib.h
 MODULE__CTYPES_DEPS=$(srcdir)/Modules/_ctypes/ctypes.h
+MODULE__CTYPES_TEST_DEPS=$(srcdir)/Modules/_ctypes/_ctypes_test_generated.c.h
 MODULE__CTYPES_MALLOC_CLOSURE=@MODULE__CTYPES_MALLOC_CLOSURE@
 MODULE__DECIMAL_DEPS=$(srcdir)/Modules/_decimal/docstrings.h @LIBMPDEC_INTERNAL@
 MODULE__ELEMENTTREE_DEPS=$(srcdir)/Modules/pyexpat.c @LIBEXPAT_INTERNAL@

Misc/NEWS.d/next/C API/2022-10-01-09-56-27.gh-issue-97588.Gvg54o.rst

@@ -0,0 +1,2 @@
+Fix creating bitfields in :mod:`ctypes` structures and unions. Fields
+no longer overlap.

Modules/_ctypes/_ctypes_test.c

@@ -22,6 +22,8 @@
 
 #define EXPORT(x) Py_EXPORTED_SYMBOL x
 
+#include "_ctypes_test_generated.c.h"
+
 /* some functions handy for testing */
 
 EXPORT(int)
@@ -343,6 +345,31 @@ _testfunc_bitfield_by_reference2(Test7 *in) {
     return result;
 }
 
+typedef struct{
+    uint16_t A ;
+    uint16_t B : 9;
+    uint16_t C : 1;
+    uint16_t D : 1;
+    uint16_t E : 1;
+    uint16_t F : 1;
+    uint16_t G : 3;
+    uint32_t H : 10;
+    uint32_t I : 20;
+    uint32_t J : 2;
+} Test9;
+
+EXPORT(long)
+_testfunc_bitfield_by_reference3(Test9 *in, long pos) {
+    long data[] = {in->A , in->B , in->C , in->D , in->E , in->F , in->G , in->H , in->I , in->J};
+    long data_length = (long) (sizeof(data)/sizeof(data[0]));
+    if(pos < 0)
+        return -1;
+    if(pos >= data_length)
+        return -1;
+
+    return data[pos];
+}
+
 typedef union {
     signed int A: 1, B:2, C:3, D:2;
 } Test8;
@@ -704,7 +731,7 @@ struct BITS {
  */
 #ifndef __xlc__
 #define SIGNED_SHORT_BITFIELDS
-     short M: 1, N: 2, O: 3, P: 4, Q: 5, R: 6, S: 7;
+    signed short M: 1, N: 2, O: 3, P: 4, Q: 5, R: 6, S: 7;
 #endif
 };
 
@@ -734,12 +761,58 @@ EXPORT(int) unpack_bitfields(struct BITS *bits, char name)
     return 999;
 }
 
+#if (defined(MS_WIN32) || ((defined(__x86_64__) || defined(__i386__) || defined(__ppc64__)) && (defined(__GNUC__) || defined(__clang__))))
+struct
+#ifndef MS_WIN32
+__attribute__ ((ms_struct))
+#endif
+BITS_msvc
+{
+    signed int A: 1, B:2, C:3, D:4, E: 5, F: 6, G: 7, H: 8, I: 9;
+/*
+ * The test case needs/uses "signed short" bitfields, but the
+ * IBM XLC compiler does not support this
+ */
+#ifndef __xlc__
+#define SIGNED_SHORT_BITFIELDS
+    signed short M: 1, N: 2, O: 3, P: 4, Q: 5, R: 6, S: 7;
+#endif
+};
+
+EXPORT(int) unpack_bitfields_msvc(struct BITS_msvc *bits, char name)
+{
+    switch (name) {
+    case 'A': return bits->A;
+    case 'B': return bits->B;
+    case 'C': return bits->C;
+    case 'D': return bits->D;
+    case 'E': return bits->E;
+    case 'F': return bits->F;
+    case 'G': return bits->G;
+    case 'H': return bits->H;
+    case 'I': return bits->I;
+
+#ifdef SIGNED_SHORT_BITFIELDS
+    case 'M': return bits->M;
+    case 'N': return bits->N;
+    case 'O': return bits->O;
+    case 'P': return bits->P;
+    case 'Q': return bits->Q;
+    case 'R': return bits->R;
+    case 'S': return bits->S;
+#endif
+    }
+    return 999;
+}
+#endif
+
 static PyMethodDef module_methods[] = {
 /*      {"get_last_tf_arg_s", get_last_tf_arg_s, METH_NOARGS},
     {"get_last_tf_arg_u", get_last_tf_arg_u, METH_NOARGS},
 */
     {"func_si", py_func_si, METH_VARARGS},
     {"func", py_func, METH_NOARGS},
+    {"get_generated_test_data", get_generated_test_data, METH_O},
     { NULL, NULL, 0, NULL},
 };
 

Modules/_ctypes/cfield.c

@@ -31,40 +31,168 @@ static void pymem_destructor(PyObject *ptr)
   PyCField_Type
 */
 
-/*
+static inline
- * Expects the size, index and offset for the current field in *psize and
+Py_ssize_t round_down(Py_ssize_t numToRound, Py_ssize_t multiple)
- * *poffset, stores the total size so far in *psize, the offset for the next
+{
- * field in *poffset, the alignment requirements for the current field in
+    assert(numToRound >= 0);
- * *palign, and returns a field descriptor for this field.
+    assert(multiple >= 0);
- */
+    if (multiple == 0)
-/*
+        return numToRound;
- * bitfields extension:
+    return (numToRound / multiple) * multiple;
- * bitsize != 0: this is a bit field.
+}
- * pbitofs points to the current bit offset, this will be updated.
+
- * prev_desc points to the type of the previous bitfield, if any.
+static inline
- */
+Py_ssize_t round_up(Py_ssize_t numToRound, Py_ssize_t multiple)
+{
+    assert(numToRound >= 0);
+    assert(multiple >= 0);
+    if (multiple == 0)
+        return numToRound;
+    return ((numToRound + multiple - 1) / multiple) * multiple;
+}
+
+static inline
+Py_ssize_t NUM_BITS(Py_ssize_t bitsize);
+static inline
+Py_ssize_t LOW_BIT(Py_ssize_t offset);
+static inline
+Py_ssize_t BUILD_SIZE(Py_ssize_t bitsize, Py_ssize_t offset);
+
+/* PyCField_FromDesc creates and returns a struct/union field descriptor.
+
+The function expects to be called repeatedly for all fields in a struct or
+union.  It uses helper functions PyCField_FromDesc_gcc and
+PyCField_FromDesc_msvc to simulate the corresponding compilers.
+
+GCC mode places fields one after another, bit by bit.  But "each bit field must
+fit within a single object of its specified type" (GCC manual, section 15.8
+"Bit Field Packing"). When it doesn't, we insert a few bits of padding to
+avoid that.
+
+MSVC mode works similar except for bitfield packing.  Adjacent bit-fields are
+packed into the same 1-, 2-, or 4-byte allocation unit if the integral types
+are the same size and if the next bit-field fits into the current allocation
+unit without crossing the boundary imposed by the common alignment requirements
+of the bit-fields.
+
+See https://gcc.gnu.org/onlinedocs/gcc/x86-Options.html#index-mms-bitfields for details.
+
+We do not support zero length bitfields.  In fact we use bitsize != 0 elsewhere
+to indicate a bitfield. Here, non-bitfields need bitsize set to size*8.
+
+PyCField_FromDesc manages:
+- *psize: the size of the structure / union so far.
+- *poffset, *pbitofs: 8* (*poffset) + *pbitofs points to where the next field
+  would start.
+- *palign: the alignment requirements of the last field we placed.
+*/
+
+static int
+PyCField_FromDesc_gcc(Py_ssize_t bitsize, Py_ssize_t *pbitofs,
+                Py_ssize_t *psize, Py_ssize_t *poffset, Py_ssize_t *palign,
+                CFieldObject* self, StgInfo* info,
+                int is_bitfield
+                )
+{
+    // We don't use poffset here, so clear it, if it has been set.
+    *pbitofs += *poffset * 8;
+    *poffset = 0;
+
+    *palign = info->align;
+
+    if (bitsize > 0) {
+        // Determine whether the bit field, if placed at the next free bit,
+        // fits within a single object of its specified type.
+        // That is: determine a "slot", sized & aligned for the specified type,
+        // which contains the bitfield's beginning:
+        Py_ssize_t slot_start_bit = round_down(*pbitofs, 8 * info->align);
+        Py_ssize_t slot_end_bit = slot_start_bit + 8 * info->size;
+        // And see if it also contains the bitfield's last bit:
+        Py_ssize_t field_end_bit = *pbitofs + bitsize;
+        if (field_end_bit > slot_end_bit) {
+            // It doesn't: add padding (bump up to the next alignment boundary)
+            *pbitofs = round_up(*pbitofs, 8*info->align);
+        }
+    }
+    assert(*poffset == 0);
+
+    self->offset = round_down(*pbitofs, 8*info->align) / 8;
+    if(is_bitfield) {
+        Py_ssize_t effective_bitsof = *pbitofs - 8 * self->offset;
+        self->size = BUILD_SIZE(bitsize, effective_bitsof);
+        assert(effective_bitsof <= info->size * 8);
+    } else {
+        self->size = info->size;
+    }
+
+    *pbitofs += bitsize;
+    *psize = round_up(*pbitofs, 8) / 8;
+
+    return 0;
+}
+
+static int
+PyCField_FromDesc_msvc(
+                Py_ssize_t *pfield_size, Py_ssize_t bitsize,
+                Py_ssize_t *pbitofs, Py_ssize_t *psize, Py_ssize_t *poffset,
+                Py_ssize_t *palign, int pack,
+                CFieldObject* self, StgInfo* info,
+                int is_bitfield
+                )
+{
+    if (pack) {
+        *palign = Py_MIN(pack, info->align);
+    } else {
+        *palign = info->align;
+    }
+
+    // *poffset points to end of current bitfield.
+    // *pbitofs is generally non-positive,
+    // and 8 * (*poffset) + *pbitofs points just behind
+    // the end of the last field we placed.
+    if (0 < *pbitofs + bitsize || 8 * info->size != *pfield_size) {
+        // Close the previous bitfield (if any).
+        // and start a new bitfield:
+        *poffset = round_up(*poffset, *palign);
+
+        *poffset += info->size;
+
+        *pfield_size = info->size * 8;
+        // Reminder: 8 * (*poffset) + *pbitofs points to where we would start a
+        // new field.  Ie just behind where we placed the last field plus an
+        // allowance for alignment.
+        *pbitofs = - *pfield_size;
+    }
+
+    assert(8 * info->size == *pfield_size);
+
+    self->offset = *poffset - (*pfield_size) / 8;
+    if(is_bitfield) {
+        assert(0 <= (*pfield_size + *pbitofs));
+        assert((*pfield_size + *pbitofs) < info->size * 8);
+        self->size = BUILD_SIZE(bitsize, *pfield_size + *pbitofs);
+    } else {
+        self->size = info->size;
+    }
+    assert(*pfield_size + *pbitofs <= info->size * 8);
+
+    *pbitofs += bitsize;
+    *psize = *poffset;
+
+    return 0;
+}
+
 PyObject *
 PyCField_FromDesc(ctypes_state *st, PyObject *desc, Py_ssize_t index,
-                Py_ssize_t *pfield_size, int bitsize, int *pbitofs,
+                Py_ssize_t *pfield_size, Py_ssize_t bitsize,
-                Py_ssize_t *psize, Py_ssize_t *poffset, Py_ssize_t *palign,
+                Py_ssize_t *pbitofs, Py_ssize_t *psize, Py_ssize_t *poffset, Py_ssize_t *palign,
-                int pack, int big_endian)
+                int pack, int big_endian, LayoutMode layout_mode)
 {
-    CFieldObject *self;
-    PyObject *proto;
-    Py_ssize_t size, align;
-    SETFUNC setfunc = NULL;
-    GETFUNC getfunc = NULL;
-    int fieldtype;
-#define NO_BITFIELD 0
-#define NEW_BITFIELD 1
-#define CONT_BITFIELD 2
-#define EXPAND_BITFIELD 3
-
     PyTypeObject *tp = st->PyCField_Type;
-    self = (CFieldObject *)tp->tp_alloc(tp, 0);
+    CFieldObject* self = (CFieldObject *)tp->tp_alloc(tp, 0);
-    if (self == NULL)
+    if (self == NULL) {
         return NULL;
-
+    }
     StgInfo *info;
     if (PyStgInfo_FromType(st, desc, &info) < 0) {
         Py_DECREF(self);
@@ -77,44 +205,13 @@ PyCField_FromDesc(ctypes_state *st, PyObject *desc, Py_ssize_t index,
         return NULL;
     }
 
-    if (bitsize /* this is a bitfield request */
+    PyObject* proto = desc;
-        && *pfield_size /* we have a bitfield open */
-#ifdef MS_WIN32
-        /* MSVC, GCC with -mms-bitfields */
-        && info->size * 8 == *pfield_size
-#else
-        /* GCC */
-        && info->size * 8 <= *pfield_size
-#endif
-        && (*pbitofs + bitsize) <= *pfield_size) {
-        /* continue bit field */
-        fieldtype = CONT_BITFIELD;
-#ifndef MS_WIN32
-    } else if (bitsize /* this is a bitfield request */
-        && *pfield_size /* we have a bitfield open */
-        && info->size * 8 >= *pfield_size
-        && (*pbitofs + bitsize) <= info->size * 8) {
-        /* expand bit field */
-        fieldtype = EXPAND_BITFIELD;
-#endif
-    } else if (bitsize) {
-        /* start new bitfield */
-        fieldtype = NEW_BITFIELD;
-        *pbitofs = 0;
-        *pfield_size = info->size * 8;
-    } else {
-        /* not a bit field */
-        fieldtype = NO_BITFIELD;
-        *pbitofs = 0;
-        *pfield_size = 0;
-    }
-
-    size = info->size;
-    proto = desc;
 
     /*  Field descriptors for 'c_char * n' are be scpecial cased to
         return a Python string instead of an Array object instance...
     */
+    SETFUNC setfunc = NULL;
+    GETFUNC getfunc = NULL;
     if (PyCArrayTypeObject_Check(st, proto)) {
         StgInfo *ainfo;
         if (PyStgInfo_FromType(st, proto, &ainfo) < 0) {
@@ -153,61 +250,43 @@ PyCField_FromDesc(ctypes_state *st, PyObject *desc, Py_ssize_t index,
 
     self->proto = Py_NewRef(proto);
 
-    switch (fieldtype) {
+    int is_bitfield = !!bitsize;
-    case NEW_BITFIELD:
+    if(!is_bitfield) {
-        if (big_endian)
+        assert(info->size >= 0);
-            self->size = (bitsize << 16) + *pfield_size - *pbitofs - bitsize;
+        // assert: no overflow;
-        else
+        assert((unsigned long long int) info->size
-            self->size = (bitsize << 16) + *pbitofs;
+            < (1ULL << (8*sizeof(Py_ssize_t)-1)) / 8);
-        *pbitofs = bitsize;
+        bitsize = 8 * info->size;
-        /* fall through */
+        // Caution: bitsize might still be 0 now.
-    case NO_BITFIELD:
-        if (pack)
-            align = min(pack, info->align);
-        else
-            align = info->align;
-        if (align && *poffset % align) {
-            Py_ssize_t delta = align - (*poffset % align);
-            *psize += delta;
-            *poffset += delta;
     }
+    assert(bitsize <= info->size * 8);
 
-        if (bitsize == 0)
+    int result;
-            self->size = size;
+    if (layout_mode == LAYOUT_MODE_MS) {
-        *psize += size;
+        result = PyCField_FromDesc_msvc(
-
+                pfield_size, bitsize, pbitofs,
-        self->offset = *poffset;
+                psize, poffset, palign,
-        *poffset += size;
+                pack,
-
+                self, info,
-        *palign = align;
+                is_bitfield
-        break;
+                );
-
+    } else {
-    case EXPAND_BITFIELD:
+        assert(pack == 0);
-        *poffset += info->size - *pfield_size/8;
+        result = PyCField_FromDesc_gcc(
-        *psize += info->size - *pfield_size/8;
+                bitsize, pbitofs,
-
+                psize, poffset, palign,
-        *pfield_size = info->size * 8;
+                self, info,
-
+                is_bitfield
-        if (big_endian)
+                );
-            self->size = (bitsize << 16) + *pfield_size - *pbitofs - bitsize;
+    }
-        else
+    if (result < 0) {
-            self->size = (bitsize << 16) + *pbitofs;
+        Py_DECREF(self);
-
+        return NULL;
-        self->offset = *poffset - size; /* poffset is already updated for the NEXT field */
+    }
-        *pbitofs += bitsize;
+    assert(!is_bitfield || (LOW_BIT(self->size) <= self->size * 8));
-        break;
+    if(big_endian && is_bitfield) {
-
+        self->size = BUILD_SIZE(NUM_BITS(self->size), 8*info->size - LOW_BIT(self->size) - bitsize);
-    case CONT_BITFIELD:
-        if (big_endian)
-            self->size = (bitsize << 16) + *pfield_size - *pbitofs - bitsize;
-        else
-            self->size = (bitsize << 16) + *pbitofs;
-
-        self->offset = *poffset - size; /* poffset is already updated for the NEXT field */
-        *pbitofs += bitsize;
-        break;
     }
-
     return (PyObject *)self;
 }
 
@@ -298,8 +377,8 @@ static PyObject *
 PyCField_repr(CFieldObject *self)
 {
     PyObject *result;
-    Py_ssize_t bits = self->size >> 16;
+    Py_ssize_t bits = NUM_BITS(self->size);
-    Py_ssize_t size = self->size & 0xFFFF;
+    Py_ssize_t size = LOW_BIT(self->size);
     const char *name;
 
     name = ((PyTypeObject *)self->proto)->tp_name;
@@ -396,8 +475,28 @@ get_ulonglong(PyObject *v, unsigned long long *p)
  */
 
 /* how to decode the size field, for integer get/set functions */
-#define LOW_BIT(x)  ((x) & 0xFFFF)
+static inline
-#define NUM_BITS(x) ((x) >> 16)
+Py_ssize_t LOW_BIT(Py_ssize_t offset) {
+    return offset & 0xFFFF;
+}
+static inline
+Py_ssize_t NUM_BITS(Py_ssize_t bitsize) {
+    return bitsize >> 16;
+}
+
+static inline
+Py_ssize_t BUILD_SIZE(Py_ssize_t bitsize, Py_ssize_t offset) {
+    assert(0 <= offset);
+    assert(offset <= 0xFFFF);
+    // We don't support zero length bitfields.
+    // And GET_BITFIELD uses NUM_BITS(size)==0,
+    // to figure out whether we are handling a bitfield.
+    assert(0 < bitsize);
+    Py_ssize_t result = (bitsize << 16) + offset;
+    assert(bitsize == NUM_BITS(result));
+    assert(offset == LOW_BIT(result));
+    return result;
+}
 
 /* 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)

Modules/_ctypes/ctypes.h

@@ -210,12 +210,17 @@ extern int PyObject_stginfo(PyObject *self, Py_ssize_t *psize, Py_ssize_t *palig
 
 extern struct fielddesc *_ctypes_get_fielddesc(const char *fmt);
 
+typedef enum {
+    LAYOUT_MODE_MS,
+    LAYOUT_MODE_GCC_SYSV,
+} LayoutMode;
 
 extern PyObject *
 PyCField_FromDesc(ctypes_state *st, PyObject *desc, Py_ssize_t index,
-                Py_ssize_t *pfield_size, int bitsize, int *pbitofs,
+                Py_ssize_t *pfield_size, Py_ssize_t bitsize,
-                Py_ssize_t *psize, Py_ssize_t *poffset, Py_ssize_t *palign,
+                Py_ssize_t *pbitofs, Py_ssize_t *psize, Py_ssize_t *poffset,
-                int pack, int is_big_endian);
+                Py_ssize_t *palign,
+                int pack, int is_big_endian, LayoutMode layout_mode);
 
 extern PyObject *PyCData_AtAddress(ctypes_state *st, PyObject *type, void *buf);
 extern PyObject *PyCData_FromBytes(ctypes_state *st, PyObject *type, char *data, Py_ssize_t length);

Modules/_ctypes/stgdict.c

@@ -243,7 +243,7 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
     Py_ssize_t len, offset, size, align, i;
     Py_ssize_t union_size, total_align, aligned_size;
     Py_ssize_t field_size = 0;
-    int bitofs;
+    Py_ssize_t bitofs = 0;
     PyObject *tmp;
     int pack;
     int forced_alignment = 1;
@@ -287,6 +287,38 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
         pack = 0;
     }
 
+    #ifdef MS_WIN32
+    LayoutMode layout_mode = LAYOUT_MODE_MS;
+    #else
+    LayoutMode layout_mode = (pack > 0) ? LAYOUT_MODE_MS : LAYOUT_MODE_GCC_SYSV;
+    #endif
+
+    if (PyObject_GetOptionalAttr(type, &_Py_ID(_layout_), &tmp) < 0) {
+        return -1;
+    }
+    if (tmp) {
+        if (!PyUnicode_Check(tmp)) {
+            PyErr_SetString(PyExc_TypeError,
+                            "_layout_ must be a string");
+            return -1;
+        }
+        if (PyUnicode_CompareWithASCIIString(tmp, "ms") == 0) {
+            layout_mode = LAYOUT_MODE_MS;
+        }
+        else if (PyUnicode_CompareWithASCIIString(tmp, "gcc-sysv") == 0) {
+            layout_mode = LAYOUT_MODE_GCC_SYSV;
+            if (pack > 0) {
+                PyErr_SetString(PyExc_ValueError,
+                                "_pack_ is not compatible with _layout_=\"gcc-sysv\"");
+                return -1;
+            }
+        }
+        else {
+            PyErr_Format(PyExc_ValueError,
+                            "unknown _layout_ %R", tmp);
+            return -1;
+        }
+    }
     if (PyObject_GetOptionalAttr(type, &_Py_ID(_align_), &tmp) < 0) {
         return -1;
     }
@@ -409,9 +441,9 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
         PyObject *name = NULL, *desc = NULL;
         PyObject *pair = PySequence_GetItem(fields, i);
         PyObject *prop;
-        int bitsize = 0;
+        Py_ssize_t bitsize = 0;
 
-        if (!pair || !PyArg_ParseTuple(pair, "UO|i", &name, &desc, &bitsize)) {
+        if (!pair || !PyArg_ParseTuple(pair, "UO|n", &name, &desc, &bitsize)) {
             PyErr_SetString(PyExc_TypeError,
                             "'_fields_' must be a sequence of (name, C type) pairs");
             Py_XDECREF(pair);
@@ -465,8 +497,9 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
                 return -1;
             }
             if (bitsize <= 0 || bitsize > info->size * 8) {
-                PyErr_SetString(PyExc_ValueError,
+                PyErr_Format(PyExc_ValueError,
-                                "number of bits invalid for bit field");
+                                "number of bits invalid for bit field %R",
+                                name);
                 Py_DECREF(pair);
                 return -1;
             }
@@ -493,7 +526,7 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
             prop = PyCField_FromDesc(st, desc, i,
                                    &field_size, bitsize, &bitofs,
                                    &size, &offset, &align,
-                                   pack, big_endian);
+                                   pack, big_endian, layout_mode);
             if (prop == NULL) {
                 Py_DECREF(pair);
                 return -1;
@@ -541,13 +574,15 @@ PyCStructUnionType_update_stginfo(PyObject *type, PyObject *fields, int isStruct
                 return -1;
             }
         } else /* union */ {
+            field_size = 0;
             size = 0;
+            bitofs = 0;
             offset = 0;
             align = 0;
             prop = PyCField_FromDesc(st, desc, i,
                                    &field_size, bitsize, &bitofs,
                                    &size, &offset, &align,
-                                   pack, big_endian);
+                                   pack, big_endian, layout_mode);
             if (prop == NULL) {
                 Py_DECREF(pair);
                 return -1;

PCbuild/_ctypes_test.vcxproj

@@ -94,6 +94,7 @@
   </PropertyGroup>
   <ItemGroup>
     <ClInclude Include="..\Modules\_ctypes\_ctypes_test.h" />
+    <ClInclude Include="..\Modules\_ctypes\_ctypes_test_generated.c.h" />
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="..\Modules\_ctypes\_ctypes_test.c" />

PCbuild/_ctypes_test.vcxproj.filters

@@ -15,6 +15,9 @@
     <ClInclude Include="..\Modules\_ctypes\_ctypes_test.h">
       <Filter>Header Files</Filter>
     </ClInclude>
+    <ClInclude Include="..\Modules\_ctypes\_ctypes_test_generated.c.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>
   <ItemGroup>
     <ClCompile Include="..\Modules\_ctypes\_ctypes_test.c">