Fix PR35902: incorrect alignment used for ubsan check.

[jyknight]

Summary

UBSan was using the complete-object align rather than nv alignment
when checking the "this" pointer of a method.

Furthermore, CGF.CXXABIThisAlignment was also being set incorrectly,
due to an incorrectly negated test. The latter doesn't appear to have
had any impact, due to it not really being used anywhere.

[rsmith]

clang/lib/CodeGen/CGCXXABI.cpp

137–138

If you're so inclined, this would be better as MD->getParent()->isEffectivelyFinal().

clang/lib/CodeGen/CGCXXABI.cpp

auto *ThisDecl = ImplicitParamDecl::Create(
       &CGM.getContext().Idents.get("this"), MD->getThisType(),
       ImplicitParamDecl::CXXThis);
   params.push_back(ThisDecl);
   CGF.CXXABIThisDecl = ThisDecl;
 
   // Compute the presumed alignment of 'this', which basically comes
   // down to whether we know it's a complete object or not.
   auto &Layout = CGF.getContext().getASTRecordLayout(MD->getParent());
   if (MD->getParent()->getNumVBases() == 0 || // avoid vcall in common case
-      MD->getParent()->hasAttr<FinalAttr>() ||
-      !isThisCompleteObject(CGF.CurGD)) {
+      MD->getParent()->isEffectivelyFinal() ||
+      isThisCompleteObject(CGF.CurGD)) {
     CGF.CXXABIThisAlignment = Layout.getAlignment();
   } else {
     CGF.CXXABIThisAlignment = Layout.getNonVirtualAlignment();
   }
 }
 
 llvm::Value *CGCXXABI::loadIncomingCXXThis(CodeGenFunction &CGF) {
   return CGF.Builder.CreateLoad(CGF.GetAddrOfLocalVar(getThisDecl(CGF)),

clang/lib/CodeGen/CodeGenFunction.cpp

if (CXXABIThisValue) {
 
       // If this is the call operator of a lambda with no capture-default, it
       // may have a static invoker function, which may call this operator with
       // a null 'this' pointer.
       if (isLambdaCallOperator(MD) &&
           MD->getParent()->getLambdaCaptureDefault() == LCD_None)
         SkippedChecks.set(SanitizerKind::Null, true);
 
-      EmitTypeCheck(isa<CXXConstructorDecl>(MD) ? TCK_ConstructorCall
-                                                : TCK_MemberCall,
-                    Loc, CXXABIThisValue, ThisTy,
-                    getContext().getTypeAlignInChars(ThisTy->getPointeeType()),
-                    SkippedChecks);
+      EmitTypeCheck(
+          isa<CXXConstructorDecl>(MD) ? TCK_ConstructorCall : TCK_MemberCall,
+          Loc, CXXABIThisValue, ThisTy, CXXABIThisAlignment, SkippedChecks);
     }
   }
 
   // If any of the arguments have a variably modified type, make sure to
   // emit the type size.
   for (FunctionArgList::const_iterator i = Args.begin(), e = Args.end();
        i != e; ++i) {
     const VarDecl *VD = *i;

clang/test/CodeGenCXX/catch-undef-behavior.cpp

void indirect_function_call(void (*p)(int)) {
 
   p(42);
 }
 
 namespace VBaseObjectSize {
   // Note: C is laid out such that offsetof(C, B) + sizeof(B) extends outside
   // the C object.
   struct alignas(16) A { void *a1, *a2; };
-  struct B : virtual A { void *b; };
-  struct C : virtual A, virtual B {};
+  struct B : virtual A { void *b; void* g(); };
+  struct C : virtual A, virtual B { };
   // CHECK-LABEL: define {{.*}} @_ZN15VBaseObjectSize1fERNS_1BE(
   B &f(B &b) {
     // Size check: check for nvsize(B) == 16 (do not require size(B) == 32)
     // CHECK: [[SIZE:%.+]] = call i{{32|64}} @llvm.objectsize.i64.p0i8(
     // CHECK: icmp uge i{{32|64}} [[SIZE]], 16,
 
     // Alignment check: check for nvalign(B) == 8 (do not require align(B) == 16)
     // CHECK: [[PTRTOINT:%.+]] = ptrtoint {{.*}} to i64,
     // CHECK: and i64 [[PTRTOINT]], 7,
     return b;
   }
+
+  // CHECK-LABEL: define {{.*}} @_ZN15VBaseObjectSize1B1gEv(
+  void *B::g() {
+    // Ensure that the check on the "this" pointer also uses the proper
+    // alignment. We should be using nvalign(B) == 8, not 16.
+    // CHECK: [[PTRTOINT:%.+]] = ptrtoint {{.*}} to i64,
+    // CHECK: and i64 [[PTRTOINT]], 7
+    return nullptr;
+  }
 }
 
 namespace FunctionSanitizerVirtualCalls {
 struct A {
   virtual void f() {}
   virtual void g() {}
   void h() {}
 };