Implement CFI for indirect calls via a member function pointer.

Similarly to CFI on virtual and indirect calls, this implementation
tries to use program type information to make the checks as precise
as possible.  The basic way that it works is as follows, where `C`
is the name of the class being defined or the target of a call and
the function type is assumed to be `void()`.

For virtual calls:
- Attach type metadata to the addresses of function pointers in vtables
  (not the functions themselves) of type `void (B::*)()` for each `B`
  that is a recursive dynamic base class of `C`, including `C` itself.
  This type metadata has an annotation that the type is for virtual
  calls (to distinguish it from the non-virtual case).
- At the call site, check that the computed address of the function
  pointer in the vtable has type `void (C::*)()`.

For non-virtual calls:
- Attach type metadata to each non-virtual member function whose address
  can be taken with a member function pointer. The type of a function
  in class `C` of type `void()` is each of the types `void (B::*)()`
  where `B` is a most-base class of `C`. A most-base class of `C`
  is defined as a recursive base class of `C`, including `C` itself,
  that does not have any bases.
- At the call site, check that the function pointer has one of the types
  `void (B::*)()` where `B` is a most-base class of `C`.

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

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@335569 91177308-0d34-0410-b5e6-96231b3b80d8

docs/ControlFlowIntegrity.rst

@@ -66,6 +66,8 @@ Available schemes are:
      wrong dynamic type.
   -  ``-fsanitize=cfi-icall``: Indirect call of a function with wrong dynamic
      type.
+  -  ``-fsanitize=cfi-mfcall``: Indirect call via a member function pointer with
+     wrong dynamic type.
 
 You can use ``-fsanitize=cfi`` to enable all the schemes and use
 ``-fno-sanitize`` flag to narrow down the set of schemes as desired.
@@ -255,6 +257,34 @@ the identity of function pointers is maintained, and calls across shared
 library boundaries are no different from calls within a single program or
 shared library.
 
+Member Function Pointer Call Checking
+=====================================
+
+This scheme checks that indirect calls via a member function pointer
+take place using an object of the correct dynamic type. Specifically, we
+check that the dynamic type of the member function referenced by the member
+function pointer matches the "function pointer" part of the member function
+pointer, and that the member function's class type is related to the base
+type of the member function. This CFI scheme can be enabled on its own using
+``-fsanitize=cfi-mfcall``.
+
+The compiler will only emit a full CFI check if the member function pointer's
+base type is complete. This is because the complete definition of the base
+type contains information that is necessary to correctly compile the CFI
+check. To ensure that the compiler always emits a full CFI check, it is
+recommended to also pass the flag ``-fcomplete-member-pointers``, which
+enables a non-conforming language extension that requires member pointer
+base types to be complete if they may be used for a call.
+
+For this scheme to work, all translation units containing the definition
+of a virtual member function (whether inline or not), other than members
+of :ref:`blacklisted <cfi-blacklist>` types or types with public :doc:`LTO
+visibility <LTOVisibility>`, must be compiled with ``-flto`` or ``-flto=thin``
+enabled and be statically linked into the program.
+
+This scheme is currently not compatible with cross-DSO CFI or the
+Microsoft ABI.
+
 .. _cfi-blacklist:
 
 Blacklist

docs/LTOVisibility.rst

@@ -11,9 +11,9 @@ linkage unit's LTO unit is empty. Each linkage unit has only a single LTO unit.
 
 The LTO visibility of a class is used by the compiler to determine which
 classes the whole-program devirtualization (``-fwhole-program-vtables``) and
-control flow integrity (``-fsanitize=cfi-vcall``) features apply to. These
-features use whole-program information, so they require the entire class
-hierarchy to be visible in order to work correctly.
+control flow integrity (``-fsanitize=cfi-vcall`` and ``-fsanitize=cfi-mfcall``)
+features apply to. These features use whole-program information, so they
+require the entire class hierarchy to be visible in order to work correctly.
 
 If any translation unit in the program uses either of the whole-program
 devirtualization or control flow integrity features, it is effectively an ODR

include/clang/Basic/Sanitizers.def

@@ -104,12 +104,13 @@ SANITIZER("dataflow", DataFlow)
 SANITIZER("cfi-cast-strict", CFICastStrict)
 SANITIZER("cfi-derived-cast", CFIDerivedCast)
 SANITIZER("cfi-icall", CFIICall)
+SANITIZER("cfi-mfcall", CFIMFCall)
 SANITIZER("cfi-unrelated-cast", CFIUnrelatedCast)
 SANITIZER("cfi-nvcall", CFINVCall)
 SANITIZER("cfi-vcall", CFIVCall)
 SANITIZER_GROUP("cfi", CFI,
-                CFIDerivedCast | CFIICall | CFIUnrelatedCast | CFINVCall |
-                CFIVCall)
+                CFIDerivedCast | CFIICall | CFIMFCall | CFIUnrelatedCast |
+                    CFINVCall | CFIVCall)
 
 // Safe Stack
 SANITIZER("safe-stack", SafeStack)

lib/CodeGen/CGClass.cpp

@@ -2688,7 +2688,9 @@ void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD,
     SSK = llvm::SanStat_CFI_UnrelatedCast;
     break;
   case CFITCK_ICall:
