Skip to content

Commit 854f5f3

Browse files
ahatanakaahatanaka
committed
[Sema] Teach -Wcast-align to compute an accurate alignment using the
alignment information on VarDecls in more cases This commit improves upon https://reviews.llvm.org/D21099. The code that computes the source alignment now understands array subscript expressions, binary operators, derived-to-base casts, and several more expressions. rdar://problem/59242343 Differential Revision: https://reviews.llvm.org/D78767
1 parent f58e78f commit 854f5f3

File tree

2 files changed

+333
-19
lines changed

2 files changed

+333
-19
lines changed

‎clang/lib/Sema/SemaChecking.cpp

Lines changed: 220 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include "clang/AST/NSAPI.h"
3131
#include "clang/AST/NonTrivialTypeVisitor.h"
3232
#include "clang/AST/OperationKinds.h"
33+
#include "clang/AST/RecordLayout.h"
3334
#include "clang/AST/Stmt.h"
3435
#include "clang/AST/TemplateBase.h"
3536
#include "clang/AST/Type.h"
@@ -13105,17 +13106,226 @@ bool Sema::CheckParmsForFunctionDef(ArrayRef<ParmVarDecl *> Parameters,
1310513106
return HasInvalidParm;
1310613107
}
1310713108

13108-
/// A helper function to get the alignment of a Decl referred to by DeclRefExpr
13109-
/// or MemberExpr.
13110-
static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign,
13111-
ASTContext &Context) {
13112-
if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
13113-
return Context.getDeclAlign(DRE->getDecl());
13109+
Optional<std::pair<CharUnits, CharUnits>>
13110+
static getBaseAlignmentAndOffsetFromPtr(const Expr *E, ASTContext &Ctx);
13111+
13112+
/// Compute the alignment and offset of the base class object given the
13113+
/// derived-to-base cast expression and the alignment and offset of the derived
13114+
/// class object.
13115+
static std::pair<CharUnits, CharUnits>
13116+
getDerivedToBaseAlignmentAndOffset(const CastExpr *CE, QualType DerivedType,
13117+
CharUnits BaseAlignment, CharUnits Offset,
13118+
ASTContext &Ctx) {
13119+
for (auto PathI = CE->path_begin(), PathE = CE->path_end(); PathI != PathE;
13120+
++PathI) {
13121+
const CXXBaseSpecifier *Base = *PathI;
13122+
const CXXRecordDecl *BaseDecl = Base->getType()->getAsCXXRecordDecl();
13123+
if (Base->isVirtual()) {
13124+
// The complete object may have a lower alignment than the non-virtual
13125+
// alignment of the base, in which case the base may be misaligned. Choose
13126+
// the smaller of the non-virtual alignment and BaseAlignment, which is a
13127+
// conservative lower bound of the complete object alignment.
13128+
CharUnits NonVirtualAlignment =
13129+
Ctx.getASTRecordLayout(BaseDecl).getNonVirtualAlignment();
13130+
BaseAlignment = std::min(BaseAlignment, NonVirtualAlignment);
13131+
Offset = CharUnits::Zero();
13132+
} else {
13133+
const ASTRecordLayout &RL =
13134+
Ctx.getASTRecordLayout(DerivedType->getAsCXXRecordDecl());
13135+
Offset += RL.getBaseClassOffset(BaseDecl);
13136+
}
13137+
DerivedType = Base->getType();
13138+
}
13139+
13140+
return std::make_pair(BaseAlignment, Offset);
13141+
}
13142+
13143+
/// Compute the alignment and offset of a binary additive operator.
13144+
static Optional<std::pair<CharUnits, CharUnits>>
13145+
getAlignmentAndOffsetFromBinAddOrSub(const Expr *PtrE, const Expr *IntE,
13146+
bool IsSub, ASTContext &Ctx) {
13147+
QualType PointeeType = PtrE->getType()->getPointeeType();
13148+
13149+
if (!PointeeType->isConstantSizeType())
13150+
return llvm::None;
13151+
13152+
auto P = getBaseAlignmentAndOffsetFromPtr(PtrE, Ctx);
13153+
13154+
if (!P)
13155+
return llvm::None;
1311413156

13115-
if (const auto *ME = dyn_cast<MemberExpr>(E))
13116-
return Context.getDeclAlign(ME->getMemberDecl());
13157+
llvm::APSInt IdxRes;
13158+
CharUnits EltSize = Ctx.getTypeSizeInChars(PointeeType);
13159+
if (IntE->isIntegerConstantExpr(IdxRes, Ctx)) {
13160+
CharUnits Offset = EltSize * IdxRes.getExtValue();
13161+
if (IsSub)
13162+
Offset = -Offset;
13163+
return std::make_pair(P->first, P->second + Offset);
13164+
}
1311713165

13118-
return TypeAlign;
13166+
// If the integer expression isn't a constant expression, compute the lower
13167+
// bound of the alignment using the alignment and offset of the pointer
13168+
// expression and the element size.
13169+
return std::make_pair(
13170+
P->first.alignmentAtOffset(P->second).alignmentAtOffset(EltSize),
13171+
CharUnits::Zero());
13172+
}
13173+
13174+
/// This helper function takes an lvalue expression and returns the alignment of
13175+
/// a VarDecl and a constant offset from the VarDecl.
13176+
Optional<std::pair<CharUnits, CharUnits>>
13177+
static getBaseAlignmentAndOffsetFromLValue(const Expr *E, ASTContext &Ctx) {
13178+
E = E->IgnoreParens();
13179+
switch (E->getStmtClass()) {
13180+
default:
13181+
break;
13182+
case Stmt::CStyleCastExprClass:
13183+
case Stmt::CXXStaticCastExprClass:
13184+
case Stmt::ImplicitCastExprClass: {
13185+
auto *CE = cast<CastExpr>(E);
13186+
const Expr *From = CE->getSubExpr();
13187+
switch (CE->getCastKind()) {
13188+
default:
13189+
break;
13190+
case CK_NoOp:
13191+
return getBaseAlignmentAndOffsetFromLValue(From, Ctx);
13192+
case CK_UncheckedDerivedToBase:
13193+
case CK_DerivedToBase: {
13194+
auto P = getBaseAlignmentAndOffsetFromLValue(From, Ctx);
13195+
if (!P)
13196+
break;
13197+
return getDerivedToBaseAlignmentAndOffset(CE, From->getType(), P->first,
13198+
P->second, Ctx);
13199+
}
13200+
}
13201+
break;
13202+
}
13203+
case Stmt::ArraySubscriptExprClass: {
13204+
auto *ASE = cast<ArraySubscriptExpr>(E);
13205+
return getAlignmentAndOffsetFromBinAddOrSub(ASE->getBase(), ASE->getIdx(),
13206+
false, Ctx);
13207+
}
13208+
case Stmt::DeclRefExprClass: {
13209+
if (auto *VD = dyn_cast<VarDecl>(cast<DeclRefExpr>(E)->getDecl())) {
13210+
// FIXME: If VD is captured by copy or is an escaping __block variable,
13211+
// use the alignment of VD's type.
13212+
if (!VD->getType()->isReferenceType())
13213+
return std::make_pair(Ctx.getDeclAlign(VD), CharUnits::Zero());
13214+
if (VD->hasInit())
13215+
return getBaseAlignmentAndOffsetFromLValue(VD->getInit(), Ctx);
13216+
}
13217+
break;
13218+
}
13219+
case Stmt::MemberExprClass: {
13220+
auto *ME = cast<MemberExpr>(E);
13221+
if (ME->isArrow())
13222+
break;
13223+
auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl());
13224+
if (!FD || FD->getType()->isReferenceType())
13225+
break;
13226+
auto P = getBaseAlignmentAndOffsetFromLValue(ME->getBase(), Ctx);
13227+
if (!P)
13228+
break;
13229+
const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(FD->getParent());
13230+
uint64_t Offset = Layout.getFieldOffset(FD->getFieldIndex());
13231+
return std::make_pair(P->first,
13232+
P->second + CharUnits::fromQuantity(Offset));
13233+
}
13234+
case Stmt::UnaryOperatorClass: {
13235+
auto *UO = cast<UnaryOperator>(E);
13236+
switch (UO->getOpcode()) {
13237+
default:
13238+
break;
13239+
case UO_Deref:
13240+
return getBaseAlignmentAndOffsetFromPtr(UO->getSubExpr(), Ctx);
13241+
}
13242+
break;
13243+
}
13244+
case Stmt::BinaryOperatorClass: {
13245+
auto *BO = cast<BinaryOperator>(E);
13246+
auto Opcode = BO->getOpcode();
13247+
switch (Opcode) {
13248+
default:
13249+
break;
13250+
case BO_Comma:
13251+
return getBaseAlignmentAndOffsetFromLValue(BO->getRHS(), Ctx);
13252+
}
13253+
break;
13254+
}
13255+
}
13256+
return llvm::None;
13257+
}
13258+
13259+
/// This helper function takes a pointer expression and returns the alignment of
13260+
/// a VarDecl and a constant offset from the VarDecl.
13261+
Optional<std::pair<CharUnits, CharUnits>>
13262+
static getBaseAlignmentAndOffsetFromPtr(const Expr *E, ASTContext &Ctx) {
13263+
E = E->IgnoreParens();
13264+
switch (E->getStmtClass()) {
13265+
default:
13266+
break;
13267+
case Stmt::CStyleCastExprClass:
13268+
case Stmt::CXXStaticCastExprClass:
13269+
case Stmt::ImplicitCastExprClass: {
13270+
auto *CE = cast<CastExpr>(E);
13271+
const Expr *From = CE->getSubExpr();
13272+
switch (CE->getCastKind()) {
13273+
default:
13274+
break;
13275+
case CK_NoOp:
13276+
return getBaseAlignmentAndOffsetFromPtr(From, Ctx);
13277+
case CK_ArrayToPointerDecay:
13278+
return getBaseAlignmentAndOffsetFromLValue(From, Ctx);
13279+
case CK_UncheckedDerivedToBase:
13280+
case CK_DerivedToBase: {
13281+
auto P = getBaseAlignmentAndOffsetFromPtr(From, Ctx);
13282+
if (!P)
13283+
break;
13284+
return getDerivedToBaseAlignmentAndOffset(
13285+
CE, From->getType()->getPointeeType(), P->first, P->second, Ctx);
13286+
}
13287+
}
13288+
break;
13289+
}
13290+
case Stmt::UnaryOperatorClass: {
13291+
auto *UO = cast<UnaryOperator>(E);
13292+
if (UO->getOpcode() == UO_AddrOf)
13293+
return getBaseAlignmentAndOffsetFromLValue(UO->getSubExpr(), Ctx);
13294+
break;
13295+
}
13296+
case Stmt::BinaryOperatorClass: {
13297+
auto *BO = cast<BinaryOperator>(E);
13298+
auto Opcode = BO->getOpcode();
13299+
switch (Opcode) {
13300+
default:
13301+
break;
13302+
case BO_Add:
13303+
case BO_Sub: {
13304+
const Expr *LHS = BO->getLHS(), *RHS = BO->getRHS();
13305+
if (Opcode == BO_Add && !RHS->getType()->isIntegralOrEnumerationType())
13306+
std::swap(LHS, RHS);
13307+
return getAlignmentAndOffsetFromBinAddOrSub(LHS, RHS, Opcode == BO_Sub,
13308+
Ctx);
13309+
}
13310+
case BO_Comma:
13311+
return getBaseAlignmentAndOffsetFromPtr(BO->getRHS(), Ctx);
13312+
}
13313+
break;
13314+
}
13315+
}
13316+
return llvm::None;
13317+
}
13318+
13319+
static CharUnits getPresumedAlignmentOfPointer(const Expr *E, Sema &S) {
13320+
// See if we can compute the alignment of a VarDecl and an offset from it.
13321+
Optional<std::pair<CharUnits, CharUnits>> P =
13322+
getBaseAlignmentAndOffsetFromPtr(E, S.Context);
13323+
13324+
if (P)
13325+
return P->first.alignmentAtOffset(P->second);
13326+
13327+
// If that failed, return the type's alignment.
13328+
return S.Context.getTypeAlignInChars(E->getType()->getPointeeType());
1311913329
}
1312013330

