https://github.com/ojhunt updated
https://github.com/llvm/llvm-project/pull/117428
>From 3e25d7ef2e223942298078dace8979905956d05c Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Fri, 22 Nov 2024 17:53:24 +0100
Subject: [PATCH 01/10] Add an off-by-default warning to complain about MSVC
bitfield padding
---
clang/include/clang/Basic/DiagnosticGroups.td | 1 +
.../clang/Basic/DiagnosticSemaKinds.td | 6 +
clang/lib/Sema/SemaDecl.cpp | 27 ++-
.../SemaCXX/ms_struct-bitfield-padding.cpp | 180 ++++++++++++++++++
4 files changed, 212 insertions(+), 2 deletions(-)
create mode 100644 clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index df9bf94b5d039..57bdda4b8f8b4 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,6 +631,7 @@ 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-packing-compatibility">;
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 eb05a6a77978a..aa13e3438d373 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
InGroup<BitFieldEnumConversion>, DefaultIgnore;
def note_change_bitfield_sign : Note<
"consider making the bitfield type %select{unsigned|signed}0">;
+def warn_ms_bitfield_mismatched_storage_packing : Warning<
+ "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
+ "preceding bit-field and may not be packed under MSVC 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 74b0e5ad23bd4..18aeca3b34f65 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector<FieldDecl*, 32> RecFields;
-
+ std::optional<const FieldDecl *> PreviousField;
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.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
+ auto IsNonDependentBitField = [](const FieldDecl *FD) {
+ if (!FD->isBitField())
+ return false;
+ if (FD->getType()->isDependentType())
+ return false;
+ return true;
+ };
+
+ if (Record && PreviousField && IsNonDependentBitField(FD) &&
+ IsNonDependentBitField(*PreviousField)) {
+ unsigned FDStorageSize =
+ Context.getTypeSizeInChars(FD->getType()).getQuantity();
+ unsigned PreviousFieldStorageSize =
+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ if (FDStorageSize != PreviousFieldStorageSize) {
+ Diag(FD->getLocation(),
+ diag::warn_ms_bitfield_mismatched_storage_packing)
+ << FD << FD->getType() << FDStorageSize <<
PreviousFieldStorageSize;
+ 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/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
new file mode 100644
index 0000000000000..001086de5bbd1
--- /dev/null
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -0,0 +1,180 @@
+
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -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@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size (8 vs 4 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size (4 vs 1 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size (8 vs 4 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
+ EnumU32_1 c : 10;
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size (4 vs 8 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size (4 vs 2 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size (2 vs 4 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size (2 vs 1 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size (1 vs 2 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-note@-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
>From 11447fea7197a0902c0b1c23071b35de3b782f13 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Sat, 30 Nov 2024 19:36:33 -0800
Subject: [PATCH 02/10] Address feedback
---
.../clang/Basic/DiagnosticSemaKinds.td | 5 ++--
clang/lib/Sema/SemaDecl.cpp | 24 ++++++++-----------
.../SemaCXX/ms_struct-bitfield-padding.cpp | 16 ++++++-------
3 files changed, 21 insertions(+), 24 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 98ed070e398a5..ce4f68bbb8bfc 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6423,8 +6423,9 @@ def warn_signed_bitfield_enum_conversion : Warning<
def note_change_bitfield_sign : Note<
"consider making the bit-field type %select{unsigned|signed}0">;
def warn_ms_bitfield_mismatched_storage_packing : Warning<
- "bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than
the "
- "preceding bit-field and may not be packed under MSVC ABI">,
+ "bit-field %0 of type %1 has a different storage size than the "
+ "preceding bit-field (%2 vs %3 bytes) and will not be packed under "
+ "the MSVC ABI">,
InGroup<MSBitfieldCompatibility>, DefaultIgnore;
def note_ms_bitfield_mismatched_storage_size_previous : Note<
"preceding bit-field %0 declared here with type %1">;
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 123e061404add..a71c31b0a5ae3 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19016,7 +19016,7 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc,
Decl *EnclosingDecl,
// Verify that all the fields are okay.
SmallVector<FieldDecl*, 32> RecFields;
- std::optional<const FieldDecl *> PreviousField;
+ const FieldDecl *PreviousField = nullptr;
for (ArrayRef<Decl *>::iterator i = Fields.begin(), end = Fields.end();
i != end; PreviousField = cast<FieldDecl>(*i), ++i) {
FieldDecl *FD = cast<FieldDecl>(*i);
@@ -19229,26 +19229,22 @@ void Sema::ActOnFields(Scope *S, SourceLocation
RecLoc, Decl *EnclosingDecl,
if (Record && FD->getType().isVolatileQualified())
Record->setHasVolatileMember(true);
auto IsNonDependentBitField = [](const FieldDecl *FD) {
- if (!FD->isBitField())
- return false;
- if (FD->getType()->isDependentType())
- return false;
- return true;
+ return FD->isBitField() && !FD->getType()->isDependentType();
};
if (Record && PreviousField && IsNonDependentBitField(FD) &&
- IsNonDependentBitField(*PreviousField)) {
- unsigned FDStorageSize =
- Context.getTypeSizeInChars(FD->getType()).getQuantity();
- unsigned PreviousFieldStorageSize =
-
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
+ 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 <<
PreviousFieldStorageSize;
- Diag((*PreviousField)->getLocation(),
+ << FD << FD->getType() << FDStorageSize.getQuantity()
+ << PreviousFieldStorageSize.getQuantity();
+ Diag(PreviousField->getLocation(),
diag::note_ms_bitfield_mismatched_storage_size_previous)
- << *PreviousField << (*PreviousField)->getType();
+ << PreviousField << PreviousField->getType();
}
}
// Keep track of the number of named members.
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
index 001086de5bbd1..11a786d47dd0a 100644
--- a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -55,7 +55,7 @@ static_assert(sizeof(F) == 4);
struct G {
EnumU32_1 a : 15;
EnumU64 b : 15;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size (8 vs 4 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
};
@@ -76,7 +76,7 @@ struct I {
EnumU8 a : 3;
EnumI8 b : 5;
EnumU32_1 c : 10;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size (4 vs 1 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'b' declared here with type
'EnumI8'}}
};
#ifdef MS_BITFIELDS
@@ -116,10 +116,10 @@ static_assert(sizeof(M) == 4);
struct N {
EnumU32_1 a : 10;
EnumU64 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size (8 vs 4 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
EnumU32_1 c : 10;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size (4 vs 8 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-5 {{preceding bit-field 'b' declared here with type
'EnumU64'}}
};
@@ -132,7 +132,7 @@ static_assert(sizeof(N) == 8);
struct O {
EnumU16 a : 10;
EnumU32_1 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size (4 vs 2 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU16'}}
};
#ifdef MS_BITFIELDS
@@ -144,7 +144,7 @@ static_assert(sizeof(O) == 4);
struct P {
EnumU32_1 a : 10;
EnumU16 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size (2 vs 4 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
};
#ifdef MS_BITFIELDS
@@ -156,7 +156,7 @@ static_assert(sizeof(P) == 4);
struct Q {
EnumU8 a : 6;
EnumU16 b : 6;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size (2 vs 1 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU8'}}
};
#ifdef MS_BITFIELDS
@@ -169,7 +169,7 @@ struct R {
EnumU16 a : 9;
EnumU16 b : 9;
EnumU8 c : 6;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size (1 vs 2 bytes) than the preceding bit-field and may not be packed
under MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed
under the MSVC ABI}}
// expected-note@-3 {{preceding bit-field 'b' declared here with type
'EnumU16'}}
};
>From a18c032c7b7b57be64d0f60aa7768b078adae992 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Sun, 1 Dec 2024 23:23:47 -0800
Subject: [PATCH 03/10] Documentation and renaming
---
clang/include/clang/Basic/DiagnosticGroups.td | 45 ++++++++++++++++++-
.../SemaCXX/ms_struct-bitfield-padding.cpp | 2 +-
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index 69fae8c650ec9..7e679169302fb 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -631,7 +631,50 @@ 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-packing-compatibility">;
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-compatibility"> {
+ code Documentation = [{
+ Under the MSVC ABI adjacemt bit-fields are not packed if the underlying
type
+ has a different storage size. This warning indicates that a pair of
adjacent
+ bit-fields may not pack in the same way due to this behavioural difference.
+
+ This can occur when mixing different 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 MSVC 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 `preferred_type` 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/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
index 11a786d47dd0a..fd1eb30bfe376 100644
--- a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify
-triple armv8 -std=c++23 %s
+// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-compatibility -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
>From 75a46fe44b70f565bbe7f1ac0ee1ddf1bb0c171e Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Sun, 1 Dec 2024 23:30:16 -0800
Subject: [PATCH 04/10] Add release note
---
clang/docs/ReleaseNotes.rst | 3 +++
1 file changed, 3 insertions(+)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index e44aefa90ab38..5605656c3df48 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -629,6 +629,9 @@ Improvements to Clang's diagnostics
- Clang now diagnoses dangling references for C++20's parenthesized aggregate
initialization (#101957).
+- A new off-by-default warning ``-Wms-bitfield-compatibility`` has been added
to alert to cases where bit-field
+ packing may differ under the MS struct ABI (#GH117428).
+
Improvements to Clang's time-trace
----------------------------------
>From bd3ffbe2c1f7853665eed436a75d0a30eb611256 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Mon, 2 Dec 2024 14:23:46 -0800
Subject: [PATCH 05/10] Add links to other docs
---
clang/include/clang/Basic/AttrDocs.td | 2 ++
clang/include/clang/Basic/DiagnosticGroups.td | 7 ++++---
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index 5de39be480560..5e87161958272 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8236,6 +8236,8 @@ its underlying representation to be a WebAssembly
``funcref``.
def PreferredTypeDocumentation : Documentation {
let Category = DocCatField;
let Content = [{
+.. _langext-preferred_type_documentation:
+
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,
but has to be specified as having the enumeration's underlying type in order to
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index daa2428b47500..fa0beef9b9fbd 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -635,7 +635,7 @@ def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-compatibility"> {
code Documentation = [{
- Under the MSVC ABI adjacemt bit-fields are not packed if the underlying
type
+ Under the MSVC ABI adjacent bit-fields are not packed if the underlying
type
has a different storage size. This warning indicates that a pair of
adjacent
bit-fields may not pack in the same way due to this behavioural difference.
@@ -673,8 +673,9 @@ def MSBitfieldCompatibility :
DiagGroup<"ms-bitfield-compatibility"> {
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 `preferred_type` attribute can be used
to
- convey the actual field type to debuggers and other tooling.
+ 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.
}];
}
>From c4b2d4a3d49c306868e81b2ba6d125bb3475d2cc Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Mon, 2 Dec 2024 18:00:13 -0800
Subject: [PATCH 06/10] Address feedback
---
clang/include/clang/Basic/DiagnosticGroups.td | 21 ++++++------
.../clang/Basic/DiagnosticSemaKinds.td | 2 +-
.../SemaCXX/ms_struct-bitfield-padding.cpp | 32 ++++++++++++++-----
3 files changed, 36 insertions(+), 19 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index fa0beef9b9fbd..0e3b3d5e165c7 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -635,9 +635,10 @@ def Padded : DiagGroup<"padded", [PaddedBitField]>;
def UnalignedAccess : DiagGroup<"unaligned-access">;
def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-compatibility"> {
code Documentation = [{
- Under the MSVC ABI adjacent bit-fields are not packed if the underlying
type
- has a different storage size. This warning indicates that a pair of
adjacent
- bit-fields may not pack in the same way due to this behavioural difference.
+ Under the MS Windows platform ABI, adjacent bit-fields are not packed if
the
+ underlying type has a different storage size. This warning indicates that a
+ pair of adjacent bit-fields may not pack in the same way due to this
behavioural
+ difference.
This can occur when mixing different types explicitly:
@@ -659,13 +660,13 @@ def MSBitfieldCompatibility :
DiagGroup<"ms-bitfield-compatibility"> {
Enum2 field2 : 1;
};
- In each of these cases under the MSVC 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.
+ In each of these cases under the MS Windows platform 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
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ce4f68bbb8bfc..cf4264eb44002 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6425,7 +6425,7 @@ def note_change_bitfield_sign : Note<
def warn_ms_bitfield_mismatched_storage_packing : Warning<
"bit-field %0 of type %1 has a different storage size than the "
"preceding bit-field (%2 vs %3 bytes) and will not be packed under "
- "the MSVC ABI">,
+ "the MS Windows platform ABI">,
InGroup<MSBitfieldCompatibility>, DefaultIgnore;
def note_ms_bitfield_mismatched_storage_size_previous : Note<
"preceding bit-field %0 declared here with type %1">;
diff --git a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
index fd1eb30bfe376..15706bdaaa97a 100644
--- a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -55,7 +55,7 @@ static_assert(sizeof(F) == 4);
struct G {
EnumU32_1 a : 15;
EnumU64 b : 15;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
};
@@ -76,7 +76,7 @@ struct I {
EnumU8 a : 3;
EnumI8 b : 5;
EnumU32_1 c : 10;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'b' declared here with type
'EnumI8'}}
};
#ifdef MS_BITFIELDS
@@ -116,10 +116,10 @@ static_assert(sizeof(M) == 4);
struct N {
EnumU32_1 a : 10;
EnumU64 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
EnumU32_1 c : 10;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-5 {{preceding bit-field 'b' declared here with type
'EnumU64'}}
};
@@ -132,7 +132,7 @@ static_assert(sizeof(N) == 8);
struct O {
EnumU16 a : 10;
EnumU32_1 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU16'}}
};
#ifdef MS_BITFIELDS
@@ -144,7 +144,7 @@ static_assert(sizeof(O) == 4);
struct P {
EnumU32_1 a : 10;
EnumU16 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
};
#ifdef MS_BITFIELDS
@@ -156,7 +156,7 @@ static_assert(sizeof(P) == 4);
struct Q {
EnumU8 a : 6;
EnumU16 b : 6;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU8'}}
};
#ifdef MS_BITFIELDS
@@ -169,7 +169,7 @@ struct R {
EnumU16 a : 9;
EnumU16 b : 9;
EnumU8 c : 6;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed
under the MSVC ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed
under the MS Windows platform ABI}}
// expected-note@-3 {{preceding bit-field 'b' declared here with type
'EnumU16'}}
};
@@ -178,3 +178,19 @@ 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@-1 {{bit-field 'x' of type 'short' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-note@-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);
>From ca50d1e00fc10c205e6488b56f5debdc4eee7b20 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Wed, 4 Dec 2024 10:57:45 -0800
Subject: [PATCH 07/10] Update documentation label
---
clang/include/clang/Basic/AttrDocs.td | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/clang/include/clang/Basic/AttrDocs.td
b/clang/include/clang/Basic/AttrDocs.td
index c4820724b9f59..4f1731e0a70c1 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -8231,9 +8231,8 @@ its underlying representation to be a WebAssembly
``funcref``.
def PreferredTypeDocumentation : Documentation {
let Category = DocCatField;
+ let Label = "langext-preferred_type_documentation";
let Content = [{
-.. _langext-preferred_type_documentation:
-
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,
but has to be specified as having the enumeration's underlying type in order to
>From ab3b7c393ff81f04042da1689f480b877709b0d2 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Mon, 3 Mar 2025 15:23:31 -0800
Subject: [PATCH 08/10] Don't try to diagnose the mismatch unless the warning
is enabled
No longer perform the field analysis unless the warning is enabled.
For safety i've updated a few of the existing bitfield tests to also run
with this diagnostic enabled just to get a bit more coverage given we're
no longer running this diagnostic code for every test.
---
clang/lib/Sema/SemaDecl.cpp | 4 ++-
clang/test/Sema/bitfield-layout.c | 44 ++++++++++++++++++++++++-----
clang/test/Sema/bitfield-layout_1.c | 1 +
clang/test/Sema/mms-bitfields.c | 6 +++-
clang/test/SemaCXX/bitfield.cpp | 1 +
5 files changed, 47 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index eebda05c1d48a..16990baa6acab 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19406,11 +19406,13 @@ 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 (Record && PreviousField && IsNonDependentBitField(FD) &&
+ if (ReportMSBitfieldStoragePacking && IsNonDependentBitField(FD) &&
IsNonDependentBitField(PreviousField)) {
CharUnits FDStorageSize = Context.getTypeSizeInChars(FD->getType());
CharUnits PreviousFieldStorageSize =
diff --git a/clang/test/Sema/bitfield-layout.c
b/clang/test/Sema/bitfield-layout.c
index 079720cc9b40b..c200ab78534f4 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-compatibility
+
// 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@-1 {{bit-field '' of type 'char' has a different storage
size than the preceding bit-field (1 vs 2 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-note@#a2x {{preceding bit-field 'x' declared here with type
'short'}}
+ int y : 17;
+ // checkms-warning@-1 {{bit-field 'y' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform 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@-1 {{bit-field '' of type 'int' has a different storage
size than the preceding bit-field (4 vs 2 bytes) and will not be packed under
the MS Windows platform 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@-1 {{bit-field '' of type 'int' has a different storage
size than the preceding bit-field (4 vs 2 bytes) and will not be packed under
the MS Windows platform 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@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform 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@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform 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@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform 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@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform 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..f925d96644a70 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-compatibility
// 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..fe327df45f8dc 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-compatibility
-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@-1 {{bit-field 'b' of type 'short' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // checkms-note@#a {{preceding bit-field 'a' declared here with type 'int'}}
} t;
// MS pads out bitfields between different types.
diff --git a/clang/test/SemaCXX/bitfield.cpp b/clang/test/SemaCXX/bitfield.cpp
index 083c28ffbb3d4..b4a4b3805c948 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-compatibility
// expected-no-diagnostics
>From d3fb2f2f0de05051421c7ba365d0eea32d90a148 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Mon, 3 Mar 2025 15:37:23 -0800
Subject: [PATCH 09/10] Sigh i really thought i had the formatting correct
---
clang/lib/Sema/SemaDecl.cpp | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 16990baa6acab..f327b2983ee65 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -19406,8 +19406,10 @@ 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());
+ 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();
};
>From 5e64d46feeb6438aee1bc0b9e7ac05952102a108 Mon Sep 17 00:00:00 2001
From: Oliver Hunt <oli...@apple.com>
Date: Fri, 11 Apr 2025 15:25:45 -0700
Subject: [PATCH 10/10] Change diagnostic spelling, switch to 'Microsoft ABI'
to describe the ABI
---
clang/docs/ReleaseNotes.rst | 2 +-
clang/include/clang/Basic/DiagnosticGroups.td | 6 +++---
.../clang/Basic/DiagnosticSemaKinds.td | 2 +-
clang/test/Sema/bitfield-layout.c | 18 ++++++++---------
clang/test/Sema/bitfield-layout_1.c | 2 +-
clang/test/Sema/mms-bitfields.c | 4 ++--
clang/test/SemaCXX/bitfield.cpp | 2 +-
.../SemaCXX/ms_struct-bitfield-padding.cpp | 20 +++++++++----------
8 files changed, 28 insertions(+), 28 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 498404aa3a220..482e5923465a3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -350,7 +350,7 @@ Improvements to Clang's diagnostics
- Now correctly diagnose a tentative definition of an array with static
storage duration in pedantic mode in C. (#GH50661)
-- A new off-by-default warning ``-Wms-bitfield-compatibility`` has been added
to alert to cases where bit-field
+- A new off-by-default warning ``-Wms-bitfield-padding`` has been added to
alert to cases where bit-field
packing may differ under the MS struct ABI (#GH117428).
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td
b/clang/include/clang/Basic/DiagnosticGroups.td
index 10ed276a69a22..2b8e977c92eb3 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -652,9 +652,9 @@ 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-compatibility"> {
+def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-padding"> {
code Documentation = [{
- Under the MS Windows platform ABI, adjacent bit-fields are not packed if
the
+ Under the Microsoft ABI, adjacent bit-fields are not packed if the
underlying type has a different storage size. This warning indicates that a
pair of adjacent bit-fields may not pack in the same way due to this
behavioural
difference.
@@ -679,7 +679,7 @@ def MSBitfieldCompatibility :
DiagGroup<"ms-bitfield-compatibility"> {
Enum2 field2 : 1;
};
- In each of these cases under the MS Windows platform ABI the second
bit-field
+ 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
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 77a10dee2d5b9..1bc862338aed4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6471,7 +6471,7 @@ def note_change_bitfield_sign : Note<
def warn_ms_bitfield_mismatched_storage_packing : Warning<
"bit-field %0 of type %1 has a different storage size than the "
"preceding bit-field (%2 vs %3 bytes) and will not be packed under "
- "the MS Windows platform ABI">,
+ "the Microsoft ABI">,
InGroup<MSBitfieldCompatibility>, DefaultIgnore;
def note_ms_bitfield_mismatched_storage_size_previous : Note<
"preceding bit-field %0 declared here with type %1">;
diff --git a/clang/test/Sema/bitfield-layout.c
b/clang/test/Sema/bitfield-layout.c
index c200ab78534f4..595b24d3e857e 100644
--- a/clang/test/Sema/bitfield-layout.c
+++ b/clang/test/Sema/bitfield-layout.c
@@ -3,7 +3,7 @@
// 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-compatibility
+// RUN: %clang_cc1 %s -fsyntax-only -verify=checkms -triple=i686-apple-darwin9
-Wms-bitfield-padding
// expected-no-diagnostics
#include <stddef.h>
@@ -29,10 +29,10 @@ CHECK_ALIGN(struct, a, 1)
struct __attribute__((packed)) a2 {
short x : 9; // #a2x
char : 0; // #a2anon
- // checkms-warning@-1 {{bit-field '' of type 'char' has a different storage
size than the preceding bit-field (1 vs 2 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field '' of type 'char' has a different 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@-1 {{bit-field 'y' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field 'y' of type 'int' has a different 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'}}
};
@@ -43,7 +43,7 @@ CHECK_ALIGN(struct, a2, 1)
struct __attribute__((packed)) a3 {
short x : 9; // #a3x
int : 0;
- // checkms-warning@-1 {{bit-field '' of type 'int' has a different storage
size than the preceding bit-field (4 vs 2 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field '' of type 'int' has a different 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'}}
};
@@ -59,7 +59,7 @@ CHECK_ALIGN(struct, a3, 1)
struct __attribute__((packed)) a4 {
short x : 9; // #a4x
int : 1;
- // checkms-warning@-1 {{bit-field '' of type 'int' has a different storage
size than the preceding bit-field (4 vs 2 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field '' of type 'int' has a different 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)
@@ -189,7 +189,7 @@ CHECK_OFFSET(struct, g4, c, 3);
struct g5 {
char : 1; // #g5
__attribute__((aligned(1))) int n : 24;
- // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different 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);
@@ -198,7 +198,7 @@ CHECK_ALIGN(struct, g5, 4);
struct __attribute__((packed)) g6 {
char : 1; // #g6
__attribute__((aligned(1))) int n : 24;
- // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different 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);
@@ -207,7 +207,7 @@ CHECK_ALIGN(struct, g6, 1);
struct g7 {
char : 1; // #g7
__attribute__((aligned(1))) int n : 25;
- // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different 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__)
@@ -220,7 +220,7 @@ CHECK_ALIGN(struct, g7, 4);
struct __attribute__((packed)) g8 {
char : 1; // #g8
__attribute__((aligned(1))) int n : 25;
- // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different storage
size than the preceding bit-field (4 vs 1 bytes) and will not be packed under
the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field 'n' of type 'int' has a different 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__)
diff --git a/clang/test/Sema/bitfield-layout_1.c
b/clang/test/Sema/bitfield-layout_1.c
index f925d96644a70..3db83c7463503 100644
--- a/clang/test/Sema/bitfield-layout_1.c
+++ b/clang/test/Sema/bitfield-layout_1.c
@@ -2,7 +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-compatibility
+// 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 fe327df45f8dc..a976578845229 100644
--- a/clang/test/Sema/mms-bitfields.c
+++ b/clang/test/Sema/mms-bitfields.c
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 -mms-bitfields -fsyntax-only -verify -triple
x86_64-apple-darwin9 %s
-// RUN: %clang_cc1 -mms-bitfields -fsyntax-only -Wms-bitfield-compatibility
-verify=checkms -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
@@ -9,7 +9,7 @@ struct
{
int a : 1; // #a
short b : 1;
- // checkms-warning@-1 {{bit-field 'b' of type 'short' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // checkms-warning@-1 {{bit-field 'b' of type 'short' has a different
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;
diff --git a/clang/test/SemaCXX/bitfield.cpp b/clang/test/SemaCXX/bitfield.cpp
index b4a4b3805c948..bb3094561bea4 100644
--- a/clang/test/SemaCXX/bitfield.cpp
+++ b/clang/test/SemaCXX/bitfield.cpp
@@ -1,5 +1,5 @@
// RUN: %clang_cc1 %s -verify
-// RUN: %clang_cc1 %s -verify -Wms-bitfield-compatibility
+// 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
index 15706bdaaa97a..c0f90f798118a 100644
--- a/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
+++ b/clang/test/SemaCXX/ms_struct-bitfield-padding.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-compatibility -verify -triple
armv8 -std=c++23 %s
+// 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
@@ -55,7 +55,7 @@ static_assert(sizeof(F) == 4);
struct G {
EnumU32_1 a : 15;
EnumU64 b : 15;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
};
@@ -76,7 +76,7 @@ struct I {
EnumU8 a : 3;
EnumI8 b : 5;
EnumU32_1 c : 10;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 1 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'b' declared here with type
'EnumI8'}}
};
#ifdef MS_BITFIELDS
@@ -116,10 +116,10 @@ static_assert(sizeof(M) == 4);
struct N {
EnumU32_1 a : 10;
EnumU64 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU64' has a different
storage size than the preceding bit-field (8 vs 4 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
EnumU32_1 c : 10;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 8 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-5 {{preceding bit-field 'b' declared here with type
'EnumU64'}}
};
@@ -132,7 +132,7 @@ static_assert(sizeof(N) == 8);
struct O {
EnumU16 a : 10;
EnumU32_1 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU32_1' has a different
storage size than the preceding bit-field (4 vs 2 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU16'}}
};
#ifdef MS_BITFIELDS
@@ -144,7 +144,7 @@ static_assert(sizeof(O) == 4);
struct P {
EnumU32_1 a : 10;
EnumU16 b : 10;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 4 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU32_1'}}
};
#ifdef MS_BITFIELDS
@@ -156,7 +156,7 @@ static_assert(sizeof(P) == 4);
struct Q {
EnumU8 a : 6;
EnumU16 b : 6;
- // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'b' of type 'EnumU16' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'a' declared here with type
'EnumU8'}}
};
#ifdef MS_BITFIELDS
@@ -169,7 +169,7 @@ struct R {
EnumU16 a : 9;
EnumU16 b : 9;
EnumU8 c : 6;
- // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'c' of type 'EnumU8' has a different
storage size than the preceding bit-field (1 vs 2 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-3 {{preceding bit-field 'b' declared here with type
'EnumU16'}}
};
@@ -185,7 +185,7 @@ struct S {
char c : 4;
char d : 4;
short x : 7;
- // expected-warning@-1 {{bit-field 'x' of type 'short' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the MS Windows platform ABI}}
+ // expected-warning@-1 {{bit-field 'x' of type 'short' has a different
storage size than the preceding bit-field (2 vs 1 bytes) and will not be packed
under the Microsoft ABI}}
// expected-note@-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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
