[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie created this revision.
dblaikie added a reviewer: rsmith.
dblaikie requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118511

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp


Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===================================================================
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -146,8 +146,18 @@
   unsigned char b : 8;
 } __attribute__((packed));
 
+struct S28_non_pod {
+ protected:
+  int i;
+};
+struct S28 {
+  char c1;
+  short s1;
+  char c2;
+  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is non-POD}}
+} __attribute__((packed));
 
 // 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*, S8*, S9*, S10*, S11*, S12*, S13*,
        S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
-       S26*, S27*){}
+       S26*, S27*, S28*){}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1888,11 +1888,17 @@
   LastBitfieldStorageUnitSize = 0;
 
   llvm::Triple Target = Context.getTargetInfo().getTriple();
-  bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
-                                 Context.getLangOpts().getClangABICompat() <=
-                                     LangOptions::ClangABI::Ver13 ||
-                                 Target.isPS4() || Target.isOSDarwin())) ||
-                     D->hasAttr<PackedAttr>();
+  bool FieldPacked = Packed;
+  if (FieldPacked && FieldClass && !FieldClass->isPOD() &&
+      Context.getLangOpts().getClangABICompat() >
+          LangOptions::ClangABI::Ver13 &&
+      !Target.isPS4() && !Target.isOSDarwin()) {
+    Diag(D->getLocation(), diag::warn_unpacked_field) << D;
+    FieldPacked = false;
+  }
+
+  if (!FieldPacked && D->hasAttr<PackedAttr>())
+    FieldPacked = true;
 
   AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
   CharUnits FieldSize;
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -590,6 +590,8 @@
   InGroup<Padded>, DefaultIgnore;
 def warn_unnecessary_packed : Warning<
   "packed attribute is unnecessary for %0">, InGroup<Packed>, DefaultIgnore;
+def warn_unpacked_field : Warning<
+  "not packing field %0 as it is non-POD">, InGroup<Packed>, DefaultIgnore;
 
 // -Wunaligned-access
 def warn_unaligned_access : Warning<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D118511.404166.patch
Type: text/x-patch
Size: 2552 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220128/8f434554/attachment-0001.bin>