Skip to content

Commit 3e25d7e

Browse files
ojhuntojhunt
committed
Add an off-by-default warning to complain about MSVC bitfield padding
1 parent 1e31a45 commit 3e25d7e

File tree

4 files changed

+212
-2
lines changed

4 files changed

+212
-2
lines changed

‎clang/include/clang/Basic/DiagnosticGroups.td‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -631,6 +631,7 @@ def Packed : DiagGroup<"packed", [PackedNonPod]>;
631631
def PaddedBitField : DiagGroup<"padded-bitfield">;
632632
def Padded : DiagGroup<"padded", [PaddedBitField]>;
633633
def UnalignedAccess : DiagGroup<"unaligned-access">;
634+
def MSBitfieldCompatibility : DiagGroup<"ms-bitfield-packing-compatibility">;
634635

635636
def PessimizingMove : DiagGroup<"pessimizing-move">;
636637
def ReturnStdMove : DiagGroup<"return-std-move">;

‎clang/include/clang/Basic/DiagnosticSemaKinds.td‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6418,6 +6418,12 @@ def warn_signed_bitfield_enum_conversion : Warning<
64186418
InGroup<BitFieldEnumConversion>, DefaultIgnore;
64196419
def note_change_bitfield_sign : Note<
64206420
"consider making the bitfield type %select{unsigned|signed}0">;
6421+
def warn_ms_bitfield_mismatched_storage_packing : Warning<
6422+
"bit-field %0 of type %1 has a different storage size (%2 vs %3 bytes) than the "
6423+
"preceding bit-field and may not be packed under MSVC ABI">,
6424+
InGroup<MSBitfieldCompatibility>, DefaultIgnore;
6425+
def note_ms_bitfield_mismatched_storage_size_previous : Note<
6426+
"preceding bit-field %0 declared here with type %1">;
64216427

64226428
def warn_missing_braces : Warning<
64236429
"suggest braces around initialization of subobject">,

‎clang/lib/Sema/SemaDecl.cpp‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19001,9 +19001,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
1900119001

