[ubsan] Reduce alignment checking of C++ object pointers

This patch teaches ubsan to insert an alignment check for the 'this'
pointer at the start of each method/lambda. This allows clang to emit
significantly fewer alignment checks overall, because if 'this' is
aligned, so are its fields.

This is essentially the same thing r295515 does, but for the alignment
check instead of the null check. One difference is that we keep the
alignment checks on member expressions where the base is a DeclRefExpr.
There's an opportunity to diagnose unaligned accesses in this situation
(as pointed out by Eli, see PR32630).

Testing: check-clang, check-ubsan, and a stage2 ubsan build.

Along with the patch from D30285, this roughly halves the amount of
alignment checks we emit when compiling X86FastISel.cpp. Here are the
numbers from patched/unpatched clangs based on r298160.

  ------------------------------------------
  | Setup          | # of alignment checks |
  ------------------------------------------
  | unpatched, -O0 |                 24326 |
  | patched, -O0   |                 12717 | (-47.7%)
  ------------------------------------------

Differential Revision: https://reviews.llvm.org/D30283

llvm-svn: 300370

4 files changed, 25 insertions, 16 deletions

diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index a0dd20b5f6a..428c8ffd2a1 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -954,10 +954,7 @@ LValue CodeGenFunction::EmitUnsupportedLValue(const Expr *E,
                         E->getType());
 }
 
-bool CodeGenFunction::IsDeclRefOrWrappedCXXThis(const Expr *Obj) {
-  if (isa<DeclRefExpr>(Obj))
-    return true;
-
+bool CodeGenFunction::IsWrappedCXXThis(const Expr *Obj) {
   const Expr *Base = Obj;
   while (!isa<CXXThisExpr>(Base)) {
     // The result of a dynamic_cast can be null.
@@ -988,9 +985,13 @@ LValue CodeGenFunction::EmitCheckedLValue(const Expr *E, TypeCheckKind TCK) {
     LV = EmitLValue(E);
   if (!isa<DeclRefExpr>(E) && !LV.isBitField() && LV.isSimple()) {
     SanitizerSet SkippedChecks;
-    if (const auto *ME = dyn_cast<MemberExpr>(E))
-      if (IsDeclRefOrWrappedCXXThis(ME->getBase()))
+    if (const auto *ME = dyn_cast<MemberExpr>(E)) {
+      bool IsBaseCXXThis = IsWrappedCXXThis(ME->getBase());
+      if (IsBaseCXXThis)
+        SkippedChecks.set(SanitizerKind::Alignment, true);
+      if (IsBaseCXXThis || isa<DeclRefExpr>(ME->getBase()))
         SkippedChecks.set(SanitizerKind::Null, true);
+    }
     EmitTypeCheck(TCK, E->getExprLoc(), LV.getPointer(),
                   E->getType(), LV.getAlignment(), SkippedChecks);
   }
@@ -3429,7 +3430,10 @@ LValue CodeGenFunction::EmitMemberExpr(const MemberExpr *E) {
     Address Addr = EmitPointerWithAlignment(BaseExpr, &AlignSource);
     QualType PtrTy = BaseExpr->getType()->getPointeeType();
     SanitizerSet SkippedChecks;
-    if (IsDeclRefOrWrappedCXXThis(BaseExpr))
+    bool IsBaseCXXThis = IsWrappedCXXThis(BaseExpr);
+    if (IsBaseCXXThis)
+      SkippedChecks.set(SanitizerKind::Alignment, true);
+    if (IsBaseCXXThis || isa<DeclRefExpr>(BaseExpr))
       SkippedChecks.set(SanitizerKind::Null, true);
     EmitTypeCheck(TCK_MemberAccess, E->getExprLoc(), Addr.getPointer(), PtrTy,
                   /*Alignment=*/CharUnits::Zero(), SkippedChecks);
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index c4db670ec4e..d02f074dd60 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -301,9 +301,14 @@ RValue CodeGenFunction::EmitCXXMemberOrOperatorMemberCallExpr(
     CallLoc = CE->getExprLoc();
 
   SanitizerSet SkippedChecks;
-  if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE))
-    if (IsDeclRefOrWrappedCXXThis(CMCE->getImplicitObjectArgument()))
+  if (const auto *CMCE = dyn_cast<CXXMemberCallExpr>(CE)) {
+    auto *IOA = CMCE->getImplicitObjectArgument();
+    bool IsImplicitObjectCXXThis = IsWrappedCXXThis(IOA);
+    if (IsImplicitObjectCXXThis)
+      SkippedChecks.set(SanitizerKind::Alignment, true);
+    if (IsImplicitObjectCXXThis || isa<DeclRefExpr>(IOA))
       SkippedChecks.set(SanitizerKind::Null, true);
+  }
   EmitTypeCheck(
       isa<CXXConstructorDecl>(CalleeDecl) ? CodeGenFunction::TCK_ConstructorCall
                                           : CodeGenFunction::TCK_MemberCall,
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 346679b08f6..6e6eb7d7f13 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -963,13 +963,14 @@ void CodeGenFunction::StartFunction(GlobalDecl GD,
       CXXThisValue = CXXABIThisValue;
     }
 
-    // Null-check the 'this' pointer once per function, if it's available.
+    // Check the 'this' pointer once per function, if it's available.
     if (CXXThisValue) {
       SanitizerSet SkippedChecks;
-      SkippedChecks.set(SanitizerKind::Alignment, true);
       SkippedChecks.set(SanitizerKind::ObjectSize, true);
-      EmitTypeCheck(TCK_Load, Loc, CXXThisValue, MD->getThisType(getContext()),
-                    /*Alignment=*/CharUnits::Zero(), SkippedChecks);
+      QualType ThisTy = MD->getThisType(getContext());
+      EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy,
+                    getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
+                    SkippedChecks);
     }
   }
 
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 3321bc86c00..fa72019eb08 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -2077,9 +2077,8 @@ public:
   llvm::BlockAddress *GetAddrOfLabel(const LabelDecl *L);
   llvm::BasicBlock *GetIndirectGotoBlock();
 
-  /// Check if \p E is a reference, or a C++ "this" pointer wrapped in value-
-  /// preserving casts.
-  static bool IsDeclRefOrWrappedCXXThis(const Expr *E);
+  /// Check if \p E is a C++ "this" pointer wrapped in value-preserving casts.
+  static bool IsWrappedCXXThis(const Expr *E);
 
   /// EmitNullInitialization - Generate code to set a value of the given type to
   /// null, If the type contains data member pointers, they will be initialized