[clang] [clang][layout] Fix unsigned char overflow in ms_struct bitfield layout (PR #181433)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Akram Hany (akramhany)

<details>
<summary>Changes</summary>

Fixes #<!-- -->161511

When using MS-struct layout rules (`#pragma ms_struct`), Clang produces incorrect memory layouts when encountering large `_BitInt` bit-fields. The reason was that `LastBitfieldStorageUnitSize` variable was declared as `unsigned char`, which caused it to overflow when the values used with `_BitInt` exceeded `255`, so I changed it from `unsigned char` to `uint64_t`.

In the added test, we can clearly see that `f2` and `f3` are both packed in the 256-bit unit.

---
Full diff: https://github.com/llvm/llvm-project/pull/181433.diff


2 Files Affected:

- (modified) clang/lib/AST/RecordLayoutBuilder.cpp (+1-1) 
- (added) clang/test/Layout/ms-x86-bitfields-overflow.c (+21) 


``````````diff
diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp
index 36b8eea92c0a4..e55ccb50608b8 100644
--- a/clang/lib/AST/RecordLayoutBuilder.cpp
+++ b/clang/lib/AST/RecordLayoutBuilder.cpp
@@ -619,7 +619,7 @@ class ItaniumRecordLayoutBuilder {
 
   /// LastBitfieldStorageUnitSize - If IsMsStruct, represents the size of the
   /// storage unit of the previous field if it was a bitfield.
-  unsigned char LastBitfieldStorageUnitSize;
+  uint64_t LastBitfieldStorageUnitSize;
 
   /// MaxFieldAlignment - The maximum allowed field alignment. This is set by
   /// #pragma pack.
diff --git a/clang/test/Layout/ms-x86-bitfields-overflow.c b/clang/test/Layout/ms-x86-bitfields-overflow.c
new file mode 100644
index 0000000000000..d8964f32cabad
--- /dev/null
+++ b/clang/test/Layout/ms-x86-bitfields-overflow.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fms-extensions -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
+// RUN:            | FileCheck %s
+
+// Test that large _BitInt fields do not overflow the internal storage unit tracker (previously unsigned char).
+// If the bug exists, this struct splits into two 256-bit units.
+// If fixed, f2 and f3 are packed into a single 256-bit unit.
+
+#pragma ms_struct on
+
+struct __attribute__((packed, aligned(1))) A {
+  _BitInt(250) f2 : 2;
+  _BitInt(250) f3 : 2;
+};
+
+// CHECK-LABEL:   0 | struct A{{$}}
+// CHECK-NEXT:0:0-1 |   _BitInt(250) f2
+// CHECK-NEXT:0:2-3 |   _BitInt(250) f3
+// CHECK-NEXT:      | [sizeof=32, align=32]
+
+// Force the compiler to layout the struct
+int x[sizeof(struct A)];

``````````

</details>


https://github.com/llvm/llvm-project/pull/181433