[llvm] r318124 - [PM] Refactor BoundsChecking further to prepare it to be exposed both as
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Mon Nov 13 17:13:59 PST 2017
Author: chandlerc
Date: Mon Nov 13 17:13:59 2017
New Revision: 318124
URL: http://llvm.org/viewvc/llvm-project?rev=318124&view=rev
Log:
[PM] Refactor BoundsChecking further to prepare it to be exposed both as
a legacy and new PM pass.
This essentially moves the class state to parameters and re-shuffles the
code to make that reasonable. It also does some minor cleanups along the
way and leaves some comments.
Differential Revision: https://reviews.llvm.org/D39081
Modified:
llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp
Modified: llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp?rev=318124&r1=318123&r2=318124&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp (original)
+++ llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp Mon Nov 13 17:13:59 2017
@@ -65,16 +65,6 @@ namespace {
void getAnalysisUsage(AnalysisUsage &AU) const override {
AU.addRequired<TargetLibraryInfoWrapperPass>();
}
-
- private:
- const TargetLibraryInfo *TLI;
- ObjectSizeOffsetEvaluator *ObjSizeEval;
- BuilderTy *Builder;
- Instruction *Inst;
- BasicBlock *TrapBB;
-
- BasicBlock *getTrapBB();
- bool instrument(Value *Ptr, Value *Val, const DataLayout &DL);
};
} // end anonymous namespace
@@ -84,41 +74,28 @@ char BoundsChecking::ID = 0;
INITIALIZE_PASS(BoundsChecking, "bounds-checking", "Run-time bounds checking",
false, false)
-/// getTrapBB - create a basic block that traps. All overflowing conditions
-/// branch to this block. There's only one trap block per function.
-BasicBlock *BoundsChecking::getTrapBB() {
- if (TrapBB && SingleTrapBB)
- return TrapBB;
-
- Function *Fn = Inst->getParent()->getParent();
- IRBuilder<>::InsertPointGuard Guard(*Builder);
- TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
- Builder->SetInsertPoint(TrapBB);
-
- Value *F = Intrinsic::getDeclaration(Fn->getParent(), Intrinsic::trap);
- CallInst *TrapCall = Builder->CreateCall(F, {});
- TrapCall->setDoesNotReturn();
- TrapCall->setDoesNotThrow();
- TrapCall->setDebugLoc(Inst->getDebugLoc());
- Builder->CreateUnreachable();
-
- return TrapBB;
-}
-
-/// instrument - adds run-time bounds checks to memory accessing instructions.
-/// Ptr is the pointer that will be read/written, and InstVal is either the
-/// result from the load or the value being stored. It is used to determine the
-/// size of memory block that is touched.
+/// Adds run-time bounds checks to memory accessing instructions.
+///
+/// \p Ptr is the pointer that will be read/written, and \p InstVal is either
+/// the result from the load or the value being stored. It is used to determine
+/// the size of memory block that is touched.
+///
+/// \p GetTrapBB is a callable that returns the trap BB to use on failure.
+///
/// Returns true if any change was made to the IR, false otherwise.
-bool BoundsChecking::instrument(Value *Ptr, Value *InstVal,
- const DataLayout &DL) {
+template <typename GetTrapBBT>
+static bool instrumentMemAccess(Value *Ptr, Value *InstVal,
+ const DataLayout &DL, TargetLibraryInfo &TLI,
+ ObjectSizeOffsetEvaluator &ObjSizeEval,
+ BuilderTy &IRB,
+ GetTrapBBT GetTrapBB) {
uint64_t NeededSize = DL.getTypeStoreSize(InstVal->getType());
DEBUG(dbgs() << "Instrument " << *Ptr << " for " << Twine(NeededSize)
<< " bytes\n");
- SizeOffsetEvalType SizeOffset = ObjSizeEval->compute(Ptr);
+ SizeOffsetEvalType SizeOffset = ObjSizeEval.compute(Ptr);
- if (!ObjSizeEval->bothKnown(SizeOffset)) {
+ if (!ObjSizeEval.bothKnown(SizeOffset)) {
++ChecksUnable;
return false;
}
@@ -137,13 +114,13 @@ bool BoundsChecking::instrument(Value *P
//
// optimization: if Size >= 0 (signed), skip 1st check
// FIXME: add NSW/NUW here? -- we dont care if the subtraction overflows
- Value *ObjSize = Builder->CreateSub(Size, Offset);
- Value *Cmp2 = Builder->CreateICmpULT(Size, Offset);
- Value *Cmp3 = Builder->CreateICmpULT(ObjSize, NeededSizeVal);
- Value *Or = Builder->CreateOr(Cmp2, Cmp3);
+ Value *ObjSize = IRB.CreateSub(Size, Offset);
+ Value *Cmp2 = IRB.CreateICmpULT(Size, Offset);
+ Value *Cmp3 = IRB.CreateICmpULT(ObjSize, NeededSizeVal);
+ Value *Or = IRB.CreateOr(Cmp2, Cmp3);
if (!SizeCI || SizeCI->getValue().slt(0)) {
- Value *Cmp1 = Builder->CreateICmpSLT(Offset, ConstantInt::get(IntTy, 0));
- Or = Builder->CreateOr(Cmp1, Or);
+ Value *Cmp1 = IRB.CreateICmpSLT(Offset, ConstantInt::get(IntTy, 0));
+ Or = IRB.CreateOr(Cmp1, Or);
}
// check if the comparison is always false
@@ -156,7 +133,7 @@ bool BoundsChecking::instrument(Value *P
}
++ChecksAdded;
- BasicBlock::iterator SplitI = Builder->GetInsertPoint();
+ BasicBlock::iterator SplitI = IRB.GetInsertPoint();
BasicBlock *OldBB = SplitI->getParent();
BasicBlock *Cont = OldBB->splitBasicBlock(SplitI);
OldBB->getTerminator()->eraseFromParent();
@@ -165,52 +142,75 @@ bool BoundsChecking::instrument(Value *P
// If we have a constant zero, unconditionally branch.
// FIXME: We should really handle this differently to bypass the splitting
// the block.
- BranchInst::Create(getTrapBB(), OldBB);
+ BranchInst::Create(GetTrapBB(IRB), OldBB);
return true;
}
// Create the conditional branch.
- BranchInst::Create(getTrapBB(), Cont, Or, OldBB);
+ BranchInst::Create(GetTrapBB(IRB), Cont, Or, OldBB);
return true;
}
bool BoundsChecking::runOnFunction(Function &F) {
const DataLayout &DL = F.getParent()->getDataLayout();
- TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
+ auto &TLI = getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
- TrapBB = nullptr;
- BuilderTy TheBuilder(F.getContext(), TargetFolder(DL));
- Builder = &TheBuilder;
- ObjectSizeOffsetEvaluator TheObjSizeEval(DL, TLI, F.getContext(),
+ ObjectSizeOffsetEvaluator ObjSizeEval(DL, &TLI, F.getContext(),
/*RoundToAlign=*/true);
- ObjSizeEval = &TheObjSizeEval;
// check HANDLE_MEMORY_INST in include/llvm/Instruction.def for memory
// touching instructions
std::vector<Instruction *> WorkList;
- for (inst_iterator i = inst_begin(F), e = inst_end(F); i != e; ++i) {
- Instruction *I = &*i;
+ for (Instruction &I : instructions(F)) {
if (isa<LoadInst>(I) || isa<StoreInst>(I) || isa<AtomicCmpXchgInst>(I) ||
isa<AtomicRMWInst>(I))
- WorkList.push_back(I);
+ WorkList.push_back(&I);
}
- bool MadeChange = false;
- for (Instruction *i : WorkList) {
- Inst = i;
+ // Create a trapping basic block on demand using a callback. Depending on
+ // flags, this will either create a single block for the entire function or
+ // will create a fresh block every time it is called.
+ BasicBlock *TrapBB = nullptr;
+ auto GetTrapBB = [&TrapBB](BuilderTy &IRB) {
+ if (TrapBB && SingleTrapBB)
+ return TrapBB;
+
+ Function *Fn = IRB.GetInsertBlock()->getParent();
+ // FIXME: This debug location doesn't make a lot of sense in the
+ // `SingleTrapBB` case.
+ auto DebugLoc = IRB.getCurrentDebugLocation();
+ IRBuilder<>::InsertPointGuard Guard(IRB);
+ TrapBB = BasicBlock::Create(Fn->getContext(), "trap", Fn);
+ IRB.SetInsertPoint(TrapBB);
+
+ auto *F = Intrinsic::getDeclaration(Fn->getParent(), Intrinsic::trap);
+ CallInst *TrapCall = IRB.CreateCall(F, {});
+ TrapCall->setDoesNotReturn();
+ TrapCall->setDoesNotThrow();
+ TrapCall->setDebugLoc(DebugLoc);
+ IRB.CreateUnreachable();
- Builder->SetInsertPoint(Inst);
+ return TrapBB;
+ };
+
+ bool MadeChange = false;
+ for (Instruction *Inst : WorkList) {
+ BuilderTy IRB(Inst->getParent(), BasicBlock::iterator(Inst), TargetFolder(DL));
if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
- MadeChange |= instrument(LI->getPointerOperand(), LI, DL);
+ MadeChange |= instrumentMemAccess(LI->getPointerOperand(), LI, DL, TLI,
+ ObjSizeEval, IRB, GetTrapBB);
} else if (StoreInst *SI = dyn_cast<StoreInst>(Inst)) {
MadeChange |=
- instrument(SI->getPointerOperand(), SI->getValueOperand(), DL);
+ instrumentMemAccess(SI->getPointerOperand(), SI->getValueOperand(),
+ DL, TLI, ObjSizeEval, IRB, GetTrapBB);
} else if (AtomicCmpXchgInst *AI = dyn_cast<AtomicCmpXchgInst>(Inst)) {
MadeChange |=
- instrument(AI->getPointerOperand(), AI->getCompareOperand(), DL);
+ instrumentMemAccess(AI->getPointerOperand(), AI->getCompareOperand(),
+ DL, TLI, ObjSizeEval, IRB, GetTrapBB);
} else if (AtomicRMWInst *AI = dyn_cast<AtomicRMWInst>(Inst)) {
MadeChange |=
- instrument(AI->getPointerOperand(), AI->getValOperand(), DL);
+ instrumentMemAccess(AI->getPointerOperand(), AI->getValOperand(), DL,
+ TLI, ObjSizeEval, IRB, GetTrapBB);
} else {
llvm_unreachable("unknown Instruction type");
}
More information about the llvm-commits
mailing list