diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td --- a/clang/include/clang/Basic/DiagnosticASTKinds.td +++ b/clang/include/clang/Basic/DiagnosticASTKinds.td @@ -934,6 +934,8 @@ InGroup, DefaultIgnore; def warn_unnecessary_packed : Warning< "packed attribute is unnecessary for %0">, InGroup, DefaultIgnore; +def warn_unpacked_field : Warning< + "not packing field %0 as it is non-POD">, InGroup, DefaultIgnore; // -Wunaligned-access def warn_unaligned_access : Warning< diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -562,7 +562,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">; diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -1890,12 +1890,6 @@ LastBitfieldStorageUnitSize = 0; llvm::Triple Target = Context.getTargetInfo().getTriple(); - bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || - FieldClass->hasAttr() || - Context.getLangOpts().getClangABICompat() <= - LangOptions::ClangABI::Ver15 || - Target.isPS() || Target.isOSDarwin())) || - D->hasAttr(); AlignRequirementKind AlignRequirement = AlignRequirementKind::None; CharUnits FieldSize; @@ -1976,6 +1970,13 @@ } } + bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() || + FieldClass->hasAttr() || + Context.getLangOpts().getClangABICompat() <= + LangOptions::ClangABI::Ver15 || + Target.isPS() || Target.isOSDarwin())) || + D->hasAttr(); + // 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 @@ -2026,28 +2027,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. @@ -2130,6 +2137,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) { diff --git a/clang/test/CodeGenCXX/warn-padded-packed.cpp b/clang/test/CodeGenCXX/warn-padded-packed.cpp --- a/clang/test/CodeGenCXX/warn-padded-packed.cpp +++ b/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*){}