Implement non-trapping variant of -fsanitize=cfi* sanitizers.

[pcc]

Summary

This causes programs compiled with this flag to print a diagnostic when
a control flow integrity check fails instead of aborting. Diagnostics are
printed using UBSan's runtime library.

The main motivation of this feature over -fsanitize=vptr is fidelity with
the -fsanitize=cfi implementation: the diagnostics are printed under exactly
the same conditions as those which would cause -fsanitize=cfi to abort the
program. This means that the same restrictions apply regarding compiling
all translation units with -fsanitize=cfi, cross-DSO virtual calls are
forbidden, etc.

[rsmith]

It doesn't make sense to me to add a "sanitizer" that controls how another sanitizer reports failures; that is not how the -fsanitize= flags are supposed to work at all.

I think you should follow the existing model for frontend sanitizers here: if -fsanitize-undefined-trap-on-error is specified, then trap, otherwise produce a diagnostic. If it really makes sense to have a per-sanitizer trap-on-error mode (as opposed to the existing per-sanitizer -fsanitize-recover= setting), we should enable that for all the frontend sanitizers, not just this one, but it seems (perhaps naively) to me like -fsanitize-recover= should be sufficient here already?

[pcc]

My rationale for making the CFI flags work differently than the UBSan flags was that the default trapping behaviors of the sanitizers ought to be different. Because the primary purpose of the -fsanitize=cfi* flags is to enable a security hardening mechanism, they should trap by default. UBSan's primary purpose is to catch and report issues during development, so it makes sense for it to diagnose by default.

-fsanitize-recover= doesn't do exactly what we want for CFI, either. UBSan with recovery disabled still emits diagnostics, and if we wanted CFI to have the same behaviour it would introduce a dependency on RTTI and a runtime library, both things we would like to avoid in CFI in order to reduce binary size overhead.

I agree with you that it does seem weird for cfi-diag to look like a sanitizer. It does seem that something like -fsanitize-trap= (with appropriate defaults for the various sanitizers) may be the best option.

[rsmith]

UBSan with recovery disabled still emits diagnostics, and if we wanted CFI to have the same behaviour it would introduce a dependency on RTTI and a runtime library, both things we would like to avoid in CFI in order to reduce binary size overhead.

I don't follow why the dependence on RTTI is linked to whether you emit diagnostics, can you explain? I would think the diagnostic handler could gracefully degrade to not using the RTTI if it's unavailable.

As for the library dependence, I would think it's preferable to produce *some* kind of diagnostic by default on the way out rather than just silently terminating, unless there are security implications to doing so (are you worried about an attacker overwriting the sanitizer runtime's callback function?). A CFI failure could show up during testing due to "normal" lifetime bugs, uninitialized variables, and the like, not just due to an attack being thwarted, and in either case I think it'd be good to have *some* indication that the bad thing happened, even if we can only print it to stderr by default. But, ultimately, I'm happy leaving the choice with you. If you think that we should trap silently by default, then I have no objection to adding a -fsanitize-trap= mechanism, and turning it on by default for CFI.

[samsonov]

FWIW, I agree with Richard here. We already have different defaults for which sanitizers are recoverable, and which are not (-fsanitize=addres, unreachable etc.).
We may also have the defaults for which sanitizers trap-on-error (-fsanitize=cfi), and which print diagnostics. I think that adding -fsanitize-trap=<list> is the way to go.
I would love to see the ugly mask -fsanitize=undefined-trap gone in one way or another. Instead, I'd vote for silently disabling vptr sanitizer if the user runs the invocation:

`clang++ -fsanitize=undefined -fsanitize-trap=undefined`.

"Well, you're asking us to skip runtime checks and trap on error instead, we'll do that, but implicitly disable sanitizers which can't be implemented w/o runtime checks".

[rsmith]

"Well, you're asking us to skip runtime checks and trap on error instead, we'll do that, but implicitly disable sanitizers which can't be implemented w/o runtime checks".

Yeah, I'd be OK with that, but we'll need to be pretty clear about it in the documentation. (It's not entirely natural that "trap on error" and "don't use the runtime library" are the same thing, but it's the latter that requires us to turn off the vptr sanitizer when the former is enabled.)

[pcc]

I don't follow why the dependence on RTTI is linked to whether you emit diagnostics, can you explain?

The CFI mechanism is designed not to depend on RTTI; see http://clang.llvm.org/docs/ControlFlowIntegrityDesign.html for details on how it works. However if diagnostics are enabled we will need RTTI, as that is the mechanism we use to get the name of the class that failed the check.

I would think the diagnostic handler could gracefully degrade to not using the RTTI if it's unavailable.

Perhaps, but without a class name its usefulness is limited.

As for the library dependence, I would think it's preferable to produce *some* kind of diagnostic by default on the way out rather than just silently terminating, unless there are security implications to doing so (are you worried about an attacker overwriting the sanitizer runtime's callback function?).

