rogfer01 created this revision.
rogfer01 added a reviewer: rsmith.
rogfer01 added a subscriber: cfe-commits.
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
http://reviews.llvm.org/D20561
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaExpr.cpp
test/Sema/address-packed.c
Index: test/Sema/address-packed.c
===================================================================
--- /dev/null
+++ test/Sema/address-packed.c
@@ -0,0 +1,201 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -xc++ -fsyntax-only -verify %s
+extern void f1(int *);
+extern void f2(char *);
+
+struct Ok {
+ char c;
+ int x;
+};
+
+struct __attribute__((packed)) Arguable {
+ char c;
+ int x;
+#ifdef __cplusplus
+ static void foo();
+#endif
+};
+
+union __attribute__((packed)) UnionArguable {
+ char c;
+ int x;
+#ifdef __cplusplus
+ static void foo();
+#endif
+};
+
+extern void f3(void());
+
+typedef struct Arguable ArguableT;
+
+#ifdef __cplusplus
+namespace Foo {
+struct __attribute__((packed)) Arguable {
+ char c;
+ int x;
+ static void foo();
+};
+}
+#endif
+
+void g0() {
+ {
+ struct Ok ok;
+ f1(&ok.x); // no-warning
+ f2(&ok.c); // no-warning
+ }
+ {
+ struct Arguable arguable;
+ f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+ f2(&arguable.c); // expected-warning {{packed member 'c' of class or structure 'Arguable'}}
+ }
+ {
+ union UnionArguable arguable;
+ f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'UnionArguable'}}
+ f2(&arguable.c); // expected-warning {{packed member 'c' of class or structure 'UnionArguable'}}
+ }
+ {
+ ArguableT arguable;
+ f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure }}
+ f2(&arguable.c); // expected-warning {{packed member 'c' of class or structure }}
+ }
+ {
+ struct Arguable *arguable;
+ f1(&(arguable->x)); // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+ f2(&(arguable->c)); // expected-warning {{packed member 'c' of class or structure 'Arguable'}}
+ }
+ {
+ ArguableT *arguable;
+ f1(&(arguable->x)); // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+ f2(&(arguable->c)); // expected-warning {{packed member 'c' of class or structure 'Arguable'}}
+ }
+
+#ifdef __cplusplus
+ {
+ Ok ok;
+ f1(&ok.x); // no-warning
+ f2(&ok.c); // no-warning
+ }
+ {
+ Arguable arguable;
+ f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Arguable'}}
+ f2(&arguable.c); // expected-warning {{packed member 'c' of class or structure 'Arguable'}}
+ f3(&arguable.foo); // no-warning
+ }
+ {
+ Foo::Arguable arguable;
+ f1(&arguable.x); // expected-warning {{packed member 'x' of class or structure 'Foo::Arguable'}}
+ f2(&arguable.c); // expected-warning {{packed member 'c' of class or structure 'Foo::Arguable'}}
+ 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); // expected-warning {{packed member 'c' of class or structure 'Arguable'}}
+ 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); // expected-warning {{packed member 'c' of class or structure 'Arguable'}}
+ f3(&arguable->foo); // no-warning
+ }
+#endif
+}
+
+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)'}}
+}
+
+#ifdef __cplusplus
+struct __attribute__((packed)) A
+{
+ char c;
+ int x;
+
+ 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'}}
+ }
+};
+
+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'}}
+ }
+};
+#endif
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -10514,6 +10514,23 @@
return QualType();
}
+ // Taking the address of a data member/field of a packed
+ // struct may be a problem if the pointer value is dereferenced
+ MemberExpr *ME = dyn_cast<MemberExpr>(op);
+ while (ME && isa<FieldDecl>(ME->getMemberDecl())) {
+ QualType BaseType = ME->getBase()->getType();
+ if (ME->isArrow())
+ BaseType = BaseType->getPointeeType();
+ RecordDecl *RD = BaseType->getAs<RecordType>()->getDecl();
+ if (RD->hasAttr<PackedAttr>() ||
+ ME->getMemberDecl()->hasAttr<PackedAttr>()) {
+ Diag(OpLoc, diag::warn_taking_address_of_packed_member)
+ << ME->getMemberDecl() << RD;
+ break;
+ }
+ ME = dyn_cast<MemberExpr>(ME->getBase());
+ }
+
return Context.getPointerType(op->getType());
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5382,6 +5382,9 @@
"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_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
