Skip to content

Commit

Permalink
Remove some false positives when taking the address of packed members
Browse files Browse the repository at this point in the history 
Differential Revision: https://reviews.llvm.org/D23657

llvm-svn: 286798
  • Loading branch information
Roger Ferrer Ibanez committed Nov 14, 2016
1 parent c79c9d8 commit e3d8026
Show file tree
Hide file tree
Showing 3 changed files with 236 additions and 25 deletions.
2 changes: 1 addition & 1 deletion clang/include/clang/Sema/Sema.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9998,7 +9998,7 @@ class Sema {
/// 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.
Expand Down
108 changes: 93 additions & 15 deletions clang/lib/Sema/SemaChecking.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11696,7 +11696,8 @@ void Sema::DiagnoseMisalignedMembers() {
}

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) {
Expand All @@ -11705,36 +11706,113 @@ void Sema::DiscardMisalignedMemberAddress(const Type *T, Expr *E) {
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 || FD->isInvalidDecl())
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;

const Expr *TopBase = TopME->getBase()->IgnoreParenImpCasts();
const auto *DRE = dyn_cast<DeclRefExpr>(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<CXXThisExpr>(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<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);
}
}

Expand Down
151 changes: 142 additions & 9 deletions clang/test/Sema/address-packed.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ typedef struct Arguable ArguableT;
struct Arguable *get_arguable();

void to_void(void *);
void to_intptr(intptr_t);

void g0(void) {
{
Expand All @@ -41,43 +42,48 @@ void g0(void) {

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
}
}

Expand Down Expand Up @@ -196,3 +202,130 @@ struct S6 {
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
}

0 comments on commit e3d8026

Please sign in to comment.