-    llvm_unreachable("not expecting CFITCK_ICall");
+  case CFITCK_NVMFCall:
+  case CFITCK_VMFCall:
+    llvm_unreachable("unexpected sanitizer kind");
   }
 
   std::string TypeName = RD->getQualifiedNameAsString();

lib/CodeGen/CGVTables.cpp

@@ -1012,30 +1012,29 @@ void CodeGenModule::EmitVTableTypeMetadata(llvm::GlobalVariable *VTable,
   CharUnits PointerWidth =
       Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
 
-  typedef std::pair<const CXXRecordDecl *, unsigned> TypeMetadata;
-  std::vector<TypeMetadata> TypeMetadatas;
-  // Create type metadata for each address point.
+  typedef std::pair<const CXXRecordDecl *, unsigned> AddressPoint;
+  std::vector<AddressPoint> AddressPoints;
   for (auto &&AP : VTLayout.getAddressPoints())
-    TypeMetadatas.push_back(std::make_pair(
+    AddressPoints.push_back(std::make_pair(
         AP.first.getBase(), VTLayout.getVTableOffset(AP.second.VTableIndex) +
                                 AP.second.AddressPointIndex));
 
-  // Sort the type metadata for determinism.
-  llvm::sort(TypeMetadatas.begin(), TypeMetadatas.end(),
-             [this](const TypeMetadata &M1, const TypeMetadata &M2) {
-    if (&M1 == &M2)
+  // Sort the address points for determinism.
+  llvm::sort(AddressPoints.begin(), AddressPoints.end(),
+             [this](const AddressPoint &AP1, const AddressPoint &AP2) {
+    if (&AP1 == &AP2)
       return false;
 
     std::string S1;
     llvm::raw_string_ostream O1(S1);
     getCXXABI().getMangleContext().mangleTypeName(
-        QualType(M1.first->getTypeForDecl(), 0), O1);
+        QualType(AP1.first->getTypeForDecl(), 0), O1);
     O1.flush();
 
     std::string S2;
     llvm::raw_string_ostream O2(S2);
     getCXXABI().getMangleContext().mangleTypeName(
-        QualType(M2.first->getTypeForDecl(), 0), O2);
+        QualType(AP2.first->getTypeForDecl(), 0), O2);
     O2.flush();
 
     if (S1 < S2)
@@ -1043,10 +1042,26 @@ void CodeGenModule::EmitVTableTypeMetadata(llvm::GlobalVariable *VTable,
     if (S1 != S2)
       return false;
 
-    return M1.second < M2.second;
+    return AP1.second < AP2.second;
   });
 
-  for (auto TypeMetadata : TypeMetadatas)
-    AddVTableTypeMetadata(VTable, PointerWidth * TypeMetadata.second,
-                          TypeMetadata.first);
+  ArrayRef<VTableComponent> Comps = VTLayout.vtable_components();
+  for (auto AP : AddressPoints) {
+    // Create type metadata for the address point.
+    AddVTableTypeMetadata(VTable, PointerWidth * AP.second, AP.first);
+
+    // The class associated with each address point could also potentially be
+    // used for indirect calls via a member function pointer, so we need to
+    // annotate the address of each function pointer with the appropriate member
+    // function pointer type.
+    for (unsigned I = 0; I != Comps.size(); ++I) {
+      if (Comps[I].getKind() != VTableComponent::CK_FunctionPointer)
+        continue;
+      llvm::Metadata *MD = CreateMetadataIdentifierForVirtualMemPtrType(
+          Context.getMemberPointerType(
+              Comps[I].getFunctionDecl()->getType(),
+              Context.getRecordType(AP.first).getTypePtr()));
+      VTable->addTypeMetadata((PointerWidth * I).getQuantity(), MD);
+    }
+  }
 }

lib/CodeGen/CodeGenFunction.h

@@ -1765,6 +1765,8 @@ public:
     CFITCK_DerivedCast,
     CFITCK_UnrelatedCast,
     CFITCK_ICall,
+    CFITCK_NVMFCall,
+    CFITCK_VMFCall,
   };
 
   /// Derived is the presumed address of an object of type T after a

lib/CodeGen/CodeGenModule.cpp

@@ -1132,6 +1132,34 @@ static bool hasUnwindExceptions(const LangOptions &LangOpts) {
   return true;
 }
 
+static bool requiresMemberFunctionPointerTypeMetadata(CodeGenModule &CGM,
+                                                      const CXXMethodDecl *MD) {
+  // Check that the type metadata can ever actually be used by a call.
+  if (!CGM.getCodeGenOpts().LTOUnit ||
+      !CGM.HasHiddenLTOVisibility(MD->getParent()))
+    return false;
+
+  // Only functions whose address can be taken with a member function pointer
+  // need this sort of type metadata.
+  return !MD->isStatic() && !MD->isVirtual() && !isa<CXXConstructorDecl>(MD) &&
+         !isa<CXXDestructorDecl>(MD);
+}
+
+std::vector<const CXXRecordDecl *>
+CodeGenModule::getMostBaseClasses(const CXXRecordDecl *RD) {
+  llvm::SetVector<const CXXRecordDecl *> MostBases;
+
+  std::function<void (const CXXRecordDecl *)> CollectMostBases;
+  CollectMostBases = [&](const CXXRecordDecl *RD) {
+    if (RD->getNumBases() == 0)
+      MostBases.insert(RD);
+    for (const CXXBaseSpecifier &B : RD->bases())
+      CollectMostBases(B.getType()->getAsCXXRecordDecl());
+  };
+  CollectMostBases(RD);
+  return MostBases.takeVector();
+}
+
 void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
                                                            llvm::Function *F) {
   llvm::AttrBuilder B;
@@ -1257,7 +1285,20 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D,
   // In the cross-dso CFI mode, we want !type attributes on definitions only.
   if (CodeGenOpts.SanitizeCfiCrossDso)
     if (auto *FD = dyn_cast<FunctionDecl>(D))
-      CreateFunctionTypeMetadata(FD, F);
+      CreateFunctionTypeMetadataForIcall(FD, F);
+
+  // Emit type metadata on member functions for member function pointer checks.
+  // These are only ever necessary on definitions; we're guaranteed that the
+  // definition will be present in the LTO unit as a result of LTO visibility.
+  auto *MD = dyn_cast<CXXMethodDecl>(D);
+  if (MD && requiresMemberFunctionPointerTypeMetadata(*this, MD)) {
+    for (const CXXRecordDecl *Base : getMostBaseClasses(MD->getParent())) {
+      llvm::Metadata *Id =
+          CreateMetadataIdentifierForType(Context.getMemberPointerType(
+              MD->getType(), Context.getRecordType(Base).getTypePtr()));
+      F->addTypeMetadata(0, Id);
+    }
+  }
 }
 
 void CodeGenModule::SetCommonAttributes(GlobalDecl GD, llvm::GlobalValue *GV) {
@@ -1378,13 +1419,14 @@ static void setLinkageForGV(llvm::GlobalValue *GV, const NamedDecl *ND) {
     GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage);
 }
 
-void CodeGenModule::CreateFunctionTypeMetadata(const FunctionDecl *FD,
-                                               llvm::Function *F) {
+void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
+                                                       llvm::Function *F) {
   // Only if we are checking indirect calls.
   if (!LangOpts.Sanitize.has(SanitizerKind::CFIICall))
     return;
 
-  // Non-static class methods are handled via vtable pointer checks elsewhere.
+  // Non-static class methods are handled via vtable or member function pointer
+  // checks elsewhere.
   if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic())
     return;
 
@@ -1476,7 +1518,7 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F,
   // Don't emit entries for function declarations in the cross-DSO mode. This
   // is handled with better precision by the receiving DSO.
   if (!CodeGenOpts.SanitizeCfiCrossDso)
-    CreateFunctionTypeMetadata(FD, F);
+    CreateFunctionTypeMetadataForIcall(FD, F);
 
   if (getLangOpts().OpenMP && FD->hasAttr<OMPDeclareSimdDeclAttr>())
     getOpenMPRuntime().emitDeclareSimdFunction(FD, F);
@@ -4925,8 +4967,10 @@ void CodeGenModule::EmitOMPThreadPrivateDecl(const OMPThreadPrivateDecl *D) {
   }
 }
 
