Index: clang/docs/ControlFlowIntegrity.rst =================================================================== --- clang/docs/ControlFlowIntegrity.rst +++ clang/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 ` types or types with public :doc:`LTO +visibility `, 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: clang/docs/LTOVisibility.rst =================================================================== --- clang/docs/LTOVisibility.rst +++ clang/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: clang/include/clang/Basic/Sanitizers.def =================================================================== --- clang/include/clang/Basic/Sanitizers.def +++ clang/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: clang/lib/CodeGen/CGClass.cpp =================================================================== --- clang/lib/CodeGen/CGClass.cpp +++ clang/lib/CodeGen/CGClass.cpp @@ -2687,7 +2687,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: clang/lib/CodeGen/CGVTables.cpp =================================================================== --- clang/lib/CodeGen/CGVTables.cpp +++ clang/lib/CodeGen/CGVTables.cpp @@ -1012,30 +1012,29 @@ CharUnits PointerWidth = Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0)); - typedef std::pair TypeMetadata; - std::vector TypeMetadatas; - // Create type metadata for each address point. + typedef std::pair AddressPoint; + std::vector 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 @@ 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 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: clang/lib/CodeGen/CodeGenFunction.h =================================================================== --- clang/lib/CodeGen/CodeGenFunction.h +++ clang/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: clang/lib/CodeGen/CodeGenModule.h =================================================================== --- clang/lib/CodeGen/CodeGenModule.h +++ clang/lib/CodeGen/CodeGenModule.h @@ -503,6 +503,7 @@ /// MDNodes. typedef llvm::DenseMap MetadataTypeMap; MetadataTypeMap MetadataIdMap; + MetadataTypeMap VirtualMetadataIdMap; MetadataTypeMap GeneralizedMetadataIdMap; public: @@ -1233,6 +1234,10 @@ /// 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). @@ -1248,6 +1253,9 @@ void AddVTableTypeMetadata(llvm::GlobalVariable *VTable, CharUnits Offset, const CXXRecordDecl *RD); + std::vector + getMostBaseClasses(const CXXRecordDecl *RD); + /// Get the declaration of std::terminate for the platform. llvm::Constant *getTerminateFn(); @@ -1409,6 +1417,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: clang/lib/CodeGen/CodeGenModule.cpp =================================================================== --- clang/lib/CodeGen/CodeGenModule.cpp +++ clang/lib/CodeGen/CodeGenModule.cpp @@ -1378,14 +1378,45 @@ GV->setLinkage(llvm::GlobalValue::ExternalWeakLinkage); } +std::vector +CodeGenModule::getMostBaseClasses(const CXXRecordDecl *RD) { + llvm::SetVector MostBases; + + std::function 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::CreateFunctionTypeMetadata(const FunctionDecl *FD, llvm::Function *F) { - // Only if we are checking indirect calls. - if (!LangOpts.Sanitize.has(SanitizerKind::CFIICall)) + auto *MD = dyn_cast(FD); + if (MD && !MD->isStatic()) { + if (!LangOpts.Sanitize.has(SanitizerKind::CFIMFCall)) + return; + + // Only functions whose address can be taken with a member function pointer + // need type metadata. + if (MD->isVirtual() || isa(MD) || + isa(MD)) + return; + + for (const CXXRecordDecl *Base : getMostBaseClasses(MD->getParent())) { + llvm::Metadata *Id = + CreateMetadataIdentifierForType(Context.getMemberPointerType( + FD->getType(), Context.getRecordType(Base).getTypePtr())); + F->addTypeMetadata(0, Id); + } return; + } - // Non-static class methods are handled via vtable pointer checks elsewhere. - if (isa(FD) && !cast(FD)->isStatic()) + // Only if we are checking indirect calls. + if (!LangOpts.Sanitize.has(SanitizerKind::CFIICall)) return; // Additionally, if building with cross-DSO support... @@ -1396,13 +1427,13 @@ return; } - llvm::Metadata *MD = CreateMetadataIdentifierForType(FD->getType()); - F->addTypeMetadata(0, MD); + llvm::Metadata *Id = CreateMetadataIdentifierForType(FD->getType()); + F->addTypeMetadata(0, Id); F->addTypeMetadata(0, CreateMetadataIdentifierGeneralized(FD->getType())); // Emit a hash-based bit set entry for cross-DSO calls. if (CodeGenOpts.SanitizeCfiCrossDso) - if (auto CrossDsoTypeId = CreateCrossDsoCfiTypeId(MD)) + if (auto CrossDsoTypeId = CreateCrossDsoCfiTypeId(Id)) F->addTypeMetadata(0, llvm::ConstantAsMetadata::get(CrossDsoTypeId)); } @@ -4924,8 +4955,10 @@ } } -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; @@ -4933,6 +4966,7 @@ std::string OutName; llvm::raw_string_ostream Out(OutName); getCXXABI().getMangleContext().mangleTypeName(T, Out); + Out << Suffix; InternalId = llvm::MDString::get(getLLVMContext(), Out.str()); } else { @@ -4943,6 +4977,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 @@ -4976,25 +5019,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()); - } - - return InternalId; + return CreateMetadataIdentifierImpl(GeneralizeFunctionType(getContext(), T), + GeneralizedMetadataIdMap, ".generalized"); } /// Returns whether this module needs the "all-vtables" type identifier. Index: clang/lib/CodeGen/ItaniumCXXABI.cpp =================================================================== --- clang/lib/CodeGen/ItaniumCXXABI.cpp +++ clang/lib/CodeGen/ItaniumCXXABI.cpp @@ -622,13 +622,53 @@ 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 @@ 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: clang/lib/Driver/SanitizerArgs.cpp =================================================================== --- clang/lib/Driver/SanitizerArgs.cpp +++ clang/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, }; @@ -217,7 +218,21 @@ SanitizerMask DiagnosedKinds = 0; // All Kinds we have diagnosed up to now. // Used to deduplicate diagnostics. SanitizerMask Kinds = 0; - const SanitizerMask Supported = setGroupBits(TC.getSupportedSanitizers()); + SanitizerMask Supported = setGroupBits(TC.getSupportedSanitizers()); + + // 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. + CfiCrossDso = Args.hasFlag(options::OPT_fsanitize_cfi_cross_dso, + options::OPT_fno_sanitize_cfi_cross_dso, false); + if (CfiCrossDso) + Supported &= ~CFIMFCall; + ToolChain::RTTIMode RTTIMode = TC.getRTTIMode(); const Driver &D = TC.getDriver(); @@ -553,8 +568,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: clang/lib/Driver/ToolChains/MSVC.cpp =================================================================== --- clang/lib/Driver/ToolChains/MSVC.cpp +++ clang/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: clang/test/CodeGenCXX/cfi-mfcall-incomplete.cpp =================================================================== --- /dev/null +++ clang/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: clang/test/CodeGenCXX/cfi-mfcall.cpp =================================================================== --- /dev/null +++ clang/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: clang/test/CodeGenCXX/type-metadata.cpp =================================================================== --- clang/test/CodeGenCXX/type-metadata.cpp +++ clang/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,14 +252,28 @@ // 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: [[FA16]] = !{i64 16, [[FA_ID:![0-9]+]]} Index: clang/test/Driver/fsanitize.c =================================================================== --- clang/test/Driver/fsanitize.c +++ clang/test/Driver/fsanitize.c @@ -476,6 +476,7 @@ // 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-WIN // 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 +485,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-WIN: -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 @@ -644,16 +646,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 Index: compiler-rt/lib/ubsan/ubsan_handlers.h =================================================================== --- compiler-rt/lib/ubsan/ubsan_handlers.h +++ compiler-rt/lib/ubsan/ubsan_handlers.h @@ -181,6 +181,8 @@ CFITCK_DerivedCast, CFITCK_UnrelatedCast, CFITCK_ICall, + CFITCK_NVMFCall, + CFITCK_VMFCall, }; struct CFICheckFailData { Index: compiler-rt/lib/ubsan/ubsan_handlers.cc =================================================================== --- compiler-rt/lib/ubsan/ubsan_handlers.cc +++ compiler-rt/lib/ubsan/ubsan_handlers.cc @@ -630,7 +630,7 @@ static void handleCFIBadIcall(CFICheckFailData *Data, ValueHandle Function, ReportOptions Opts) { - if (Data->CheckKind != CFITCK_ICall) + if (Data->CheckKind != CFITCK_ICall && Data->CheckKind != CFITCK_NVMFCall) Die(); SourceLocation Loc = Data->Loc.acquire(); @@ -641,9 +641,12 @@ ScopedReport R(Opts, Loc, ET); - Diag(Loc, DL_Error, "control flow integrity check for type %0 failed during " - "indirect function call") - << Data->Type; + const char *CheckKindStr = Data->CheckKind == CFITCK_NVMFCall + ? "non-virtual member function call" + : "indirect function call"; + Diag(Loc, DL_Error, + "control flow integrity check for type %0 failed during %1") + << Data->Type << CheckKindStr; SymbolizedStackHolder FLoc(getSymbolizedLocation(Function)); const char *FName = FLoc.get()->info.function; @@ -685,7 +688,7 @@ ValueHandle Value, uptr ValidVtable) { GET_REPORT_OPTIONS(false); - if (Data->CheckKind == CFITCK_ICall) + if (Data->CheckKind == CFITCK_ICall || Data->CheckKind == CFITCK_NVMFCall) handleCFIBadIcall(Data, Value, Opts); else __ubsan_handle_cfi_bad_type(Data, Value, ValidVtable, Opts); @@ -695,7 +698,7 @@ ValueHandle Value, uptr ValidVtable) { GET_REPORT_OPTIONS(true); - if (Data->CheckKind == CFITCK_ICall) + if (Data->CheckKind == CFITCK_ICall || Data->CheckKind == CFITCK_NVMFCall) handleCFIBadIcall(Data, Value, Opts); else __ubsan_handle_cfi_bad_type(Data, Value, ValidVtable, Opts); Index: compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc =================================================================== --- compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc +++ compiler-rt/lib/ubsan/ubsan_handlers_cxx.cc @@ -122,7 +122,11 @@ case CFITCK_UnrelatedCast: CheckKindStr = "cast to unrelated type"; break; + case CFITCK_VMFCall: + CheckKindStr = "virtual member function call"; + break; case CFITCK_ICall: + case CFITCK_NVMFCall: Die(); } Index: compiler-rt/test/cfi/mfcall.cpp =================================================================== --- /dev/null +++ compiler-rt/test/cfi/mfcall.cpp @@ -0,0 +1,94 @@ +// RUN: %clangxx_cfi -o %t %s +// RUN: %expect_crash %run %t a +// RUN: %expect_crash %run %t b +// RUN: %expect_crash %run %t c +// RUN: %expect_crash %run %t d +// RUN: %expect_crash %run %t e +// RUN: %run %t f +// RUN: %run %t g + +// RUN: %clangxx_cfi_diag -o %t2 %s +// RUN: %run %t2 a 2>&1 | FileCheck --check-prefix=A %s +// RUN: %run %t2 b 2>&1 | FileCheck --check-prefix=B %s +// RUN: %run %t2 c 2>&1 | FileCheck --check-prefix=C %s +// RUN: %run %t2 d 2>&1 | FileCheck --check-prefix=D %s +// RUN: %run %t2 e 2>&1 | FileCheck --check-prefix=E %s + +#include +#include + +struct SBase1 { + void b1() {} +}; + +struct SBase2 { + void b2() {} +}; + +struct S : SBase1, SBase2 { + void f1() {} + int f2() { return 1; } + virtual void g1() {} + virtual int g2() { return 1; } + virtual int g3() { return 1; } +}; + +struct T { + void f1() {} + int f2() { return 2; } + virtual void g1() {} + virtual int g2() { return 2; } + virtual void g3() {} +}; + +typedef void (S::*S_void)(); + +typedef int (S::*S_int)(); +typedef int (T::*T_int)(); + +template +To bitcast(From f) { + assert(sizeof(To) == sizeof(From)); + To t; + memcpy(&t, &f, sizeof(f)); + return t; +} + +int main(int argc, char **argv) { + S s; + T t; + + switch (argv[1][0]) { + case 'a': + // A: runtime error: control flow integrity check for type 'int (S::*)()' failed during non-virtual member function call + // A: note: S::f1() defined here + (s.*bitcast(&S::f1))(); + break; + case 'b': + // B: runtime error: control flow integrity check for type 'int (T::*)()' failed during non-virtual member function call + // B: note: S::f2() defined here + (t.*bitcast(&S::f2))(); + break; + case 'c': + // C: runtime error: control flow integrity check for type 'int (S::*)()' failed during virtual member function call + // C: note: vtable is of type 'S' + (s.*bitcast(&S::g1))(); + break; + case 'd': + // D: runtime error: control flow integrity check for type 'int (S::*)()' failed during virtual member function call + // D: note: vtable is of type 'T' + (reinterpret_cast(t).*&S::g2)(); + break; + case 'e': + // E: runtime error: control flow integrity check for type 'void (S::*)()' failed during virtual member function call + // E: note: vtable is of type 'S' + (s.*bitcast(&T::g3))(); + break; + case 'f': + (s.*&SBase1::b1)(); + break; + case 'g': + (s.*&SBase2::b2)(); + break; + } +}