Skip to content

Commit 43feb3e

Browse files
authored
[clang] Separate bit-field padding diagnostics into -Wpadded-bitfield (#70978)
The `-Wpadded` diagnostics are usually very noisy and generally not helpful. However, reporting padding that was introduced in bit-fields is rather helpful. For example, yesterday in SerenityOS's discord we had very unpleasant experience of debugging Windows portability issue, and its root cause was that under `x86_64-pc-windows-msvc` a padding was introduced for one of the bit-fields. So, this PR separates bit-field-related padding diagnostics into a new `-Wpadded-bitfield`. The diagnostic group is also enabled by `-Wpadded` for compatibility reasons.
1 parent 8f59c16 commit 43feb3e

File tree

5 files changed

+67
-12
lines changed

5 files changed

+67
-12
lines changed

clang/include/clang/Basic/DiagnosticASTKinds.td

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -998,14 +998,24 @@ def warn_npot_ms_struct : Warning<
998998
"data types with sizes that aren't a power of two">,
999999
DefaultError, InGroup<IncompatibleMSStruct>;
10001000

1001+
// -Wpadded-bitfield
1002+
def warn_padded_struct_bitfield : Warning<
1003+
"padding %select{struct|interface|class}0 %1 with %2 "
1004+
"%select{byte|bit}3%s2 to align %4">,
1005+
InGroup<PaddedBitField>, DefaultIgnore;
1006+
def warn_padded_struct_anon_bitfield : Warning<
1007+
"padding %select{struct|interface|class}0 %1 with %2 "
1008+
"%select{byte|bit}3%s2 to align anonymous bit-field">,
1009+
InGroup<PaddedBitField>, DefaultIgnore;
1010+
10011011
// -Wpadded, -Wpacked
10021012
def warn_padded_struct_field : Warning<
10031013
"padding %select{struct|interface|class}0 %1 with %2 "
10041014
"%select{byte|bit}3%s2 to align %4">,
10051015
InGroup<Padded>, DefaultIgnore;
10061016
def warn_padded_struct_anon_field : Warning<
10071017
"padding %select{struct|interface|class}0 %1 with %2 "
1008-
"%select{byte|bit}3%s2 to align anonymous bit-field">,
1018+
"%select{byte|bit}3%s2 to align anonymous field">,
10091019
InGroup<Padded>, DefaultIgnore;
10101020
def warn_padded_struct_size : Warning<
10111021
"padding size of %0 with %1 %select{byte|bit}2%s1 to alignment boundary">,

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,8 @@ def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
585585
def OrderedCompareFunctionPointers : DiagGroup<"ordered-compare-function-pointers">;
586586
def PackedNonPod : DiagGroup<"packed-non-pod">;
587587
def Packed : DiagGroup<"packed", [PackedNonPod]>;
588-
def Padded : DiagGroup<"padded">;
588+
def PaddedBitField : DiagGroup<"padded-bitfield">;
589+
def Padded : DiagGroup<"padded", [PaddedBitField]>;
589590
def UnalignedAccess : DiagGroup<"unaligned-access">;
590591

591592
def PessimizingMove : DiagGroup<"pessimizing-move">;

clang/lib/AST/RecordLayoutBuilder.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2297,19 +2297,22 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding(
22972297
PadSize = PadSize / CharBitNum;
22982298
InBits = false;
22992299
}
2300-
if (D->getIdentifier())
2301-
Diag(D->getLocation(), diag::warn_padded_struct_field)
2300+
if (D->getIdentifier()) {
2301+
auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield
2302+
: diag::warn_padded_struct_field;
2303+
Diag(D->getLocation(), Diagnostic)
23022304
<< getPaddingDiagFromTagKind(D->getParent()->getTagKind())
2303-
<< Context.getTypeDeclType(D->getParent())
2304-
<< PadSize
2305+
<< Context.getTypeDeclType(D->getParent()) << PadSize
23052306
<< (InBits ? 1 : 0) // (byte|bit)
23062307
<< D->getIdentifier();
2307-
else
2308-
Diag(D->getLocation(), diag::warn_padded_struct_anon_field)
2308+
} else {
2309+
auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield
2310+
: diag::warn_padded_struct_anon_field;
2311+
Diag(D->getLocation(), Diagnostic)
23092312
<< getPaddingDiagFromTagKind(D->getParent()->getTagKind())
2310-
<< Context.getTypeDeclType(D->getParent())
2311-
<< PadSize
2313+
<< Context.getTypeDeclType(D->getParent()) << PadSize
23122314
<< (InBits ? 1 : 0); // (byte|bit)
2315+
}
23132316
}
23142317
if (isPacked && Offset != UnpackedOffset) {
23152318
HasPackedField = true;

clang/test/CodeGenCXX/warn-all-padded-packed-packed-non-pod.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,top %s -emit-llvm-only
2-
// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
1+
// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -Wno-padded-bitfield -verify=expected,top %s -emit-llvm-only
2+
// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -Wno-padded-bitfield -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only
33
// -Wpacked-non-pod itself should not emit the "packed attribute is unnecessary" warnings.
44
// RUN: %clang_cc1 -triple=x86_64-none-none -Wpacked-non-pod -verify=top %s -emit-llvm-only
55
// -Wall should not emit the "packed attribute is unnecessary" warnings without -Wpacked.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded-bitfield -verify=expected %s -emit-llvm-only
2+
3+
struct S1 {
4+
unsigned a : 1;
5+
unsigned long long : 0; // expected-warning {{padding struct 'S1' with 63 bits to align anonymous bit-field}}
6+
};
7+
8+
struct S2 {
9+
unsigned a : 1;
10+
unsigned long long b : 64; // expected-warning {{padding struct 'S2' with 63 bits to align 'b'}}
11+
};
12+
13+
struct S3 {
14+
char a : 1;
15+
short b : 16; // expected-warning {{padding struct 'S3' with 15 bits to align 'b'}}
16+
};
17+
18+
struct [[gnu::packed]] S4 {
19+
char a : 1;
20+
short b : 16;
21+
};
22+
23+
struct S5 {
24+
unsigned a : 1;
25+
unsigned long long b : 63;
26+
};
27+
28+
struct S6 {
29+
unsigned a : 1;
30+
unsigned long long b;
31+
};
32+
33+
struct S7 {
34+
int word;
35+
struct {
36+
int filler __attribute__ ((aligned (8)));
37+
};
38+
};
39+
40+
// The warnings are emitted when the layout of the structs is computed, so we have to use them.
41+
void f(S1, S2, S3, S4, S5, S6, S7){}

0 commit comments

Comments
 (0)