-llvm::Metadata *CodeGenModule::CreateMetadataIdentifierForType(QualType T) {
-  llvm::Metadata *&InternalId = MetadataIdMap[T.getCanonicalType()];
+llvm::Metadata *
+CodeGenModule::CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap &Map,
+                                            StringRef Suffix) {
+  llvm::Metadata *&InternalId = Map[T.getCanonicalType()];
   if (InternalId)
     return InternalId;
 
@@ -4934,6 +4978,7 @@ llvm::Metadata *CodeGenModule::CreateMetadataIdentifierForType(QualType T) {
     std::string OutName;
     llvm::raw_string_ostream Out(OutName);
     getCXXABI().getMangleContext().mangleTypeName(T, Out);
+    Out << Suffix;
 
     InternalId = llvm::MDString::get(getLLVMContext(), Out.str());
   } else {
@@ -4944,6 +4989,15 @@ llvm::Metadata *CodeGenModule::CreateMetadataIdentifierForType(QualType T) {
   return InternalId;
 }
 
+llvm::Metadata *CodeGenModule::CreateMetadataIdentifierForType(QualType T) {
+  return CreateMetadataIdentifierImpl(T, MetadataIdMap, "");
+}
+
+llvm::Metadata *
+CodeGenModule::CreateMetadataIdentifierForVirtualMemPtrType(QualType T) {
+  return CreateMetadataIdentifierImpl(T, VirtualMetadataIdMap, ".virtual");
+}
+
 // Generalize pointer types to a void pointer with the qualifiers of the
 // originally pointed-to type, e.g. 'const char *' and 'char * const *'
 // generalize to 'const void *' while 'char *' and 'const char **' generalize to
@@ -4977,25 +5031,8 @@ static QualType GeneralizeFunctionType(ASTContext &Ctx, QualType Ty) {
 }
 
 llvm::Metadata *CodeGenModule::CreateMetadataIdentifierGeneralized(QualType T) {
-  T = GeneralizeFunctionType(getContext(), T);
-
-  llvm::Metadata *&InternalId = GeneralizedMetadataIdMap[T.getCanonicalType()];
-  if (InternalId)
-    return InternalId;
-
-  if (isExternallyVisible(T->getLinkage())) {
-    std::string OutName;
-    llvm::raw_string_ostream Out(OutName);
-    getCXXABI().getMangleContext().mangleTypeName(T, Out);
-    Out << ".generalized";
-
-    InternalId = llvm::MDString::get(getLLVMContext(), Out.str());
-  } else {
-    InternalId = llvm::MDNode::getDistinct(getLLVMContext(),
-                                           llvm::ArrayRef<llvm::Metadata *>());
-  }
-
-  return InternalId;
+  return CreateMetadataIdentifierImpl(GeneralizeFunctionType(getContext(), T),
+                                      GeneralizedMetadataIdMap, ".generalized");
 }
 
 /// Returns whether this module needs the "all-vtables" type identifier.

lib/CodeGen/CodeGenModule.h

@@ -503,6 +503,7 @@ private:
   /// MDNodes.
   typedef llvm::DenseMap<QualType, llvm::Metadata *> MetadataTypeMap;
   MetadataTypeMap MetadataIdMap;
+  MetadataTypeMap VirtualMetadataIdMap;
   MetadataTypeMap GeneralizedMetadataIdMap;
 
 public:
@@ -1232,13 +1233,18 @@ public:
   /// internal identifiers).
   llvm::Metadata *CreateMetadataIdentifierForType(QualType T);
 
+  /// Create a metadata identifier that is intended to be used to check virtual
+  /// calls via a member function pointer.
+  llvm::Metadata *CreateMetadataIdentifierForVirtualMemPtrType(QualType T);
+
   /// Create a metadata identifier for the generalization of the given type.
   /// This may either be an MDString (for external identifiers) or a distinct
   /// unnamed MDNode (for internal identifiers).
   llvm::Metadata *CreateMetadataIdentifierGeneralized(QualType T);
 
   /// Create and attach type metadata to the given function.
-  void CreateFunctionTypeMetadata(const FunctionDecl *FD, llvm::Function *F);
+  void CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD,
+                                          llvm::Function *F);
 
   /// Returns whether this module needs the "all-vtables" type identifier.
   bool NeedAllVtablesTypeId() const;
@@ -1247,6 +1253,14 @@ public:
   void AddVTableTypeMetadata(llvm::GlobalVariable *VTable, CharUnits Offset,
                              const CXXRecordDecl *RD);
 
+  /// Return a vector of most-base classes for RD. This is used to implement
+  /// control flow integrity checks for member function pointers.
+  ///
+  /// A most-base class of a class C is defined as a recursive base class of C,
+  /// including C itself, that does not have any bases.
+  std::vector<const CXXRecordDecl *>
+  getMostBaseClasses(const CXXRecordDecl *RD);
+
   /// Get the declaration of std::terminate for the platform.
   llvm::Constant *getTerminateFn();
 
@@ -1408,6 +1422,9 @@ private:
   void ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
                                   bool AttrOnCallSite,
                                   llvm::AttrBuilder &FuncAttrs);
