rogfer01 updated this revision to Diff 74638.
rogfer01 added a comment.
Updated patch, now we check if the innermost base of a MemberExpr is a
DeclRefExpr and check for its declaration in case it provides stronger
alignment guarantees.
https://reviews.llvm.org/D23657
Files:
include/clang/Sema/Sema.h
lib/Sema/SemaChecking.cpp
test/Sema/address-packed.c
Index: test/Sema/address-packed.c
===================================================================
--- test/Sema/address-packed.c
+++ test/Sema/address-packed.c
@@ -26,6 +26,7 @@
struct Arguable *get_arguable();
void to_void(void *);
+void to_intptr(intptr_t);
void g0(void) {
{
@@ -41,43 +42,48 @@
f1((int *)(void *)&arguable.x); // no-warning
to_void(&arguable.x); // no-warning
- void *p = &arguable.x; // no-warning;
+ void *p = &arguable.x; // no-warning
to_void(p);
+ to_intptr((intptr_t)p); // no-warning
}
{
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
+ f1((int *)(void *)&arguable.x); // no-warning
+ to_void(&arguable.x); // no-warning
+ to_intptr((intptr_t)&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
+ f1((int *)(void *)&arguable.x); // no-warning
+ to_void(&arguable.x); // no-warning
+ to_intptr((intptr_t)&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
+ f1((int *)(void *)&arguable->x); // no-warning
+ to_void(&arguable->c1); // no-warning
+ to_intptr((intptr_t)&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
+ f1((int *)(void *)&(arguable->x)); // no-warning
+ to_void(&(arguable->c1)); // no-warning
+ to_intptr((intptr_t)&(arguable->c1)); // no-warning
}
}
@@ -161,3 +167,120 @@
{
return (struct AlignedTo2Bis*)&s->x; // no-warning
}
+
+struct S6 {
+ int a;
+ int _;
+ int c;
+ char __;
+ int d;
+} __attribute__((packed, aligned(16))) s6;
+
+void g8()
+{
+ f1(&s6.a); // no-warning
+ f1(&s6.c); // no-warning
+ f1(&s6.d); // expected-warning {{packed member 'd' of class or structure 'S6'}}
+}
+
+struct __attribute__((packed, aligned(1))) MisalignedContainee { double d; };
+struct __attribute__((aligned(8))) AlignedContainer { struct MisalignedContainee b; };
+
+struct AlignedContainer *p;
+double* g9() {
+ return &p->b.d; // no-warning
+}
+
+union OneUnion
+{
+ uint32_t a;
+ uint32_t b:1;
+};
+
+struct __attribute__((packed)) S7 {
+ uint8_t length;
+ uint8_t stuff;
+ uint8_t padding[2];
+ union OneUnion one_union;
+};
+
+union AnotherUnion {
+ long data;
+ struct S7 s;
+} *au;
+
+union OneUnion* get_OneUnion(void)
+{
+ return &au->s.one_union; // no-warning
+}
+
+struct __attribute__((packed)) S8 {
+ uint8_t data1;
+ uint8_t data2;
+ uint16_t wider_data;
+};
+
+#define LE_READ_2(p) \
+ ((uint16_t) \
+ ((((const uint8_t *)(p))[0] ) | \
+ (((const uint8_t *)(p))[1] << 8)))
+
+uint32_t get_wider_data(struct S8 *s)
+{
+ return LE_READ_2(&s->wider_data); // no-warning
+}
+
+struct S9 {
+ uint32_t x;
+ uint8_t y[2];
+ uint16_t z;
+} __attribute__((__packed__));
+
+typedef struct S9 __attribute__((__aligned__(16))) aligned_S9;
+
+void g10() {
+ struct S9 x;
+ struct S9 __attribute__((__aligned__(8))) y;
+ aligned_S9 z;
+
+ uint32_t *p32;
+ p32 = &x.x; // expected-warning {{packed member 'x' of class or structure 'S9'}}
+ p32 = &y.x; // no-warning
+ p32 = &z.x; // no-warning
+}
+
+typedef struct {
+ uint32_t msgh_bits;
+ uint32_t msgh_size;
+ int32_t msgh_voucher_port;
+ int32_t msgh_id;
+} S10Header;
+
+typedef struct {
+ uint32_t t;
+ uint64_t m;
+ uint32_t p;
+ union {
+ struct {
+ uint32_t a;
+ double z;
+ } __attribute__((aligned(8), packed)) a;
+ struct {
+ uint32_t b;
+ double z;
+ uint32_t a;
+ } __attribute__((aligned(8), packed)) b;
+ };
+} __attribute__((aligned(8), packed)) S10Data;
+
+typedef struct {
+ S10Header hdr;
+ uint32_t size;
+ uint8_t count;
+ S10Data data[] __attribute__((aligned(8)));
+} __attribute__((aligned(8), packed)) S10;
+
+void g11(S10Header *hdr);
+void g12(S10 *s) {
+ g11(&s->hdr); // no-warning
+}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -11286,45 +11286,115 @@
}
void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) {
- if (!T->isPointerType())
+ E = E->IgnoreParens();
+ if (!T->isPointerType() && !T->isIntegerType())
return;
if (isa<UnaryOperator>(E) &&
cast<UnaryOperator>(E)->getOpcode() == UO_AddrOf) {
auto *Op = cast<UnaryOperator>(E)->getSubExpr()->IgnoreParens();
if (isa<MemberExpr>(Op)) {
auto MA = std::find(MisalignedMembers.begin(), MisalignedMembers.end(),
MisalignedMember(Op));
if (MA != MisalignedMembers.end() &&
- Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment)
+ (T->isIntegerType() ||
+ (T->isPointerType() &&
+ Context.getTypeAlignInChars(T->getPointeeType()) <= MA->Alignment)))
MisalignedMembers.erase(MA);
}
}
}
void Sema::RefersToMemberWithReducedAlignment(
Expr *E,
- std::function<void(Expr *, RecordDecl *, ValueDecl *, CharUnits)> Action) {
+ std::function<void(Expr *, RecordDecl *, FieldDecl *, CharUnits)> Action) {
const auto *ME = dyn_cast<MemberExpr>(E);
- while (ME && isa<FieldDecl>(ME->getMemberDecl())) {
+ if (!ME)
+ return;
+
+ // For a chain of MemberExpr like "a.b.c.d" this list
+ // will keep FieldDecl's like [d, c, b].
+ SmallVector<FieldDecl *, 4> ReverseMemberChain;
+ const MemberExpr *TopME = nullptr;
+ bool AnyIsPacked = false;
+ do {
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;
+ auto *FD = dyn_cast<FieldDecl>(MD);
+ // We do not care about non-data members.
+ if (!FD)
+ return;
- if (!ByteAligned &&
- (RD->hasAttr<PackedAttr>() || (MD->hasAttr<PackedAttr>()))) {
- CharUnits Alignment = std::min(Context.getTypeAlignInChars(MD->getType()),
- Context.getTypeAlignInChars(BaseType));
- // Notify that this expression designates a member with reduced alignment
- Action(E, RD, MD, Alignment);
- break;
+ AnyIsPacked =
+ AnyIsPacked || (RD->hasAttr<PackedAttr>() || MD->hasAttr<PackedAttr>());
+ ReverseMemberChain.push_back(FD);
+
+ TopME = ME;
+ ME = dyn_cast<MemberExpr>(ME->getBase()->IgnoreParens());
+ } while (ME);
+ assert(TopME && "We did not compute a topmost MemberExpr!");
+
+ // Not the scope of this diagnostic.
+ if (!AnyIsPacked)
+ return;
+
+ // Alignment expected by the whole expression.
+ CharUnits ExpectedAlignment = Context.getTypeAlignInChars(E->getType());
+
+ // No need to do anything else with this case.
+ if (ExpectedAlignment.isOne())
+ return;
+
+ // Synthesize offset of the whole access.
+ CharUnits Offset;
+ for (auto I = ReverseMemberChain.rbegin(); I != ReverseMemberChain.rend();
+ I++) {
+ Offset += Context.toCharUnitsFromBits(Context.getFieldOffset(*I));
+ }
+
+ // Compute the EffectiveAlignment as the alignment of the whole chain.
+ CharUnits EffectiveAlignment = Context.getTypeAlignInChars(
+ ReverseMemberChain.back()->getParent()->getTypeForDecl());
+ // The base expression of the innermost MemberExpr may give
+ // stronger guarantees than the class containing the member.
+ const Expr *TopBase = TopME->getBase();
+ if (const auto *DRE = dyn_cast<DeclRefExpr>(TopBase)) {
+ const ValueDecl *VD = DRE->getDecl();
+ if (!VD->getType()->isReferenceType())
+ EffectiveAlignment =
+ std::max(EffectiveAlignment, Context.getDeclAlign(VD));
+ }
+
+ // Check if the synthesized offset fulfills the alignment.
+ if (Offset % ExpectedAlignment != 0
+ // It may fulfill the offset it but the effective alignment may still be
+ // lower than the expected expression alignment.
+ || EffectiveAlignment < ExpectedAlignment) {
+ // If this happens, we want to determine a sensible culprit of this.
+ // Intuitively, watching the chain of member expressions from right to
+ // left, we start with the required alignment (as required by the field
+ // type) but some packed attribute in that chain has reduced the alignment.
+ // It may happen that another packed structure increases it again. But if
+ // we are here such increase has not been enough. So pointing the first
+ // FieldDecl that either is packed or else its RecordDecl is,
+ // seems reasonable.
+ FieldDecl *FD = nullptr;
+ CharUnits Alignment;
+ for (FieldDecl *FDI : ReverseMemberChain) {
+ if (FDI->hasAttr<PackedAttr>() ||
+ FDI->getParent()->hasAttr<PackedAttr>()) {
+ FD = FDI;
+ Alignment = std::min(
+ Context.getTypeAlignInChars(FD->getType()),
+ Context.getTypeAlignInChars(FD->getParent()->getTypeForDecl()));
+ break;
+ }
}
- ME = dyn_cast<MemberExpr>(ME->getBase());
+ assert(FD && "We did not find a packed FieldDecl!");
+ Action(E, FD->getParent(), FD, Alignment);
}
}
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -9784,7 +9784,7 @@
/// local diagnostics like in reference binding.
void RefersToMemberWithReducedAlignment(
Expr *E,
- std::function<void(Expr *, RecordDecl *, ValueDecl *, CharUnits)> Action);
+ std::function<void(Expr *, RecordDecl *, FieldDecl *, CharUnits)> Action);
};
/// \brief RAII object that enters a new expression evaluation context.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
