Author: Fangrui Song Date: 2022-11-17T22:45:30Z New Revision: fc91c705937d7ba3b92da38f3a883dde720c41f2
URL: https://github.com/llvm/llvm-project/commit/fc91c705937d7ba3b92da38f3a883dde720c41f2 DIFF: https://github.com/llvm/llvm-project/commit/fc91c705937d7ba3b92da38f3a883dde720c41f2.diff LOG: Revert D135411 "Add generic KCFI operand bundle lowering" This reverts commit eb2a57ebc7aaad551af30462097a9e06c96db925. llvm/include/llvm/Transforms/Instrumentation/KCFI.h including llvm/CodeGen is a layering violation. We should use an approach where Instrumementation/ doesn't need to include CodeGen/. Sorry for not spotting this in the review. Added: Modified: clang/lib/CodeGen/BackendUtil.cpp clang/lib/Driver/ToolChain.cpp llvm/include/llvm/InitializePasses.h llvm/lib/Passes/PassBuilder.cpp llvm/lib/Passes/PassRegistry.def llvm/lib/Transforms/Instrumentation/CMakeLists.txt llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn Removed: llvm/include/llvm/Transforms/Instrumentation/KCFI.h llvm/lib/Transforms/Instrumentation/KCFI.cpp llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll llvm/test/Transforms/KCFI/kcfi-supported.ll llvm/test/Transforms/KCFI/kcfi.ll ################################################################################ diff --git a/clang/lib/CodeGen/BackendUtil.cpp b/clang/lib/CodeGen/BackendUtil.cpp index 7082fb74ab59..8498ca2e6338 100644 --- a/clang/lib/CodeGen/BackendUtil.cpp +++ b/clang/lib/CodeGen/BackendUtil.cpp @@ -72,7 +72,6 @@ #include "llvm/Transforms/Instrumentation/GCOVProfiler.h" #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" -#include "llvm/Transforms/Instrumentation/KCFI.h" #include "llvm/Transforms/Instrumentation/MemProfiler.h" #include "llvm/Transforms/Instrumentation/MemorySanitizer.h" #include "llvm/Transforms/Instrumentation/SanitizerBinaryMetadata.h" @@ -645,31 +644,6 @@ static OptimizationLevel mapToLevel(const CodeGenOptions &Opts) { } } -static void addKCFIPass(TargetMachine *TM, const Triple &TargetTriple, - const LangOptions &LangOpts, PassBuilder &PB) { - // If the back-end supports KCFI operand bundle lowering, skip KCFIPass. - if (TargetTriple.getArch() == llvm::Triple::x86_64 || - TargetTriple.isAArch64(64)) - return; - - // Ensure we lower KCFI operand bundles with -O0. - PB.registerOptimizerLastEPCallback( - [&, TM](ModulePassManager &MPM, OptimizationLevel Level) { - if (Level == OptimizationLevel::O0 && - LangOpts.Sanitize.has(SanitizerKind::KCFI)) - MPM.addPass(createModuleToFunctionPassAdaptor(KCFIPass(TM))); - }); - - // When optimizations are requested, run KCIFPass after InstCombine to - // avoid unnecessary checks. - PB.registerPeepholeEPCallback( - [&, TM](FunctionPassManager &FPM, OptimizationLevel Level) { - if (Level != OptimizationLevel::O0 && - LangOpts.Sanitize.has(SanitizerKind::KCFI)) - FPM.addPass(KCFIPass(TM)); - }); -} - static void addSanitizers(const Triple &TargetTriple, const CodeGenOptions &CodeGenOpts, const LangOptions &LangOpts, PassBuilder &PB) { @@ -972,10 +946,8 @@ void EmitAssemblyHelper::RunOptimizationPipeline( // Don't add sanitizers if we are here from ThinLTO PostLink. That already // done on PreLink stage. - if (!IsThinLTOPostLink) { + if (!IsThinLTOPostLink) addSanitizers(TargetTriple, CodeGenOpts, LangOpts, PB); - addKCFIPass(TM.get(), TargetTriple, LangOpts, PB); - } if (Optional<GCOVOptions> Options = getGCOVOptions(CodeGenOpts, LangOpts)) PB.registerPipelineStartEPCallback( diff --git a/clang/lib/Driver/ToolChain.cpp b/clang/lib/Driver/ToolChain.cpp index 1cb95065ade2..695741f73dcc 100644 --- a/clang/lib/Driver/ToolChain.cpp +++ b/clang/lib/Driver/ToolChain.cpp @@ -1089,7 +1089,7 @@ SanitizerMask ToolChain::getSupportedSanitizers() const { ~SanitizerKind::Function) | (SanitizerKind::CFI & ~SanitizerKind::CFIICall) | SanitizerKind::CFICastStrict | SanitizerKind::FloatDivideByZero | - SanitizerKind::KCFI | SanitizerKind::UnsignedIntegerOverflow | + SanitizerKind::UnsignedIntegerOverflow | SanitizerKind::UnsignedShiftBase | SanitizerKind::ImplicitConversion | SanitizerKind::Nullability | SanitizerKind::LocalBounds; if (getTriple().getArch() == llvm::Triple::x86 || @@ -1097,6 +1097,9 @@ SanitizerMask ToolChain::getSupportedSanitizers() const { getTriple().getArch() == llvm::Triple::arm || getTriple().isWasm() || getTriple().isAArch64() || getTriple().isRISCV()) Res |= SanitizerKind::CFIICall; + if (getTriple().getArch() == llvm::Triple::x86_64 || + getTriple().isAArch64(64)) + Res |= SanitizerKind::KCFI; if (getTriple().getArch() == llvm::Triple::x86_64 || getTriple().isAArch64(64) || getTriple().isRISCV()) Res |= SanitizerKind::ShadowCallStack; diff --git a/llvm/include/llvm/InitializePasses.h b/llvm/include/llvm/InitializePasses.h index 2e6b497d0693..2c9106461563 100644 --- a/llvm/include/llvm/InitializePasses.h +++ b/llvm/include/llvm/InitializePasses.h @@ -258,7 +258,6 @@ void initializeLowerInvokeLegacyPassPass(PassRegistry&); void initializeLowerSwitchLegacyPassPass(PassRegistry &); void initializeLowerMatrixIntrinsicsLegacyPassPass(PassRegistry &); void initializeLowerMatrixIntrinsicsMinimalLegacyPassPass(PassRegistry &); -void initializeKCFIPass(PassRegistry &); void initializeMIRAddFSDiscriminatorsPass(PassRegistry &); void initializeMIRCanonicalizerPass(PassRegistry &); void initializeMIRNamerPass(PassRegistry &); diff --git a/llvm/include/llvm/Transforms/Instrumentation/KCFI.h b/llvm/include/llvm/Transforms/Instrumentation/KCFI.h deleted file mode 100644 index f8d549e237bf..000000000000 --- a/llvm/include/llvm/Transforms/Instrumentation/KCFI.h +++ /dev/null @@ -1,31 +0,0 @@ -//===-- KCFI.h - Generic KCFI operand bundle lowering -----------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This pass emits generic KCFI indirect call checks for targets that don't -// support lowering KCFI operand bundles in the back-end. -// -//===----------------------------------------------------------------------===// - -#ifndef LLVM_TRANSFORMS_INSTRUMENTATION_KCFI_H -#define LLVM_TRANSFORMS_INSTRUMENTATION_KCFI_H - -#include "llvm/IR/PassManager.h" - -namespace llvm { -class TargetMachine; - -class KCFIPass : public PassInfoMixin<KCFIPass> { - TargetMachine *TM; - -public: - KCFIPass(TargetMachine *TM) : TM(TM) {} - static bool isRequired() { return true; } - PreservedAnalyses run(Function &F, FunctionAnalysisManager &AM); -}; -} // namespace llvm -#endif // LLVM_TRANSFORMS_INSTRUMENTATION_KCFI_H diff --git a/llvm/lib/Passes/PassBuilder.cpp b/llvm/lib/Passes/PassBuilder.cpp index 8802980efa4e..b84f0db188da 100644 --- a/llvm/lib/Passes/PassBuilder.cpp +++ b/llvm/lib/Passes/PassBuilder.cpp @@ -137,7 +137,6 @@ #include "llvm/Transforms/Instrumentation/HWAddressSanitizer.h" #include "llvm/Transforms/Instrumentation/InstrOrderFile.h" #include "llvm/Transforms/Instrumentation/InstrProfiling.h" -#include "llvm/Transforms/Instrumentation/KCFI.h" #include "llvm/Transforms/Instrumentation/MemProfiler.h" #include "llvm/Transforms/Instrumentation/MemorySanitizer.h" #include "llvm/Transforms/Instrumentation/PGOInstrumentation.h" diff --git a/llvm/lib/Passes/PassRegistry.def b/llvm/lib/Passes/PassRegistry.def index 314d45867dd9..7d8656ee41d7 100644 --- a/llvm/lib/Passes/PassRegistry.def +++ b/llvm/lib/Passes/PassRegistry.def @@ -322,7 +322,6 @@ FUNCTION_PASS("nary-reassociate", NaryReassociatePass()) FUNCTION_PASS("newgvn", NewGVNPass()) FUNCTION_PASS("jump-threading", JumpThreadingPass()) FUNCTION_PASS("partially-inline-libcalls", PartiallyInlineLibCallsPass()) -FUNCTION_PASS("kcfi", KCFIPass(TM)) FUNCTION_PASS("lcssa", LCSSAPass()) FUNCTION_PASS("loop-data-prefetch", LoopDataPrefetchPass()) FUNCTION_PASS("loop-load-elim", LoopLoadEliminationPass()) diff --git a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt index cb4af35aef17..d54175e9fe06 100644 --- a/llvm/lib/Transforms/Instrumentation/CMakeLists.txt +++ b/llvm/lib/Transforms/Instrumentation/CMakeLists.txt @@ -11,7 +11,6 @@ add_llvm_component_library(LLVMInstrumentation Instrumentation.cpp InstrOrderFile.cpp InstrProfiling.cpp - KCFI.cpp PGOInstrumentation.cpp PGOMemOPSizeOpt.cpp PoisonChecking.cpp diff --git a/llvm/lib/Transforms/Instrumentation/KCFI.cpp b/llvm/lib/Transforms/Instrumentation/KCFI.cpp deleted file mode 100644 index 577a7eafe31d..000000000000 --- a/llvm/lib/Transforms/Instrumentation/KCFI.cpp +++ /dev/null @@ -1,115 +0,0 @@ -//===-- KCFI.cpp - Generic KCFI operand bundle lowering ---------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// -// -// This pass emits generic KCFI indirect call checks for targets that don't -// support lowering KCFI operand bundles in the back-end. -// -//===----------------------------------------------------------------------===// - -#include "llvm/Transforms/Instrumentation/KCFI.h" -#include "llvm/ADT/Statistic.h" -#include "llvm/CodeGen/TargetLowering.h" -#include "llvm/CodeGen/TargetSubtargetInfo.h" -#include "llvm/IR/Constants.h" -#include "llvm/IR/DiagnosticInfo.h" -#include "llvm/IR/DiagnosticPrinter.h" -#include "llvm/IR/Function.h" -#include "llvm/IR/GlobalObject.h" -#include "llvm/IR/IRBuilder.h" -#include "llvm/IR/InstIterator.h" -#include "llvm/IR/Instructions.h" -#include "llvm/IR/Intrinsics.h" -#include "llvm/IR/MDBuilder.h" -#include "llvm/IR/Module.h" -#include "llvm/InitializePasses.h" -#include "llvm/Pass.h" -#include "llvm/Target/TargetMachine.h" -#include "llvm/Transforms/Instrumentation.h" -#include "llvm/Transforms/Utils/BasicBlockUtils.h" - -using namespace llvm; - -#define DEBUG_TYPE "kcfi" - -STATISTIC(NumKCFIChecks, "Number of kcfi operands transformed into checks"); - -namespace { -class DiagnosticInfoKCFI : public DiagnosticInfo { - const Twine &Msg; - -public: - DiagnosticInfoKCFI(const Twine &DiagMsg, - DiagnosticSeverity Severity = DS_Error) - : DiagnosticInfo(DK_Linker, Severity), Msg(DiagMsg) {} - void print(DiagnosticPrinter &DP) const override { DP << Msg; } -}; -} // namespace - -PreservedAnalyses KCFIPass::run(Function &F, FunctionAnalysisManager &AM) { - Module &M = *F.getParent(); - if (!M.getModuleFlag("kcfi") || - (TM && - TM->getSubtargetImpl(F)->getTargetLowering()->supportKCFIBundles())) - return PreservedAnalyses::all(); - - // Find call instructions with KCFI operand bundles. - SmallVector<CallInst *> KCFICalls; - for (Instruction &I : instructions(F)) { - if (auto *CI = dyn_cast<CallInst>(&I)) - if (CI->getOperandBundle(LLVMContext::OB_kcfi)) - KCFICalls.push_back(CI); - } - - if (KCFICalls.empty()) - return PreservedAnalyses::all(); - - LLVMContext &Ctx = M.getContext(); - // patchable-function-prefix emits nops between the KCFI type identifier - // and the function start. As we don't know the size of the emitted nops, - // don't allow this attribute with generic lowering. - if (F.hasFnAttribute("patchable-function-prefix")) - Ctx.diagnose( - DiagnosticInfoKCFI("-fpatchable-function-entry=N,M, where M>0 is not " - "compatible with -fsanitize=kcfi on this target")); - - IntegerType *Int32Ty = Type::getInt32Ty(Ctx); - MDNode *VeryUnlikelyWeights = - MDBuilder(Ctx).createBranchWeights(1, (1U << 20) - 1); - - for (CallInst *CI : KCFICalls) { - // Get the expected hash value. - const uint32_t ExpectedHash = - cast<ConstantInt>(CI->getOperandBundle(LLVMContext::OB_kcfi)->Inputs[0]) - ->getZExtValue(); - - // Drop the KCFI operand bundle. - CallBase *Call = - CallBase::removeOperandBundle(CI, LLVMContext::OB_kcfi, CI); - assert(Call != CI); - Call->copyMetadata(*CI); - CI->replaceAllUsesWith(Call); - CI->eraseFromParent(); - - if (!Call->isIndirectCall()) - continue; - - // Emit a check and trap if the target hash doesn't match. - IRBuilder<> Builder(Call); - Value *HashPtr = Builder.CreateConstInBoundsGEP1_32( - Int32Ty, Call->getCalledOperand(), -1); - Value *Test = Builder.CreateICmpNE(Builder.CreateLoad(Int32Ty, HashPtr), - ConstantInt::get(Int32Ty, ExpectedHash)); - Instruction *ThenTerm = - SplitBlockAndInsertIfThen(Test, Call, false, VeryUnlikelyWeights); - Builder.SetInsertPoint(ThenTerm); - Builder.CreateCall(Intrinsic::getDeclaration(&M, Intrinsic::trap)); - ++NumKCFIChecks; - } - - return PreservedAnalyses::none(); -} diff --git a/llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll b/llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll deleted file mode 100644 index 15f1cf452d18..000000000000 --- a/llvm/test/Transforms/KCFI/kcfi-patchable-function-prefix.ll +++ /dev/null @@ -1,12 +0,0 @@ -; RUN: not opt -S -passes=kcfi %s 2>&1 | FileCheck %s - -; CHECK: error: -fpatchable-function-entry=N,M, where M>0 is not compatible with -fsanitize=kcfi on this target -define void @f1(ptr noundef %x) #0 { - call void %x() [ "kcfi"(i32 12345678) ] - ret void -} - -attributes #0 = { "patchable-function-prefix"="1" } - -!llvm.module.flags = !{!0} -!0 = !{i32 4, !"kcfi", i32 1} diff --git a/llvm/test/Transforms/KCFI/kcfi-supported.ll b/llvm/test/Transforms/KCFI/kcfi-supported.ll deleted file mode 100644 index 3d5f00510ab6..000000000000 --- a/llvm/test/Transforms/KCFI/kcfi-supported.ll +++ /dev/null @@ -1,18 +0,0 @@ -; REQUIRES: x86-registered-target -; RUN: opt -S -passes=kcfi %s | FileCheck %s - -;; If the back-end supports KCFI operand bundle lowering, KCFIPass should be a no-op. - -target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" -target triple = "x86_64-unknown-linux-gnu" - -; CHECK-LABEL: define void @f1 -define void @f1(ptr noundef %x) { - ; CHECK-NOT: call void @llvm.trap() - ; CHECK: call void %x() [ "kcfi"(i32 12345678) ] - call void %x() [ "kcfi"(i32 12345678) ] - ret void -} - -!llvm.module.flags = !{!0} -!0 = !{i32 4, !"kcfi", i32 1} diff --git a/llvm/test/Transforms/KCFI/kcfi.ll b/llvm/test/Transforms/KCFI/kcfi.ll deleted file mode 100644 index 49c311b1927a..000000000000 --- a/llvm/test/Transforms/KCFI/kcfi.ll +++ /dev/null @@ -1,21 +0,0 @@ -; RUN: opt -S -passes=kcfi %s | FileCheck %s - -; CHECK-LABEL: define void @f1( -define void @f1(ptr noundef %x) { - ; CHECK: %[[#GEPI:]] = getelementptr inbounds i32, ptr %x, i32 -1 - ; CHECK-NEXT: %[[#LOAD:]] = load i32, ptr %[[#GEPI]], align 4 - ; CHECK-NEXT: %[[#ICMP:]] = icmp ne i32 %[[#LOAD]], 12345678 - ; CHECK-NEXT: br i1 %[[#ICMP]], label %[[#TRAP:]], label %[[#CALL:]], !prof ![[#WEIGHTS:]] - ; CHECK: [[#TRAP]]: - ; CHECK-NEXT: call void @llvm.trap() - ; CHECK-NEXT: br label %[[#CALL]] - ; CHECK: [[#CALL]]: - ; CHECK-NEXT: call void %x() - ; CHECK-NOT: [ "kcfi"(i32 12345678) ] - call void %x() [ "kcfi"(i32 12345678) ] - ret void -} - -!llvm.module.flags = !{!0} -!0 = !{i32 4, !"kcfi", i32 1} -; CHECK: ![[#WEIGHTS]] = !{!"branch_weights", i32 1, i32 1048575} diff --git a/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn b/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn index 0e4fb250bca3..b3a52aa7ba12 100644 --- a/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn +++ b/llvm/utils/gn/secondary/llvm/lib/Transforms/Instrumentation/BUILD.gn @@ -20,7 +20,6 @@ static_library("Instrumentation") { "InstrOrderFile.cpp", "InstrProfiling.cpp", "Instrumentation.cpp", - "KCFI.cpp", "MemProfiler.cpp", "MemorySanitizer.cpp", "PGOInstrumentation.cpp", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