I believe that the default behavior ought to be that which is recommended for binaries shipped to end users. A diagnostic is no more actionable to a user than a crash, and even the function call to emit the diagnostic has a small file size cost. As for security implications, the runtime library (as with any code linked into the program) forms part of an attack surface which we should seek to minimize.

If you think that we should trap silently by default, then I have no objection to adding a -fsanitize-trap= mechanism, and turning it on by default for CFI.

Sounds good then, I'll work on that approach.

[pcc]

• Re-implement in terms of -fsanitize-trap

[pcc]

• Document -fsanitize-trap precedence over -fsanitize-recover

[pcc]

Reverting the documentation change, moving it to D10464

[pcc]

Refresh

[pcc]

• Fix assertion failure

[samsonov]

I'm OK with this change, but let Richard have a look as well.

[rsmith]

LGTM too.

include/clang/Basic/DiagnosticDriverKinds.td

def warn_drv_pch_not_first_include : Warning<
   "precompiled header '%0' was ignored because '%1' is not first '-include'">;
 def warn_missing_sysroot : Warning<"no such sysroot directory: '%0'">,
   InGroup<DiagGroup<"missing-sysroot">>;
 def warn_debug_compression_unavailable : Warning<"cannot compress debug sections (zlib not installed)">,
   InGroup<DiagGroup<"debug-compression-unavailable">>;
 def warn_drv_enabling_rtti_with_exceptions : Warning<
   "implicitly enabling rtti for exception handling">,
   InGroup<DiagGroup<"rtti-for-exceptions">>;
-def warn_drv_disabling_vptr_no_rtti_default : Warning<
-  "implicitly disabling vptr sanitizer because rtti wasn't enabled">,
+def warn_drv_disabling_sanitizer_no_rtti_default : Warning<
+  "implicitly disabling %0 sanitizer because rtti wasn't enabled">,
   InGroup<DiagGroup<"auto-disable-vptr-sanitizer">>;
 
 def note_drv_command_failed_diag_msg : Note<
   "diagnostic msg: %0">;
 def note_drv_t_option_is_global : Note<
   "The last /TC or /TP option takes precedence over earlier instances">;
 def note_drv_address_sanitizer_debug_runtime : Note<
   "AddressSanitizer doesn't support linking with debug runtime libraries yet">;

include/clang/Basic/Sanitizers.def

 // IntegerSanitizer
 SANITIZER("unsigned-integer-overflow", UnsignedIntegerOverflow)
 
 // DataFlowSanitizer
 SANITIZER("dataflow", DataFlow)
 
 // Control Flow Integrity
 SANITIZER("cfi-cast-strict", CFICastStrict)
+SANITIZER("cfi-diag", CFIDiag)
 SANITIZER("cfi-derived-cast", CFIDerivedCast)
 SANITIZER("cfi-unrelated-cast", CFIUnrelatedCast)
 SANITIZER("cfi-nvcall", CFINVCall)
 SANITIZER("cfi-vcall", CFIVCall)
 SANITIZER_GROUP("cfi", CFI,
                 CFIDerivedCast | CFIUnrelatedCast | CFINVCall | CFIVCall)
 
 // -fsanitize=undefined-trap includes sanitizers from -fsanitize=undefined

lib/CodeGen/CGCXXABI.h

