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,50 @@ // 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; + + MisalignedMember() : E(), RD(), MD() { } + MisalignedMember(Expr *E, RecordDecl *RD, ValueDecl *MD) : E(E), RD(RD), MD(MD) { } + explicit MisalignedMember(Expr *E) : MisalignedMember(E, nullptr, nullptr) { } + + 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 States whether this expression refers to a misaligned member + /// due to the packed attribute. + bool IsMisalignedMember(Expr *E); + + /// \brief Removes an expression from the set of gathered misaligned members. + void RemoveMisalignedMember(Expr *E); + + /// \brief Adds an expression to the set of gathered misaligned members. + void AddPotentialMisalignedMembers(Expr *E, RecordDecl *RD, ValueDecl *MD); + +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. If so it removes it. This is used when we do not want + /// to diagnose such misaligned access (e.g. in casts to void*). + void DiscardMisalignedMemberAddress(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,8 @@ Op.CheckConstCast(); if (Op.SrcExpr.isInvalid()) return ExprError(); + if (DestType->isVoidPointerType()) + DiscardMisalignedMemberAddress(E); } return Op.complete(CXXConstCastExpr::Create(Context, Op.ResultType, Op.ValueKind, Op.SrcExpr.get(), DestTInfo, @@ -279,6 +281,8 @@ Op.CheckReinterpretCast(); if (Op.SrcExpr.isInvalid()) return ExprError(); + if (DestType->isVoidPointerType()) + DiscardMisalignedMemberAddress(E); } return Op.complete(CXXReinterpretCastExpr::Create(Context, Op.ResultType, Op.ValueKind, Op.Kind, Op.SrcExpr.get(), @@ -291,6 +295,8 @@ Op.CheckStaticCast(); if (Op.SrcExpr.isInvalid()) return ExprError(); + if (DestType->isVoidPointerType()) + DiscardMisalignedMemberAddress(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,9 @@ DiagnoseNullConversion(S, E, T, CC); + if (Target->isVoidPointerType()) + S.DiscardMisalignedMemberAddress(E); + if (!Source->isIntegerType() || !Target->isIntegerType()) return; @@ -9148,6 +9151,7 @@ CheckUnsequencedOperations(E); if (!IsConstexpr && !E->isValueDependent()) CheckForIntOverflow(E); + DiagnoseMisalignedMembers(); } void Sema::CheckBitFieldInitialization(SourceLocation InitLoc, @@ -10704,3 +10708,67 @@ << ArgumentExpr->getSourceRange() << TypeTagExpr->getSourceRange(); } + +void Sema::AddPotentialMisalignedMembers(Expr *E, RecordDecl *RD, ValueDecl *MD) { + MisalignedMembers.emplace_back(E, RD, MD); +} + +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(); +} + +bool Sema::IsMisalignedMember(Expr *E) { + return std::find(MisalignedMembers.begin(), MisalignedMembers.end(), + MisalignedMember(E)) != MisalignedMembers.end(); +} + +void Sema::RemoveMisalignedMember(Expr *E) { + MisalignedMembers.erase(std::remove(MisalignedMembers.begin(), + MisalignedMembers.end(), + MisalignedMember(E)), + MisalignedMembers.end()); +} + +void Sema::DiscardMisalignedMemberAddress(Expr *E) { + if (isa(E) && + cast(E)->getOpcode() == UO_AddrOf) { + auto *Op = cast(E)->getSubExpr()->IgnoreParens(); + if (isa(Op) && IsMisalignedMember(Op)) + RemoveMisalignedMember(Op); + } +} + +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()))) { + // Notify that this expression designates a member with reduced alignment + Action(E, RD, MD); + 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)); +} + Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -5999,6 +5999,9 @@ CheckTollFreeBridgeCast(castType, CastExpr); CheckObjCBridgeRelatedCast(castType, CastExpr); + + if (castType->isVoidPointerType()) + DiscardMisalignedMemberAddress(CastExpr); return BuildCStyleCastExpr(LParenLoc, castTInfo, RParenLoc, CastExpr); } @@ -10514,6 +10517,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,13 @@ ExtendingEntity->getDecl()); CheckForNullPointerDereference(S, CurInit.get()); + + S.RefersToMemberWithReducedAlignment( + CurInit.get(), [&](Expr *E, RecordDecl *RD, ValueDecl *MD) { + S.Diag(Kind.getLocation(), diag::err_binding_reference_to_packed_member) + << MD << RD << E->getSourceRange(); + }); + break; case SK_BindReferenceToTemporary: { 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,143 @@ +// 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)'}} +} 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'}} +}