Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5387,6 +5387,11 @@ "dereference of type %1 that was reinterpret_cast from type %0 has undefined " "behavior">, InGroup, DefaultIgnore; +def warn_taking_address_of_packed_member : Warning< + "taking address of packed member %0 of class or structure %q1 may result in an unaligned pointer value">, + InGroup>; +def err_binding_reference_to_packed_member : Error< + "binding reference to packed member %0 of class or structure %q1">; def err_objc_object_assignment : Error< "cannot assign to class object (%0 invalid)">; Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -9417,6 +9417,10 @@ void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr, const Expr * const *ExprArgs); + /// \brief Check if we are taking the address of a packed field + /// as this may be a problem if the pointer value is dereferenced. + void CheckAddressOfPackedMember(Expr *rhs); + /// \brief The parser's current scope. /// /// The parser maintains this state here. @@ -9495,6 +9499,51 @@ // Emitting members of dllexported classes is delayed until the class // (including field initializers) is fully parsed. SmallVector DelayedDllExportClasses; + +private: + /// \brief Helper class that collects misaligned member designations and + /// their location info for delayed diagnostics. + struct MisalignedMember { + Expr *E; + RecordDecl *RD; + ValueDecl *MD; + CharUnits Alignment; + + MisalignedMember() : E(), RD(), MD(), Alignment() {} + MisalignedMember(Expr *E, RecordDecl *RD, ValueDecl *MD, + CharUnits Alignment) + : E(E), RD(RD), MD(MD), Alignment(Alignment) {} + explicit MisalignedMember(Expr *E) + : MisalignedMember(E, nullptr, nullptr, CharUnits()) {} + + bool operator==(const MisalignedMember &m) { return this->E == m.E; } + }; + /// \brief Small set of gathered accesses to potentially misaligned members + /// due to the packed attribute. + SmallVector MisalignedMembers; + + /// \brief Adds an expression to the set of gathered misaligned members. + void AddPotentialMisalignedMembers(Expr *E, RecordDecl *RD, ValueDecl *MD, + CharUnits Alignment); + +public: + /// \brief Diagnoses the current set of gathered accesses. This typically + /// happens at full expression level. The set is cleared after emitting the + /// diagnostics. + void DiagnoseMisalignedMembers(); + + /// \brief This function checks if the expression is in the sef of potentially + /// misaligned members and it is converted to some pointer type T with lower + /// or equal alignment requirements. If so it removes it. This is used when + /// we do not want to diagnose such misaligned access (e.g. in conversions to void*). + void DiscardMisalignedMemberAddress(const Type *T, Expr *E); + + /// \brief This function calls Action when it determines that E designates a + /// misaligned member due to the packed attribute. This is used to emit + /// local diagnostics like in reference binding. + void RefersToMemberWithReducedAlignment( + Expr *E, + std::function Action); }; /// \brief RAII object that enters a new expression evaluation context. Index: lib/Sema/SemaCast.cpp =================================================================== --- lib/Sema/SemaCast.cpp +++ lib/Sema/SemaCast.cpp @@ -256,6 +256,7 @@ Op.CheckConstCast(); if (Op.SrcExpr.isInvalid()) return ExprError(); + DiscardMisalignedMemberAddress(DestType.getTypePtr(), E); } return Op.complete(CXXConstCastExpr::Create(Context, Op.ResultType, Op.ValueKind, Op.SrcExpr.get(), DestTInfo, @@ -279,6 +280,7 @@ Op.CheckReinterpretCast(); if (Op.SrcExpr.isInvalid()) return ExprError(); + DiscardMisalignedMemberAddress(DestType.getTypePtr(), E); } return Op.complete(CXXReinterpretCastExpr::Create(Context, Op.ResultType, Op.ValueKind, Op.Kind, Op.SrcExpr.get(), @@ -291,6 +293,7 @@ Op.CheckStaticCast(); if (Op.SrcExpr.isInvalid()) return ExprError(); + DiscardMisalignedMemberAddress(DestType.getTypePtr(), E); } return Op.complete(CXXStaticCastExpr::Create(Context, Op.ResultType, Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -8101,6 +8101,8 @@ DiagnoseNullConversion(S, E, T, CC); + S.DiscardMisalignedMemberAddress(Target, E); + if (!Source->isIntegerType() || !Target->isIntegerType()) return; @@ -9148,6 +9150,7 @@ CheckUnsequencedOperations(E); if (!IsConstexpr && !E->isValueDependent()) CheckForIntOverflow(E); + DiagnoseMisalignedMembers(); } void Sema::CheckBitFieldInitialization(SourceLocation InitLoc, @@ -10704,3 +10707,67 @@ << ArgumentExpr->getSourceRange() << TypeTagExpr->getSourceRange(); } + +void Sema::AddPotentialMisalignedMembers(Expr *E, RecordDecl *RD, ValueDecl *MD, + CharUnits Alignment) { + MisalignedMembers.emplace_back(E, RD, MD, Alignment); +} + +void Sema::DiagnoseMisalignedMembers() { + for (MisalignedMember &m : MisalignedMembers) { + Diag(m.E->getLocStart(), diag::warn_taking_address_of_packed_member) + << m.MD << m.RD << m.E->getSourceRange(); + } + MisalignedMembers.clear(); +} + +void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) { + if (!T->isPointerType()) + return; + if (isa(E) && + cast(E)->getOpcode() == UO_AddrOf) { + auto *Op = cast(E)->getSubExpr()->IgnoreParens(); + if (isa(Op)) { + auto MA = std::find(MisalignedMembers.begin(), MisalignedMembers.end(), + MisalignedMember(Op)); + if (MA != MisalignedMembers.end() && + Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment) + MisalignedMembers.erase(MA); + } + } +} + +void Sema::RefersToMemberWithReducedAlignment( + Expr *E, + std::function Action) { + const auto *ME = dyn_cast(E); + while (ME && isa(ME->getMemberDecl())) { + QualType BaseType = ME->getBase()->getType(); + if (ME->isArrow()) + BaseType = BaseType->getPointeeType(); + RecordDecl *RD = BaseType->getAs()->getDecl(); + + ValueDecl *MD = ME->getMemberDecl(); + bool ByteAligned = Context.getTypeAlignInChars(MD->getType()).isOne(); + if (ByteAligned) // Attribute packed does not have any effect. + break; + + if (!ByteAligned && + (RD->hasAttr() || (MD->hasAttr()))) { + CharUnits Alignment = std::min(Context.getTypeAlignInChars(MD->getType()), + Context.getTypeAlignInChars(BaseType)); + // Notify that this expression designates a member with reduced alignment + Action(E, RD, MD, Alignment); + break; + } + ME = dyn_cast(ME->getBase()); + } +} + +void Sema::CheckAddressOfPackedMember(Expr *rhs) { + using namespace std::placeholders; + RefersToMemberWithReducedAlignment( + rhs, std::bind(&Sema::AddPotentialMisalignedMembers, std::ref(*this), _1, + _2, _3, _4)); +} + Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -5999,7 +5999,9 @@ CheckTollFreeBridgeCast(castType, CastExpr); CheckObjCBridgeRelatedCast(castType, CastExpr); - + + DiscardMisalignedMemberAddress(castType.getTypePtr(), CastExpr); + return BuildCStyleCastExpr(LParenLoc, castTInfo, RParenLoc, CastExpr); } @@ -10514,6 +10516,8 @@ return QualType(); } + CheckAddressOfPackedMember(op); + return Context.getPointerType(op->getType()); } Index: lib/Sema/SemaInit.cpp =================================================================== --- lib/Sema/SemaInit.cpp +++ lib/Sema/SemaInit.cpp @@ -6435,6 +6435,15 @@ ExtendingEntity->getDecl()); CheckForNullPointerDereference(S, CurInit.get()); + + S.RefersToMemberWithReducedAlignment(CurInit.get(), [&](Expr *E, + RecordDecl *RD, + ValueDecl *MD, + CharUnits) { + S.Diag(Kind.getLocation(), diag::err_binding_reference_to_packed_member) + << MD << RD << E->getSourceRange(); + }); + break; case SK_BindReferenceToTemporary: { @@ -6623,12 +6632,16 @@ getAssignmentAction(Entity), CCK); if (CurInitExprRes.isInvalid()) return ExprError(); + + S.DiscardMisalignedMemberAddress(Step->Type.getTypePtr(), CurInit.get()); + CurInit = CurInitExprRes; if (Step->Kind == SK_ConversionSequenceNoNarrowing && S.getLangOpts().CPlusPlus && !CurInit.get()->isValueDependent()) DiagnoseNarrowingInInitList(S, *Step->ICS, SourceType, Entity.getType(), CurInit.get()); + break; } Index: test/Sema/address-packed-member-memops.c =================================================================== --- /dev/null +++ test/Sema/address-packed-member-memops.c @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics + +struct B { + int x, y, z, w; +} b; + +struct __attribute__((packed)) A { + struct B b; +} a; + +typedef __typeof__(sizeof(int)) size_t; + +void *memcpy(void *dest, const void *src, size_t n); +int memcmp(const void *s1, const void *s2, size_t n); +void *memmove(void *dest, const void *src, size_t n); +void *memset(void *s, int c, size_t n); + +int x; + +void foo(void) { + memcpy(&a.b, &b, sizeof(b)); + memmove(&a.b, &b, sizeof(b)); + memset(&a.b, 0, sizeof(b)); + x = memcmp(&a.b, &b, sizeof(b)); +} Index: test/Sema/address-packed.c =================================================================== --- /dev/null +++ test/Sema/address-packed.c @@ -0,0 +1,160 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +extern void f1(int *); +extern void f2(char *); + +struct Ok { + char c; + int x; +}; + +struct __attribute__((packed)) Arguable { + char c0; + int x; + char c1; +}; + +union __attribute__((packed)) UnionArguable { + char c; + int x; +}; + +typedef struct Arguable ArguableT; + +struct Arguable *get_arguable(); + +void to_void(void *); + +void g0(void) { + { + struct Ok ok; + f1(&ok.x); // no-warning + f2(&ok.c); // no-warning + } + { + struct Arguable arguable; + f2(&arguable.c0); // no-warning + f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}} + f2(&arguable.c1); // no-warning + + f1((int *)(void *)&arguable.x); // no-warning + to_void(&arguable.x); // no-warning + void *p = &arguable.x; // no-warning; + to_void(p); + } + { + union UnionArguable arguable; + f2(&arguable.c); // no-warning + f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'UnionArguable'}} + + f1((int *)(void *)&arguable.x); // no-warning + to_void(&arguable.x); // no-warning + } + { + ArguableT arguable; + f2(&arguable.c0); // no-warning + f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}} + f2(&arguable.c1); // no-warning + + f1((int *)(void *)&arguable.x); // no-warning + to_void(&arguable.x); // no-warning + } + { + struct Arguable *arguable = get_arguable(); + f2(&arguable->c0); // no-warning + f1(&arguable->x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}} + f2(&arguable->c1); // no-warning + + f1((int *)(void *)&arguable->x); // no-warning + to_void(&arguable->c1); // no-warning + } + { + ArguableT *arguable = get_arguable(); + f2(&(arguable->c0)); // no-warning + f1(&(arguable->x)); // expected-warning {{packed member 'x' of class or structure 'Arguable'}} + f2(&(arguable->c1)); // no-warning + + f1((int *)(void *)&(arguable->x)); // no-warning + to_void(&(arguable->c1)); // no-warning + } +} + +struct S1 { + char c; + int i __attribute__((packed)); +}; + +int *g1(struct S1 *s1) { + return &s1->i; // expected-warning {{packed member 'i' of class or structure 'S1'}} +} + +struct S2_i { + int i; +}; +struct __attribute__((packed)) S2 { + char c; + struct S2_i inner; +}; + +int *g2(struct S2 *s2) { + return &s2->inner.i; // expected-warning {{packed member 'inner' of class or structure 'S2'}} +} + +struct S2_a { + char c; + struct S2_i inner __attribute__((packed)); +}; + +int *g2_a(struct S2_a *s2_a) { + return &s2_a->inner.i; // expected-warning {{packed member 'inner' of class or structure 'S2_a'}} +} + +struct __attribute__((packed)) S3 { + char c; + struct { + int i; + } inner; +}; + +int *g3(struct S3 *s3) { + return &s3->inner.i; // expected-warning {{packed member 'inner' of class or structure 'S3'}} +} + +struct S4 { + char c; + struct __attribute__((packed)) { + int i; + } inner; +}; + +int *g4(struct S4 *s4) { + return &s4->inner.i; // expected-warning {{packed member 'i' of class or structure 'S4::(anonymous)'}} +} + +struct S5 { + char c; + struct { + char c1; + int i __attribute__((packed)); + } inner; +}; + +int *g5(struct S5 *s5) { + return &s5->inner.i; // expected-warning {{packed member 'i' of class or structure 'S5::(anonymous)'}} +} + +struct __attribute__((packed, aligned(2))) AlignedTo2 { + int x; +}; + +char *g6(struct AlignedTo2 *s) { + return (char *)&s->x; // no-warning +} + +struct __attribute__((packed, aligned(2))) AlignedTo2Bis { + int x; +}; + +struct AlignedTo2Bis* g7(struct AlignedTo2 *s) +{ + return (struct AlignedTo2Bis*)&s->x; // no-warning +} Index: test/SemaCXX/address-packed-member-memops.cpp =================================================================== --- /dev/null +++ test/SemaCXX/address-packed-member-memops.cpp @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s +// expected-no-diagnostics + +struct B { + int x, y, z, w; +} b; + +struct __attribute__((packed)) A { + struct B b; +} a; + +typedef __typeof__(sizeof(int)) size_t; + +extern "C" { +void *memcpy(void *dest, const void *src, size_t n); +int memcmp(const void *s1, const void *s2, size_t n); +void *memmove(void *dest, const void *src, size_t n); +void *memset(void *s, int c, size_t n); +} + +int x; + +void foo() { + memcpy(&a.b, &b, sizeof(b)); + memmove(&a.b, &b, sizeof(b)); + memset(&a.b, 0, sizeof(b)); + x = memcmp(&a.b, &b, sizeof(b)); +} Index: test/SemaCXX/address-packed.cpp =================================================================== --- /dev/null +++ test/SemaCXX/address-packed.cpp @@ -0,0 +1,118 @@ +// RUN: %clang_cc1 -std=c++11 -fsyntax-only -verify %s +extern void f1(int *); +extern void f2(char *); + +struct __attribute__((packed)) Arguable { + int x; + char c; + static void foo(); +}; + +extern void f3(void()); + +namespace Foo { +struct __attribute__((packed)) Arguable { + char c; + int x; + static void foo(); +}; +} + +struct Arguable *get_arguable(); + +void f4(int &); + +void to_void(void *); + +template +void sink(T...); + +void g0() { + { + Foo::Arguable arguable; + f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Foo::Arguable'}} + f2(&arguable.c); // no-warning + f3(&arguable.foo); // no-warning + + int &w = arguable.x; // expected-error {{binding reference to packed member 'x' of class or structure 'Foo::Arguable'}} + sink(w); + f4(arguable.x); // expected-error {{binding reference to packed member 'x' of class or structure 'Foo::Arguable'}} + + to_void(&arguable.x); // no-warning + void *p1 = &arguable.x; // no-warning + void *p2 = static_cast(&arguable.x); // no-warning + void *p3 = reinterpret_cast(&arguable.x); // no-warning + void *p4 = (void *)&arguable.x; // no-warning + sink(p1, p2, p3, p4); + } + { + Arguable arguable1; + Arguable &arguable(arguable1); + f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}} + f2(&arguable.c); // no-warning + f3(&arguable.foo); // no-warning + } + { + Arguable *arguable1; + Arguable *&arguable(arguable1); + f1(&arguable->x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}} + f2(&arguable->c); // no-warning + f3(&arguable->foo); // no-warning + } +} + +struct __attribute__((packed)) A { + int x; + char c; + + int *f0() { + return &this->x; // expected-warning {{packed member 'x' of class or structure 'A'}} + } + + int *g0() { + return &x; // expected-warning {{packed member 'x' of class or structure 'A'}} + } + + char *h0() { + return &c; // no-warning + } +}; + +struct B : A { + int *f1() { + return &this->x; // expected-warning {{packed member 'x' of class or structure 'A'}} + } + + int *g1() { + return &x; // expected-warning {{packed member 'x' of class or structure 'A'}} + } + + char *h1() { + return &c; // no-warning + } +}; + +template +class __attribute__((packed)) S { + Ty X; + +public: + const Ty *get() const { + return &X; // expected-warning {{packed member 'X' of class or structure 'S'}} + // expected-warning@-1 {{packed member 'X' of class or structure 'S'}} + } +}; + +template +void h(Ty *); + +void g1() { + S s1; + s1.get(); // expected-note {{in instantiation of member function 'S::get'}} + + S s2; + s2.get(); + + S s3; + s3.get(); // expected-note {{in instantiation of member function 'S::get'}} +}