1900219002
// Verify that all the fields are okay.
1900319003
SmallVector<FieldDecl*, 32> RecFields;
19004-
19004+
std::optional<const FieldDecl *> PreviousField;
1900519005
for (ArrayRef<Decl *>::iterator i = Fields.begin(), end = Fields.end();
19006-
i != end; ++i) {
19006+
i != end; PreviousField = cast<FieldDecl>(*i), ++i) {
1900719007
FieldDecl *FD = cast<FieldDecl>(*i);
1900819008

1900919009
// Get the type for the field.
@@ -19213,6 +19213,29 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
1921319213

1921419214
if (Record && FD->getType().isVolatileQualified())
1921519215
Record->setHasVolatileMember(true);
19216+
auto IsNonDependentBitField = [](const FieldDecl *FD) {
19217+
if (!FD->isBitField())
19218+
return false;
19219+
if (FD->getType()->isDependentType())
19220+
return false;
19221+
return true;
19222+
};
19223+
19224+
if (Record && PreviousField && IsNonDependentBitField(FD) &&
19225+
IsNonDependentBitField(*PreviousField)) {
19226+
unsigned FDStorageSize =
19227+
Context.getTypeSizeInChars(FD->getType()).getQuantity();
19228+
unsigned PreviousFieldStorageSize =
19229+
Context.getTypeSizeInChars((*PreviousField)->getType()).getQuantity();
19230+
if (FDStorageSize != PreviousFieldStorageSize) {
19231+
Diag(FD->getLocation(),
19232+
diag::warn_ms_bitfield_mismatched_storage_packing)
19233+
<< FD << FD->getType() << FDStorageSize << PreviousFieldStorageSize;
19234+
Diag((*PreviousField)->getLocation(),
19235+
diag::note_ms_bitfield_mismatched_storage_size_previous)
19236+
<< *PreviousField << (*PreviousField)->getType();
19237+
}
19238+
}
1921619239
// Keep track of the number of named members.
1921719240
if (FD->getIdentifier())
1921819241
++NumNamedMembers;

‎clang/test/SemaCXX/ms_struct-bitfield-padding.cpp‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
2+
// RUN: %clang_cc1 -fsyntax-only -Wms-bitfield-packing-compatibility -verify -triple armv8 -std=c++23 %s
3+
// RUN: %clang_cc1 -fsyntax-only -DMS_BITFIELDS -mms-bitfields -verify=msbitfields -triple armv8-apple-macos10.15 -std=c++23 %s
4+
5+
// msbitfields-no-diagnostics
6+
7+
enum Enum1 { Enum1_A, Enum1_B };
8+
enum Enum2 { Enum2_A, Enum2_B };
9+
10+
enum class EnumU32_1 : unsigned { A, B };
11+
enum class EnumU32_2 : unsigned { A, B };
12+
enum class EnumU64 : unsigned long long { A, B };
13+
enum class EnumI32 : int { A, B };
14+
enum class EnumU8 : unsigned char { A, B };
15+
enum class EnumI8 : char { A, B };
16+
enum class EnumU16 : unsigned short { A, B };
17+
enum class EnumI16 : short { A, B };
18+
19+
struct A {
20+
unsigned int a : 15;
21+
unsigned int b : 15;
22+
};
23+
static_assert(sizeof(A) == 4);
24+
25+
struct B {
26+
unsigned int a : 15;
27+
int b : 15;
28+
};
29+
static_assert(sizeof(B) == 4);
30+
31+
struct C {
32+
unsigned int a : 15;
33+
int b : 15;
34+
};
35+
static_assert(sizeof(C) == 4);
36+
37+
struct D {
38+
Enum1 a : 15;
39+
Enum1 b : 15;
40+
};
41+
static_assert(sizeof(D) == 4);
42+
43+
struct E {
44+
Enum1 a : 15;
45+
Enum2 b : 15;
46+
};
47+
static_assert(sizeof(E) == 4);
48+
49+
struct F {
50+
EnumU32_1 a : 15;
51+
EnumU32_2 b : 15;
52+
};
53+
static_assert(sizeof(F) == 4);
54+
55+
struct G {
56+
EnumU32_1 a : 15;
57+
EnumU64 b : 15;
58+
// 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}}
59+
// expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
60+
};
61+
62+
#ifdef MS_BITFIELDS
63+
static_assert(sizeof(G) == 16);
64+
#else
65+
static_assert(sizeof(G) == 8);
66+
#endif
67+
68+
struct H {
69+
EnumU32_1 a : 10;
70+
EnumI32 b : 10;
71+
EnumU32_1 c : 10;
72+
};
73+
static_assert(sizeof(H) == 4);
74+
75+
struct I {
76+
EnumU8 a : 3;
77+
EnumI8 b : 5;
78+
EnumU32_1 c : 10;
79+
// 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}}
80+
// expected-note@-3 {{preceding bit-field 'b' declared here with type 'EnumI8'}}
81+
};
82+
#ifdef MS_BITFIELDS
83+
static_assert(sizeof(I) == 8);
84+
#else
85+
static_assert(sizeof(I) == 4);
86+
#endif
87+
88+
struct J {
89+
EnumU8 : 0;
90+
EnumU8 b : 4;
91+
};
92+
static_assert(sizeof(J) == 1);
93+
94+
struct K {
95+
EnumU8 a : 4;
96+
EnumU8 : 0;
97+
};
98+
static_assert(sizeof(K) == 1);
99+
100+
struct L {
101+
EnumU32_1 a : 10;
102+
EnumU32_2 b : 10;
103+
EnumU32_1 c : 10;
104+
};
105+
106+
static_assert(sizeof(L) == 4);
107+
108+
struct M {
109+
EnumU32_1 a : 10;
110+
EnumI32 b : 10;
111+
EnumU32_1 c : 10;
112+
};
113+
114+
static_assert(sizeof(M) == 4);
115+
116+
struct N {
117+
EnumU32_1 a : 10;
118+
EnumU64 b : 10;
119+
// 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}}
120+
// expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
121+
EnumU32_1 c : 10;
122+
// 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}}
123+
// expected-note@-5 {{preceding bit-field 'b' declared here with type 'EnumU64'}}
124+
};
125+
126+
#ifdef MS_BITFIELDS
127+
static_assert(sizeof(N) == 24);
128+
#else
129+
static_assert(sizeof(N) == 8);
130+
#endif
131+
132+
struct O {
133+
EnumU16 a : 10;
134+
EnumU32_1 b : 10;
135+
// 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}}
136+
// expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU16'}}
137+
};
138+
#ifdef MS_BITFIELDS
139+
static_assert(sizeof(O) == 8);
140+
#else
141+
static_assert(sizeof(O) == 4);
142+
#endif
143+
144+
struct P {
145+
EnumU32_1 a : 10;
146+
EnumU16 b : 10;
147+
// 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}}
148+
// expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU32_1'}}
149+
};
150+
#ifdef MS_BITFIELDS
151+
static_assert(sizeof(P) == 8);
152+
#else
153+
static_assert(sizeof(P) == 4);
154+
#endif
155+
156+
struct Q {
157+
EnumU8 a : 6;
158+
EnumU16 b : 6;
159+
// 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}}
160+
// expected-note@-3 {{preceding bit-field 'a' declared here with type 'EnumU8'}}
161+
};
162+
#ifdef MS_BITFIELDS
163+
static_assert(sizeof(Q) == 4);
164+
#else
165+
static_assert(sizeof(Q) == 2);
166+
#endif
167+
168+
struct R {
169+
EnumU16 a : 9;
170+
EnumU16 b : 9;
171+
EnumU8 c : 6;
172+
// 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}}
173+
// expected-note@-3 {{preceding bit-field 'b' declared here with type 'EnumU16'}}
174+
};
175+
176+
#ifdef MS_BITFIELDS
177+
static_assert(sizeof(R) == 6);
178+
#else
179+
static_assert(sizeof(R) == 4);
180+
#endif

0 commit comments

Comments
 (0)