dblaikie updated this revision to Diff 452023.
dblaikie added a comment.
rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118511/new/
https://reviews.llvm.org/D118511
Files:
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/include/clang/Basic/DiagnosticGroups.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,28 @@
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));
+
+struct S29_non_pod_align_1 {
+ protected:
+ char c;
+};
+struct S29 {
+ S29_non_pod_align_1 p1;
+ int i;
+} __attribute__((packed)); // no warning
+static_assert(alignof(S29) == 1, "");
// 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*, S29*){}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===================================================================
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1890,11 +1890,6 @@
LastBitfieldStorageUnitSize = 0;
llvm::Triple Target = Context.getTargetInfo().getTriple();
- bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
- Context.getLangOpts().getClangABICompat() <=
- LangOptions::ClangABI::Ver14 ||
- Target.isPS() || Target.isOSDarwin())) ||
- D->hasAttr<PackedAttr>();
AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
CharUnits FieldSize;
@@ -1975,6 +1970,12 @@
}
}
+ bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver14 ||
+ Target.isPS() || Target.isOSDarwin())) ||
+ D->hasAttr<PackedAttr>();
+
// When used as part of a typedef, or together with a 'packed' attribute, the
// 'aligned' attribute can be used to decrease alignment. In that case, it
// overrides any computed alignment we have, and there is no need to upgrade
@@ -2025,28 +2026,34 @@
// The align if the field is not packed. This is to check if the attribute
// was unnecessary (-Wpacked).
- CharUnits UnpackedFieldAlign =
- !DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
+ CharUnits UnpackedFieldAlign = FieldAlign;
+ CharUnits PackedFieldAlign = CharUnits::One();
CharUnits UnpackedFieldOffset = FieldOffset;
CharUnits OriginalFieldAlign = UnpackedFieldAlign;
- if (FieldPacked) {
- FieldAlign = CharUnits::One();
- PreferredAlign = CharUnits::One();
- }
CharUnits MaxAlignmentInChars =
Context.toCharUnitsFromBits(D->getMaxAlignment());
- FieldAlign = std::max(FieldAlign, MaxAlignmentInChars);
+ PackedFieldAlign = std::max(PackedFieldAlign, MaxAlignmentInChars);
PreferredAlign = std::max(PreferredAlign, MaxAlignmentInChars);
UnpackedFieldAlign = std::max(UnpackedFieldAlign, MaxAlignmentInChars);
// The maximum field alignment overrides the aligned attribute.
if (!MaxFieldAlignment.isZero()) {
- FieldAlign = std::min(FieldAlign, MaxFieldAlignment);
+ PackedFieldAlign = std::min(PackedFieldAlign, MaxFieldAlignment);
PreferredAlign = std::min(PreferredAlign, MaxFieldAlignment);
UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment);
}
+
+ if (!FieldPacked)
+ FieldAlign = UnpackedFieldAlign;
+ if (DefaultsToAIXPowerAlignment)
+ UnpackedFieldAlign = PreferredAlign;
+ if (FieldPacked) {
+ PreferredAlign = PackedFieldAlign;
+ FieldAlign = PackedFieldAlign;
+ }
+
CharUnits AlignTo =
!DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
// Round up the current record size to the field's alignment boundary.
@@ -2129,6 +2136,9 @@
<< Context.getTypeDeclType(RD) << D->getName() << D->getType();
}
}
+
+ if (Packed && !FieldPacked && PackedFieldAlign < FieldAlign)
+ Diag(D->getLocation(), diag::warn_unpacked_field) << D;
}
void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -554,7 +554,8 @@
def DeprecatedObjCIsaUsage : DiagGroup<"deprecated-objc-isa-usage">;
def ExplicitInitializeCall : DiagGroup<"explicit-initialize-call">;
def OrderedCompareFunctionPointers : DiagGroup<"ordered-compare-function-pointers">;
-def Packed : DiagGroup<"packed">;
+def PackedNonPod : DiagGroup<"packed-non-pod">;
+def Packed : DiagGroup<"packed", [PackedNonPod]>;
def Padded : DiagGroup<"padded">;
def UnalignedAccess : DiagGroup<"unaligned-access">;
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -595,6 +595,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<PackedNonPod>, DefaultIgnore;
// -Wunaligned-access
def warn_unaligned_access : Warning<
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
