[clang] [Clang][CodeGen] Fix assertion failure in bitfield access unit clipping with #pragma pack and ms_struct (PR #181508)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang-codegen

Author: Giovanni B. (Z3rox-dev)

<details>
<summary>Changes</summary>

When `#pragma pack(N)` with N < `alignof(int)` is combined with `ms_struct` layout
and a zero-width bitfield, the bitfield storage unit could extend past the zero-width
bitfield boundary. This caused the subsequent member's storage to overlap, triggering
the `"Bitfield access unit is not clipped"` assertion in `checkBitfieldClipping()`.

The fix detects when a zero-width bitfield terminates a bitfield run whose storage
extends beyond it, and clips the storage type to a byte array (`[N x i8]`) of the
exact size needed. We use `getByteArrayType()` instead of `getIntNType()` because
LLVM's `TypeAllocSize` for `iN` types rounds up (e.g. `i24` -> 4 bytes), which would
not actually clip the storage.

Fixes #<!-- -->181505

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


2 Files Affected:

- (modified) clang/lib/CodeGen/CGRecordLayoutBuilder.cpp (+23) 
- (added) clang/test/CodeGen/ms_struct-pack-bitfield-clip.c (+62) 


``````````diff
diff --git a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
index e9205c68c2812..d47929fb1cde3 100644
--- a/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
+++ b/clang/lib/CodeGen/CGRecordLayoutBuilder.cpp
@@ -424,6 +424,29 @@ CGRecordLowering::accumulateBitFields(bool isNonVirtualBaseType,
     for (; Field != FieldEnd && Field->isBitField(); ++Field) {
       // Zero-width bitfields end runs.
       if (Field->isZeroLengthBitField()) {
+        // If the current run's storage extends beyond this zero-width
+        // bitfield, clip it to prevent overlap with subsequent members.
+        // This can happen with #pragma pack when the packed alignment is
+        // smaller than the bitfield's formal type, causing the storage
+        // unit to extend into the next run's territory.
+        if (Run != FieldEnd) {
+          uint64_t BitOffset = getFieldBitOffset(*Field);
+          if (Tail > BitOffset) {
+            CharUnits RunBytes = bitsToCharUnits(BitOffset) -
+                                 bitsToCharUnits(StartBitOffset);
+            llvm::Type *ClippedType = getByteArrayType(RunBytes);
+            // Walk backward through Members to find and replace the
+            // storage member for the current run.
+            CharUnits RunStart = bitsToCharUnits(StartBitOffset);
+            for (size_t I = Members.size(); I > 0; --I) {
+              if (Members[I - 1].Data &&
+                  Members[I - 1].Offset == RunStart) {
+                Members[I - 1].Data = ClippedType;
+                break;
+              }
+            }
+          }
+        }
         Run = FieldEnd;
         continue;
       }
diff --git a/clang/test/CodeGen/ms_struct-pack-bitfield-clip.c b/clang/test/CodeGen/ms_struct-pack-bitfield-clip.c
new file mode 100644
index 0000000000000..dc49cbb1f183a
--- /dev/null
+++ b/clang/test/CodeGen/ms_struct-pack-bitfield-clip.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -emit-llvm-only -triple x86_64-w64-windows-gnu %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i686-w64-windows-gnu %s
+// RUN: %clang_cc1 -emit-llvm-only -triple i386-apple-darwin9 %s
+
+// Regression test: when #pragma pack(N) with N < alignof(int) is combined
+// with ms_struct layout and a zero-width bitfield, the bitfield storage unit
+// could extend past the zero-width bitfield boundary, overlapping the next
+// member's storage and triggering the "Bitfield access unit is not clipped"
+// assertion in checkBitfieldClipping().
+
+// Minimal case: pack(1) + char + bitfield + :0 + bitfield
+#pragma pack(1)
+struct S0 {
+  char f1;
+  unsigned f3 : 1;
+  unsigned : 0;
+  unsigned f4 : 1;
+} __attribute__((__ms_struct__));
+struct S0 s0;
+
+// Variation: pack(1) with more complex bitfield layout (original CSmith case)
+struct S1 {
+  signed f0 : 28;
+  const signed char f1;
+  const volatile signed f2 : 25;
+  const unsigned f3 : 23;
+  unsigned : 0;
+  unsigned f4 : 12;
+  volatile unsigned f5 : 29;
+} __attribute__((__ms_struct__));
+struct S1 s1;
+
+// Variation: pack(2) with short-sized first member
+#pragma pack(2)
+struct S2 {
+  short f1;
+  unsigned f3 : 1;
+  unsigned : 0;
+  unsigned f4 : 1;
+} __attribute__((__ms_struct__));
+struct S2 s2;
+
+// Variation: pack(1), multiple zero-width bitfields
+#pragma pack(1)
+struct S3 {
+  char f1;
+  unsigned f3 : 1;
+  unsigned : 0;
+  unsigned f4 : 1;
+  unsigned : 0;
+  unsigned f5 : 1;
+} __attribute__((__ms_struct__));
+struct S3 s3;
+
+// Variation: pack(1), zero-width of different type
+struct S4 {
+  char f1;
+  unsigned f3 : 1;
+  short : 0;
+  unsigned f4 : 1;
+} __attribute__((__ms_struct__));
+struct S4 s4;

``````````

</details>


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