Index: llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp =================================================================== --- llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp +++ llvm/trunk/lib/Transforms/Instrumentation/BoundsChecking.cpp @@ -47,21 +47,17 @@ using BuilderTy = IRBuilder; -/// Adds run-time bounds checks to memory accessing instructions. +/// Gets the conditions under which memory accessing instructions will overflow. /// /// \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. -template -static bool instrumentMemAccess(Value *Ptr, Value *InstVal, - const DataLayout &DL, TargetLibraryInfo &TLI, - ObjectSizeOffsetEvaluator &ObjSizeEval, - BuilderTy &IRB, GetTrapBBT GetTrapBB, - ScalarEvolution &SE) { +/// Returns the condition under which the access will overflow. +static Value *getBoundsCheckCond(Value *Ptr, Value *InstVal, + const DataLayout &DL, TargetLibraryInfo &TLI, + ObjectSizeOffsetEvaluator &ObjSizeEval, + BuilderTy &IRB, ScalarEvolution &SE) { uint64_t NeededSize = DL.getTypeStoreSize(InstVal->getType()); LLVM_DEBUG(dbgs() << "Instrument " << *Ptr << " for " << Twine(NeededSize) << " bytes\n"); @@ -70,7 +66,7 @@ if (!ObjSizeEval.bothKnown(SizeOffset)) { ++ChecksUnable; - return false; + return nullptr; } Value *Size = SizeOffset.first; @@ -107,13 +103,23 @@ Or = IRB.CreateOr(Cmp1, Or); } + return Or; +} + +/// Adds run-time bounds checks to memory accessing instructions. +/// +/// \p Or is the condition that should guard the trap. +/// +/// \p GetTrapBB is a callable that returns the trap BB to use on failure. +template +static void insertBoundsCheck(Value *Or, BuilderTy IRB, GetTrapBBT GetTrapBB) { // check if the comparison is always false ConstantInt *C = dyn_cast_or_null(Or); if (C) { ++ChecksSkipped; // If non-zero, nothing to do. if (!C->getZExtValue()) - return true; + return; } ++ChecksAdded; @@ -127,12 +133,11 @@ // FIXME: We should really handle this differently to bypass the splitting // the block. BranchInst::Create(GetTrapBB(IRB), OldBB); - return true; + return; } // Create the conditional branch. BranchInst::Create(GetTrapBB(IRB), Cont, Or, OldBB); - return true; } static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI, @@ -143,11 +148,25 @@ // check HANDLE_MEMORY_INST in include/llvm/Instruction.def for memory // touching instructions - std::vector WorkList; + SmallVector, 4> TrapInfo; for (Instruction &I : instructions(F)) { - if (isa(I) || isa(I) || isa(I) || - isa(I)) - WorkList.push_back(&I); + Value *Or = nullptr; + BuilderTy IRB(I.getParent(), BasicBlock::iterator(&I), TargetFolder(DL)); + if (LoadInst *LI = dyn_cast(&I)) { + Or = getBoundsCheckCond(LI->getPointerOperand(), LI, DL, TLI, + ObjSizeEval, IRB, SE); + } else if (StoreInst *SI = dyn_cast(&I)) { + Or = getBoundsCheckCond(SI->getPointerOperand(), SI->getValueOperand(), + DL, TLI, ObjSizeEval, IRB, SE); + } else if (AtomicCmpXchgInst *AI = dyn_cast(&I)) { + Or = getBoundsCheckCond(AI->getPointerOperand(), AI->getCompareOperand(), + DL, TLI, ObjSizeEval, IRB, SE); + } else if (AtomicRMWInst *AI = dyn_cast(&I)) { + Or = getBoundsCheckCond(AI->getPointerOperand(), AI->getValOperand(), DL, + TLI, ObjSizeEval, IRB, SE); + } + if (Or) + TrapInfo.push_back(std::make_pair(&I, Or)); } // Create a trapping basic block on demand using a callback. Depending on @@ -176,29 +195,14 @@ return TrapBB; }; - bool MadeChange = false; - for (Instruction *Inst : WorkList) { + // Add the checks. + for (const auto &Entry : TrapInfo) { + Instruction *Inst = Entry.first; BuilderTy IRB(Inst->getParent(), BasicBlock::iterator(Inst), TargetFolder(DL)); - if (LoadInst *LI = dyn_cast(Inst)) { - MadeChange |= instrumentMemAccess(LI->getPointerOperand(), LI, DL, TLI, - ObjSizeEval, IRB, GetTrapBB, SE); - } else if (StoreInst *SI = dyn_cast(Inst)) { - MadeChange |= - instrumentMemAccess(SI->getPointerOperand(), SI->getValueOperand(), - DL, TLI, ObjSizeEval, IRB, GetTrapBB, SE); - } else if (AtomicCmpXchgInst *AI = dyn_cast(Inst)) { - MadeChange |= - instrumentMemAccess(AI->getPointerOperand(), AI->getCompareOperand(), - DL, TLI, ObjSizeEval, IRB, GetTrapBB, SE); - } else if (AtomicRMWInst *AI = dyn_cast(Inst)) { - MadeChange |= - instrumentMemAccess(AI->getPointerOperand(), AI->getValOperand(), DL, - TLI, ObjSizeEval, IRB, GetTrapBB, SE); - } else { - llvm_unreachable("unknown Instruction type"); - } + insertBoundsCheck(Entry.second, IRB, GetTrapBB); } - return MadeChange; + + return !TrapInfo.empty(); } PreservedAnalyses BoundsCheckingPass::run(Function &F, FunctionAnalysisManager &AM) { Index: llvm/trunk/test/Instrumentation/BoundsChecking/many-traps-2.ll =================================================================== --- llvm/trunk/test/Instrumentation/BoundsChecking/many-traps-2.ll +++ llvm/trunk/test/Instrumentation/BoundsChecking/many-traps-2.ll @@ -0,0 +1,65 @@ +; RUN: opt < %s -bounds-checking -S | FileCheck %s +@array = internal global [1819 x i16] zeroinitializer, section ".bss,bss" +@offsets = external dso_local global [10 x i16] + +; CHECK-LABEL: @test +define dso_local void @test() { +bb1: + br label %bb19 + +bb20: + %_tmp819 = load i16, i16* null +; CHECK: br {{.*}} %trap + %_tmp820 = sub nsw i16 9, %_tmp819 + %_tmp821 = sext i16 %_tmp820 to i64 + %_tmp822 = getelementptr [10 x i16], [10 x i16]* @offsets, i16 0, i64 %_tmp821 + %_tmp823 = load i16, i16* %_tmp822 + br label %bb33 + +bb34: + %_tmp907 = zext i16 %i__7.107.0 to i64 + %_tmp908 = getelementptr [1819 x i16], [1819 x i16]* @array, i16 0, i64 %_tmp907 + store i16 0, i16* %_tmp908 +; CHECK: br {{.*}} %trap + %_tmp910 = add i16 %i__7.107.0, 1 + br label %bb33 + +bb33: + %i__7.107.0 = phi i16 [ undef, %bb20 ], [ %_tmp910, %bb34 ] + %_tmp913 = add i16 %_tmp823, 191 + %_tmp914 = icmp ult i16 %i__7.107.0, %_tmp913 + br i1 %_tmp914, label %bb34, label %bb19 + +bb19: + %_tmp976 = icmp slt i16 0, 10 + br i1 %_tmp976, label %bb20, label %bb39 + +bb39: + ret void +} + +@e = dso_local local_unnamed_addr global [1 x i16] zeroinitializer, align 1 + +; CHECK-LABEL: @test2 +define dso_local void @test2() local_unnamed_addr { +entry: + br label %while.cond1.preheader + +while.cond1.preheader: + %0 = phi i16 [ undef, %entry ], [ %inc, %while.end ] + %1 = load i16, i16* undef, align 1 +; CHECK: br {{.*}} %trap + br label %while.end + +while.end: + %inc = add nsw i16 %0, 1 + %arrayidx = getelementptr inbounds [1 x i16], [1 x i16]* @e, i16 0, i16 + %0 + %2 = load i16, i16* %arrayidx, align 1 +; CHECK: or i1 +; CHECK-NEXT: br {{.*}} %trap + br i1 false, label %while.end6, label %while.cond1.preheader + +while.end6: + ret void +}