[llvm-commits] [llvm] r157822 - in /llvm/trunk: lib/Transforms/Scalar/BoundsChecking.cpp test/Transforms/BoundsChecking/phi.ll test/Transforms/BoundsChecking/simple.ll
Nuno Lopes
nunoplopes at sapo.pt
Fri Jun 1 10:43:31 PDT 2012
Author: nlopes
Date: Fri Jun 1 12:43:31 2012
New Revision: 157822
URL: http://llvm.org/viewvc/llvm-project?rev=157822&view=rev
Log:
BoundsChecking: fix a bug when the handling of recursive PHIs failed and could leave dangling references in the cache
add regression tests for this problem.
Can already compile & run: PHP, PCRE, and ICU (i.e., all the software I tried)
Added:
llvm/trunk/test/Transforms/BoundsChecking/phi.ll
Modified:
llvm/trunk/lib/Transforms/Scalar/BoundsChecking.cpp
llvm/trunk/test/Transforms/BoundsChecking/simple.ll
Modified: llvm/trunk/lib/Transforms/Scalar/BoundsChecking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/BoundsChecking.cpp?rev=157822&r1=157821&r2=157822&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/BoundsChecking.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/BoundsChecking.cpp Fri Jun 1 12:43:31 2012
@@ -55,8 +55,13 @@
APInt Size;
Value *SizeValue;
bool ReturnVal;
+ CacheData() {}
+ CacheData(APInt Off, Value *OffVal, APInt Sz, Value *SzVal, bool Ret) :
+ Offset(Off), OffsetValue(OffVal), Size(Sz), SizeValue(SzVal),
+ ReturnVal(Ret) {}
};
typedef DenseMap<Value*, CacheData> CacheMapTy;
+ typedef SmallPtrSet<Value*, 8> PtrSetTy;
struct BoundsChecking : public FunctionPass {
static char ID;
@@ -82,6 +87,7 @@
BasicBlock *TrapBB;
unsigned Penalty;
CacheMapTy CacheMap;
+ PtrSetTy SeenPtrs;
BasicBlock *getTrapBB();
void emitBranchToTrap(Value *Cmp = 0);
@@ -163,6 +169,10 @@
return Cache.ReturnVal;
}
+ // record the pointers that were handled in this run, so that they can be
+ // cleaned later if something fails
+ SeenPtrs.insert(Ptr);
+
IntegerType *IntTy = TD->getIntPtrType(Fn->getContext());
unsigned IntTyBits = IntTy->getBitWidth();
bool ReturnVal;
@@ -194,11 +204,12 @@
RETURN(true);
}
OffsetValue = ConstantInt::get(IntTy, Offset);
- } else {
+ } else if (Penalty > 1) {
OffsetValue = EmitGEPOffset(Builder, *TD, GEP);
- }
+ GET_VALUE(PtrOffsetValue, PtrOffset);
+ } else
+ RETURN(false);
- GET_VALUE(PtrOffsetValue, PtrOffset);
OffsetValue = Builder->CreateAdd(PtrOffsetValue, OffsetValue);
RETURN(true);
@@ -259,7 +270,7 @@
bool FalseAlloc = computeAllocSize(SI->getFalseValue(), OffsetFalse,
OffsetValueFalse, SizeFalse,
SizeValueFalse);
- if (!TrueAlloc && !FalseAlloc)
+ if (!TrueAlloc || !FalseAlloc)
RETURN(false);
// fold constant sizes & offsets if they are equal
@@ -378,22 +389,24 @@
PHINode *SizePHI = Builder->CreatePHI(IntTy, PHI->getNumIncomingValues());
// insert right away in the cache to handle recursive PHIs
- CacheData CacheEntry;
- CacheEntry.Offset = CacheEntry.Size = 0;
- CacheEntry.OffsetValue = OffsetPHI;
- CacheEntry.SizeValue = SizePHI;
- CacheEntry.ReturnVal = true;
+ CacheData CacheEntry(APInt(), OffsetPHI, APInt(), SizePHI, true);
CacheMap[Ptr] = CacheEntry;
// compute offset/size for each PHI incoming pointer
- bool someOk = false;
for (unsigned i = 0, e = PHI->getNumIncomingValues(); i != e; ++i) {
Builder->SetInsertPoint(PHI->getIncomingBlock(i)->getFirstInsertionPt());
APInt PhiOffset(IntTyBits, 0), PhiSize(IntTyBits, 0);
Value *PhiOffsetValue = 0, *PhiSizeValue = 0;
- someOk |= computeAllocSize(PHI->getIncomingValue(i), PhiOffset,
- PhiOffsetValue, PhiSize, PhiSizeValue);
+
+ if (!computeAllocSize(PHI->getIncomingValue(i), PhiOffset, PhiOffsetValue,
+ PhiSize, PhiSizeValue)) {
+ OffsetPHI->replaceAllUsesWith(UndefValue::get(IntTy));
+ OffsetPHI->eraseFromParent();
+ SizePHI->replaceAllUsesWith(UndefValue::get(IntTy));
+ SizePHI->eraseFromParent();
+ RETURN(false);
+ }
GET_VALUE(PhiOffsetValue, PhiOffset);
GET_VALUE(PhiSizeValue, PhiSize);
@@ -402,10 +415,6 @@
SizePHI->addIncoming(PhiSizeValue, PHI->getIncomingBlock(i));
}
- // fail here if we couldn't compute the size/offset in any incoming edge
- if (!someOk)
- RETURN(false);
-
OffsetValue = OffsetPHI;
SizeValue = SizePHI;
RETURN(true);
@@ -423,14 +432,13 @@
cache_and_return:
// cache the result and return
- CacheData CacheEntry;
- CacheEntry.Offset = Offset;
- CacheEntry.OffsetValue = OffsetValue;
- CacheEntry.Size = Size;
- CacheEntry.SizeValue = SizeValue;
- CacheEntry.ReturnVal = ReturnVal;
+ CacheData CacheEntry(Offset, OffsetValue, Size, SizeValue, ReturnVal);
CacheMap[Ptr] = CacheEntry;
+ // non-computable results can be safely cached
+ if (!ReturnVal)
+ SeenPtrs.erase(Ptr);
+
Builder->SetInsertPoint(PrevInsertPoint);
return ReturnVal;
}
@@ -454,6 +462,15 @@
if (!computeAllocSize(Ptr, Offset, OffsetValue, Size, SizeValue)) {
DEBUG(dbgs() << "computeAllocSize failed:\n" << *Ptr << "\n");
+
+ // erase everything that was computed in this iteration from the cache, so
+ // that no dangling references are left behind. We could be a bit smarter if
+ // we kept a dependency graph. It's probably not worth the complexity,
+ // though.
+ for (PtrSetTy::iterator I=SeenPtrs.begin(), E=SeenPtrs.end(); I != E; ++I)
+ CacheMap.erase(*I);
+ SeenPtrs.clear();
+
++ChecksUnable;
return false;
}
Added: llvm/trunk/test/Transforms/BoundsChecking/phi.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BoundsChecking/phi.ll?rev=157822&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/BoundsChecking/phi.ll (added)
+++ llvm/trunk/test/Transforms/BoundsChecking/phi.ll Fri Jun 1 12:43:31 2012
@@ -0,0 +1,48 @@
+; RUN: opt < %s -bounds-checking -S | FileCheck %s
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
+
+ at global = private unnamed_addr constant [10 x i8] c"ola\00mundo\00", align 1
+
+; CHECK: f1
+; no checks are possible here
+; CHECK-NOT: trap
+define void @f1(i8* nocapture %c) {
+entry:
+ %0 = load i8* %c, align 1
+ %tobool1 = icmp eq i8 %0, 0
+ br i1 %tobool1, label %while.end, label %while.body
+
+while.body:
+ %c.addr.02 = phi i8* [ %incdec.ptr, %while.body ], [ %c, %entry ]
+ %incdec.ptr = getelementptr inbounds i8* %c.addr.02, i64 -1
+ store i8 100, i8* %c.addr.02, align 1
+ %1 = load i8* %incdec.ptr, align 1
+ %tobool = icmp eq i8 %1, 0
+ br i1 %tobool, label %while.end, label %while.body
+
+while.end:
+ ret void
+}
+
+
+; CHECK: f2
+define void @f2() {
+while.body.i.preheader:
+ %addr = getelementptr inbounds [10 x i8]* @global, i64 0, i64 9
+ br label %while.body.i
+
+while.body.i:
+; CHECK: phi
+; CHECK-NEXT: phi
+; CHECK-NEXT: phi
+; CHECK: trap
+ %c.addr.02.i = phi i8* [ %incdec.ptr.i, %while.body.i ], [ %addr, %while.body.i.preheader ]
+ %incdec.ptr.i = getelementptr inbounds i8* %c.addr.02.i, i64 -1
+ store i8 100, i8* %c.addr.02.i, align 1
+ %0 = load i8* %incdec.ptr.i, align 1
+ %tobool.i = icmp eq i8 %0, 0
+ br i1 %tobool.i, label %fn.exit, label %while.body.i
+
+fn.exit:
+ ret void
+}
Modified: llvm/trunk/test/Transforms/BoundsChecking/simple.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/BoundsChecking/simple.ll?rev=157822&r1=157821&r2=157822&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/BoundsChecking/simple.ll (original)
+++ llvm/trunk/test/Transforms/BoundsChecking/simple.ll Fri Jun 1 12:43:31 2012
@@ -91,7 +91,7 @@
define void @f9(i128* %arg) nounwind {
%1 = alloca i128
%2 = select i1 undef, i128* %arg, i128* %1
-; CHECK: br i1 false
+; CHECK-NOT: trap
%3 = load i128* %2, align 4
ret void
}
More information about the llvm-commits
mailing list