public:
   /// used for the vptr at the given offset in RD.
   virtual llvm::GlobalVariable *getAddrOfVTable(const CXXRecordDecl *RD,
                                                 CharUnits VPtrOffset) = 0;
 
   /// Build a virtual function pointer in the ABI-specific way.
   virtual llvm::Value *getVirtualFunctionPointer(CodeGenFunction &CGF,
                                                  GlobalDecl GD,
                                                  llvm::Value *This,
-                                                 llvm::Type *Ty) = 0;
+                                                 llvm::Type *Ty,
+                                                 SourceLocation Loc) = 0;
 
   /// Emit the ABI-specific virtual destructor call.
   virtual llvm::Value *
   EmitVirtualDestructorCall(CodeGenFunction &CGF, const CXXDestructorDecl *Dtor,
                             CXXDtorType DtorType, llvm::Value *This,
                             const CXXMemberCallExpr *CE) = 0;
 
   virtual void adjustCallArgsForDestructorThunk(CodeGenFunction &CGF,

lib/CodeGen/CGClass.cpp

for (const CXXMethodDecl *MD : RD->methods()) {
     }
   }
 
   return LeastDerivedClassWithSameLayout(
       RD->bases_begin()->getType()->getAsCXXRecordDecl());
 }
 
 void CodeGenFunction::EmitVTablePtrCheckForCall(const CXXMethodDecl *MD,
-                                                llvm::Value *VTable) {
+                                                llvm::Value *VTable,
+                                                CFITypeCheckKind TCK,
+                                                SourceLocation Loc) {
   const CXXRecordDecl *ClassDecl = MD->getParent();
   if (!SanOpts.has(SanitizerKind::CFICastStrict))
     ClassDecl = LeastDerivedClassWithSameLayout(ClassDecl);
 
-  EmitVTablePtrCheck(ClassDecl, VTable);
+  EmitVTablePtrCheck(ClassDecl, VTable, TCK, Loc);
 }
 
 void CodeGenFunction::EmitVTablePtrCheckForCast(QualType T,
                                                 llvm::Value *Derived,
-                                                bool MayBeNull) {
+                                                bool MayBeNull,
+                                                CFITypeCheckKind TCK,
+                                                SourceLocation Loc) {
   if (!getLangOpts().CPlusPlus)
     return;
 
   auto *ClassTy = T->getAs<RecordType>();
   if (!ClassTy)
     return;
 
   const CXXRecordDecl *ClassDecl = cast<CXXRecordDecl>(ClassTy->getDecl());
if (MayBeNull) {
     ContBlock = createBasicBlock("cast.cont");
 
     Builder.CreateCondBr(DerivedNotNull, CheckBlock, ContBlock);
 
     EmitBlock(CheckBlock);
   }
 
   llvm::Value *VTable = GetVTablePtr(Derived, Int8PtrTy);
-  EmitVTablePtrCheck(ClassDecl, VTable);
+  EmitVTablePtrCheck(ClassDecl, VTable, TCK, Loc);
 
   if (MayBeNull) {
     Builder.CreateBr(ContBlock);
     EmitBlock(ContBlock);
   }
 }
 
 void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD,
-                                         llvm::Value *VTable) {
+                                         llvm::Value *VTable,
+                                         CFITypeCheckKind TCK,
+                                         SourceLocation Loc) {
   // FIXME: Add blacklisting scheme.
   if (RD->isInStdNamespace())
     return;
 
   std::string OutName;
   llvm::raw_string_ostream Out(OutName);
   CGM.getCXXABI().getMangleContext().mangleCXXVTableBitSet(RD, Out);
 
   llvm::Value *BitSetName = llvm::MetadataAsValue::get(
       getLLVMContext(), llvm::MDString::get(getLLVMContext(), Out.str()));
 
-  llvm::Value *BitSetTest = Builder.CreateCall(
-      CGM.getIntrinsic(llvm::Intrinsic::bitset_test),
-      {Builder.CreateBitCast(VTable, CGM.Int8PtrTy), BitSetName});
+  llvm::Value *CastedVTable = Builder.CreateBitCast(VTable, Int8PtrTy);
+  llvm::Value *BitSetTest =
+      Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::bitset_test),
+                         {CastedVTable, BitSetName});
 
   llvm::BasicBlock *ContBlock = createBasicBlock("vtable.check.cont");
   llvm::BasicBlock *TrapBlock = createBasicBlock("vtable.check.trap");
 
   Builder.CreateCondBr(BitSetTest, ContBlock, TrapBlock);
 
   EmitBlock(TrapBlock);
+  if (SanOpts.has(SanitizerKind::CFIDiag)) {
+    QualType Ty(RD->getTypeForDecl(), 0);
+    llvm::Constant *StaticData[] = {
+      EmitCheckSourceLocation(Loc),
+      EmitCheckTypeDescriptor(Ty),
+      llvm::ConstantInt::get(Int8Ty, TCK),
+    };
+    llvm::Constant *StaticDataConst = llvm::ConstantStruct::getAnon(StaticData);
+    auto *StaticDataGlobal = new llvm::GlobalVariable(
+        CGM.getModule(), StaticDataConst->getType(), false,
+        llvm::GlobalVariable::PrivateLinkage, StaticDataConst);
+    StaticDataGlobal->setUnnamedAddr(true);
+    CGM.getSanitizerMetadata()->disableSanitizerForGlobal(StaticDataGlobal);
+
+    llvm::Type *ArgTypes[] = {
+      StaticDataGlobal->getType(),
+      Int8PtrTy,
+    };
+
+    llvm::FunctionType *FnType =
+        llvm::FunctionType::get(CGM.VoidTy, ArgTypes, false);
+
+    llvm::Value *Fn =
+        CGM.CreateRuntimeFunction(FnType, "__cfi_handle_bad_type");
+    EmitNounwindRuntimeCall(Fn, {StaticDataGlobal, CastedVTable});
+    Builder.CreateBr(ContBlock);
+  } else {
     Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::trap), {});
     Builder.CreateUnreachable();
