This revision was automatically updated to reflect the committed changes.
Closed by commit rC335569: Implement CFI for indirect calls via a member 
function pointer. (authored by pcc, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47567?vs=151272&id=152819#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47567

Files:
  docs/ControlFlowIntegrity.rst
  docs/LTOVisibility.rst
  include/clang/Basic/Sanitizers.def
  lib/CodeGen/CGClass.cpp
  lib/CodeGen/CGVTables.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.cpp
  lib/CodeGen/CodeGenModule.h
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Driver/SanitizerArgs.cpp
  lib/Driver/ToolChains/MSVC.cpp
  test/CodeGenCXX/cfi-mfcall-incomplete.cpp
  test/CodeGenCXX/cfi-mfcall.cpp
  test/CodeGenCXX/type-metadata-memfun.cpp
  test/CodeGenCXX/type-metadata.cpp
  test/Driver/fsanitize.c

Index: lib/CodeGen/CGClass.cpp
===================================================================
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -2688,7 +2688,9 @@
     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();
Index: lib/CodeGen/CodeGenModule.cpp
===================================================================
--- lib/CodeGen/CodeGenModule.cpp
+++ lib/CodeGen/CodeGenModule.cpp
@@ -1132,6 +1132,34 @@
   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 @@
   // 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 @@
     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 @@
   // 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,15 +4967,18 @@
   }
 }
 
-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;
 
   if (isExternallyVisible(T->getLinkage())) {
     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 @@
   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 @@
 }
 
 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.
Index: lib/CodeGen/CodeGenModule.h
===================================================================
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -503,6 +503,7 @@
   /// MDNodes.
   typedef llvm::DenseMap<QualType, llvm::Metadata *> MetadataTypeMap;
   MetadataTypeMap MetadataIdMap;
+  MetadataTypeMap VirtualMetadataIdMap;
   MetadataTypeMap GeneralizedMetadataIdMap;
 
 public:
@@ -1232,21 +1233,34 @@
   /// 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;
 
   /// Create and attach type metadata for the given vtable.
   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 @@
   void ConstructDefaultFnAttrList(StringRef Name, bool HasOptnone,
                                   bool AttrOnCallSite,
                                   llvm::AttrBuilder &FuncAttrs);
+
+  llvm::Metadata *CreateMetadataIdentifierImpl(QualType T, MetadataTypeMap &Map,
+                                               StringRef Suffix);
 };
 
 }  // end namespace CodeGen
Index: lib/CodeGen/CodeGenFunction.h
===================================================================
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -1765,6 +1765,8 @@
     CFITCK_DerivedCast,
     CFITCK_UnrelatedCast,
     CFITCK_ICall,
+    CFITCK_NVMFCall,
+    CFITCK_VMFCall,
   };
 
   /// Derived is the presumed address of an object of type T after a
Index: lib/CodeGen/CGVTables.cpp
===================================================================
--- lib/CodeGen/CGVTables.cpp
+++ lib/CodeGen/CGVTables.cpp
@@ -1012,41 +1012,56 @@
   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)
       return true;
     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);
+    }
+  }
 }
Index: lib/CodeGen/ItaniumCXXABI.cpp
===================================================================
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -622,21 +622,98 @@
     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
   // function pointer.
   CGF.EmitBlock(FnNonVirtual);
   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);
Index: lib/Driver/SanitizerArgs.cpp
===================================================================
--- lib/Driver/SanitizerArgs.cpp
+++ lib/Driver/SanitizerArgs.cpp
@@ -44,7 +44,8 @@
   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 @@
                                      // 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 @@
         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 @@
       if (MinimalRuntime) {
         Add &= ~NotAllowedWithMinimalRuntime;
       }
+      if (CfiCrossDso)
+        Add &= ~CFIMFCall;
       Add &= Supported;
 
       if (Add & Fuzzer)
@@ -554,8 +579,6 @@
   }
 
   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;
Index: lib/Driver/ToolChains/MSVC.cpp
===================================================================
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -1297,6 +1297,7 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }
 
Index: docs/ControlFlowIntegrity.rst
===================================================================
--- docs/ControlFlowIntegrity.rst
+++ docs/ControlFlowIntegrity.rst
@@ -66,6 +66,8 @@
      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 @@
 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
Index: docs/LTOVisibility.rst
===================================================================
--- docs/LTOVisibility.rst
+++ docs/LTOVisibility.rst
@@ -11,9 +11,9 @@
 
 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
Index: include/clang/Basic/Sanitizers.def
===================================================================
--- include/clang/Basic/Sanitizers.def
+++ include/clang/Basic/Sanitizers.def
@@ -104,12 +104,13 @@
 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)
Index: test/CodeGenCXX/cfi-mfcall.cpp
===================================================================
--- test/CodeGenCXX/cfi-mfcall.cpp
+++ 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)();
+}
Index: test/CodeGenCXX/type-metadata-memfun.cpp
===================================================================
--- test/CodeGenCXX/type-metadata-memfun.cpp
+++ 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"}
Index: test/CodeGenCXX/type-metadata.cpp
===================================================================
--- test/CodeGenCXX/type-metadata.cpp
+++ test/CodeGenCXX/type-metadata.cpp
@@ -14,44 +14,69 @@
 
 // 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" }
 // DIAG: @[[BADTYPESTATIC:.*]] = private unnamed_addr global { i8, { [{{.*}} x i8]*, i32, i32 }, { i16, i16, [4 x i8] }* } { i8 0, { [{{.*}} x i8]*, i32, i32 } { [{{.*}} x i8]* @[[SRC]], i32 123, i32 3 }, { i16, i16, [4 x i8] }* @[[TYPE]] }
 
 // 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]]
 // ITANIUM-SAME: !type [[B32]]
 // ITANIUM-DIAG-SAME: !type [[ALL32]]
 
 // 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 @@
 
 // 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@@"}
Index: test/CodeGenCXX/cfi-mfcall-incomplete.cpp
===================================================================
--- test/CodeGenCXX/cfi-mfcall-incomplete.cpp
+++ 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
Index: test/Driver/fsanitize.c
===================================================================
--- test/Driver/fsanitize.c
+++ test/Driver/fsanitize.c
@@ -476,15 +476,18 @@
 
 // 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
 // RUN: %clang -target x86_64-linux-gnu -flto -fvisibility=hidden -fsanitize=cfi-vcall -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-VCALL
 // RUN: %clang -target arm-linux-gnu -fvisibility=hidden -fsanitize=cfi -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI
 // 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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to