[CLANG-CL] ignores Wpadded (#134426)

[clang] add support for -Wpadded on Windows

Implements the -Wpadded warning for --target=x86_64-windows-msvc etc.

Fixes #61702 .

Author: theomagellan

clang/docs/ReleaseNotes.rst

@@ -217,6 +217,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
 -------------------------
 

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,23 +2300,31 @@ 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)
           << D->getIdentifier();
     } 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,
@@ -2555,7 +2563,8 @@ struct MicrosoftRecordLayoutBuilder {
   typedef llvm::DenseMap<const CXXRecordDecl *, CharUnits> BaseOffsetsMapTy;
   MicrosoftRecordLayoutBuilder(const ASTContext &Context,
                                EmptySubobjectMap *EmptySubobjects)
-      : Context(Context), EmptySubobjects(EmptySubobjects) {}
+      : Context(Context), EmptySubobjects(EmptySubobjects),
+        RemainingBitsInField(0) {}
 
 private:
   MicrosoftRecordLayoutBuilder(const MicrosoftRecordLayoutBuilder &) = delete;
@@ -2642,8 +2651,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 +3011,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 +3065,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 +3096,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 +3228,14 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
 }
 
 void MicrosoftRecordLayoutBuilder::finalizeLayout(const RecordDecl *RD) {
+  uint64_t UnpaddedSizeInBits = Context.toBits(DataSize);
+  UnpaddedSizeInBits -= RemainingBitsInField;
+
+  // MS ABI allocates 1 byte for empty class
+  // (not padding)
+  if (Size.isZero())
+    UnpaddedSizeInBits += 8;
+
   // 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 +3264,23 @@ 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)
   }
 }
 

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;
+}

clang/test/SemaCXX/windows-Wpadded.cpp

@@ -0,0 +1,57 @@
+// 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;
+};
+
+struct __attribute__((ms_struct)) EmptyStruct {};
+
+struct __attribute__((ms_struct)) AlignedMemberStruct { // expected-warning {{padding size of 'AlignedMemberStruct' with 28 bytes to alignment boundary}}
+    alignas(32) int x;
+};
+
+struct alignas(32) __attribute__((ms_struct)) AlignedNonEmptyStruct { // expected-warning {{padding size of 'AlignedNonEmptyStruct' with 28 bytes to alignment boundary}}
+    int x;
+};
+
+
+struct alignas(16) __attribute__((ms_struct)) AlignedEmptyStruct {}; // expected-warning {{padding size of 'AlignedEmptyStruct' with 15 bytes to alignment boundary}}
+
+int main() {
+  Foo f;
+  AlignedStruct a;
+  Derived d;
+  StructWithUnion swu;
+  EmptyStruct e;
+  AlignedNonEmptyStruct anes;
+  AlignedMemberStruct ams;
+  AlignedEmptyStruct aes;
+}