+  }
 
   EmitBlock(ContBlock);
 }
 
 // FIXME: Ideally Expr::IgnoreParenNoopCasts should do this, but it doesn't do
 // quite what we want.
 static const Expr *skipNoOpCastsAndParens(const Expr *E) {
   while (true) {

lib/CodeGen/CGExpr.cpp

case CK_BaseToDerived: {
 
     // C++11 [expr.static.cast]p2: Behavior is undefined if a downcast is
     // performed and the object is not of the derived type.
     if (sanitizePerformTypeCheck())
       EmitTypeCheck(TCK_DowncastReference, E->getExprLoc(),
                     Derived, E->getType());
 
     if (SanOpts.has(SanitizerKind::CFIDerivedCast))
-      EmitVTablePtrCheckForCast(E->getType(), Derived, /*MayBeNull=*/false);
+      EmitVTablePtrCheckForCast(E->getType(), Derived, /*MayBeNull=*/false,
+                                CFITCK_DerivedCast, E->getLocStart());
 
     return MakeAddrLValue(Derived, E->getType());
   }
   case CK_LValueBitCast: {
     // This must be a reinterpret_cast (or c-style equivalent).
     const auto *CE = cast<ExplicitCastExpr>(E);
 
     LValue LV = EmitLValue(E->getSubExpr());
     llvm::Value *V = Builder.CreateBitCast(LV.getAddress(),
                                            ConvertType(CE->getTypeAsWritten()));
 
     if (SanOpts.has(SanitizerKind::CFIUnrelatedCast))
-      EmitVTablePtrCheckForCast(E->getType(), V, /*MayBeNull=*/false);
+      EmitVTablePtrCheckForCast(E->getType(), V, /*MayBeNull=*/false,
+                                CFITCK_UnrelatedCast, E->getLocStart());
 
     return MakeAddrLValue(V, E->getType());
   }
   case CK_ObjCObjectLValueCast: {
     LValue LV = EmitLValue(E->getSubExpr());
     QualType ToType = getContext().getLValueReferenceType(E->getType());
     llvm::Value *V = Builder.CreateBitCast(LV.getAddress(),
                                            ConvertType(ToType));

lib/CodeGen/CGExprCXX.cpp

if (UseVirtualCall) {
                                   /*ImplicitParam=*/nullptr, QualType(), CE);
     }
     return RValue::get(nullptr);
   }
   
   if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(MD)) {
     Callee = CGM.GetAddrOfFunction(GlobalDecl(Ctor, Ctor_Complete), Ty);
   } else if (UseVirtualCall) {
-    Callee = CGM.getCXXABI().getVirtualFunctionPointer(*this, MD, This, Ty);
+    Callee = CGM.getCXXABI().getVirtualFunctionPointer(*this, MD, This, Ty,
+                                                       CE->getLocStart());
   } else {
     if (SanOpts.has(SanitizerKind::CFINVCall) &&
         MD->getParent()->isDynamicClass()) {
       llvm::Value *VTable = GetVTablePtr(This, Int8PtrTy);
-      EmitVTablePtrCheckForCall(MD, VTable);
+      EmitVTablePtrCheckForCall(MD, VTable, CFITCK_NVCall, CE->getLocStart());
     }
 
     if (getLangOpts().AppleKext && MD->isVirtual() && HasQualifier)
       Callee = BuildAppleKextVirtualCall(MD, Qualifier, Ty);
     else if (!DevirtualizedMethod)
       Callee = CGM.GetAddrOfFunction(MD, Ty);
     else {
       Callee = CGM.GetAddrOfFunction(DevirtualizedMethod, Ty);

lib/CodeGen/CGExprScalar.cpp

if (SrcTy->isPtrOrPtrVectorTy() && DstTy->isPtrOrPtrVectorTy() &&
         SrcTy->getPointerAddressSpace() != DstTy->getPointerAddressSpace()) {
       llvm_unreachable("wrong cast for pointers in different address spaces"
                        "(must be an address space cast)!");
     }
 
     if (CGF.SanOpts.has(SanitizerKind::CFIUnrelatedCast)) {
       if (auto PT = DestTy->getAs<PointerType>())
         CGF.EmitVTablePtrCheckForCast(PT->getPointeeType(), Src,
-                                      /*MayBeNull=*/true);
+                                      /*MayBeNull=*/true,
+                                      CodeGenFunction::CFITCK_UnrelatedCast,
+                                      CE->getLocStart());
     }
 
     return Builder.CreateBitCast(Src, DstTy);
   }
   case CK_AddressSpaceConversion: {
     Value *Src = Visit(const_cast<Expr*>(E));
     return Builder.CreateAddrSpaceCast(Src, ConvertType(DestTy));
   }
