Author: Theo de Magalhaes Date: 2025-04-02T14:46:58-07:00 New Revision: 76fa9530c9ac7f81a49b840556f51f4838efbfe1
URL: https://github.com/llvm/llvm-project/commit/76fa9530c9ac7f81a49b840556f51f4838efbfe1 DIFF: https://github.com/llvm/llvm-project/commit/76fa9530c9ac7f81a49b840556f51f4838efbfe1.diff LOG: [clang] add support for -Wpadded on Windows (#130182) Implements the -Wpadded warning for --target=x86_64-windows-msvc etc. Fixes #61702 . Added: clang/test/SemaCXX/windows-Wpadded-bitfield.cpp clang/test/SemaCXX/windows-Wpadded.cpp Modified: clang/docs/ReleaseNotes.rst clang/lib/AST/RecordLayoutBuilder.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 3f7f723bc96ce..7fb6b0baae16b 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -190,6 +190,8 @@ Modified Compiler Flags - The compiler flag `-fbracket-depth` default value is increased from 256 to 2048. (#GH94728) +- `-Wpadded` option implemented for the `x86_64-windows-msvc` target. Fixes #61702 + Removed Compiler Flags ------------------------- diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 3e756ab9b9bfe..41e7198cb7581 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2274,9 +2274,9 @@ static unsigned getPaddingDiagFromTagKind(TagTypeKind Tag) { } } -void ItaniumRecordLayoutBuilder::CheckFieldPadding( - uint64_t Offset, uint64_t UnpaddedOffset, uint64_t UnpackedOffset, - unsigned UnpackedAlign, bool isPacked, const FieldDecl *D) { +static void CheckFieldPadding(const ASTContext &Context, bool IsUnion, + uint64_t Offset, uint64_t UnpaddedOffset, + const FieldDecl *D) { // We let objc ivars without warning, objc interfaces generally are not used // for padding tricks. if (isa<ObjCIvarDecl>(D)) @@ -2300,7 +2300,8 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( if (D->getIdentifier()) { auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_bitfield : diag::warn_padded_struct_field; - Diag(D->getLocation(), Diagnostic) + Context.getDiagnostics().Report(D->getLocation(), + Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0) // (byte|bit) @@ -2308,15 +2309,22 @@ void ItaniumRecordLayoutBuilder::CheckFieldPadding( } else { auto Diagnostic = D->isBitField() ? diag::warn_padded_struct_anon_bitfield : diag::warn_padded_struct_anon_field; - Diag(D->getLocation(), Diagnostic) + Context.getDiagnostics().Report(D->getLocation(), + Diagnostic) << getPaddingDiagFromTagKind(D->getParent()->getTagKind()) << Context.getTypeDeclType(D->getParent()) << PadSize << (InBits ? 1 : 0); // (byte|bit) } - } - if (isPacked && Offset != UnpackedOffset) { - HasPackedField = true; - } + } +} + +void ItaniumRecordLayoutBuilder::CheckFieldPadding( + uint64_t Offset, uint64_t UnpaddedOffset, uint64_t UnpackedOffset, + unsigned UnpackedAlign, bool isPacked, const FieldDecl *D) { + ::CheckFieldPadding(Context, IsUnion, Offset, UnpaddedOffset, D); + if (isPacked && Offset != UnpackedOffset) { + HasPackedField = true; + } } static const CXXMethodDecl *computeKeyFunction(ASTContext &Context, @@ -2642,8 +2650,6 @@ struct MicrosoftRecordLayoutBuilder { /// virtual base classes and their offsets in the record. ASTRecordLayout::VBaseOffsetsMapTy VBases; /// The number of remaining bits in our last bitfield allocation. - /// This value isn't meaningful unless LastFieldIsNonZeroWidthBitfield is - /// true. unsigned RemainingBitsInField; bool IsUnion : 1; /// True if the last field laid out was a bitfield and was not 0 @@ -3004,6 +3010,15 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) { } else { FieldOffset = Size.alignTo(Info.Alignment); } + + uint64_t UnpaddedFielddOffsetInBits = + Context.toBits(DataSize) - RemainingBitsInField; + + ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), + UnpaddedFielddOffsetInBits, FD); + + RemainingBitsInField = 0; + placeFieldAtOffset(FieldOffset); if (!IsOverlappingEmptyField) @@ -3049,10 +3064,14 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) { } else { // Allocate a new block of memory and place the bitfield in it. CharUnits FieldOffset = Size.alignTo(Info.Alignment); + uint64_t UnpaddedFieldOffsetInBits = + Context.toBits(DataSize) - RemainingBitsInField; placeFieldAtOffset(FieldOffset); Size = FieldOffset + Info.Size; Alignment = std::max(Alignment, Info.Alignment); RemainingBitsInField = Context.toBits(Info.Size) - Width; + ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), + UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3076,9 +3095,14 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) { } else { // Round up the current record size to the field's alignment boundary. CharUnits FieldOffset = Size.alignTo(Info.Alignment); + uint64_t UnpaddedFieldOffsetInBits = + Context.toBits(DataSize) - RemainingBitsInField; placeFieldAtOffset(FieldOffset); + RemainingBitsInField = 0; Size = FieldOffset; Alignment = std::max(Alignment, Info.Alignment); + ::CheckFieldPadding(Context, IsUnion, Context.toBits(FieldOffset), + UnpaddedFieldOffsetInBits, FD); } DataSize = Size; } @@ -3203,6 +3227,9 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) { } void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { + uint64_t UnpaddedSizeInBits = Context.toBits(DataSize); + UnpaddedSizeInBits -= RemainingBitsInField; + // Respect required alignment. Note that in 32-bit mode Required alignment // may be 0 and cause size not to be updated. DataSize = Size; @@ -3231,6 +3258,22 @@ void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) { Size = Context.toCharUnitsFromBits(External.Size); if (External.Align) Alignment = Context.toCharUnitsFromBits(External.Align); + return; + } + unsigned CharBitNum = Context.getTargetInfo().getCharWidth(); + uint64_t SizeInBits = Context.toBits(Size); + if (SizeInBits > UnpaddedSizeInBits) { + unsigned int PadSize = SizeInBits - UnpaddedSizeInBits; + bool InBits = true; + if (PadSize % CharBitNum == 0) { + PadSize = PadSize / CharBitNum; + InBits = false; + } + + Context.getDiagnostics().Report(RD->getLocation(), + diag::warn_padded_struct_size) + << Context.getTypeDeclType(RD) << PadSize + << (InBits ? 1 : 0); // (byte|bit) } } diff --git a/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp new file mode 100644 index 0000000000000..ee5a57124eca5 --- /dev/null +++ b/clang/test/SemaCXX/windows-Wpadded-bitfield.cpp @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded %s + +struct __attribute__((ms_struct)) BitfieldStruct { // expected-warning {{padding size of 'BitfieldStruct' with 3 bytes to alignment boundary}} + char c : 1; + int : 0; // expected-warning {{padding struct 'BitfieldStruct' with 31 bits to align anonymous bit-field}} + char i; +}; + +struct __attribute__((ms_struct)) SevenBitfieldStruct { // expected-warning {{padding size of 'SevenBitfieldStruct' with 3 bytes to alignment boundary}} + char c : 7; + int : 0; // expected-warning {{padding struct 'SevenBitfieldStruct' with 25 bits to align anonymous bit-field}} + char i; +}; + +struct __attribute__((ms_struct)) SameUnitSizeBitfield { + char c : 7; + char : 1; // Same unit size attributes fall in the same unit + they fill the unit -> no padding + char i; +}; + +struct __attribute__((ms_struct)) DifferentUnitSizeBitfield { // expected-warning {{padding size of 'DifferentUnitSizeBitfield' with 3 bytes to alignment boundary}} + char c : 7; + int : 1; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' with 25 bits to align anonymous bit-field}} + char i; // expected-warning {{padding struct 'DifferentUnitSizeBitfield' with 31 bits to align 'i'}} +}; + +int main() { + BitfieldStruct b; + SevenBitfieldStruct s; + SameUnitSizeBitfield su; + DifferentUnitSizeBitfield du; +} diff --git a/clang/test/SemaCXX/windows-Wpadded.cpp b/clang/test/SemaCXX/windows-Wpadded.cpp new file mode 100644 index 0000000000000..da3f2bf08c6b8 --- /dev/null +++ b/clang/test/SemaCXX/windows-Wpadded.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fsyntax-only -verify -Wpadded %s +// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify -Wpadded %s + +struct __attribute__((ms_struct)) Foo { // expected-warning {{padding size of 'Foo' with 3 bytes to alignment boundary}} + int b : 1; + char a; // expected-warning {{padding struct 'Foo' with 31 bits to align 'a'}} +}; + +struct __attribute__((ms_struct)) AlignedStruct { // expected-warning {{padding size of 'AlignedStruct' with 4 bytes to alignment boundary}} + char c; + alignas(8) int i; // expected-warning {{padding struct 'AlignedStruct' with 7 bytes to align 'i'}} +}; + + +struct Base { + int b; +}; + +struct Derived : public Base { // expected-warning {{padding size of 'Derived' with 3 bytes to alignment boundary}} + char c; +}; + +union __attribute__((ms_struct)) Union { + char c; + long long u; +}; + +struct __attribute__((ms_struct)) StructWithUnion { // expected-warning {{padding size of 'StructWithUnion' with 6 bytes to alignment boundary}} + char c; + int : 0; + Union t; // expected-warning {{padding struct 'StructWithUnion' with 7 bytes to align 't'}} + short i; +}; + +int main() { + Foo f; + AlignedStruct a; + Derived d; + StructWithUnion swu; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits