https://github.com/mattst88 updated https://github.com/llvm/llvm-project/pull/219096
>From f58fc16a9a2d12fcb6a6e3e489d2ed6ecb2394ea Mon Sep 17 00:00:00 2001 From: Matt Turner <[email protected]> Date: Mon, 14 Sep 2026 20:39:25 -0400 Subject: [PATCH] [Sema] Diagnose taking the address of a #pragma pack member -Waddress-of-packed-member only looked for PackedAttr, so members packed by #pragma pack went undiagnosed: #pragma pack(1) struct S { char c; int x; }; int *f(struct S *p) { return &p->x; } // not diagnosed Replace the attribute checks with a helper that returns the alignment limit record layout puts on a field: #pragma pack, packed, alignment. The field to blame is the first one limited below the alignment of its type. Report the least alignment of any link in the chain, since an outer packed record can reduce a field further than its own record did. Fixes #97091. --- clang/docs/ReleaseNotes.md | 4 + clang/lib/Sema/SemaChecking.cpp | 65 +++++++++--- clang/test/Sema/address-packed-layout.c | 64 ++++++++++++ clang/test/Sema/address-packed.c | 125 ++++++++++++++++++++++++ clang/test/SemaCXX/address-packed.cpp | 28 ++++++ 5 files changed, 271 insertions(+), 15 deletions(-) create mode 100644 clang/test/Sema/address-packed-layout.c diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 7ec126a065ae5ab..dfe68dc91a3955d 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -559,6 +559,10 @@ features cannot lower the translation-unit ABI level; - Improve Clang diagnoses when unary `__imag` operator with non-complex type operand is used as lvalue. (#GH222383) +- `-Waddress-of-packed-member` now also diagnoses members packed by + `#pragma pack`, `#pragma options align=mac68k`, or `-fpack-struct`, and + members whose typedef lowers their alignment. (#GH97091) + ### Improvements to Clang's time-trace ### Improvements to Coverage Mapping diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 0531dfa877fbdf4..e61da7c24a48aa8 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -17102,6 +17102,37 @@ void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) { } } +/// Return the maximum alignment record layout allows for \p FD, if packing or +/// a typedef with a lowered alignment limits it. Mirrors RecordLayoutBuilder. +static std::optional<CharUnits> getFieldAlignmentLimit(const ASTContext &Ctx, + const FieldDecl *FD) { + std::optional<CharUnits> Limit; + const RecordDecl *RD = FD->getParent(); + const auto *MFAA = RD->getAttr<MaxFieldAlignmentAttr>(); + // The Microsoft layout ignores #pragma pack wider than a pointer. + if (MFAA && Ctx.getTargetInfo().hasMicrosoftRecordLayout() && + MFAA->getAlignment() > + Ctx.getTargetInfo().getPointerWidth(LangAS::Default)) + MFAA = nullptr; + + if (FD->hasAttr<PackedAttr>() || RD->hasAttr<PackedAttr>()) + Limit = CharUnits::One(); + else if (RD->hasAttr<AlignMac68kAttr>()) + Limit = CharUnits::fromQuantity(2); + else if (MFAA) + Limit = Ctx.toCharUnitsFromBits(MFAA->getAlignment()); + else if (unsigned PackStruct = Ctx.getLangOpts().PackStruct) + Limit = CharUnits::fromQuantity(PackStruct); + + QualType T = FD->getType(); + if (!T.isCanonical()) { + CharUnits TypeAlignment = Ctx.getTypeAlignInChars(T); + if (TypeAlignment < Ctx.getTypeAlignInChars(T.getCanonicalType())) + Limit = Limit ? std::min(*Limit, TypeAlignment) : TypeAlignment; + } + return Limit; +} + void Sema::RefersToMemberWithReducedAlignment( Expr *E, llvm::function_ref<void(Expr *, RecordDecl *, FieldDecl *, CharUnits)> @@ -17118,7 +17149,7 @@ void Sema::RefersToMemberWithReducedAlignment( // will keep FieldDecl's like [d, c, b]. SmallVector<FieldDecl *, 4> ReverseMemberChain; const MemberExpr *TopME = nullptr; - bool AnyIsPacked = false; + bool AnyIsLimited = false; do { QualType BaseType = ME->getBase()->getType(); if (BaseType->isDependentType()) @@ -17135,8 +17166,8 @@ void Sema::RefersToMemberWithReducedAlignment( if (!FD || FD->isInvalidDecl()) return; - AnyIsPacked = - AnyIsPacked || (RD->hasAttr<PackedAttr>() || MD->hasAttr<PackedAttr>()); + AnyIsLimited = + AnyIsLimited || getFieldAlignmentLimit(Context, FD).has_value(); ReverseMemberChain.push_back(FD); TopME = ME; @@ -17145,7 +17176,7 @@ void Sema::RefersToMemberWithReducedAlignment( assert(TopME && "We did not compute a topmost MemberExpr!"); // Not the scope of this diagnostic. - if (!AnyIsPacked) + if (!AnyIsLimited) return; const Expr *TopBase = TopME->getBase()->IgnoreParenImpCasts(); @@ -17192,21 +17223,25 @@ void Sema::RefersToMemberWithReducedAlignment( // type) but some packed attribute in that chain has reduced the alignment. // It may happen that another packed structure increases it again. But if // we are here such increase has not been enough. So pointing the first - // FieldDecl that either is packed or else its RecordDecl is, - // seems reasonable. + // FieldDecl whose alignment was reduced seems reasonable. FieldDecl *FD = nullptr; - CharUnits Alignment; + // An outer link may reduce the alignment further than the culprit did. + CharUnits Alignment = ExpectedAlignment; for (FieldDecl *FDI : ReverseMemberChain) { - if (FDI->hasAttr<PackedAttr>() || - FDI->getParent()->hasAttr<PackedAttr>()) { - FD = FDI; - Alignment = std::min(Context.getTypeAlignInChars(FD->getType()), - Context.getTypeAlignInChars( - Context.getCanonicalTagType(FD->getParent()))); - break; + CharUnits FieldAlignment = + Context.getTypeAlignInChars(FDI->getType().getCanonicalType()); + std::optional<CharUnits> Limit = getFieldAlignmentLimit(Context, FDI); + if (Limit && *Limit < FieldAlignment) { + if (!FD) + FD = FDI; + FieldAlignment = + std::min(Context.getTypeAlignInChars(FDI->getType()), + Context.getTypeAlignInChars( + Context.getCanonicalTagType(FDI->getParent()))); } + Alignment = std::min(Alignment, FieldAlignment); } - assert(FD && "We did not find a packed FieldDecl!"); + assert(FD && "We did not find a FieldDecl with reduced alignment!"); Action(E, FD->getParent(), FD, Alignment); } } diff --git a/clang/test/Sema/address-packed-layout.c b/clang/test/Sema/address-packed-layout.c new file mode 100644 index 000000000000000..76aad5352aa629d --- /dev/null +++ b/clang/test/Sema/address-packed-layout.c @@ -0,0 +1,64 @@ +// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-gnu -fpack-struct=1 -DPACK_STRUCT -verify=pack-struct %s +// RUN: %clang_cc1 -fsyntax-only -triple i386-apple-darwin -DMAC68K -verify=mac68k %s +// RUN: %clang_cc1 -fsyntax-only -triple x86_64-linux-gnu -target-feature +avx512f -DPACK16 -verify=itanium %s +// RUN: %clang_cc1 -fsyntax-only -triple x86_64-windows-msvc -target-feature +avx512f -DPACK16 -verify=ms %s + +extern void f1(int *); + +#ifdef PACK_STRUCT +struct S { + char c; + int x; +}; + +void g0(struct S *p) { + f1(&p->x); // pack-struct-warning {{packed member 'x' of class or structure 'S'}} +} + +// #pragma pack overrides -fpack-struct. +#pragma pack(push, 4) +struct Pack4 { + char c; + int x; +}; +#pragma pack(pop) + +void g1(struct Pack4 *p) { + f1(&p->x); // no-warning +} +#endif + +#ifdef MAC68K +#pragma options align=mac68k +struct S { + char c; + int x; +}; +#pragma options align=reset + +void g0(struct S *p) { + f1(&p->x); // mac68k-warning {{packed member 'x' of class or structure 'S'}} +} +#endif + +#ifdef PACK16 +typedef float V512 __attribute__((vector_size(64))); + +// Only the Microsoft layout ignores a pack wider than a pointer. +#pragma pack(push, 16) +struct Pack16 { + char c; + V512 x; +}; +#pragma pack(pop) + +struct __attribute__((packed)) Outer { + char c; + struct Pack16 inner; +}; + +void g0(struct Outer *p) { + V512 *q = &p->inner.x; // itanium-warning {{packed member 'x' of class or structure 'Pack16'}} + // ms-warning@-1 {{packed member 'inner' of class or structure 'Outer'}} +} +#endif diff --git a/clang/test/Sema/address-packed.c b/clang/test/Sema/address-packed.c index f826b7d57d91cf5..e4b0813823dc9dd 100644 --- a/clang/test/Sema/address-packed.c +++ b/clang/test/Sema/address-packed.c @@ -346,3 +346,128 @@ void g15(void) { to_void_with_expr(&arguable.x, 3); // no-warning to_void_with_expr(&arguable.x, ({3;})); // no-warning } + +#pragma pack(push, 1) +struct PragmaPack1 { + char c0; + int x; +}; +#pragma pack(pop) + +void g16(struct PragmaPack1 *p) { + f1(&p->x); // expected-warning {{packed member 'x' of class or structure 'PragmaPack1'}} + f2(&p->c0); // no-warning +} + +// #pragma pack(4) does not reduce an int. +#pragma pack(push, 4) +struct PragmaPack4 { + char c0; + int x; + char c1; +}; +#pragma pack(pop) + +void g17(struct PragmaPack4 *p) { + f1(&p->x); // no-warning +} + +// The record is two-aligned, so x is misaligned despite an offset of four. +#pragma pack(push, 2) +struct PragmaPack2 { + char c0; + char c1; + int x; +}; +#pragma pack(pop) + +void g18(struct PragmaPack2 *p) { + f1(&p->x); // expected-warning {{packed member 'x' of class or structure 'PragmaPack2'}} +} + +struct PragmaPackOuter { + char c0; + struct PragmaPack1 inner; +}; + +void g19(struct PragmaPackOuter *p) { + f1(&p->inner.x); // expected-warning {{packed member 'x' of class or structure 'PragmaPack1'}} +} + +extern void f3(short *); + +// short is two-aligned and so is PragmaPack2. +void g20(struct PragmaPack2 *p) { + f3((short *)&p->x); // no-warning +} + +// The outer packed record lowers it to one. +struct __attribute__((packed)) PragmaPackInPacked { + char c0; + struct PragmaPack2 inner; +}; + +void g21(struct PragmaPackInPacked *p) { + f3((short *)&p->inner.x); // expected-warning {{packed member 'x' of class or structure 'PragmaPack2'}} +} + +// #pragma pack(4) does not reduce x, so the outer packed record takes the +// blame. +struct __attribute__((packed)) PragmaPack4InPacked { + char c0; + struct PragmaPack4 inner; +}; + +void g22(struct PragmaPack4InPacked *p) { + f1(&p->inner.x); // expected-warning {{packed member 'inner' of class or structure 'PragmaPack4InPacked'}} +} + +// The typedef lowers inner to one, whatever the packing. +struct Natural { + char c0; + int x; +}; +typedef struct Natural __attribute__((aligned(1))) UnalignedNatural; + +struct __attribute__((packed)) PackedHoldsUnaligned { + char c0; + UnalignedNatural inner; +}; + +#pragma pack(push, 1) +struct PragmaPackHoldsUnaligned { + char c0; + UnalignedNatural inner; +}; +#pragma pack(pop) + +void g23(struct PackedHoldsUnaligned *p, struct PragmaPackHoldsUnaligned *q) { + f1(&p->inner.x); // expected-warning {{packed member 'inner' of class or structure 'PackedHoldsUnaligned'}} + f1(&q->inner.x); // expected-warning {{packed member 'inner' of class or structure 'PragmaPackHoldsUnaligned'}} +} + +// A typedef lowers alignment without any packing. +struct HoldsUnaligned { + char c0; + UnalignedNatural inner; +}; + +struct HoldsHoldsUnaligned { + char c0; + struct HoldsUnaligned holder; +}; + +typedef int __attribute__((aligned(1))) UnalignedInt; + +struct HoldsUnalignedInt { + char c0; + UnalignedInt x; +}; + +void g24(struct HoldsUnaligned *p, struct HoldsHoldsUnaligned *q, + struct HoldsUnalignedInt *r) { + f1(&p->inner.x); // expected-warning {{packed member 'inner' of class or structure 'HoldsUnaligned'}} + f1(&q->holder.inner.x); // expected-warning {{packed member 'inner' of class or structure 'HoldsUnaligned'}} + f2(&p->inner.c0); // no-warning + UnalignedInt *ui = &r->x; // no-warning +} diff --git a/clang/test/SemaCXX/address-packed.cpp b/clang/test/SemaCXX/address-packed.cpp index f0d1496fd892861..f020f0b1355e9cf 100644 --- a/clang/test/SemaCXX/address-packed.cpp +++ b/clang/test/SemaCXX/address-packed.cpp @@ -121,3 +121,31 @@ struct S2 { Incomplete *e() const; } __attribute__((packed)); Incomplete *S2::e() const { return (Incomplete *)&d; } // no-warning + +#pragma pack(push, 1) +struct PragmaPacked { + char c; + int x; + int *get() { return &x; } // expected-warning {{packed member 'x' of class or structure 'PragmaPacked'}} +}; +#pragma pack(pop) + +void g2(PragmaPacked *p) { + f1(&p->x); // expected-warning {{packed member 'x' of class or structure 'PragmaPacked'}} + f2(&p->c); // no-warning +} + +// #pragma pack caps the base subobject too, so alignof(DerivedFromUnpacked) +// is one and &p->x really is a one-aligned int *. Not diagnosed: the chain +// has a link per member access and none for the derived-to-base conversion. +struct Unpacked { + char c; + int x; +}; +#pragma pack(push, 1) +struct DerivedFromUnpacked : Unpacked {}; +#pragma pack(pop) + +void g3(DerivedFromUnpacked *p) { + f1(&p->x); // no-warning +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
