[clang] fc91c70 - Revert D135411 "Add generic KCFI operand bundle lowering"
Fangrui Song via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 17 14:45:37 PST 2022
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",
More information about the cfe-commits
mailing list