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 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
