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

Reply via email to