ahatanak updated this revision to Diff 263238. ahatanak marked an inline comment as done. ahatanak added a comment.
Define two helper functions in Sema that compute the alignment of a VarDecl and a constant offset from it and use them instead of the classes for evaluating lvalue and pointer expressions in ExprConstant.cpp. Using the classes in ExprConstant.cpp to compute an expression alignment as I did in the previous patch is probably not a good idea since they are for evaluating constant expressions. They don't return the lvalue base variables and offsets in some cases ( for example, `(A *)&m_ref` in the test case) and using them for this purpose can make it harder to make changes to the way constant expressions are evaluated. The current patch is far from perfect as it misses many cases that could be detected by the classes in ExprConstant.cpp, but is at least an improvement over what we have now. I was also thinking about fixing the alignment computation of captured variables, but I think I should do that separately in a follow-up as it might not be trivial. It will probably require looking up the captured variables in all the enclosing `CapturingScopeInfo`s. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78767/new/ https://reviews.llvm.org/D78767 Files: clang/lib/Sema/SemaChecking.cpp clang/test/SemaCXX/warn-cast-align.cpp
Index: clang/test/SemaCXX/warn-cast-align.cpp =================================================================== --- clang/test/SemaCXX/warn-cast-align.cpp +++ clang/test/SemaCXX/warn-cast-align.cpp @@ -43,3 +43,53 @@ typedef int *IntPtr; c = IntPtr(P); } + +struct __attribute__((aligned(16))) A { + char m0[16]; + char m1[16]; +}; + +struct B { + char m0[16]; +}; + +struct C { + A &m0; + B &m1; + A m2; +}; + +struct D { + char m0[8]; +}; + +void test2(int n, A *a2) { + __attribute__((aligned(16))) char m[sizeof(A) * 2]; + char(&m_ref)[sizeof(A) * 2] = m; + __attribute__((aligned(16))) char vararray[10][n]; + A t0; + B t1; + C t2 = {.m0 = t0, .m1 = t1}; + __attribute__((aligned(16))) char t3[5][5][5]; + + A *a; + a = (A *)&m; + a = (A *)(m + sizeof(A)); + a = (A *)(sizeof(A) + m); + a = (A *)((sizeof(A) * 2 + m) - sizeof(A)); + a = (A *)((sizeof(A) * 2 + m) - 1); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)(m + 1); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)(1 + m); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)(m + n); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)&m[sizeof(A)]; + a = (A *)&m[n]; // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)&m_ref; + a = (A *)(&vararray[4][0]); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)(a2->m0 + sizeof(A)); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)(&t2.m0); + a = (A *)(&t2.m1); // expected-warning {{cast from 'B *' to 'A *'}} + a = (A *)(&t2.m2); + a = (A *)(t2.m2.m1); + a = (A *)(&t3[3][3][0]); // expected-warning {{cast from 'char *' to 'A *'}} + a = (A *)(&t3[2][2][4]); +} Index: clang/lib/Sema/SemaChecking.cpp =================================================================== --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -30,6 +30,7 @@ #include "clang/AST/NSAPI.h" #include "clang/AST/NonTrivialTypeVisitor.h" #include "clang/AST/OperationKinds.h" +#include "clang/AST/RecordLayout.h" #include "clang/AST/Stmt.h" #include "clang/AST/TemplateBase.h" #include "clang/AST/Type.h" @@ -13055,17 +13056,126 @@ return HasInvalidParm; } -/// A helper function to get the alignment of a Decl referred to by DeclRefExpr -/// or MemberExpr. -static CharUnits getDeclAlign(Expr *E, CharUnits TypeAlign, - ASTContext &Context) { - if (const auto *DRE = dyn_cast<DeclRefExpr>(E)) - return Context.getDeclAlign(DRE->getDecl()); +std::tuple<bool, CharUnits, CharUnits> static getBaseAlignmentAndOffsetFromPtr( + const Expr *E, ASTContext &Ctx); - if (const auto *ME = dyn_cast<MemberExpr>(E)) - return Context.getDeclAlign(ME->getMemberDecl()); +/// This helper function takes an lvalue expression and returns the alignment of +/// a VarDecl and a constant offset from the VarDecl. The first element of the +/// tuple it returns is true if a VarDecl is found and the offset is constant. +std::tuple<bool, CharUnits, CharUnits> +static getBaseAlignmentAndOffsetFromLValue(const Expr *E, ASTContext &Ctx) { + E = E->IgnoreParens(); + switch (E->getStmtClass()) { + default: + break; + case Stmt::ArraySubscriptExprClass: { + if (!E->getType()->isConstantSizeType()) + break; + auto *ASE = cast<ArraySubscriptExpr>(E); + auto P = getBaseAlignmentAndOffsetFromPtr(ASE->getBase(), Ctx); + if (std::get<0>(P)) { + llvm::APSInt IdxRes; + if (ASE->getIdx()->isIntegerConstantExpr(IdxRes, Ctx)) { + CharUnits EltSize = Ctx.getTypeSizeInChars(E->getType()); + return std::make_tuple(true, std::get<1>(P), + std::get<2>(P) + EltSize * IdxRes.getExtValue()); + } + } + break; + } + case Stmt::DeclRefExprClass: { + if (auto *VD = dyn_cast<VarDecl>(cast<DeclRefExpr>(E)->getDecl())) { + // FIXME: If VD is captured by copy or is an escaping __block variable, + // use the alignment of VD's type. + if (VD->getType()->isReferenceType()) + return getBaseAlignmentAndOffsetFromLValue(VD->getInit(), Ctx); + return std::make_tuple(true, Ctx.getDeclAlign(VD), CharUnits::Zero()); + } + break; + } + case Stmt::MemberExprClass: { + auto *ME = cast<MemberExpr>(E); + if (ME->isArrow()) + break; + auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl()); + if (!FD || FD->getType()->isReferenceType()) + break; + auto P = getBaseAlignmentAndOffsetFromLValue(ME->getBase(), Ctx); + if (!std::get<0>(P)) + break; + const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(FD->getParent()); + uint64_t Offset = Layout.getFieldOffset(FD->getFieldIndex()); + return std::make_tuple(true, std::get<1>(P), + std::get<2>(P) + CharUnits::fromQuantity(Offset)); + } + } + return std::make_tuple(false, CharUnits::Zero(), CharUnits::Zero()); +} - return TypeAlign; +/// This helper function takes a pointer expression and returns the alignment of +/// a VarDecl and a constant offset from the VarDecl. The first element of the +/// tuple it returns is true if a VarDecl is found and the offset is constant. +std::tuple<bool, CharUnits, CharUnits> static getBaseAlignmentAndOffsetFromPtr( + const Expr *E, ASTContext &Ctx) { + E = E->IgnoreParens(); + switch (E->getStmtClass()) { + default: + break; + case Stmt::UnaryOperatorClass: { + auto *UO = cast<UnaryOperator>(E); + if (UO->getOpcode() == UO_AddrOf) + return getBaseAlignmentAndOffsetFromLValue(UO->getSubExpr(), Ctx); + break; + } + case Stmt::ImplicitCastExprClass: { + auto *ICE = cast<ImplicitCastExpr>(E); + if (ICE->getCastKind() == CK_ArrayToPointerDecay) + return getBaseAlignmentAndOffsetFromLValue(ICE->getSubExpr(), Ctx); + break; + } + case Stmt::BinaryOperatorClass: { + auto HandleBinOp = [&](const Expr *LHS, const Expr *RHS, + BinaryOperatorKind Kind, + std::tuple<bool, CharUnits, CharUnits> &P) { + auto LHSRes = getBaseAlignmentAndOffsetFromPtr(LHS, Ctx); + if (!std::get<0>(LHSRes)) + return true; + llvm::APSInt RHSRes; + if (RHS->isIntegerConstantExpr(RHSRes, Ctx)) { + CharUnits Offset = std::get<2>(LHSRes); + if (Kind == BO_Add) + Offset += CharUnits::fromQuantity(RHSRes.getExtValue()); + else + Offset -= CharUnits::fromQuantity(RHSRes.getExtValue()); + P = std::make_tuple(true, std::get<1>(LHSRes), Offset); + } + return false; + }; + auto *BO = cast<BinaryOperator>(E); + auto Opcode = BO->getOpcode(); + if (Opcode == BO_Add || Opcode == BO_Sub) { + std::tuple<bool, CharUnits, CharUnits> P; + if (HandleBinOp(BO->getLHS(), BO->getRHS(), Opcode, P) && + Opcode == BO_Add) + HandleBinOp(BO->getRHS(), BO->getLHS(), Opcode, P); + return P; + } + break; + } + } + return std::make_tuple(false, CharUnits::Zero(), CharUnits::Zero()); +} + +static CharUnits getPresumedAlignmentOfPointer(const Expr *E, Sema &S) { + // See if we can compute the alignment of a VarDecl and an offset from it. + std::tuple<bool, CharUnits, CharUnits> P = + getBaseAlignmentAndOffsetFromPtr(E, S.Context); + + if (std::get<0>(P)) + return std::get<1>(P).alignmentAtOffset(std::get<2>(P)); + + // If that failed, return the type's alignment. + return S.Context.getTypeAlignInChars(E->getType()->getPointeeType()); } /// CheckCastAlign - Implements -Wcast-align, which warns when a @@ -13101,15 +13211,7 @@ // includes 'void'. if (SrcPointee->isIncompleteType()) return; - CharUnits SrcAlign = Context.getTypeAlignInChars(SrcPointee); - - if (auto *CE = dyn_cast<CastExpr>(Op)) { - if (CE->getCastKind() == CK_ArrayToPointerDecay) - SrcAlign = getDeclAlign(CE->getSubExpr(), SrcAlign, Context); - } else if (auto *UO = dyn_cast<UnaryOperator>(Op)) { - if (UO->getOpcode() == UO_AddrOf) - SrcAlign = getDeclAlign(UO->getSubExpr(), SrcAlign, Context); - } + CharUnits SrcAlign = getPresumedAlignmentOfPointer(Op, *this); if (SrcAlign >= DestAlign) return;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits