llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Oliver Hunt (ojhunt) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/117428.diff 4 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticGroups.td (+1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+6) - (modified) clang/lib/Sema/SemaDecl.cpp (+25-2) - (added) clang/test/SemaCXX/ms_struct-bitfield-padding.cpp (+180) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index df9bf94b5d0398..57bdda4b8f8b47 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 eb05a6a77978af..aa13e3438d3739 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 74b0e5ad23bd48..18aeca3b34f659 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 00000000000000..001086de5bbd10 --- /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 `````````` </details> https://github.com/llvm/llvm-project/pull/117428 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
