[SROA] Fix PR18615 with some long overdue simplifications to the bounds

checking in SROA.

The primary change is to just rely on uge for checking that the offset
is within the allocation size. This removes the explicit checks against
isNegative which were terribly error prone (including the reversed logic
that led to PR18615) and prevented us from supporting stack allocations
larger than half the address space.... Ok, so maybe the latter isn't
*common* but it's a silly restriction to have.

Also, we used to try to support a PHI node which loaded from before the
start of the allocation if any of the loaded bytes were within the
allocation. This doesn't make any sense, we have never really supported
loading or storing *before* the allocation starts. The simplified logic
just doesn't care.

We continue to allow loading past the end of the allocation in part to
support cases where there is a PHI and some loads are larger than others
and the larger ones reach past the end of the allocation. We could solve
this a different and more conservative way, but I'm still somewhat
paranoid about this.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@202224 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
Chandler Carruth 2014-02-26 03:14:14 +00:00
parent 5820a9389e
commit 50bc165c54
2 changed files with 18 additions and 9 deletions

View File

@ -356,7 +356,7 @@ private:
bool IsSplittable = false) {
// Completely skip uses which have a zero size or start either before or
// past the end of the allocation.
if (Size == 0 || Offset.isNegative() || Offset.uge(AllocSize)) {
if (Size == 0 || Offset.uge(AllocSize)) {
DEBUG(dbgs() << "WARNING: Ignoring " << Size << " byte use @" << Offset
<< " which has zero size or starts outside of the "
<< AllocSize << " byte alloca:\n"
@ -480,8 +480,7 @@ private:
// risk of overflow.
// FIXME: We should instead consider the pointer to have escaped if this
// function is being instrumented for addressing bugs or race conditions.
if (Offset.isNegative() || Size > AllocSize ||
Offset.ugt(AllocSize - Size)) {
if (Size > AllocSize || Offset.ugt(AllocSize - Size)) {
DEBUG(dbgs() << "WARNING: Ignoring " << Size << " byte store @" << Offset
<< " which extends past the end of the " << AllocSize
<< " byte alloca:\n"
@ -500,7 +499,7 @@ private:
assert(II.getRawDest() == *U && "Pointer use is not the destination?");
ConstantInt *Length = dyn_cast<ConstantInt>(II.getLength());
if ((Length && Length->getValue() == 0) ||
(IsOffsetKnown && !Offset.isNegative() && Offset.uge(AllocSize)))
(IsOffsetKnown && Offset.uge(AllocSize)))
// Zero-length mem transfer intrinsics can be ignored entirely.
return markAsDead(II);
@ -532,7 +531,7 @@ private:
// if already added to our partitions.
// FIXME: Yet another place we really should bypass this when
// instrumenting for ASan.
if (!Offset.isNegative() && Offset.uge(AllocSize)) {
if (Offset.uge(AllocSize)) {
SmallDenseMap<Instruction *, unsigned>::iterator MTPI = MemTransferSliceMap.find(&II);
if (MTPI != MemTransferSliceMap.end())
S.Slices[MTPI->second].kill();
@ -667,8 +666,7 @@ private:
// themselves which should be replaced with undef.
// FIXME: This should instead be escaped in the event we're instrumenting
// for address sanitization.
if ((Offset.isNegative() && (-Offset).uge(PHISize)) ||
(!Offset.isNegative() && Offset.uge(AllocSize))) {
if (Offset.uge(AllocSize)) {
S.DeadOperands.push_back(U);
return;
}
@ -708,8 +706,7 @@ private:
// themselves which should be replaced with undef.
// FIXME: This should instead be escaped in the event we're instrumenting
// for address sanitization.
if ((Offset.isNegative() && Offset.uge(SelectSize)) ||
(!Offset.isNegative() && Offset.uge(AllocSize))) {
if (Offset.uge(AllocSize)) {
S.DeadOperands.push_back(U);
return;
}

View File

@ -1393,3 +1393,15 @@ entry:
call void @llvm.memcpy.p0i8.p0i8.i32(i8* %cast1, i8* %cast0, i32 4, i32 1, i1 false)
ret void
}
define void @PR18615() {
; CHECK-LABEL: @PR18615(
; CHECK-NOT: alloca
; CHECK: ret void
entry:
%f = alloca i8
%gep = getelementptr i8* %f, i64 -1
call void @llvm.memcpy.p0i8.p0i8.i32(i8* undef, i8* %gep, i32 1, i32 1, i1 false)
ret void
}