Index: cfe/trunk/include/clang/Sema/Sema.h =================================================================== --- cfe/trunk/include/clang/Sema/Sema.h +++ cfe/trunk/include/clang/Sema/Sema.h @@ -9998,7 +9998,7 @@ /// local diagnostics like in reference binding. void RefersToMemberWithReducedAlignment( Expr *E, - std::function Action); + std::function Action); }; /// \brief RAII object that enters a new expression evaluation context. Index: cfe/trunk/lib/Sema/SemaChecking.cpp =================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp +++ cfe/trunk/lib/Sema/SemaChecking.cpp @@ -11696,7 +11696,8 @@ } void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) { - if (!T->isPointerType()) + E = E->IgnoreParens(); + if (!T->isPointerType() && !T->isIntegerType()) return; if (isa(E) && cast(E)->getOpcode() == UO_AddrOf) { @@ -11705,7 +11706,9 @@ 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); } } @@ -11713,28 +11716,103 @@ void Sema::RefersToMemberWithReducedAlignment( Expr *E, - std::function Action) { + std::function Action) { const auto *ME = dyn_cast(E); - while (ME && isa(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 ReverseMemberChain; + const MemberExpr *TopME = nullptr; + bool AnyIsPacked = false; + do { 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; + auto *FD = dyn_cast(MD); + // We do not care about non-data members. + if (!FD || FD->isInvalidDecl()) + return; - if (!ByteAligned && - (RD->hasAttr() || (MD->hasAttr()))) { - 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() || MD->hasAttr()); + ReverseMemberChain.push_back(FD); + + TopME = ME; + ME = dyn_cast(ME->getBase()->IgnoreParens()); + } while (ME); + assert(TopME && "We did not compute a topmost MemberExpr!"); + + // Not the scope of this diagnostic. + if (!AnyIsPacked) + return; + + const Expr *TopBase = TopME->getBase()->IgnoreParenImpCasts(); + const auto *DRE = dyn_cast(TopBase); + // TODO: The innermost base of the member expression may be too complicated. + // For now, just disregard these cases. This is left for future + // improvement. + if (!DRE && !isa(TopBase)) + 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 CompleteObjectAlignment as the alignment of the whole chain. + CharUnits CompleteObjectAlignment = Context.getTypeAlignInChars( + ReverseMemberChain.back()->getParent()->getTypeForDecl()); + + // The base expression of the innermost MemberExpr may give + // stronger guarantees than the class containing the member. + if (DRE && !TopME->isArrow()) { + const ValueDecl *VD = DRE->getDecl(); + if (!VD->getType()->isReferenceType()) + CompleteObjectAlignment = + std::max(CompleteObjectAlignment, 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. + CompleteObjectAlignment < 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() || + FDI->getParent()->hasAttr()) { + FD = FDI; + Alignment = std::min( + Context.getTypeAlignInChars(FD->getType()), + Context.getTypeAlignInChars(FD->getParent()->getTypeForDecl())); + break; + } } - ME = dyn_cast(ME->getBase()); + assert(FD && "We did not find a packed FieldDecl!"); + Action(E, FD->getParent(), FD, Alignment); } } Index: cfe/trunk/test/Sema/address-packed.c =================================================================== --- cfe/trunk/test/Sema/address-packed.c +++ cfe/trunk/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,16 +42,18 @@ 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; @@ -58,8 +61,9 @@ 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(); @@ -67,8 +71,9 @@ 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(); @@ -76,8 +81,9 @@ 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 } } @@ -196,3 +202,130 @@ int *anonymousInnerUnion(struct S6 *s) { return &s->x; // expected-warning {{packed member 'x' of class or structure 'S6::(anonymous)'}} } + +struct S6a { + 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 'S6a'}} +} + +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 +} + +struct S11 { + uint32_t x; +} __attribute__((__packed__)); + +void g13(void) { + struct S11 __attribute__((__aligned__(4))) a[4]; + uint32_t *p32; + p32 = &a[0].x; // no-warning +}