Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5387,6 +5387,9 @@ "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_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 @@ -465,6 +465,11 @@ /// that's used to parse every top-level function. SmallVector FunctionScopes; + /// \brief Stack containing the current LHS of a direct function call being + /// parsed. It is empty if we are not parsing an expression of the argument + /// list of a function call. + SmallVector FunctionCallLHSStack; + typedef LazyVector ExtVectorDeclsType; @@ -1204,6 +1209,10 @@ void PushCompoundScope(); void PopCompoundScope(); + void PushFunctionCallLHS(Expr*); + void PopFunctionCallLHS(); + Expr* CurrentFunctionCallLHS() const; + sema::CompoundScopeInfo &getCurCompoundScope() const; bool hasAnyUnrecoverableErrorsInThisFunction() const; @@ -9417,6 +9426,10 @@ void CheckArgumentWithTypeTag(const ArgumentWithTypeTagAttr *Attr, const Expr * const *ExprArgs); + /// \brief Diagnose 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, clang::SourceLocation &OpLoc); + /// \brief The parser's current scope. /// /// The parser maintains this state here. Index: lib/Parse/ParseExpr.cpp =================================================================== --- lib/Parse/ParseExpr.cpp +++ lib/Parse/ParseExpr.cpp @@ -1561,15 +1561,17 @@ if (OpKind == tok::l_paren || !LHS.isInvalid()) { if (Tok.isNot(tok::r_paren)) { + Actions.PushFunctionCallLHS(LHS.get()); if (ParseExpressionList(ArgExprs, CommaLocs, [&] { Actions.CodeCompleteCall(getCurScope(), LHS.get(), ArgExprs); - })) { + })) { (void)Actions.CorrectDelayedTyposInExpr(LHS); LHS = ExprError(); } else if (LHS.isInvalid()) { for (auto &E : ArgExprs) Actions.CorrectDelayedTyposInExpr(E); } + Actions.PopFunctionCallLHS(); } } Index: lib/Sema/SemaChecking.cpp =================================================================== --- lib/Sema/SemaChecking.cpp +++ lib/Sema/SemaChecking.cpp @@ -10704,3 +10704,63 @@ << ArgumentExpr->getSourceRange() << TypeTagExpr->getSourceRange(); } + +void Sema::CheckAddressOfPackedMember(Expr *rhs, clang::SourceLocation &OpLoc) { + Expr *FunctionCallLHS = CurrentFunctionCallLHS(); + if (FunctionCallLHS) + FunctionCallLHS = FunctionCallLHS->IgnoreParens(); + + auto WhiteListedFunction = [](FunctionDecl *FDecl) -> bool { + if (FDecl) { + unsigned CMId = FDecl->getMemoryFunctionKind(); + switch (CMId) { + case Builtin::BImemcmp: + case Builtin::BImemcpy: + case Builtin::BImemmove: + case Builtin::BImemset: + // We skip the diagnostic if we are calling these functions since they + // are a source of false positives. + return true; + default: + break; + } + } + return false; + }; + + if (auto *ULE = dyn_cast_or_null(FunctionCallLHS)) { + for (NamedDecl *D : ULE->decls()) { + if (auto *FDecl = dyn_cast(D)) + if (WhiteListedFunction(FDecl)) + return; + } + } else if (auto *DRE = dyn_cast_or_null(FunctionCallLHS)) { + FunctionDecl *FDecl = nullptr; + if (DRE) + FDecl = dyn_cast_or_null(DRE->getDecl()); + if (FDecl && WhiteListedFunction(FDecl)) + return; + } + + const auto *ME = dyn_cast(rhs); + 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()))) { + Diag(OpLoc, diag::warn_taking_address_of_packed_member) + << MD << RD << rhs->getSourceRange(); + break; + } + ME = dyn_cast(ME->getBase()); + } +} + Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -10514,6 +10514,8 @@ return QualType(); } + CheckAddressOfPackedMember(OrigOp.get(), OpLoc); + return Context.getPointerType(op->getType()); } @@ -15023,3 +15025,14 @@ return new (Context) ObjCBoolLiteralExpr(Kind == tok::kw___objc_yes, BoolT, OpLoc); } + +void Sema::PushFunctionCallLHS(Expr *e) { FunctionCallLHSStack.push_back(e); } + +void Sema::PopFunctionCallLHS() { + assert(!FunctionCallLHSStack.empty() && "Empty stack"); + FunctionCallLHSStack.pop_back(); +} + +Expr *Sema::CurrentFunctionCallLHS() const { + return !FunctionCallLHSStack.empty() ? FunctionCallLHSStack.back() : nullptr; +} Index: test/Sema/address-packed-member-whitelist.c =================================================================== --- /dev/null +++ test/Sema/address-packed-member-whitelist.c @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +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); + +void *my_memcpy(void *dest, const void *src, size_t n); +int my_memcmp(const void *s1, const void *s2, size_t n); +void *my_memmove(void *dest, const void *src, size_t n); +void *my_memset(void *s, int c, size_t n); + +int x; + +void foo(void) { + memcpy(&a.b, &b, sizeof(b)); // no-warning + memmove(&a.b, &b, sizeof(b)); // no-warning + memset(&a.b, 0, sizeof(b)); // no-warning + x = memcmp(&a.b, &b, sizeof(b)); // no-warning + + (memcpy)(&a.b, &b, sizeof(b)); // no-warning + (memmove)(&a.b, &b, sizeof(b)); // no-warning + (memset)(&a.b, 0, sizeof(b)); // no-warning + x = (memcmp)(&a.b, &b, sizeof(b)); // no-warning + + my_memcpy(&a.b, &b, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} + my_memmove(&a.b, &b, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} + my_memset(&a.b, 0, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} + x = my_memcmp(&a.b, &b, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} +} Index: test/Sema/address-packed.c =================================================================== --- /dev/null +++ test/Sema/address-packed.c @@ -0,0 +1,126 @@ +// 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 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 + } + { + union UnionArguable arguable; + f2(&arguable.c); // no-warning + f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'UnionArguable'}} + } + { + 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 + } + { + struct Arguable *arguable = get_arguable(); + // These do not produce any warning because of the parentheses. + f2(&(arguable->c0)); // no-warning + f1(&(arguable->x)); // no-warning + f2(&(arguable->c1)); // no-warning + } + { + ArguableT *arguable = get_arguable(); + // These do not produce any warning because of the parentheses. + f2(&(arguable->c0)); // no-warning + f1(&(arguable->x)); // no-warning + f2(&(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-whitelist.cpp =================================================================== --- /dev/null +++ test/SemaCXX/address-packed-member-whitelist.cpp @@ -0,0 +1,52 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +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); +} + +void *my_memcpy(void *dest, const void *src, size_t n); +int my_memcmp(const void *s1, const void *s2, size_t n); +void *my_memmove(void *dest, const void *src, size_t n); +void *my_memset(void *s, int c, size_t n); + +int x; + +void foo() { + memcpy(&a.b, &b, sizeof(b)); // no-warning + memmove(&a.b, &b, sizeof(b)); // no-warning + memset(&a.b, 0, sizeof(b)); // no-warning + x = memcmp(&a.b, &b, sizeof(b)); // no-warning + + (memcpy)(&a.b, &b, sizeof(b)); // no-warning + (memmove)(&a.b, &b, sizeof(b)); // no-warning + (memset)(&a.b, 0, sizeof(b)); // no-warning + x = (memcmp)(&a.b, &b, sizeof(b)); // no-warning + + ::memcpy(&a.b, &b, sizeof(b)); // no-warning + ::memmove(&a.b, &b, sizeof(b)); // no-warning + ::memset(&a.b, 0, sizeof(b)); // no-warning + x = ::memcmp(&a.b, &b, sizeof(b)); // no-warning + + (::memcpy)(&a.b, &b, sizeof(b)); // no-warning + (::memmove)(&a.b, &b, sizeof(b)); // no-warning + (::memset)(&a.b, 0, sizeof(b)); // no-warning + x = (::memcmp)(&a.b, &b, sizeof(b)); // no-warning + + my_memcpy(&a.b, &b, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} + my_memmove(&a.b, &b, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} + my_memset(&a.b, 0, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} + x = my_memcmp(&a.b, &b, sizeof(b)); // expected-warning {{packed member 'b' of class or structure 'A'}} +} Index: test/SemaCXX/address-packed.cpp =================================================================== --- /dev/null +++ test/SemaCXX/address-packed.cpp @@ -0,0 +1,100 @@ +// RUN: %clang_cc1 -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 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 + } + { + 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'}} +}