1312113331
/// CheckCastAlign - Implements -Wcast-align, which warns when a
@@ -13151,15 +13361,7 @@ void Sema::CheckCastAlign(Expr *Op, QualType T, SourceRange TRange) {
1315113361
// includes 'void'.
1315213362
if (SrcPointee->isIncompleteType()) return;
1315313363

13154-
CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointee);
13155-
13156-
if (auto *CE = dyn_cast<CastExpr>(Op)) {
13157-
if (CE->getCastKind() == CK_ArrayToPointerDecay)
13158-
SrcAlign = getDeclAlign(CE->getSubExpr(), SrcAlign, Context);
13159-
} else if (auto *UO = dyn_cast<UnaryOperator>(Op)) {
13160-
if (UO->getOpcode() == UO_AddrOf)
13161-
SrcAlign = getDeclAlign(UO->getSubExpr(), SrcAlign, Context);
13162-
}
13364+
CharUnits SrcAlign = getPresumedAlignmentOfPointer(Op, *this);
1316313365

1316413366
if (SrcAlign >= DestAlign) return;
1316513367

‎clang/test/SemaCXX/warn-cast-align.cpp

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wcast-align -verify %s
1+
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fsyntax-only -Wno-unused-value -Wcast-align -verify %s
22

33
// Simple casts.
44
void test0(char *P) {
@@ -43,3 +43,115 @@ void test1(void *P) {
4343
typedef int *IntPtr;
4444
c = IntPtr(P);
4545
}
46+
47+
struct __attribute__((aligned(16))) A {
48+
char m0[16];
49+
char m1[16];
50+
};
51+
52+
struct B0 {
53+
char m0[16];
54+
};
55+
56+
struct B1 {
57+
char m0[16];
58+
};
59+
60+
struct C {
61+
A &m0;
62+
B0 &m1;
63+
A m2;
64+
};
65+
66+
struct __attribute__((aligned(16))) D0 : B0, B1 {
67+
};
68+
69+
struct __attribute__((aligned(16))) D1 : virtual B0 {
70+
};
71+
72+
struct B2 {
73+
char m0[8];
74+
};
75+
76+
struct B3 {
77+
char m0[8];
78+
};
79+
80+
struct B4 {
81+
char m0[8];
82+
};
83+
84+
struct D2 : B2, B3 {
85+
};
86+
87+
struct __attribute__((aligned(16))) D3 : B4, D2 {
88+
};
89+
90+
struct __attribute__((aligned(16))) D4 : virtual D2 {
91+
};
92+
93+
struct D5 : virtual D0 {
94+
char m0[16];
95+
};
96+
97+
struct D6 : virtual D5 {
98+
};
99+
100+
struct D7 : virtual D3 {
101+
};
102+
103+
void test2(int n, A *a2) {
104+
__attribute__((aligned(16))) char m[sizeof(A) * 2];
105+
char(&m_ref)[sizeof(A) * 2] = m;
106+
extern char(&m_ref_noinit)[sizeof(A) * 2];
107+
__attribute__((aligned(16))) char vararray[10][n];
108+
A t0;
109+
B0 t1;
110+
C t2 = {.m0 = t0, .m1 = t1};
111+
__attribute__((aligned(16))) char t3[5][5][5];
112+
__attribute__((aligned(16))) char t4[4][16];
113+
D0 t5;
114+
D1 t6;
115+
D3 t7;
116+
D4 t8;
117+
D6 t9;
118+
__attribute__((aligned(1))) D7 t10;
119+
120+
A *a;
121+
a = (A *)&m;
122+
a = (A *)(m + sizeof(A));
123+
a = (A *)(sizeof(A) + m);
124+
a = (A *)((sizeof(A) * 2 + m) - sizeof(A));
125+
a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}}
126+
a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}}
127+
a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}}
128+
a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}}
129+
a = (A *)&*&m[sizeof(A)];
130+
a = (A *)(0, 0, &m[sizeof(A)]);
131+
a = (A *)&(0, 0, *&m[sizeof(A)]);
132+
a = (A *)&m[n]; // expected-warning {{cast from 'char *' to 'A *'}}
133+
a = (A *)&m_ref;
134+
a = (A *)&m_ref_noinit; // expected-warning {{cast from 'char (*)[64]' to 'A *'}}
135+
a = (A *)(&vararray[4][0]); // expected-warning {{cast from 'char *' to 'A *'}}
136+
a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}}
137+
a = (A *)(&t2.m0);
138+
a = (A *)(&t2.m1); // expected-warning {{cast from 'B0 *' to 'A *'}}
139+
a = (A *)(&t2.m2);
140+
a = (A *)(t2.m2.m1);
141+
a = (A *)(&t3[3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}}
142+
a = (A *)(&t3[2][2][4]);
143+
a = (A *)(&t3[0][n][0]); // expected-warning {{cast from 'char *' to 'A *'}}
144+
a = (A *)&t4[n][0];
145+
a = (A *)&t4[n][1]; // expected-warning {{cast from 'char *' to 'A *'}}
146+
a = (A *)(t4 + 1);
147+
a = (A *)(t4 + n);
148+
a = (A *)(static_cast<B1 *>(&t5));
149+
a = (A *)(&(static_cast<B1 &>(t5)));
150+
a = (A *)(static_cast<B0 *>(&t6)); // expected-warning {{cast from 'B0 *' to 'A *'}}
151+
a = (A *)(static_cast<B2 *>(&t7)); // expected-warning {{cast from 'B2 *' to 'A *'}}
152+
a = (A *)(static_cast<B3 *>(&t7));
153+
a = (A *)(static_cast<B2 *>(&t8)); // expected-warning {{cast from 'B2 *' to 'A *'}}
154+
a = (A *)(static_cast<B3 *>(&t8)); // expected-warning {{cast from 'B3 *' to 'A *'}}
155+
a = (A *)(static_cast<D5 *>(&t9)); // expected-warning {{cast from 'D5 *' to 'A *'}}
156+
a = (A *)(static_cast<D3 *>(&t10)); // expected-warning {{cast from 'D3 *' to 'A *'}}
157+
}

0 commit comments

Comments
 (0)