+
+  llvm::Metadata *CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap &Map,
+                                               StringRef Suffix);
 };
 
 }  // end namespace CodeGen

lib/CodeGen/ItaniumCXXABI.cpp

@@ -622,13 +622,53 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(
     VTableOffset = Builder.CreateTrunc(VTableOffset, CGF.Int32Ty);
     VTableOffset = Builder.CreateZExt(VTableOffset, CGM.PtrDiffTy);
   }
-  VTable = Builder.CreateGEP(VTable, VTableOffset);
+  // Compute the address of the virtual function pointer.
+  llvm::Value *VFPAddr = Builder.CreateGEP(VTable, VTableOffset);
+
+  // Check the address of the function pointer if CFI on member function
+  // pointers is enabled.
+  llvm::Constant *CheckSourceLocation;
+  llvm::Constant *CheckTypeDesc;
+  bool ShouldEmitCFICheck = CGF.SanOpts.has(SanitizerKind::CFIMFCall) &&
+                            CGM.HasHiddenLTOVisibility(RD);
+  if (ShouldEmitCFICheck) {
+    CodeGenFunction::SanitizerScope SanScope(&CGF);
+
+    CheckSourceLocation = CGF.EmitCheckSourceLocation(E->getLocStart());
+    CheckTypeDesc = CGF.EmitCheckTypeDescriptor(QualType(MPT, 0));
+    llvm::Constant *StaticData[] = {
+        llvm::ConstantInt::get(CGF.Int8Ty, CodeGenFunction::CFITCK_VMFCall),
+        CheckSourceLocation,
+        CheckTypeDesc,
+    };
+
+    llvm::Metadata *MD =
+        CGM.CreateMetadataIdentifierForVirtualMemPtrType(QualType(MPT, 0));
+    llvm::Value *TypeId = llvm::MetadataAsValue::get(CGF.getLLVMContext(), MD);
+
+    llvm::Value *TypeTest = Builder.CreateCall(
+        CGM.getIntrinsic(llvm::Intrinsic::type_test), {VFPAddr, TypeId});
+
+    if (CGM.getCodeGenOpts().SanitizeTrap.has(SanitizerKind::CFIMFCall)) {
+      CGF.EmitTrapCheck(TypeTest);
+    } else {
+      llvm::Value *AllVtables = llvm::MetadataAsValue::get(
+          CGM.getLLVMContext(),
+          llvm::MDString::get(CGM.getLLVMContext(), "all-vtables"));
+      llvm::Value *ValidVtable = Builder.CreateCall(
+          CGM.getIntrinsic(llvm::Intrinsic::type_test), {VTable, AllVtables});
+      CGF.EmitCheck(std::make_pair(TypeTest, SanitizerKind::CFIMFCall),
+                    SanitizerHandler::CFICheckFail, StaticData,
+                    {VTable, ValidVtable});
+    }
+
+    FnVirtual = Builder.GetInsertBlock();
+  }
 
   // Load the virtual function to call.
