Implement struct bitfield layout rules according to GCC (#1926)

BCSharp · BCSharp · commit b160c3e530d9 · 2025-03-25T15:07:14.000-07:00
diff --git a/src/core/IronPython.Modules/_ctypes/StructType.cs b/src/core/IronPython.Modules/_ctypes/StructType.cs
@@ -12,6 +12,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Numerics;
 using System.Reflection.Emit;
+using System.Runtime.InteropServices;
 using System.Text;
 
 using Microsoft.Scripting;
@@ -261,22 +262,20 @@ private void SetFields(object? fields) {
                     INativeType? lastType = null;
                     List<Field> allFields = GetBaseSizeAlignmentAndFields(out int size, out int alignment);
 
-                    IList<object>? anonFields = GetAnonymousFields(this);
+                    IList<string>? anonFields = GetAnonymousFields(this);
 
                     foreach (object fieldDef in fieldDefList) {
                         GetFieldInfo(this, fieldDef, out string fieldName, out INativeType cdata, out bitCount);
 
-                        int prevSize = UpdateSizeAndAlignment(cdata, bitCount, lastType, ref size, ref alignment, ref curBitCount);
+                        int fieldOffset = UpdateSizeAndAlignment(cdata, bitCount, ref lastType, ref size, ref alignment, ref curBitCount);
 
-                        var newField = new Field(fieldName, cdata, prevSize, allFields.Count, bitCount, curBitCount - bitCount);
+                        var newField = new Field(fieldName, cdata, fieldOffset, allFields.Count, bitCount, curBitCount - bitCount);
                         allFields.Add(newField);
                         AddSlot(fieldName, newField);
 
                         if (anonFields != null && anonFields.Contains(fieldName)) {
                             AddAnonymousFields(this, allFields, cdata, newField);
                         }
-
-                        lastType = cdata;
                     }
 
                     CheckAnonymousFields(allFields, anonFields);
@@ -293,7 +292,7 @@ private void SetFields(object? fields) {
                 }
             }
 
-            internal static void CheckAnonymousFields(List<Field> allFields, IList<object>? anonFields) {
+            internal static void CheckAnonymousFields(List<Field> allFields, IList<string>? anonFields) {
                 if (anonFields != null) {
                     foreach (string s in anonFields) {
                         bool found = false;
@@ -311,18 +310,26 @@ internal static void CheckAnonymousFields(List<Field> allFields, IList<object>?
                 }
             }
 
-            internal static IList<object>? GetAnonymousFields(PythonType type) {
-                object anonymous;
-                IList<object>? anonFields = null;
-                if (type.TryGetBoundAttr(type.Context.SharedContext, type, "_anonymous_", out anonymous)) {
-                    anonFields = anonymous as IList<object>;
-                    if (anonFields == null) {
+
+            internal static IList<string>? GetAnonymousFields(PythonType type) {
+                IList<string>? anonFieldNames = null;
+                if (type.TryGetBoundAttr(type.Context.SharedContext, type, "_anonymous_", out object anonymous)) {
+                    if (anonymous is not IList<object> anonFields) {
                         throw PythonOps.TypeError("_anonymous_ must be a sequence");
                     }
+                    anonFieldNames = [];
+                    foreach (object anonField in anonFields) {
+                        if (Converter.TryConvertToString(anonField, out string? anonFieldStr)) {
+                            anonFieldNames.Add(anonFieldStr);
+                        } else {
+                            throw PythonOps.TypeErrorForBadInstance("anonymous field must be a string, not '{0}'", anonField);
+                        }
+                    }
                 }
-                return anonFields;
+                return anonFieldNames;
             }
 
+
             internal static void AddAnonymousFields(PythonType type, List<Field> allFields, INativeType cdata, Field newField) {
                 Field[] childFields;
                 if (cdata is StructType st) {
@@ -348,6 +355,7 @@ internal static void AddAnonymousFields(PythonType type, List<Field> allFields,
                 }
             }
 
+
             private List<Field> GetBaseSizeAlignmentAndFields(out int size, out int alignment) {
                 size = 0;
                 alignment = 1;
@@ -359,63 +367,125 @@ private List<Field> GetBaseSizeAlignmentAndFields(out int size, out int alignmen
                         st.EnsureFinal();
                         foreach (Field f in st._fields) {
                             allFields.Add(f);
-                            UpdateSizeAndAlignment(f.NativeType, f.BitCount, lastType, ref size, ref alignment, ref totalBitCount);
+                            UpdateSizeAndAlignment(f.NativeType, f.BitCount, ref lastType, ref size, ref alignment, ref totalBitCount);
 
                             if (f.NativeType == this) {
                                 throw StructureCannotContainSelf();
                             }
-
-                            lastType = f.NativeType;
                         }
                     }
                 }
                 return allFields;
             }
 
-            private int UpdateSizeAndAlignment(INativeType cdata, int? bitCount, INativeType? lastType, ref int size, ref int alignment, ref int? totalBitCount) {
-                Debug.Assert(totalBitCount == null || lastType != null); // lastType is null only on the first iteration, when totalBitCount is null as well
-                int prevSize = size;
-                if (bitCount != null) {
-                    if (lastType != null && lastType.Size != cdata.Size) {
-                        totalBitCount = null;
-                        prevSize = size += lastType.Size;
-                    }
-
-                    size = PythonStruct.Align(size, cdata.Alignment);
 
-                    if (totalBitCount != null) {
-                        if ((bitCount + totalBitCount + 7) / 8 <= cdata.Size) {
-                            totalBitCount = bitCount + totalBitCount;
+            /// <summary>
+            /// Processes one field definition and allocates its data payload within the struct.
+            /// </summary>
+            /// <param name="cdata">
+            /// The type of the field to process.</param>
+            /// <param name="bitCount">
+            /// Width of the bitfield in bits. If the fields to process is not a bitfield, this value is null</param>
+            /// <param name="lastType">
+            /// The type of the last field (or container unit) processed in the struct. If processing the first field in the struct, this value is null.
+            /// On return, this value is updated with the processed field's type, or, if the processed field was a bitfield, with its container unit type.</param>
+            /// <param name="size">
+            /// The total size of the struct in bytes excluding an open bitfield container, if any.
+            /// On input, the size of the struct before the field was allocated.
+            /// On return, the size of the struct after the field has been processed.
+            /// If the processed field was a bitfield, the size may not have been increased yet, depending whether the bitfield fit in the current container unit.
+            /// So the full (current) struct size in bits is size * 8 + totalBitCount. </param>
+            /// <param name="alignment">
+            /// The total alignment of the struct (the common denominator of all fields).
+            /// This value is being updated as necessary with the alignment of the processed field.</param>
+            /// <param name="totalBitCount">
+            /// The number of already occupied bits in the currently open containment unit for bitfields.
+            /// If the previous field is not a bitfield, this value is null.
+            /// On return, the count is updated with the number of occupied bits.</param>
+            /// <returns>
+            /// The offset of the processed field within the struct. If the processed field was a bitfield, this is the offset of its container unit.</returns>
+            private int UpdateSizeAndAlignment(INativeType cdata, int? bitCount, ref INativeType? lastType, ref int size, ref int alignment, ref int? totalBitCount) {
+                int fieldOffset;
+                if (bitCount != null) {
+                    // process a bitfield
+                    Debug.Assert(bitCount <= cdata.Size * 8);
+                    Debug.Assert(totalBitCount == null || lastType != null);
+
+                    if (_pack != null) throw new NotImplementedException("pack with bitfields");  // TODO: implement
+
+                    if (UseMsvcBitfieldAlignmentRules) {
+                        if (totalBitCount != null) { // there is already a bitfield container open
+                            // under the MSVC rules, only bitfields of type that has the same size/alignment, are packed into the same container unit
+                            if (lastType!.Size != cdata.Size || lastType.Alignment != cdata.Alignment) {
+                                // if the bitfield type is not compatible with the type of the previous container unit, close the previous container unit
+                                size += lastType.Size;
+                                fieldOffset = size = PythonStruct.Align(size, cdata.Alignment);  // TODO: _pack
+                                totalBitCount = null;
+                            }
+                        }
+                        if (totalBitCount != null) {
+                            // container unit open
+                            if ((bitCount + totalBitCount + 7) / 8 <= cdata.Size) {
+                                // new bitfield fits into the container unit
+                                fieldOffset = size;
+                                totalBitCount += bitCount;
+                            } else {
+                                // new bitfield does not fit into the container unit, close it
+                                size += lastType!.Size;
+                                // and open a new container unit for the bitfield
+                                fieldOffset = size = PythonStruct.Align(size, cdata.Alignment);  // TODO: _pack
+                                totalBitCount = bitCount;
+                                lastType = cdata;
+                            }
                         } else {
-                            size += lastType!.Size;
-                            prevSize = size;
+                            // open a new container unit for the bitfield
+                            fieldOffset = size = PythonStruct.Align(size, cdata.Alignment);  // TODO: _pack
                             totalBitCount = bitCount;
+                            lastType = cdata;
                         }
-                    } else {
-                        totalBitCount = bitCount;
+                    } else {  // GCC bitfield alignment rules
+                        // under the GCC rules, all bitfields are packed into the same container unit or an overlapping container unit of a different type,
+                        // as long as they fit and match the alignment
+                        int containerOffset = AlignBack(size, cdata.Alignment);  // TODO: _pack
+                        int containerBitCount = (totalBitCount ?? 0) + (size - containerOffset) * 8;
+                        if (containerBitCount + bitCount > cdata.Size * 8) {
+                            // the bitfield does not fit into the container unit at this offset, find the nearest allowed offset
+                            int deltaOffset = cdata.Alignment; // TODO: _pack 
+                            int numOffsets = Math.Max(1, (containerBitCount + bitCount.Value - 1) / (deltaOffset * 8));
+                            containerOffset += numOffsets * deltaOffset;
+                            containerBitCount = Math.Max(0, containerBitCount - numOffsets * deltaOffset * 8);
+                        }
+                        // the bitfield now fits into the container unit at this offset
+                        Debug.Assert(containerBitCount + bitCount <= cdata.Size * 8);
+                        fieldOffset = size = containerOffset;
+                        totalBitCount = containerBitCount + bitCount;
+                        lastType = cdata;
                     }
+                    alignment = Math.Max(alignment, lastType!.Alignment);  // TODO: _pack
                 } else {
+                    // process a regular field
                     if (totalBitCount != null) {
+                        // last field was a bitfield; close its container unit to prepare for the next regular field
                         size += lastType!.Size;
-                        prevSize = size;
                         totalBitCount = null;
                     }
 
                     if (_pack != null) {
                         alignment = _pack.Value;
-                        prevSize = size = PythonStruct.Align(size, _pack.Value);
-
+                        fieldOffset = size = PythonStruct.Align(size, _pack.Value);
                         size += cdata.Size;
                     } else {
                         alignment = Math.Max(alignment, cdata.Alignment);
-                        prevSize = size = PythonStruct.Align(size, cdata.Alignment);
+                        fieldOffset = size = PythonStruct.Align(size, cdata.Alignment);
                         size += cdata.Size;
                     }
+                    lastType = cdata;
                 }
 
-                return prevSize;
+                return fieldOffset;
             }
 
+
             [MemberNotNull(nameof(_fields), nameof(_size), nameof(_alignment))]
             internal void EnsureFinal() {
                 if (_fields == null) {
@@ -452,6 +522,12 @@ private void EnsureSizeAndAlignment() {
                     throw new InvalidOperationException("size and alignment should always be initialized together");
                 }
             }
+
+            private static int AlignBack(int length, int size)
+                => length & ~(size - 1);
+
+            private static bool UseMsvcBitfieldAlignmentRules
+                => RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
         }
     }
 }
diff --git a/src/core/IronPython.Modules/_ctypes/UnionType.cs b/src/core/IronPython.Modules/_ctypes/UnionType.cs
@@ -166,7 +166,7 @@ string INativeType.TypeFormat {
             private void SetFields(object? fields) {
                 lock (this) {
                     IList<object> fieldDefList = GetFieldsList(fields);
-                    IList<object>? anonFields = StructType.GetAnonymousFields(this);
+                    IList<string>? anonFields = StructType.GetAnonymousFields(this);
 
                     int size = 0, alignment = 1;
                     List<Field> allFields = new List<Field>();//GetBaseSizeAlignmentAndFields(out size, out alignment);
diff --git a/src/core/IronPython.Modules/_ctypes/_ctypes.cs b/src/core/IronPython.Modules/_ctypes/_ctypes.cs
@@ -597,21 +597,17 @@ private static void GetFieldInfo(INativeType type, object o, out string fieldNam
             if (pt.Count != 3) {
                 bitCount = null;
             } else {
-                bitCount = CheckBits(cdata, pt);
+                bitCount = Converter.ConvertToInt32(pt[2]);
+                CheckBits(fieldName, cdata, bitCount.Value);
             }
         }
 
         /// <summary>
         /// Verifies that the provided bit field settings are valid for this type.
         /// </summary>
-        private static int CheckBits(INativeType cdata, PythonTuple pt) {
-            int bitCount = Converter.ConvertToInt32(pt[2]);
-
-            if (!(cdata is SimpleType simpType)) {
-                throw PythonOps.TypeError("bit fields not allowed for type {0}", ((PythonType)cdata).Name);
-            }
-
-            switch (simpType._type) {
+        private static void CheckBits(string fieldName, INativeType cdata, int bitCount) {
+            switch ((cdata as SimpleType)?._type) {
+                case null:
                 case SimpleTypeKind.Object:
                 case SimpleTypeKind.Pointer:
                 case SimpleTypeKind.Single:
@@ -620,13 +616,13 @@ private static int CheckBits(INativeType cdata, PythonTuple pt) {
                 case SimpleTypeKind.CharPointer:
                 case SimpleTypeKind.WChar:
                 case SimpleTypeKind.WCharPointer:
+                case SimpleTypeKind.BStr:
                     throw PythonOps.TypeError("bit fields not allowed for type {0}", ((PythonType)cdata).Name);
             }
 
             if (bitCount <= 0 || bitCount > cdata.Size * 8) {
-                throw PythonOps.ValueError("number of bits invalid for bit field");
+                throw PythonOps.ValueError("number of bits invalid for bit field '{0}'", fieldName);
             }
-            return bitCount;
         }
 
         /// <summary>
diff --git a/src/core/IronPython.StdLib/lib/ctypes/test/test_bitfields.py b/src/core/IronPython.StdLib/lib/ctypes/test/test_bitfields.py
@@ -143,10 +143,11 @@ def test_c_wchar(self):
     def test_single_bitfield_size(self):
         for c_typ in int_types:
             result = self.fail_fields(("a", c_typ, -1))
-            self.assertEqual(result, (ValueError, 'number of bits invalid for bit field'))
+            # IronPython: error message modified to match CPython 3.14
+            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)]
@@ -157,7 +158,7 @@ class X(Structure):
             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):
diff --git a/tests/IronPython.Tests/Cases/CPythonCasesManifest.ini b/tests/IronPython.Tests/Cases/CPythonCasesManifest.ini
@@ -7,9 +7,6 @@ Timeout=120000 # 2 minute timeout
 [CPython.ctypes.test_as_parameter]
 Ignore=true
 
-[CPython.ctypes.test_bitfields] # IronPython.modules.type_related.test_bitfields_ctypes_stdlib
-Ignore=true
-
 [CPython.ctypes.test_errno]
 Ignore=true
 Reason=Current implementation of get_last_error needs to be debugged
diff --git a/tests/suite/modules/type_related/test_bitfields_ctypes_stdlib.py b/tests/suite/modules/type_related/test_bitfields_ctypes_stdlib.py
diff --git a/tests/suite/modules/type_related/test_ctypes.py b/tests/suite/modules/type_related/test_ctypes.py