case CK_BaseToDerived: {
     // C++11 [expr.static.cast]p11: Behavior is undefined if a downcast is
     // performed and the object is not of the derived type.
     if (CGF.sanitizePerformTypeCheck())
       CGF.EmitTypeCheck(CodeGenFunction::TCK_DowncastPointer, CE->getExprLoc(),
                         Derived, DestTy->getPointeeType());
 
     if (CGF.SanOpts.has(SanitizerKind::CFIDerivedCast))
       CGF.EmitVTablePtrCheckForCast(DestTy->getPointeeType(), Derived,
-                                    /*MayBeNull=*/true);
+                                    /*MayBeNull=*/true,
+                                    CodeGenFunction::CFITCK_DerivedCast,
+                                    CE->getLocStart());
 
     return Derived;
   }
   case CK_UncheckedDerivedToBase:
   case CK_DerivedToBase: {
     const CXXRecordDecl *DerivedClassDecl =
       E->getType()->getPointeeCXXRecordDecl();
     assert(DerivedClassDecl && "DerivedToBase arg isn't a C++ object pointer!");

lib/CodeGen/CodeGenFunction.h

void InitializeVTablePointers(BaseSubobject Base,
                                 VisitedVirtualBasesSetTy& VBases);
 
   void InitializeVTablePointers(const CXXRecordDecl *ClassDecl);
 
   /// GetVTablePtr - Return the Value of the vtable pointer member pointed
   /// to by This.
   llvm::Value *GetVTablePtr(llvm::Value *This, llvm::Type *Ty);
 
+  enum CFITypeCheckKind {
+    CFITCK_VCall,
+    CFITCK_NVCall,
+    CFITCK_DerivedCast,
+    CFITCK_UnrelatedCast,
+  };
+
   /// \brief Derived is the presumed address of an object of type T after a
   /// cast. If T is a polymorphic class type, emit a check that the virtual
   /// table for Derived belongs to a class derived from T.
   void EmitVTablePtrCheckForCast(QualType T, llvm::Value *Derived,
-                                 bool MayBeNull);
+                                 bool MayBeNull, CFITypeCheckKind TCK,
+                                 SourceLocation Loc);
 
   /// EmitVTablePtrCheckForCall - Virtual method MD is being called via VTable.
   /// If vptr CFI is enabled, emit a check that VTable is valid.
-  void EmitVTablePtrCheckForCall(const CXXMethodDecl *MD, llvm::Value *VTable);
+  void EmitVTablePtrCheckForCall(const CXXMethodDecl *MD, llvm::Value *VTable,
+                                 CFITypeCheckKind TCK, SourceLocation Loc);
 
   /// EmitVTablePtrCheck - Emit a check that VTable is a valid virtual table for
   /// RD using llvm.bitset.test.
-  void EmitVTablePtrCheck(const CXXRecordDecl *RD, llvm::Value *VTable);
+  void EmitVTablePtrCheck(const CXXRecordDecl *RD, llvm::Value *VTable,
+                          CFITypeCheckKind TCK, SourceLocation Loc);
 
   /// CanDevirtualizeMemberFunctionCalls - Checks whether virtual calls on given
   /// expr can be devirtualized.
   bool CanDevirtualizeMemberFunctionCall(const Expr *Base,
                                          const CXXMethodDecl *MD);
 
   /// EnterDtorCleanups - Enter the cleanups necessary to complete the
   /// given phase of destruction for a destructor.  The end result

lib/CodeGen/ItaniumCXXABI.cpp

public:
   getVTableAddressPointForConstExpr(BaseSubobject Base,
                                     const CXXRecordDecl *VTableClass) override;
 
   llvm::GlobalVariable *getAddrOfVTable(const CXXRecordDecl *RD,
                                         CharUnits VPtrOffset) override;
 
   llvm::Value *getVirtualFunctionPointer(CodeGenFunction &CGF, GlobalDecl GD,
                                          llvm::Value *This,
-                                         llvm::Type *Ty) override;
+                                         llvm::Type *Ty,
+                                         SourceLocation Loc) override;
 
   llvm::Value *EmitVirtualDestructorCall(CodeGenFunction &CGF,
                                          const CXXDestructorDecl *Dtor,
                                          CXXDtorType DtorType,
                                          llvm::Value *This,
                                          const CXXMemberCallExpr *CE) override;
 
   void emitVirtualInheritanceTables(const CXXRecordDecl *RD) override;
