Author: Dan Klishch Date: 2023-11-02T11:52:57-04:00 New Revision: 43feb3eeb1ba4de920d6eaa12b7a5c21f6dd6229
URL: https://github.com/llvm/llvm-project/commit/43feb3eeb1ba4de920d6eaa12b7a5c21f6dd6229 DIFF: https://github.com/llvm/llvm-project/commit/43feb3eeb1ba4de920d6eaa12b7a5c21f6dd6229.diff LOG: [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. Added: clang/test/CodeGenCXX/warn-padded-bitfields.cpp Modified: clang/include/clang/Basic/DiagnosticASTKinds.td clang/include/clang/Basic/DiagnosticGroups.td clang/lib/AST/RecordLayoutBuilder.cpp clang/test/CodeGenCXX/warn-all-padded-packed-packed-non-pod.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td index 031117f2c4137a4..c81d17ed641084a 100644 --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -998,6 +998,16 @@ def warn_npot_ms_struct : Warning< "data types with sizes that aren't a power of two">, DefaultError, InGroup<IncompatibleMSStruct>; +// -Wpadded-bitfield +def warn_padded_struct_bitfield : Warning< + "padding %select{struct|interface|class}0 %1 with %2 " + "%select{byte|bit}3%s2 to align %4">, + InGroup<PaddedBitField>, DefaultIgnore; +def warn_padded_struct_anon_bitfield : Warning< + "padding %select{struct|interface|class}0 %1 with %2 " + "%select{byte|bit}3%s2 to align anonymous bit-field">, + InGroup<PaddedBitField>, DefaultIgnore; + // -Wpadded, -Wpacked def warn_padded_struct_field : Warning< "padding %select{struct|interface|class}0 %1 with %2 " @@ -1005,7 +1015,7 @@ def warn_padded_struct_field : Warning< InGroup<Padded>, DefaultIgnore; def warn_padded_struct_anon_field : Warning< "padding %select{struct|interface|class}0 %1 with %2 " - "%select{byte|bit}3%s2 to align anonymous bit-field">, + "%select{byte|bit}3%s2 to align anonymous field">, InGroup<Padded>, DefaultIgnore; def warn_padded_struct_size : Warning< "padding size of %0 with %1 %select{byte|bit}2%s1 to alignment boundary">, diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 4c4820d17b7184c..37559c7ff772468 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -585,7 +585,8 @@ def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">; def OrderedCompareFunctionPointers : DiagGroup<"ordered-compare-function-pointers">; def PackedNonPod : DiagGroup<"packed-non-pod">; def Packed : DiagGroup<"packed", [PackedNonPod]>; -def Padded : DiagGroup<"padded">; +def PaddedBitField : DiagGroup<"padded-bitfield">; +def Padded : DiagGroup<"padded", [PaddedBitField]>; def UnalignedAccess : DiagGroup<"unaligned-access">; def PessimizingMove : DiagGroup<"pessimizing-move">; diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index f1f2275da44dcad..5d4f930fca50e2b 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2297,19 +2297,22 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( PadSize = PadSize / CharBitNum; InBits = false; } - if (D->getIdentifier()) - Diag(D->getLocation(), diag::warn_padded_struct_field) + if (D->getIdentifier()) { + auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield + : diag::warn_padded_struct_field; + Diag(D->getLocation(), Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) - << Context.getTypeDeclType(D->getParent()) - << PadSize + << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0) // (byte|bit) << D->getIdentifier(); - else - Diag(D->getLocation(), diag::warn_padded_struct_anon_field) + } else { + auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield + : diag::warn_padded_struct_anon_field; + Diag(D->getLocation(), Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) - << Context.getTypeDeclType(D->getParent()) - << PadSize + << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) + } } if (isPacked && Offset != UnpackedOffset) { HasPackedField = true; diff --git a/clang/test/CodeGenCXX/warn-all-padded-packed-packed-non-pod.cpp b/clang/test/CodeGenCXX/warn-all-padded-packed-packed-non-pod.cpp index 2a75498d87197a4..5e166deba0a3f05 100644 --- a/clang/test/CodeGenCXX/warn-all-padded-packed-packed-non-pod.cpp +++ b/clang/test/CodeGenCXX/warn-all-padded-packed-packed-non-pod.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,top %s -emit-llvm-only -// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only +// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -Wno-padded-bitfield -verify=expected,top %s -emit-llvm-only +// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded -Wpacked -Wno-padded-bitfield -verify=expected,abi15 -fclang-abi-compat=15 %s -emit-llvm-only // -Wpacked-non-pod itself should not emit the "packed attribute is unnecessary" warnings. // RUN: %clang_cc1 -triple=x86_64-none-none -Wpacked-non-pod -verify=top %s -emit-llvm-only // -Wall should not emit the "packed attribute is unnecessary" warnings without -Wpacked. diff --git a/clang/test/CodeGenCXX/warn-padded-bitfields.cpp b/clang/test/CodeGenCXX/warn-padded-bitfields.cpp new file mode 100644 index 000000000000000..f9882d03564fe6e --- /dev/null +++ b/clang/test/CodeGenCXX/warn-padded-bitfields.cpp @@ -0,0 +1,41 @@ +// RUN: %clang_cc1 -triple=x86_64-none-none -Wpadded-bitfield -verify=expected %s -emit-llvm-only + +struct S1 { + unsigned a : 1; + unsigned long long : 0; // expected-warning {{padding struct 'S1' with 63 bits to align anonymous bit-field}} +}; + +struct S2 { + unsigned a : 1; + unsigned long long b : 64; // expected-warning {{padding struct 'S2' with 63 bits to align 'b'}} +}; + +struct S3 { + char a : 1; + short b : 16; // expected-warning {{padding struct 'S3' with 15 bits to align 'b'}} +}; + +struct [[gnu::packed]] S4 { + char a : 1; + short b : 16; +}; + +struct S5 { + unsigned a : 1; + unsigned long long b : 63; +}; + +struct S6 { + unsigned a : 1; + unsigned long long b; +}; + +struct S7 { + int word; + struct { + int filler __attribute__ ((aligned (8))); + }; +}; + +// The warnings are emitted when the layout of the structs is computed, so we have to use them. +void f(S1, S2, S3, S4, S5, S6, S7){} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
