[clang] 8a05c20 - Add an off-by-default warning to complain about MSVC bitfield padding (#117428)
via cfe-commits
cfe-commits at lists.llvm.org
Tue May 13 14:00:24 PDT 2025
Author: Oliver Hunt
Date: 2025-05-13T14:00:20-07:00
New Revision: 8a05c20c963db27db0c93b422dab061a0d53a91f
URL: https://github.com/llvm/llvm-project/commit/8a05c20c963db27db0c93b422dab061a0d53a91f
DIFF: https://github.com/llvm/llvm-project/commit/8a05c20c963db27db0c93b422dab061a0d53a91f.diff
LOG: Add an off-by-default warning to complain about MSVC bitfield padding (#117428)
This just adds a warning for bitfields placed next to other bitfields
where the underlying type has different storage. Under the MS struct
bitfield packing ABI such bitfields are not packed.
Added:
clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
Modified:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/bitfield-layout.c
clang/test/Sema/bitfield-layout_1.c
clang/test/Sema/mms-bitfields.c
clang/test/SemaCXX/bitfield.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index bc13d02e2d20b..44c38396c764f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -524,6 +524,10 @@ Improvements to Clang's diagnostics
- An error is now emitted when OpenMP ``collapse`` and ``ordered`` clauses have an
argument larger than what can fit within a 64-bit integer.
+- A new off-by-default warning ``-Wms-bitfield-padding`` has been added to alert to cases where bit-field
+ packing may
diff er under the MS struct ABI (#GH117428).
+
+
Improvements to Clang's time-trace
----------------------------------
diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td
index 5fb5f16680b41..65d66dd398ad1 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8680,6 +8680,7 @@ its underlying representation to be a WebAssembly ``funcref``.
def PreferredTypeDocumentation : Documentation {
let Category = DocCatField;
+ let Label = "langext-preferred_type_documentation";
let Content = [{
This attribute allows adjusting the type of a bit-field in debug information.
This can be helpful when a bit-field is intended to store an enumeration value,
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 7b0dcde44296e..5a3e756f07ecc 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -690,6 +690,52 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
def PaddedBitField : DiagGroup<"padded-bitfield">;
def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-padding"> {
+ code Documentation = [{
+ Under the Microsoft ABI, adjacent bit-fields are not packed if the
+ underlying type has a
diff erent storage size. This warning indicates that a
+ pair of adjacent bit-fields may not pack in the same way due to this behavioural
+
diff erence.
+
+ This can occur when mixing
diff erent types explicitly:
+
+ .. code-block:: c++
+
+ struct S {
+ uint16_t field1 : 1;
+ uint32_t field2 : 1;
+ };
+
+ or more subtly through enums
+
+ .. code-block:: c++
+
+ enum Enum1 { /* ... */ };
+ enum class Enum2 : unsigned char { /* ... */ };
+ struct S {
+ Enum1 field1 : 1;
+ Enum2 field2 : 1;
+ };
+
+ In each of these cases under the Microsoft ABI the second bit-field
+ will not be packed with the preceding bit-field, and instead will be aligned
+ as if the fields were each separately defined integer fields of their respective
+ storage size. For binary compatibility this is obviously and observably
+ incompatible, however where bit-fields are being used solely for memory use
+ reduction this incomplete packing may silently increase the size of objects vs
+ what is expected.
+
+ This issue can be addressed by ensuring the storage type of each bit-field is
+ the same, either by explicitly using the same integer type, or in the case of
+ enum types declaring the enum types with the same storage size. For enum types
+ where you cannot specify the underlying type, the options are to either switch
+ to int sized storage for all specifiers or to resort to declaring the
+ bit-fields with explicit integer storage types and cast in and out of the field.
+ If such a solution is required the
+ :ref:`preferred_type <langext-preferred_type_documentation>` attribute can be
+ used to convey the actual field type to debuggers and other tooling.
+ }];
+}
def PessimizingMove : DiagGroup<"pessimizing-move">;
def ReturnStdMove : DiagGroup<"return-std-move">;
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 3efe9593b8633..d6afd5aebdb53 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6541,6 +6541,13 @@ def note_change_bitfield_sign : Note<
"consider making the bit-field type %select{unsigned|signed}0">;
def note_bitfield_preferred_type
: Note<"preferred type for bit-field %0 specified here">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a
diff erent storage size than the "
+ "preceding bit-field (%2 vs %3 bytes) and will not be packed under "
+ "the Microsoft ABI">,
+ InGroup<MSBitfieldCompatibility>, DefaultIgnore;
+def note_ms_bitfield_mismatched_storage_size_previous : Note<
+ "preceding bit-field %0 declared here with type %1">;
def warn_missing_braces : Warning<
"suggest braces around initialization of subobject">,
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 152f3f340cd50..a7d59ec232b64 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19399,9 +19399,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector<FieldDecl*, 32> RecFields;
-
+ const FieldDecl *PreviousField = nullptr;
for (ArrayRef<Decl *>::iterator i = Fields.begin(), end = Fields.end();
- i != end; ++i) {
+ i != end; PreviousField = cast<FieldDecl>(*i), ++i) {
FieldDecl *FD = cast<FieldDecl>(*i);
// Get the type for the field.
@@ -19617,6 +19617,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+ bool ReportMSBitfieldStoragePacking =
+ Record && PreviousField &&
+ !Diags.isIgnored(diag::warn_ms_bitfield_mismatched_storage_packing,
+ Record->getLocation());
+ auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ return FD->isBitField() && !FD->getType()->isDependentType();
+ };
+
+ if (ReportMSBitfieldStoragePacking && IsNonDependentBitField(FD) &&
+ IsNonDependentBitField(PreviousField)) {
+ CharUnits FDStorageSize = Context.getTypeSizeInChars(FD->getType());
+ CharUnits PreviousFieldStorageSize =
+ Context.getTypeSizeInChars(PreviousField->getType());
+ if (FDStorageSize != PreviousFieldStorageSize) {
+ Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+ << FD << FD->getType() << FDStorageSize.getQuantity()
+ << PreviousFieldStorageSize.getQuantity();
+ Diag(PreviousField->getLocation(),
+ diag::note_ms_bitfield_mismatched_storage_size_previous)
+ << PreviousField << PreviousField->getType();
+ }
+ }
// Keep track of the number of named members.
if (FD->getIdentifier())
++NumNamedMembers;
diff --git a/clang/test/Sema/bitfield-layout.c b/clang/test/Sema/bitfield-layout.c
index 079720cc9b40b..595b24d3e857e 100644
--- a/clang/test/Sema/bitfield-layout.c
+++ b/clang/test/Sema/bitfield-layout.c
@@ -3,6 +3,8 @@
// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=aarch64-linux-gnu
// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=x86_64-pc-linux-gnu
// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=x86_64-scei-ps4
+// RUN: %clang_cc1 %s -fsyntax-only -verify=checkms -triple=i686-apple-darwin9 -Wms-bitfield-padding
+
// expected-no-diagnostics
#include <stddef.h>
@@ -24,12 +26,27 @@ CHECK_ALIGN(struct, a, 1)
#endif
// Zero-width bit-fields with packed
-struct __attribute__((packed)) a2 { short x : 9; char : 0; int y : 17; };
+struct __attribute__((packed)) a2 {
+ short x : 9; // #a2x
+ char : 0; // #a2anon
+ // checkms-warning at -1 {{bit-field '' of type 'char' has a
diff erent storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#a2x {{preceding bit-field 'x' declared here with type 'short'}}
+ int y : 17;
+ // checkms-warning at -1 {{bit-field 'y' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#a2anon {{preceding bit-field '' declared here with type 'char'}}
+};
+
CHECK_SIZE(struct, a2, 5)
CHECK_ALIGN(struct, a2, 1)
// Zero-width bit-fields at the end of packed struct
-struct __attribute__((packed)) a3 { short x : 9; int : 0; };
+struct __attribute__((packed)) a3 {
+ short x : 9; // #a3x
+ int : 0;
+ // checkms-warning at -1 {{bit-field '' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#a3x {{preceding bit-field 'x' declared here with type 'short'}}
+};
+
#if defined(__arm__) || defined(__aarch64__)
CHECK_SIZE(struct, a3, 4)
CHECK_ALIGN(struct, a3, 4)
@@ -39,7 +56,12 @@ CHECK_ALIGN(struct, a3, 1)
#endif
// For comparison, non-zero-width bit-fields at the end of packed struct
-struct __attribute__((packed)) a4 { short x : 9; int : 1; };
+struct __attribute__((packed)) a4 {
+ short x : 9; // #a4x
+ int : 1;
+ // checkms-warning at -1 {{bit-field '' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#a4x {{preceding bit-field 'x' declared here with type 'short'}}
+};
CHECK_SIZE(struct, a4, 2)
CHECK_ALIGN(struct, a4, 1)
@@ -165,22 +187,28 @@ CHECK_OFFSET(struct, g4, c, 3);
#endif
struct g5 {
- char : 1;
+ char : 1; // #g5
__attribute__((aligned(1))) int n : 24;
+ // checkms-warning at -1 {{bit-field 'n' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#g5 {{preceding bit-field '' declared here with type 'char'}}
};
CHECK_SIZE(struct, g5, 4);
CHECK_ALIGN(struct, g5, 4);
struct __attribute__((packed)) g6 {
- char : 1;
+ char : 1; // #g6
__attribute__((aligned(1))) int n : 24;
+ // checkms-warning at -1 {{bit-field 'n' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#g6 {{preceding bit-field '' declared here with type 'char'}}
};
CHECK_SIZE(struct, g6, 4);
CHECK_ALIGN(struct, g6, 1);
struct g7 {
- char : 1;
+ char : 1; // #g7
__attribute__((aligned(1))) int n : 25;
+ // checkms-warning at -1 {{bit-field 'n' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#g7 {{preceding bit-field '' declared here with type 'char'}}
};
#if defined(__ORBIS__)
CHECK_SIZE(struct, g7, 4);
@@ -190,8 +218,10 @@ CHECK_SIZE(struct, g7, 8);
CHECK_ALIGN(struct, g7, 4);
struct __attribute__((packed)) g8 {
- char : 1;
+ char : 1; // #g8
__attribute__((aligned(1))) int n : 25;
+ // checkms-warning at -1 {{bit-field 'n' of type 'int' has a
diff erent storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#g8 {{preceding bit-field '' declared here with type 'char'}}
};
#if defined(__ORBIS__)
CHECK_SIZE(struct, g8, 4);
diff --git a/clang/test/Sema/bitfield-layout_1.c b/clang/test/Sema/bitfield-layout_1.c
index 24277c3911495..3db83c7463503 100644
--- a/clang/test/Sema/bitfield-layout_1.c
+++ b/clang/test/Sema/bitfield-layout_1.c
@@ -2,6 +2,7 @@
// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=arm-linux-gnueabihf
// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=aarch64-linux-gnu
// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=x86_64-pc-linux-gnu
+// RUN: %clang_cc1 %s -fsyntax-only -verify -triple=i686-apple-darwin9 -Wms-bitfield-padding
// expected-no-diagnostics
#define CHECK_SIZE(name, size) \
diff --git a/clang/test/Sema/mms-bitfields.c b/clang/test/Sema/mms-bitfields.c
index cee5b0669d252..a976578845229 100644
--- a/clang/test/Sema/mms-bitfields.c
+++ b/clang/test/Sema/mms-bitfields.c
@@ -1,12 +1,16 @@
// RUN: %clang_cc1 -mms-bitfields -fsyntax-only -verify -triple x86_64-apple-darwin9 %s
+// RUN: %clang_cc1 -mms-bitfields -fsyntax-only -Wms-bitfield-padding -verify=checkms -triple x86_64-apple-darwin9 %s
+
// expected-no-diagnostics
// The -mms-bitfields commandline parameter should behave the same
// as the ms_struct attribute.
struct
{
- int a : 1;
+ int a : 1; // #a
short b : 1;
+ // checkms-warning at -1 {{bit-field 'b' of type 'short' has a
diff erent storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed under the Microsoft ABI}}
+ // checkms-note@#a {{preceding bit-field 'a' declared here with type 'int'}}
} t;
// MS pads out bitfields between
diff erent types.
diff --git a/clang/test/SemaCXX/bitfield.cpp b/clang/test/SemaCXX/bitfield.cpp
index 083c28ffbb3d4..bb3094561bea4 100644
--- a/clang/test/SemaCXX/bitfield.cpp
+++ b/clang/test/SemaCXX/bitfield.cpp
@@ -1,4 +1,5 @@
// RUN: %clang_cc1 %s -verify
+// RUN: %clang_cc1 %s -verify -Wms-bitfield-padding
// expected-no-diagnostics
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 0000000000000..c0f90f798118a
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,196 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-padding -verify -triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields -verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
+
+// msbitfields-no-diagnostics
+
+enum Enum1 { Enum1_A, Enum1_B };
+enum Enum2 { Enum2_A, Enum2_B };
+
+enum class EnumU32_1 : unsigned { A, B };
+enum class EnumU32_2 : unsigned { A, B };
+enum class EnumU64 : unsigned long long { A, B };
+enum class EnumI32 : int { A, B };
+enum class EnumU8 : unsigned char { A, B };
+enum class EnumI8 : char { A, B };
+enum class EnumU16 : unsigned short { A, B };
+enum class EnumI16 : short { A, B };
+
+struct A {
+ unsigned int a : 15;
+ unsigned int b : 15;
+};
+static_assert(sizeof(A) == 4);
+
+struct B {
+ unsigned int a : 15;
+ int b : 15;
+};
+static_assert(sizeof(B) == 4);
+
+struct C {
+ unsigned int a : 15;
+ int b : 15;
+};
+static_assert(sizeof(C) == 4);
+
+struct D {
+ Enum1 a : 15;
+ Enum1 b : 15;
+};
+static_assert(sizeof(D) == 4);
+
+struct E {
+ Enum1 a : 15;
+ Enum2 b : 15;
+};
+static_assert(sizeof(E) == 4);
+
+struct F {
+ EnumU32_1 a : 15;
+ EnumU32_2 b : 15;
+};
+static_assert(sizeof(F) == 4);
+
+struct G {
+ EnumU32_1 a : 15;
+ EnumU64 b : 15;
+ // expected-warning at -1 {{bit-field 'b' of type 'EnumU64' has a
diff erent storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
+};
+
+#ifdef MS_BITFIELDS
+ static_assert(sizeof(G) == 16);
+#else
+ static_assert(sizeof(G) == 8);
+#endif
+
+struct H {
+ EnumU32_1 a : 10;
+ EnumI32 b : 10;
+ EnumU32_1 c : 10;
+};
+static_assert(sizeof(H) == 4);
+
+struct I {
+ EnumU8 a : 3;
+ EnumI8 b : 5;
+ EnumU32_1 c : 10;
+ // expected-warning at -1 {{bit-field 'c' of type 'EnumU32_1' has a
diff erent storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'b' declared here with type 'EnumI8'}}
+};
+#ifdef MS_BITFIELDS
+static_assert(sizeof(I) == 8);
+#else
+static_assert(sizeof(I) == 4);
+#endif
+
+struct J {
+ EnumU8 : 0;
+ EnumU8 b : 4;
+};
+static_assert(sizeof(J) == 1);
+
+struct K {
+ EnumU8 a : 4;
+ EnumU8 : 0;
+};
+static_assert(sizeof(K) == 1);
+
+struct L {
+ EnumU32_1 a : 10;
+ EnumU32_2 b : 10;
+ EnumU32_1 c : 10;
+};
+
+static_assert(sizeof(L) == 4);
+
+struct M {
+ EnumU32_1 a : 10;
+ EnumI32 b : 10;
+ EnumU32_1 c : 10;
+};
+
+static_assert(sizeof(M) == 4);
+
+struct N {
+ EnumU32_1 a : 10;
+ EnumU64 b : 10;
+ // expected-warning at -1 {{bit-field 'b' of type 'EnumU64' has a
diff erent storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
+ EnumU32_1 c : 10;
+ // expected-warning at -1 {{bit-field 'c' of type 'EnumU32_1' has a
diff erent storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -5 {{preceding bit-field 'b' declared here with type 'EnumU64'}}
+};
+
+#ifdef MS_BITFIELDS
+static_assert(sizeof(N) == 24);
+#else
+static_assert(sizeof(N) == 8);
+#endif
+
+struct O {
+ EnumU16 a : 10;
+ EnumU32_1 b : 10;
+ // expected-warning at -1 {{bit-field 'b' of type 'EnumU32_1' has a
diff erent storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'a' declared here with type 'EnumU16'}}
+};
+#ifdef MS_BITFIELDS
+static_assert(sizeof(O) == 8);
+#else
+static_assert(sizeof(O) == 4);
+#endif
+
+struct P {
+ EnumU32_1 a : 10;
+ EnumU16 b : 10;
+ // expected-warning at -1 {{bit-field 'b' of type 'EnumU16' has a
diff erent storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
+};
+#ifdef MS_BITFIELDS
+static_assert(sizeof(P) == 8);
+#else
+static_assert(sizeof(P) == 4);
+#endif
+
+struct Q {
+ EnumU8 a : 6;
+ EnumU16 b : 6;
+ // expected-warning at -1 {{bit-field 'b' of type 'EnumU16' has a
diff erent storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'a' declared here with type 'EnumU8'}}
+};
+#ifdef MS_BITFIELDS
+static_assert(sizeof(Q) == 4);
+#else
+static_assert(sizeof(Q) == 2);
+#endif
+
+struct R {
+ EnumU16 a : 9;
+ EnumU16 b : 9;
+ EnumU8 c : 6;
+ // expected-warning at -1 {{bit-field 'c' of type 'EnumU8' has a
diff erent storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'b' declared here with type 'EnumU16'}}
+};
+
+#ifdef MS_BITFIELDS
+static_assert(sizeof(R) == 6);
+#else
+static_assert(sizeof(R) == 4);
+#endif
+
+struct S {
+ char a : 4;
+ char b : 4;
+ char c : 4;
+ char d : 4;
+ short x : 7;
+ // expected-warning at -1 {{bit-field 'x' of type 'short' has a
diff erent storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed under the Microsoft ABI}}
+ // expected-note at -3 {{preceding bit-field 'd' declared here with type 'char'}}
+ // This is a false positive. Reporting this correctly requires duplicating the record layout process
+ // in target and MS layout modes, and it's also unclear if that's the correct choice for users of
+ // this diagnostic.
+ short y : 9;
+};
+
+static_assert(sizeof(S) == 4);
More information about the cfe-commits
mailing list