-  VTable = Builder.CreateBitCast(VTable, FTy->getPointerTo()->getPointerTo());
-  llvm::Value *VirtualFn =
-    Builder.CreateAlignedLoad(VTable, CGF.getPointerAlign(),
-                              "memptr.virtualfn");
+  VFPAddr = Builder.CreateBitCast(VFPAddr, FTy->getPointerTo()->getPointerTo());
+  llvm::Value *VirtualFn = Builder.CreateAlignedLoad(
+      VFPAddr, CGF.getPointerAlign(), "memptr.virtualfn");
   CGF.EmitBranch(FnEnd);
 
   // In the non-virtual path, the function pointer is actually a
@@ -637,6 +677,43 @@ CGCallee ItaniumCXXABI::EmitLoadOfMemberFunctionPointer(
   llvm::Value *NonVirtualFn =
     Builder.CreateIntToPtr(FnAsInt, FTy->getPointerTo(), "memptr.nonvirtualfn");
 
+  // Check the function pointer if CFI on member function pointers is enabled.
+  if (ShouldEmitCFICheck) {
+    CXXRecordDecl *RD = MPT->getClass()->getAsCXXRecordDecl();
+    if (RD->hasDefinition()) {
+      CodeGenFunction::SanitizerScope SanScope(&CGF);
+
+      llvm::Constant *StaticData[] = {
+          llvm::ConstantInt::get(CGF.Int8Ty, CodeGenFunction::CFITCK_NVMFCall),
+          CheckSourceLocation,
+          CheckTypeDesc,
+      };
+
+      llvm::Value *Bit = Builder.getFalse();
+      llvm::Value *CastedNonVirtualFn =
+          Builder.CreateBitCast(NonVirtualFn, CGF.Int8PtrTy);
+      for (const CXXRecordDecl *Base : CGM.getMostBaseClasses(RD)) {
+        llvm::Metadata *MD = CGM.CreateMetadataIdentifierForType(
+            getContext().getMemberPointerType(
+                MPT->getPointeeType(),
+                getContext().getRecordType(Base).getTypePtr()));
+        llvm::Value *TypeId =
+            llvm::MetadataAsValue::get(CGF.getLLVMContext(), MD);
+
+        llvm::Value *TypeTest =
+            Builder.CreateCall(CGM.getIntrinsic(llvm::Intrinsic::type_test),
+                               {CastedNonVirtualFn, TypeId});
+        Bit = Builder.CreateOr(Bit, TypeTest);
+      }
+
+      CGF.EmitCheck(std::make_pair(Bit, SanitizerKind::CFIMFCall),
+                    SanitizerHandler::CFICheckFail, StaticData,
+                    {CastedNonVirtualFn, llvm::UndefValue::get(CGF.IntPtrTy)});
+
+      FnNonVirtual = Builder.GetInsertBlock();
+    }
+  }
+
   // We're done.
   CGF.EmitBlock(FnEnd);
   llvm::PHINode *CalleePtr = Builder.CreatePHI(FTy->getPointerTo(), 2);

lib/Driver/SanitizerArgs.cpp

@@ -44,7 +44,8 @@ enum : SanitizerMask {
   TrappingSupported = (Undefined & ~Vptr) | UnsignedIntegerOverflow |
                       Nullability | LocalBounds | CFI,
   TrappingDefault = CFI,
-  CFIClasses = CFIVCall | CFINVCall | CFIDerivedCast | CFIUnrelatedCast,
+  CFIClasses =
+      CFIVCall | CFINVCall | CFIMFCall | CFIDerivedCast | CFIUnrelatedCast,
   CompatibleWithMinimalRuntime = TrappingSupported | Scudo,
 };
 
@@ -219,6 +220,10 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
                                      // Used to deduplicate diagnostics.
   SanitizerMask Kinds = 0;
   const SanitizerMask Supported = setGroupBits(TC.getSupportedSanitizers());
+
+  CfiCrossDso = Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso,
+                             options::OPT_fno_sanitize_cfi_cross_dso, false);
+
   ToolChain::RTTIMode RTTIMode = TC.getRTTIMode();
 
   const Driver &D = TC.getDriver();
@@ -278,6 +283,24 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
         Add &= ~NotAllowedWithMinimalRuntime;
       }
 
