https://github.com/thurstond updated https://github.com/llvm/llvm-project/pull/135438
>From 936a510b004f7b1b9a04e833e4f96210e7d7b3e6 Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Fri, 11 Apr 2025 20:48:40 +0000 Subject: [PATCH 1/2] [cfi] Fix one -fno-sanitize-merge case, and add two TODOs -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. --- clang/lib/CodeGen/CGClass.cpp | 3 ++- clang/lib/CodeGen/CGExpr.cpp | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CGClass.cpp b/clang/lib/CodeGen/CGClass.cpp index 0f2422a6a665a..e7c704f9da188 100644 --- a/clang/lib/CodeGen/CGClass.cpp +++ b/clang/lib/CodeGen/CGClass.cpp @@ -2961,7 +2961,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 4d6e776f42660..91ab542579282 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -3887,7 +3887,10 @@ 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); @@ -3926,7 +3929,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(); >From 8feef30c8d809ddf4375744fd81ca669d580c7f7 Mon Sep 17 00:00:00 2001 From: Thurston Dang <thurs...@google.com> Date: Fri, 11 Apr 2025 20:58:56 +0000 Subject: [PATCH 2/2] clang-format --- clang/lib/CodeGen/CGExpr.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 91ab542579282..93166ebcc3e79 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -3890,7 +3890,8 @@ void CodeGenFunction::EmitCfiCheckFail() { // 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); + EmitTrapCheck(DataIsNotNullPtr, SanitizerHandler::CFICheckFail, + /*NoMerge=*/false); llvm::StructType *SourceLocationTy = llvm::StructType::get(VoidPtrTy, Int32Ty, Int32Ty); @@ -3933,7 +3934,7 @@ void CodeGenFunction::EmitCfiCheckFail() { // 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); + 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