else if (RD->hasAttr<DLLExportAttr>())
     VTable->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass);
 
   return VTable;
 }
 
 llvm::Value *ItaniumCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
                                                       GlobalDecl GD,
                                                       llvm::Value *This,
-                                                      llvm::Type *Ty) {
+                                                      llvm::Type *Ty,
+                                                      SourceLocation Loc) {
   GD = GD.getCanonicalDecl();
   Ty = Ty->getPointerTo()->getPointerTo();
   llvm::Value *VTable = CGF.GetVTablePtr(This, Ty);
 
   if (CGF.SanOpts.has(SanitizerKind::CFIVCall))
-    CGF.EmitVTablePtrCheckForCall(cast<CXXMethodDecl>(GD.getDecl()), VTable);
+    CGF.EmitVTablePtrCheckForCall(cast<CXXMethodDecl>(GD.getDecl()), VTable,
+                                  CodeGenFunction::CFITCK_VCall, Loc);
 
   uint64_t VTableIndex = CGM.getItaniumVTableContext().getMethodVTableIndex(GD);
   llvm::Value *VFuncPtr =
       CGF.Builder.CreateConstInBoundsGEP1_64(VTable, VTableIndex, "vfn");
   return CGF.Builder.CreateLoad(VFuncPtr);
 }
 
 llvm::Value *ItaniumCXXABI::EmitVirtualDestructorCall(
     CodeGenFunction &CGF, const CXXDestructorDecl *Dtor, CXXDtorType DtorType,
     llvm::Value *This, const CXXMemberCallExpr *CE) {
   assert(CE == nullptr || CE->arg_begin() == CE->arg_end());
   assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete);
 
   const CGFunctionInfo *FInfo = &CGM.getTypes().arrangeCXXStructorDeclaration(
       Dtor, getFromDtorType(DtorType));
   llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
   llvm::Value *Callee =
-      getVirtualFunctionPointer(CGF, GlobalDecl(Dtor, DtorType), This, Ty);
+      getVirtualFunctionPointer(CGF, GlobalDecl(Dtor, DtorType), This, Ty,
+                                CE ? CE->getLocStart() : SourceLocation());
 
   CGF.EmitCXXMemberOrOperatorCall(Dtor, Callee, ReturnValueSlot(), This,
                                   /*ImplicitParam=*/nullptr, QualType(), CE);
   return nullptr;
 }
 
 void ItaniumCXXABI::emitVirtualInheritanceTables(const CXXRecordDecl *RD) {
   CodeGenVTables &VTables = CGM.getVTables();

lib/CodeGen/MicrosoftCXXABI.cpp

public:
   llvm::Constant *
   getVTableAddressPointForConstExpr(BaseSubobject Base,
                                     const CXXRecordDecl *VTableClass) override;
 
   llvm::GlobalVariable *getAddrOfVTable(const CXXRecordDecl *RD,
                                         CharUnits VPtrOffset) override;
 
   llvm::Value *getVirtualFunctionPointer(CodeGenFunction &CGF, GlobalDecl GD,
-                                         llvm::Value *This,
-                                         llvm::Type *Ty) override;
+                                         llvm::Value *This, llvm::Type *Ty,
+                                         SourceLocation Loc) override;
 
   llvm::Value *EmitVirtualDestructorCall(CodeGenFunction &CGF,
                                          const CXXDestructorDecl *Dtor,
                                          CXXDtorType DtorType,
                                          llvm::Value *This,
                                          const CXXMemberCallExpr *CE) override;
 
   void adjustCallArgsForDestructorThunk(CodeGenFunction &CGF, GlobalDecl GD,
#endif
 
   VFTablesMap[ID] = VFTable;
   return VTable;
 }
 
 llvm::Value *MicrosoftCXXABI::getVirtualFunctionPointer(CodeGenFunction &CGF,
                                                         GlobalDecl GD,
                                                         llvm::Value *This,
-                                                        llvm::Type *Ty) {
+                                                        llvm::Type *Ty,
+                                                        SourceLocation Loc) {
   GD = GD.getCanonicalDecl();
   CGBuilderTy &Builder = CGF.Builder;
 
   Ty = Ty->getPointerTo()->getPointerTo();
   llvm::Value *VPtr =
       adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
   llvm::Value *VTable = CGF.GetVTablePtr(VPtr, Ty);
 
llvm::Value *MicrosoftCXXABI::EmitVirtualDestructorCall(
   assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete);
 
   // We have only one destructor in the vftable but can get both behaviors
   // by passing an implicit int parameter.
   GlobalDecl GD(Dtor, Dtor_Deleting);
   const CGFunctionInfo *FInfo = &CGM.getTypes().arrangeCXXStructorDeclaration(
       Dtor, StructorType::Deleting);
   llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo);