+      // FIXME: Make CFI on member function calls compatible with cross-DSO CFI.
+      // There are currently two problems:
+      // - Virtual function call checks need to pass a pointer to the function
+      //   address to llvm.type.test and a pointer to the address point to the
+      //   diagnostic function. Currently we pass the same pointer to both
+      //   places.
+      // - Non-virtual function call checks may need to check multiple type
+      //   identifiers.
+      // Fixing both of those may require changes to the cross-DSO CFI
+      // interface.
+      if (CfiCrossDso && (Add & CFIMFCall & ~DiagnosedKinds)) {
+        D.Diag(diag::err_drv_argument_not_allowed_with)
+            << "-fsanitize=cfi-mfcall"
+            << "-fsanitize-cfi-cross-dso";
+        Add &= ~CFIMFCall;
+        DiagnosedKinds |= CFIMFCall;
+      }
+
       if (SanitizerMask KindsToDiagnose = Add & ~Supported & ~DiagnosedKinds) {
         std::string Desc = describeSanitizeArg(*I, KindsToDiagnose);
         D.Diag(diag::err_drv_unsupported_opt_for_target)
@@ -316,6 +339,8 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
       if (MinimalRuntime) {
         Add &= ~NotAllowedWithMinimalRuntime;
       }
+      if (CfiCrossDso)
+        Add &= ~CFIMFCall;
       Add &= Supported;
 
       if (Add & Fuzzer)
@@ -554,8 +579,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
   }
 
   if (AllAddedKinds & CFI) {
-    CfiCrossDso = Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso,
-                               options::OPT_fno_sanitize_cfi_cross_dso, false);
     // Without PIE, external function address may resolve to a PLT record, which
     // can not be verified by the target module.
     NeedPIE |= CfiCrossDso;

lib/Driver/ToolChains/MSVC.cpp

@@ -1297,6 +1297,7 @@ MSVCToolChain::ComputeEffectiveClangTriple(const ArgList &Args,
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }
 

test/CodeGenCXX/cfi-mfcall-incomplete.cpp

@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-mfcall -fsanitize-trap=cfi-mfcall -fvisibility hidden -emit-llvm -o - %s | FileCheck %s
+
+struct S;
+
+void f(S *s, void (S::*p)()) {
+  // CHECK-NOT: llvm.type.test
+  // CHECK: llvm.type.test{{.*}}!"_ZTSM1SFvvE.virtual"
+  // CHECK-NOT: llvm.type.test
+  (s->*p)();
+}
+
+// CHECK: declare i1 @llvm.type.test

test/CodeGenCXX/cfi-mfcall.cpp

@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-mfcall -fsanitize-trap=cfi-mfcall -fvisibility hidden -emit-llvm -o - %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-mfcall -fsanitize-trap=cfi-mfcall -fvisibility default -emit-llvm -o - %s | FileCheck --check-prefix=DEFAULT %s
+
+struct B1 {};
+struct B2 {};
+struct B3 : B2 {};
+struct S : B1, B3 {};
+
+// DEFAULT-NOT: llvm.type.test
+
+void f(S *s, void (S::*p)()) {
+  // CHECK: [[OFFSET:%.*]] = sub i64 {{.*}}, 1
+  // CHECK: [[VFPTR:%.*]] = getelementptr i8, i8* %{{.*}}, i64 [[OFFSET]]
+  // CHECK: [[TT:%.*]] = call i1 @llvm.type.test(i8* [[VFPTR]], metadata !"_ZTSM1SFvvE.virtual")
+  // CHECK: br i1 [[TT]], label {{.*}}, label %[[TRAP1:[^,]*]]
+
+  // CHECK: [[TRAP1]]:
+  // CHECK-NEXT: llvm.trap
+
+  // CHECK: [[NVFPTR:%.*]] = bitcast void (%struct.S*)* {{.*}} to i8*
+  // CHECK: [[TT1:%.*]] = call i1 @llvm.type.test(i8* [[NVFPTR]], metadata !"_ZTSM2B1FvvE")
+  // CHECK: [[OR1:%.*]] = or i1 false, [[TT1]]
+  // CHECK: [[TT2:%.*]] = call i1 @llvm.type.test(i8* [[NVFPTR]], metadata !"_ZTSM2B2FvvE")
+  // CHECK: [[OR2:%.*]] = or i1 [[OR1]], [[TT2]]
+  // CHECK: br i1 [[OR2]], label {{.*}}, label %[[TRAP2:[^,]*]]
+
+  // CHECK: [[TRAP2]]:
+  // CHECK-NEXT: llvm.trap
+  (s->*p)();
+}

test/CodeGenCXX/type-metadata-memfun.cpp

@@ -0,0 +1,31 @@
+// RUN: %clang_cc1 -flto -flto-unit -triple x86_64-unknown-linux -fvisibility hidden -emit-llvm -o - %s | FileCheck %s
+
+struct S1 {
+  S1();
+  ~S1();
+  virtual void vf();
+  void f();
+  void fdecl();
+};
+
+struct [[clang::lto_visibility_public]] S2 {
+  void f();
+};
+
+// CHECK-NOT: declare{{.*}}!type
+// CHECK-NOT: define{{.*}}!type
+
+S1::S1() {}
+S1::~S1() {}
+void S1::vf() {}
+// CHECK: define hidden void @_ZN2S11fEv{{.*}} !type [[S2F:![0-9]+]]
+void S1::f() {
+  fdecl();
+}
+
+void S2::f() {}
+
+// CHECK-NOT: declare{{.*}}!type
+// CHECK-NOT: define{{.*}}!type
+
+// CHECK: [[S2F]] = !{i64 0, !"_ZTSM2S1FvvE"}

