[llvm] r358146 - Fix a hang when lowering __builtin_dynamic_object_size

Erik Pilkington via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 10 16:42:11 PDT 2019


Author: epilk
Date: Wed Apr 10 16:42:11 2019
New Revision: 358146

URL: http://llvm.org/viewvc/llvm-project?rev=358146&view=rev
Log:
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

Modified:
    llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h
    llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
    llvm/trunk/test/Instrumentation/BoundsChecking/phi.ll
    llvm/trunk/test/Transforms/InstCombine/builtin-dynamic-object-size.ll

Modified: llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h?rev=358146&r1=358145&r2=358146&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h (original)
+++ llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h Wed Apr 10 16:42:11 2019
@@ -250,7 +250,7 @@ using SizeOffsetEvalType = std::pair<Val
 /// May create code to compute the result at run-time.
 class ObjectSizeOffsetEvaluator
   : public InstVisitor<ObjectSizeOffsetEvaluator, SizeOffsetEvalType> {
-  using BuilderTy = IRBuilder<TargetFolder>;
+  using BuilderTy = IRBuilder<TargetFolder, IRBuilderCallbackInserter>;
   using WeakEvalType = std::pair<WeakTrackingVH, WeakTrackingVH>;
   using CacheMapTy = DenseMap<const Value *, WeakEvalType>;
   using PtrSetTy = SmallPtrSet<const Value *, 8>;
@@ -264,6 +264,7 @@ class ObjectSizeOffsetEvaluator
   CacheMapTy CacheMap;
   PtrSetTy SeenVals;
   ObjectSizeOpts EvalOpts;
+  SmallPtrSet<Instruction *, 8> InsertedInstructions;
 
   SizeOffsetEvalType compute_(Value *V);
 

Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=358146&r1=358145&r2=358146&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
+++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Wed Apr 10 16:42:11 2019
@@ -765,7 +765,10 @@ SizeOffsetType ObjectSizeOffsetVisitor::
 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 ObjectSizeOffsetEvalu
       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 ObjectSizeOffsetEvalu
     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);
 }

Modified: llvm/trunk/test/Instrumentation/BoundsChecking/phi.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/BoundsChecking/phi.ll?rev=358146&r1=358145&r2=358146&view=diff
==============================================================================
--- llvm/trunk/test/Instrumentation/BoundsChecking/phi.ll (original)
+++ llvm/trunk/test/Instrumentation/BoundsChecking/phi.ll Wed Apr 10 16:42:11 2019
@@ -58,8 +58,6 @@ define void @f1_as1(i8 addrspace(1)* noc
 ; 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

Modified: llvm/trunk/test/Transforms/InstCombine/builtin-dynamic-object-size.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/builtin-dynamic-object-size.ll?rev=358146&r1=358145&r2=358146&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstCombine/builtin-dynamic-object-size.ll (original)
+++ llvm/trunk/test/Transforms/InstCombine/builtin-dynamic-object-size.ll Wed Apr 10 16:42:11 2019
@@ -71,6 +71,40 @@ entry:
 
 ; CHECK: ret i64 0
 
+ at d = common global i8 0, align 1
+ at 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)
 




More information about the llvm-commits mailing list