-  llvm::Value *Callee = getVirtualFunctionPointer(CGF, GD, This, Ty);
+  llvm::Value *Callee = getVirtualFunctionPointer(
+      CGF, GD, This, Ty, CE ? CE->getLocStart() : SourceLocation());
 
   ASTContext &Context = getContext();
   llvm::Value *ImplicitParam = llvm::ConstantInt::get(
       llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()),
       DtorType == Dtor_Deleting);
 
   This = adjustThisArgumentForVirtualFunctionCall(CGF, GD, This, true);
   RValue RV = CGF.EmitCXXStructorCall(Dtor, Callee, ReturnValueSlot(), This,

lib/Driver/SanitizerArgs.cpp

 #include <memory>
 
 using namespace clang;
 using namespace clang::SanitizerKind;
 using namespace clang::driver;
 using namespace llvm::opt;
 
 enum : SanitizerMask {
-  NeedsUbsanRt = Undefined | Integer,
+  NeedsUbsanRt = Undefined | Integer | CFIDiag,
   NotAllowedWithTrap = Vptr,
   RequiresPIE = Memory | DataFlow,
   NeedsUnwindTables = Address | Thread | Memory | DataFlow,
   SupportsCoverage = Address | Memory | Leak | Undefined | Integer | DataFlow,
   RecoverableByDefault = Undefined | Integer,
   Unrecoverable = Address | Unreachable | Return,
   LegacyFsanitizeRecoverMask = Undefined | Integer,
   NeedsLTO = CFI,
if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
         // Only diagnose the new kinds.
         std::string Desc = describeSanitizeArg(*I, KindsToDiagnose);
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << Desc << TC.getTriple().str();
         DiagnosedKinds |= KindsToDiagnose;
       }
       Add &= ~NotSupported;
 
-      // Test for -fno-rtti + explicit -fsanitizer=vptr before expanding groups
-      // so we don't error out if -fno-rtti and -fsanitize=undefined were
-      // passed.
-      if (Add & Vptr &&
+      auto diagnoseRTTIConflict = [&](SanitizerMask Mask, StringRef Name) {
+        if (Add & Mask &&
             (RTTIMode == ToolChain::RM_DisabledImplicitly ||
              RTTIMode == ToolChain::RM_DisabledExplicitly)) {
           if (RTTIMode == ToolChain::RM_DisabledImplicitly)
-          // Warn about not having rtti enabled if the vptr sanitizer is
-          // explicitly enabled
-          D.Diag(diag::warn_drv_disabling_vptr_no_rtti_default);
+            // Warn about not having rtti enabled if the sanitizer is explicitly
+            // enabled
+            D.Diag(diag::warn_drv_disabling_sanitizer_no_rtti_default) << Name;
           else {
             const llvm::opt::Arg *NoRTTIArg = TC.getRTTIArg();
             assert(NoRTTIArg &&
                    "RTTI disabled explicitly but we have no argument!");
+            std::string FlagName = "-fsanitize=";
+            FlagName += Name;
             D.Diag(diag::err_drv_argument_not_allowed_with)
-              << "-fsanitize=vptr" << NoRTTIArg->getAsString(Args);
+                << FlagName << NoRTTIArg->getAsString(Args);
           }
 
-        // Take out the Vptr sanitizer from the enabled sanitizers
-        AllRemove |= Vptr;
+          // Take out the sanitizer from the enabled sanitizers
+          AllRemove |= Mask;
         }
+      };
+
+      // Test for -fno-rtti + explicit -fsanitizer=vptr before expanding groups
+      // so we don't error out if -fno-rtti and -fsanitize=undefined were
+      // passed.
+      diagnoseRTTIConflict(Vptr, "vptr");
+      diagnoseRTTIConflict(CFIDiag, "cfi-diag");
 
       Add = expandSanitizerGroups(Add);
       // Group expansion may have enabled a sanitizer which is disabled later.
       Add &= ~AllRemove;
       // Silently discard any unsupported sanitizers implicitly enabled through
       // group expansion.
       Add &= ~NotSupported;
 

test/CodeGenCXX/cfi-vcall.cpp

-// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-vcall -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=NDIAG %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-vcall,cfi-diag -emit-llvm -o - %s | FileCheck --check-prefix=CHECK --check-prefix=DIAG %s
 
 struct A {
   A();
   virtual void f();
 };
 
 struct B : virtual A {
   B();

 D::D() {}
 
 void A::f() {
 }
 
 void D::f() {
 }
 
+// DIAG: @[[SRC:.*]] = private unnamed_addr constant [{{.*}} x i8] c"{{.*}}test/CodeGenCXX/cfi-vcall.cpp\00", align 1
+// DIAG: @[[TYPE:.*]] = private unnamed_addr constant { i16, i16, [4 x i8] } { i16 -1, i16 0, [4 x i8] c"'A'\00" }
+// DIAG: @[[BADTYPESTATIC:.*]] = private unnamed_addr global { { [{{.*}} x i8]*, i32, i32 }, { i16, i16, [4 x i8] }*, i8 } { { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 54, i32 3 }, { i16, i16, [4 x i8] }* @[[TYPE]], i8 0 }
+
 // CHECK: define void @_Z2afP1A
 void af(A *a) {
-  // CHECK: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* {{%[^ ]*}}, metadata !"1A")
+  // CHECK: [[P:%[^ ]*]] = call i1 @llvm.bitset.test(i8* [[VT:%[^ ]*]], metadata !"1A")
   // CHECK-NEXT: br i1 [[P]], label %[[CONTBB:[^ ]*]], label %[[TRAPBB:[^ ]*]]
 
   // CHECK: [[TRAPBB]]
-  // CHECK-NEXT: call void @llvm.trap()
-  // CHECK-NEXT: unreachable
+  // NDIAG-NEXT: call void @llvm.trap()
+  // NDIAG-NEXT: unreachable
+  // DIAG-NEXT: call void @__cfi_handle_bad_type({{.*}} @[[BADTYPESTATIC]], i8* [[VT]])
+  // DIAG-NEXT: br label %[[CONTBB]]
 
   // CHECK: [[CONTBB]]
   // CHECK: call void %
   a->f();
 }
 
 // CHECK: define internal void @_Z3df1PN12_GLOBAL__N_11DE
 void df1(D *d) {

test/Driver/rtti-options.cpp

 // -fsanitize=vptr
 // Make sure we only error/warn once, when trying to enable vptr and
 // undefined and have -fno-rtti
 // RUN: %clang -### -c -fsanitize=undefined -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR -check-prefix=CHECK-OK %s
 
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-WARN %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s
+// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=cfi-diag %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-CFI-WARN %s
+// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=cfi-diag -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
+// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=cfi-diag -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-CFI-ERROR %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=undefined -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=vptr %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s
+// RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=cfi-diag %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
+// RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=cfi-diag -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
+// RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=cfi-diag -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-CFI-ERROR %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=undefined %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fsanitize=undefined -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 
 // Exceptions + no/default rtti
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fcxx-exceptions -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-EXC-ERROR-CXX %s
 // RUN: %clang -### -c -target x86_64-scei-ps4 -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-EXC-WARN %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fcxx-exceptions -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fcxx-exceptions %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s

 // RUN: %clang -### -c -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RTTI %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-RTTI %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-NO-RTTI %s
 // RUN: %clang -### -c -target x86_64-unknown-unknown %s 2>&1 | FileCheck -check-prefix=CHECK-RTTI %s
 
 // CHECK-UNUSED: warning: argument unused during compilation: '-fcxx-exceptions'
 // CHECK-SAN-WARN: implicitly disabling vptr sanitizer because rtti wasn't enabled
 // CHECK-SAN-ERROR: invalid argument '-fsanitize=vptr' not allowed with '-fno-rtti'
+// CHECK-SAN-CFI-WARN: implicitly disabling cfi-diag sanitizer because rtti wasn't enabled
+// CHECK-SAN-CFI-ERROR: invalid argument '-fsanitize=cfi-diag' not allowed with '-fno-rtti'
 // CHECK-EXC-WARN: implicitly enabling rtti for exception handling
 // CHECK-EXC-ERROR: invalid argument '-fno-rtti' not allowed with '-fexceptions'
 // CHECK-EXC-ERROR-CXX: invalid argument '-fno-rtti' not allowed with '-fcxx-exceptions'
 // CHECK-RTTI-NOT: "-fno-rtti"
 // CHECK-NO-RTTI-NOT: "-frtti"
 
 // CHECK-OK-NOT: {{warning:|error:}}