test/CodeGenCXX/type-metadata.cpp

@@ -14,16 +14,25 @@
 
 // ITANIUM: @_ZTV1A = {{[^!]*}}, !type [[A16:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL16:![0-9]+]]
+// ITANIUM-SAME: !type [[AF16:![0-9]+]]
 
 // ITANIUM: @_ZTV1B = {{[^!]*}}, !type [[A32:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL32:![0-9]+]]
+// ITANIUM-SAME: !type [[AF32:![0-9]+]]
+// ITANIUM-SAME: !type [[AF40:![0-9]+]]
+// ITANIUM-SAME: !type [[AF48:![0-9]+]]
 // ITANIUM-SAME: !type [[B32:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[BF32:![0-9]+]]
+// ITANIUM-SAME: !type [[BF40:![0-9]+]]
+// ITANIUM-SAME: !type [[BF48:![0-9]+]]
 
 // ITANIUM: @_ZTV1C = {{[^!]*}}, !type [[A32]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[AF32]]
 // ITANIUM-SAME: !type [[C32:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[CF32:![0-9]+]]
 
 // DIAG: @[[SRC:.*]] = private unnamed_addr constant [{{.*}} x i8] c"{{.*}}type-metadata.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" }
@@ -31,12 +40,24 @@
 
 // ITANIUM: @_ZTVN12_GLOBAL__N_11DE = {{[^!]*}}, !type [[A32]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[AF32]]
+// ITANIUM-SAME: !type [[AF40]]
+// ITANIUM-SAME: !type [[AF48]]
 // ITANIUM-SAME: !type [[B32]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[BF32]]
+// ITANIUM-SAME: !type [[BF40]]
+// ITANIUM-SAME: !type [[BF48]]
 // ITANIUM-SAME: !type [[C88:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL88:![0-9]+]]
+// ITANIUM-SAME: !type [[CF32]]
+// ITANIUM-SAME: !type [[CF40:![0-9]+]]
+// ITANIUM-SAME: !type [[CF48:![0-9]+]]
 // ITANIUM-SAME: !type [[D32:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[DF32:![0-9]+]]
+// ITANIUM-SAME: !type [[DF40:![0-9]+]]
+// ITANIUM-SAME: !type [[DF48:![0-9]+]]
 
 // ITANIUM: @_ZTCN12_GLOBAL__N_11DE0_1B = {{[^!]*}}, !type [[A32]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
@@ -45,13 +66,17 @@
 
 // ITANIUM: @_ZTCN12_GLOBAL__N_11DE8_1C = {{[^!]*}}, !type [[A64:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL64:![0-9]+]]
+// ITANIUM-SAME: !type [[AF64:![0-9]+]]
 // ITANIUM-SAME: !type [[C32]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
+// ITANIUM-SAME: !type [[CF64:![0-9]+]]
 
 // ITANIUM: @_ZTVZ3foovE2FA = {{[^!]*}}, !type [[A16]]
 // ITANIUM-DIAG-SAME: !type [[ALL16]]
+// ITANIUM-SAME: !type [[AF16]]
 // ITANIUM-SAME: !type [[FA16:![0-9]+]]
 // ITANIUM-DIAG-SAME: !type [[ALL16]]
+// ITANIUM-SAME: !type [[FAF16:![0-9]+]]
 
 // MS: comdat($"??_7A@@6B@"), !type [[A8:![0-9]+]]
 // MS: comdat($"??_7B@@6B0@@"), !type [[B8:![0-9]+]]
