From f78c27b74d01fc9a4f099de5188867a17b0ccd7f Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Wed, 10 Apr 2019 23:42:11 +0000 Subject: [PATCH] Fix a hang when lowering __builtin_dynamic_object_size If the ObjectSizeOffsetEvaluator fails to fold the object size call, then it may litter some unused instructions in the function. When done repeatably in InstCombine, this results in an infinite loop. Fix this by tracking the set of instructions that were inserted, then removing them on failure. rdar://49172227 Differential revision: https://reviews.llvm.org/D60298 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358146 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Analysis/MemoryBuiltins.h | 3 +- lib/Analysis/MemoryBuiltins.cpp | 22 +++++++++--- test/Instrumentation/BoundsChecking/phi.ll | 2 -- .../builtin-dynamic-object-size.ll | 34 +++++++++++++++++++ 4 files changed, 54 insertions(+), 7 deletions(-) diff --git a/include/llvm/Analysis/MemoryBuiltins.h b/include/llvm/Analysis/MemoryBuiltins.h index 49b87a43a48..9a5c77d2c1b 100644 --- a/include/llvm/Analysis/MemoryBuiltins.h +++ b/include/llvm/Analysis/MemoryBuiltins.h @@ -250,7 +250,7 @@ using SizeOffsetEvalType = std::pair; /// May create code to compute the result at run-time. class ObjectSizeOffsetEvaluator : public InstVisitor { - using BuilderTy = IRBuilder; + using BuilderTy = IRBuilder; using WeakEvalType = std::pair; using CacheMapTy = DenseMap; using PtrSetTy = SmallPtrSet; @@ -264,6 +264,7 @@ class ObjectSizeOffsetEvaluator CacheMapTy CacheMap; PtrSetTy SeenVals; ObjectSizeOpts EvalOpts; + SmallPtrSet InsertedInstructions; SizeOffsetEvalType compute_(Value *V); diff --git a/lib/Analysis/MemoryBuiltins.cpp b/lib/Analysis/MemoryBuiltins.cpp index 56332bb5fe8..08254266482 100644 --- a/lib/Analysis/MemoryBuiltins.cpp +++ b/lib/Analysis/MemoryBuiltins.cpp @@ -765,7 +765,10 @@ SizeOffsetType ObjectSizeOffsetVisitor::visitInstruction(Instruction &I) { ObjectSizeOffsetEvaluator::ObjectSizeOffsetEvaluator( const DataLayout &DL, const TargetLibraryInfo *TLI, LLVMContext &Context, ObjectSizeOpts EvalOpts) - : DL(DL), TLI(TLI), Context(Context), Builder(Context, TargetFolder(DL)), + : DL(DL), TLI(TLI), Context(Context), + Builder(Context, TargetFolder(DL), + IRBuilderCallbackInserter( + [&](Instruction *I) { InsertedInstructions.insert(I); })), EvalOpts(EvalOpts) { // IntTy and Zero must be set for each compute() since the address space may // be different for later objects. @@ -788,9 +791,16 @@ SizeOffsetEvalType ObjectSizeOffsetEvaluator::compute(Value *V) { if (CacheIt != CacheMap.end() && anyKnown(CacheIt->second)) CacheMap.erase(CacheIt); } + + // Erase any instructions we inserted as part of the traversal. + for (Instruction *I : InsertedInstructions) { + I->replaceAllUsesWith(UndefValue::get(I->getType())); + I->eraseFromParent(); + } } SeenVals.clear(); + InsertedInstructions.clear(); return Result; } @@ -934,24 +944,28 @@ SizeOffsetEvalType ObjectSizeOffsetEvaluator::visitPHINode(PHINode &PHI) { if (!bothKnown(EdgeData)) { OffsetPHI->replaceAllUsesWith(UndefValue::get(IntTy)); OffsetPHI->eraseFromParent(); + InsertedInstructions.erase(OffsetPHI); SizePHI->replaceAllUsesWith(UndefValue::get(IntTy)); SizePHI->eraseFromParent(); + InsertedInstructions.erase(SizePHI); return unknown(); } SizePHI->addIncoming(EdgeData.first, PHI.getIncomingBlock(i)); OffsetPHI->addIncoming(EdgeData.second, PHI.getIncomingBlock(i)); } - Value *Size = SizePHI, *Offset = OffsetPHI, *Tmp; - if ((Tmp = SizePHI->hasConstantValue())) { + Value *Size = SizePHI, *Offset = OffsetPHI; + if (Value *Tmp = SizePHI->hasConstantValue()) { Size = Tmp; SizePHI->replaceAllUsesWith(Size); SizePHI->eraseFromParent(); + InsertedInstructions.erase(SizePHI); } - if ((Tmp = OffsetPHI->hasConstantValue())) { + if (Value *Tmp = OffsetPHI->hasConstantValue()) { Offset = Tmp; OffsetPHI->replaceAllUsesWith(Offset); OffsetPHI->eraseFromParent(); + InsertedInstructions.erase(OffsetPHI); } return std::make_pair(Size, Offset); } diff --git a/test/Instrumentation/BoundsChecking/phi.ll b/test/Instrumentation/BoundsChecking/phi.ll index 15361b64d77..6f1c753b88f 100644 --- a/test/Instrumentation/BoundsChecking/phi.ll +++ b/test/Instrumentation/BoundsChecking/phi.ll @@ -58,8 +58,6 @@ define void @f1_as1(i8 addrspace(1)* nocapture %c) { ; CHECK: @f1_as1 ; no checks are possible here ; CHECK-NOT: trap -; CHECK: add i16 undef, -1 -; CHECK-NOT: trap entry: %0 = load i8, i8 addrspace(1)* %c, align 1 %tobool1 = icmp eq i8 %0, 0 diff --git a/test/Transforms/InstCombine/builtin-dynamic-object-size.ll b/test/Transforms/InstCombine/builtin-dynamic-object-size.ll index a0c2d139916..eabe3a4c4b7 100644 --- a/test/Transforms/InstCombine/builtin-dynamic-object-size.ll +++ b/test/Transforms/InstCombine/builtin-dynamic-object-size.ll @@ -71,6 +71,40 @@ entry: ; CHECK: ret i64 0 +@d = common global i8 0, align 1 +@c = common global i32 0, align 4 + +; Function Attrs: nounwind +define void @f() { +entry: + %.pr = load i32, i32* @c, align 4 + %tobool4 = icmp eq i32 %.pr, 0 + br i1 %tobool4, label %for.end, label %for.body + +for.body: ; preds = %entry, %for.body + %dp.05 = phi i8* [ %add.ptr, %for.body ], [ @d, %entry ] + %0 = tail call i64 @llvm.objectsize.i64.p0i8(i8* %dp.05, i1 false, i1 true, i1 true) + %conv = trunc i64 %0 to i32 + tail call void @bury(i32 %conv) #3 + %1 = load i32, i32* @c, align 4 + %idx.ext = sext i32 %1 to i64 + %add.ptr.offs = add i64 %idx.ext, 0 + %2 = add i64 undef, %add.ptr.offs + %add.ptr = getelementptr inbounds i8, i8* %dp.05, i64 %idx.ext + %add = shl nsw i32 %1, 1 + store i32 %add, i32* @c, align 4 + %tobool = icmp eq i32 %1, 0 + br i1 %tobool, label %for.end, label %for.body + +for.end: ; preds = %for.body, %entry + ret void +} + +; CHECK: define void @f() +; CHECK: call i64 @llvm.objectsize.i64.p0i8( + +declare void @bury(i32) local_unnamed_addr #2 + ; Function Attrs: nounwind allocsize(0) declare i8* @malloc(i64) -- 2.50.1