rogfer01 updated the summary for this revision.
rogfer01 updated this revision to Diff 60951.
rogfer01 added a comment.
Thanks @rsmith for the suggestions. I removed the whitelisting totally and
changed the strategy. This patch implements your second suggestion which seemed
more obvious to me. Since I expect the set of gathered misaligned member
designations be small, I am using a plain SmallVector but maybe there is a more
suitable structure.
I am now building Firefox and will post an update with the results.
http://reviews.llvm.org/D20561
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaCast.cpp
lib/Sema/SemaChecking.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaInit.cpp
test/Sema/address-packed-member-memops.c
test/Sema/address-packed.c
test/SemaCXX/address-packed-member-memops.cpp
test/SemaCXX/address-packed.cpp
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 <typename... T>
+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<void *>(&arguable.x); // no-warning
+ void *p3 = reinterpret_cast<void *>(&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 <typename Ty>
+class __attribute__((packed)) S {
+ Ty X;
+
+public:
+ const Ty *get() const {
+ return &X; // expected-warning {{packed member 'X' of class or structure 'S<int>'}}
+ // expected-warning@-1 {{packed member 'X' of class or structure 'S<float>'}}
+ }
+};
+
+template <typename Ty>
+void h(Ty *);
+
+void g1() {
+ S<int> s1;
+ s1.get(); // expected-note {{in instantiation of member function 'S<int>::get'}}
+
+ S<char> s2;
+ s2.get();
+
+ S<float> s3;
+ s3.get(); // expected-note {{in instantiation of member function 'S<float>::get'}}
+}
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/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/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: 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: 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/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<UnaryOperator>(E) &&
+ cast<UnaryOperator>(E)->getOpcode() == UO_AddrOf) {
+ auto *Op = cast<UnaryOperator>(E)->getSubExpr()->IgnoreParens();
+ if (isa<MemberExpr>(Op) && IsMisalignedMember(Op))
+ RemoveMisalignedMember(Op);
+ }
+}
+
+void Sema::RefersToMemberWithReducedAlignment(
+ Expr *E, std::function<void(Expr *, RecordDecl *, ValueDecl *)> Action) {
+ const auto *ME = dyn_cast<MemberExpr>(E);
+ while (ME && isa<FieldDecl>(ME->getMemberDecl())) {
+ QualType BaseType = ME->getBase()->getType();
+ if (ME->isArrow())
+ BaseType = BaseType->getPointeeType();
+ RecordDecl *RD = BaseType->getAs<RecordType>()->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<PackedAttr>() || (MD->hasAttr<PackedAttr>()))) {
+ // Notify that this expression designates a member with reduced alignment
+ Action(E, RD, MD);
+ break;
+ }
+ ME = dyn_cast<MemberExpr>(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/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: 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<CXXRecordDecl*, 4> 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<MisalignedMember, 4> 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<void(Expr *, RecordDecl *, ValueDecl *)> Action);
};
/// \brief RAII object that enters a new expression evaluation context.
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<UndefinedReinterpretCast>, 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<DiagGroup<"address-of-packed-member">>;
+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)">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