@@ -227,18 +252,36 @@ void f(D *d) {
 
 // ITANIUM: [[A16]] = !{i64 16, !"_ZTS1A"}
 // ITANIUM-DIAG: [[ALL16]] = !{i64 16, !"all-vtables"}
+// ITANIUM: [[AF16]] = !{i64 16, !"_ZTSM1AFvvE.virtual"}
 // ITANIUM: [[A32]] = !{i64 32, !"_ZTS1A"}
 // ITANIUM-DIAG: [[ALL32]] = !{i64 32, !"all-vtables"}
+// ITANIUM: [[AF32]] = !{i64 32, !"_ZTSM1AFvvE.virtual"}
+// ITANIUM: [[AF40]] = !{i64 40, !"_ZTSM1AFvvE.virtual"}
+// ITANIUM: [[AF48]] = !{i64 48, !"_ZTSM1AFvvE.virtual"}
 // ITANIUM: [[B32]] = !{i64 32, !"_ZTS1B"}
+// ITANIUM: [[BF32]] = !{i64 32, !"_ZTSM1BFvvE.virtual"}
+// ITANIUM: [[BF40]] = !{i64 40, !"_ZTSM1BFvvE.virtual"}
+// ITANIUM: [[BF48]] = !{i64 48, !"_ZTSM1BFvvE.virtual"}
 // ITANIUM: [[C32]] = !{i64 32, !"_ZTS1C"}
+// ITANIUM: [[CF32]] = !{i64 32, !"_ZTSM1CFvvE.virtual"}
 // ITANIUM: [[C88]] = !{i64 88, !"_ZTS1C"}
 // ITANIUM-DIAG: [[ALL88]] = !{i64 88, !"all-vtables"}
+// ITANIUM: [[CF40]] = !{i64 40, !"_ZTSM1CFvvE.virtual"}
+// ITANIUM: [[CF48]] = !{i64 48, !"_ZTSM1CFvvE.virtual"}
 // ITANIUM: [[D32]] = !{i64 32, [[D_ID:![0-9]+]]}
 // ITANIUM: [[D_ID]] = distinct !{}
+// ITANIUM: [[DF32]] = !{i64 32, [[DF_ID:![0-9]+]]}
+// ITANIUM: [[DF_ID]] = distinct !{}
+// ITANIUM: [[DF40]] = !{i64 40, [[DF_ID]]}
+// ITANIUM: [[DF48]] = !{i64 48, [[DF_ID]]}
 // ITANIUM: [[A64]] = !{i64 64, !"_ZTS1A"}
 // ITANIUM-DIAG: [[ALL64]] = !{i64 64, !"all-vtables"}
+// ITANIUM: [[AF64]] = !{i64 64, !"_ZTSM1AFvvE.virtual"}
+// ITANIUM: [[CF64]] = !{i64 64, !"_ZTSM1CFvvE.virtual"}
 // ITANIUM: [[FA16]] = !{i64 16, [[FA_ID:![0-9]+]]}
 // ITANIUM: [[FA_ID]] = distinct !{}
+// ITANIUM: [[FAF16]] = !{i64 16, [[FAF_ID:![0-9]+]]}
+// ITANIUM: [[FAF_ID]] = distinct !{}
 
 // MS: [[A8]] = !{i64 8, !"?AUA@@"}
 // MS: [[B8]] = !{i64 8, !"?AUB@@"}

test/Driver/fsanitize.c

@@ -476,6 +476,8 @@
 
 // RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target x86_64-apple-darwin10 -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
+// RUN: %clang -target x86_64-pc-win32 -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOMFCALL
+// RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi -fsanitize-cfi-cross-dso -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOMFCALL
 // RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi-derived-cast -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-DCAST
 // RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi-unrelated-cast -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-UCAST
 // RUN: %clang -target x86_64-linux-gnu -flto -fvisibility=hidden -fsanitize=cfi-nvcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NVCALL
@@ -484,7 +486,8 @@
 // RUN: %clang -target aarch64-linux-gnu -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target arm-linux-android -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // RUN: %clang -target aarch64-linux-android -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
-// CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
+// CHECK-CFI: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
+// CHECK-CFI-NOMFCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall
 // CHECK-CFI-DCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-derived-cast
 // CHECK-CFI-UCAST: -emit-llvm-bc{{.*}}-fsanitize=cfi-unrelated-cast
 // CHECK-CFI-NVCALL: -emit-llvm-bc{{.*}}-fsanitize=cfi-nvcall
@@ -526,6 +529,9 @@
 // CHECK-CFI-NO-CROSS-DSO: -emit-llvm-bc
 // CHECK-CFI-NO-CROSS-DSO-NOT: -fsanitize-cfi-cross-dso
 
+// RUN: %clang -target x86_64-linux-gnu -fvisibility=hidden -fsanitize=cfi-mfcall -fsanitize-cfi-cross-dso -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-MFCALL-CROSS-DSO
+// CHECK-CFI-MFCALL-CROSS-DSO: '-fsanitize=cfi-mfcall' not allowed with '-fsanitize-cfi-cross-dso'
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fsanitize-cfi-icall-generalize-pointers -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-GENERALIZE-POINTERS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-CFI-GENERALIZE-POINTERS
 // CHECK-CFI-GENERALIZE-POINTERS: -fsanitize-cfi-icall-generalize-pointers
@@ -644,16 +650,16 @@
 // CHECK-HWASAN-MINIMAL: error: invalid argument '-fsanitize-minimal-runtime' not allowed with '-fsanitize=hwaddress'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -flto -fvisibility=hidden -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-MINIMAL
-// CHECK-CFI-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
-// CHECK-CFI-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-icall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
+// CHECK-CFI-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
+// CHECK-CFI-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-icall,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-MINIMAL: "-fsanitize-minimal-runtime"
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fno-sanitize-trap=cfi-icall -flto -fvisibility=hidden -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOTRAP-MINIMAL
 // CHECK-CFI-NOTRAP-MINIMAL: error: invalid argument 'fsanitize-minimal-runtime' only allowed with 'fsanitize-trap=cfi'
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fno-sanitize-trap=cfi-icall -fno-sanitize=cfi-icall -flto -fvisibility=hidden -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-NOICALL-MINIMAL
-// CHECK-CFI-NOICALL-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
-// CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
+// CHECK-CFI-NOICALL-MINIMAL: "-fsanitize=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
+// CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-trap=cfi-derived-cast,cfi-mfcall,cfi-unrelated-cast,cfi-nvcall,cfi-vcall"
 // CHECK-CFI-NOICALL-MINIMAL: "-fsanitize-minimal-runtime"
 
 // RUN: %clang -target aarch64-linux-gnu -fsanitize=scudo %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SCUDO