Author: Thurston Dang Date: 2025-04-23T22:00:11-07:00 New Revision: 31c7997a4acb838c94d5ab40baaf154556532ad9
URL: https://github.com/llvm/llvm-project/commit/31c7997a4acb838c94d5ab40baaf154556532ad9 DIFF: https://github.com/llvm/llvm-project/commit/31c7997a4acb838c94d5ab40baaf154556532ad9.diff LOG: [cfi] Fix one -fno-sanitize-merge case, and add two TODOs (#135438) -fno-sanitize-merge (introduced in https://github.com/llvm/llvm-project/pull/120464) nearly works for CFI: code that calls EmitCheck will already check the merge options. This patch fixes one EmitTrapCheck call, which did not check the merge options, and for two other EmitTrapChecks, adds two TODOs that explain why it is difficult to fix them. Added: Modified: clang/lib/CodeGen/CGClass.cpp clang/lib/CodeGen/CGExpr.cpp Removed: ################################################################################ diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 7176fe025b386..a8c48237977c2 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -2895,7 +2895,8 @@ void CodeGenFunction::EmitVTablePtrCheck(const CXXRecordDecl *RD, } if (CGM.getCodeGenOpts().SanitizeTrap.has(M)) { - EmitTrapCheck(TypeTest, SanitizerHandler::CFICheckFail); + bool NoMerge = !CGM.getCodeGenOpts().SanitizeMergeHandlers.has(M); + EmitTrapCheck(TypeTest, SanitizerHandler::CFICheckFail, NoMerge); return; } diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 786a56eed7ed5..bba7d1e805f3f 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -3923,7 +3923,11 @@ void CodeGenFunction::EmitCfiCheckFail() { // Data == nullptr means the calling module has trap behaviour for this check. llvm::Value *DataIsNotNullPtr = Builder.CreateICmpNE(Data, llvm::ConstantPointerNull::get(Int8PtrTy)); - EmitTrapCheck(DataIsNotNullPtr, SanitizerHandler::CFICheckFail); + // TODO: since there is no data, we don't know the CheckKind, and therefore + // cannot inspect CGM.getCodeGenOpts().SanitizeMergeHandlers. We default to + // NoMerge = false. Users can disable merging by disabling optimization. + EmitTrapCheck(DataIsNotNullPtr, SanitizerHandler::CFICheckFail, + /*NoMerge=*/false); llvm::StructType *SourceLocationTy = llvm::StructType::get(VoidPtrTy, Int32Ty, Int32Ty); @@ -3962,7 +3966,11 @@ void CodeGenFunction::EmitCfiCheckFail() { EmitCheck(std::make_pair(Cond, Ordinal), SanitizerHandler::CFICheckFail, {}, {Data, Addr, ValidVtable}); else - EmitTrapCheck(Cond, SanitizerHandler::CFICheckFail); + // TODO: we can't rely on CGM.getCodeGenOpts().SanitizeMergeHandlers. + // Although the compiler allows SanitizeMergeHandlers to be set + // independently of CGM.getLangOpts().Sanitize, Driver/SanitizerArgs.cpp + // requires that SanitizeMergeHandlers is a subset of Sanitize. + EmitTrapCheck(Cond, SanitizerHandler::CFICheckFail, /*NoMerge=*/false); } FinishFunction(); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits