Fix PR35902: incorrect alignment used for ubsan check.
authorJames Y Knight <jyknight@google.com>
Thu, 10 Dec 2020 23:03:16 +0000 (18:03 -0500)
committerJames Y Knight <jyknight@google.com>
Mon, 28 Dec 2020 23:11:17 +0000 (18:11 -0500)
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.

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

clang/lib/CodeGen/CGCXXABI.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
clang/test/CodeGenCXX/catch-undef-behavior.cpp

index 9d5ebde..9714730 100644 (file)
@@ -135,8 +135,8 @@ void CGCXXABI::buildThisParam(CodeGenFunction &CGF, FunctionArgList &params) {
   // 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();
index 005ee74..a8a91c5 100644 (file)
@@ -1137,11 +1137,9 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
           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);
     }
   }
 
index 28c92ba..a75b9d4 100644 (file)
@@ -430,8 +430,8 @@ 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)
@@ -443,6 +443,15 @@ namespace VBaseObjectSize {
     // 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 {