From ac6617b288eda8958d060328163ae05050a59b25 Mon Sep 17 00:00:00 2001 From: Roger Ferrer Ibanez Date: Mon, 13 Jun 2016 15:41:40 +0000 Subject: [PATCH] Warn when taking address of a packed member This patch implements PR#22821. Taking the address of a packed member is dangerous since the reduced alignment of the pointee is lost. This can lead to memory alignment faults in some architectures if the pointer value is dereferenced. This change adds a new warning to clang emitted when taking the address of a packed member. A packed member is either a field/data member declared as attribute((packed)) or belonging to a struct/class declared as such. The associated flag is -Waddress-of-packed-member Differential Revision: http://reviews.llvm.org/D20561 llvm-svn: 272552 --- .../clang/Basic/DiagnosticSemaKinds.td | 3 + clang/lib/Sema/SemaExpr.cpp | 24 ++++ clang/test/Sema/address-packed.c | 126 ++++++++++++++++++ clang/test/SemaCXX/address-packed.cpp | 100 ++++++++++++++ 4 files changed, 253 insertions(+) create mode 100644 clang/test/Sema/address-packed.c create mode 100644 clang/test/SemaCXX/address-packed.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index bc6e2024a5339..a607690a14c7e 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5387,6 +5387,9 @@ def warn_pointer_indirection_from_incompatible_type : Warning< "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)">; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index f4a5dea8f94c9..f766091e54e8e 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -10514,6 +10514,30 @@ QualType Sema::CheckAddressOfOperand(ExprResult &OrigOp, SourceLocation OpLoc) { return QualType(); } + // Taking the address of a data member/field of a packed + // struct may be a problem if the pointer value is dereferenced. + Expr *rhs = OrigOp.get(); + 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()); + } + return Context.getPointerType(op->getType()); } diff --git a/clang/test/Sema/address-packed.c b/clang/test/Sema/address-packed.c new file mode 100644 index 0000000000000..4c13fe6306c38 --- /dev/null +++ b/clang/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)'}} +} diff --git a/clang/test/SemaCXX/address-packed.cpp b/clang/test/SemaCXX/address-packed.cpp new file mode 100644 index 0000000000000..313120c6084dd --- /dev/null +++ b/clang/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'}} +}