Skip to content

Commit bd4fef4

Browse files
committed
IR: Do not canonicalize constant GEPs into an out-of-bounds array access
Summary: Consider a GEP of: i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c, i32 0, i32 0, i64 0) If we proceeded to GEP the aforementioned object by 8, would form a GEP of: i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c, i32 0, i32 0, i64 8) Note that we would go through the first array member, causing an out-of-bounds accesses. This is problematic because we might get fooled if we are trying to evaluate loads using this GEP, for example, based off of an object with a constant initializer where the array is zero. This fixes PR17732. Reviewers: nicholas, chandlerc, void Reviewed By: void CC: llvm-commits, echristo, void, aemerson Differential Revision: http://llvm-reviews.chandlerc.com/D2093 llvm-svn: 194220
1 parent b805c68 commit bd4fef4

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

llvm/lib/IR/ConstantFold.cpp

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1940,7 +1940,43 @@ static Constant *ConstantFoldGetElementPtrImpl(Constant *C,
19401940
I != E; ++I)
19411941
LastTy = *I;
19421942

1943-
if ((LastTy && isa<SequentialType>(LastTy)) || Idx0->isNullValue()) {
1943+
// We cannot combine indices if doing so would take us outside of an
1944+
// array or vector. Doing otherwise could trick us if we evaluated such a
1945+
// GEP as part of a load.
1946+
//
1947+
// e.g. Consider if the original GEP was:
1948+
// i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,
1949+
// i32 0, i32 0, i64 0)
1950+
//
1951+
// If we then tried to offset it by '8' to get to the third element,
1952+
// an i8, we should *not* get:
1953+
// i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,
1954+
// i32 0, i32 0, i64 8)
1955+
//
1956+
// This GEP tries to index array element '8 which runs out-of-bounds.
1957+
// Subsequent evaluation would get confused and produce erroneous results.
1958+
//
1959+
// The following prohibits such a GEP from being formed by checking to see
1960+
// if the index is in-range with respect to an array or vector.
1961+
bool IsSequentialAccessInRange = false;
1962+
if (LastTy && isa<SequentialType>(LastTy)) {
1963+
uint64_t NumElements = 0;
1964+
if (ArrayType *ATy = dyn_cast<ArrayType>(LastTy))
1965+
NumElements = ATy->getNumElements();
1966+
else if (VectorType *VTy = dyn_cast<VectorType>(LastTy))
1967+
NumElements = VTy->getNumElements();
1968+
1969+
if (ConstantInt *CI = dyn_cast<ConstantInt>(Idx0)) {
1970+
int64_t Idx0Val = CI->getSExtValue();
1971+
if (NumElements > 0 && Idx0Val >= 0 &&
1972+
(uint64_t)Idx0Val < NumElements)
1973+
IsSequentialAccessInRange = true;
1974+
} else if (PointerType *PTy = dyn_cast<PointerType>(LastTy))
1975+
// Only handle pointers to sized types, not pointers to functions.
1976+
if (PTy->getElementType()->isSized())
1977+
IsSequentialAccessInRange = true;
1978+
}
1979+
if (IsSequentialAccessInRange || Idx0->isNullValue()) {
19441980
SmallVector<Value*, 16> NewIndices;
19451981
NewIndices.reserve(Idxs.size() + CE->getNumOperands());
19461982
for (unsigned i = 1, e = CE->getNumOperands()-1; i != e; ++i)

llvm/test/Transforms/GVN/pr17732.ll

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
; RUN: opt -gvn -S -o - < %s | FileCheck %s
2+
3+
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"
4+
target triple = "x86_64-unknown-linux-gnu"
5+
6+
%struct.with_array = type { [2 x i8], i32, i8 }
7+
%struct.with_vector = type { <2 x i8>, i32, i8 }
8+
9+
@main.obj_with_array = private unnamed_addr constant { [2 x i8], i32, i8, [3 x i8] } { [2 x i8] zeroinitializer, i32 0, i8 1, [3 x i8] undef }, align 4
10+
@array_with_zeroinit = common global %struct.with_array zeroinitializer, align 4
11+
12+
@main.obj_with_vector = private unnamed_addr constant { <2 x i8>, i32, i8, [3 x i8] } { <2 x i8> zeroinitializer, i32 0, i8 1, [3 x i8] undef }, align 4
13+
@vector_with_zeroinit = common global %struct.with_vector zeroinitializer, align 4
14+
15+
define i32 @main() {
16+
entry:
17+
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds (%struct.with_array* @array_with_zeroinit, i64 0, i32 0, i64 0), i8* getelementptr inbounds ({ [2 x i8], i32, i8, [3 x i8] }* @main.obj_with_array, i64 0, i32 0, i64 0), i64 12, i32 4, i1 false)
18+
%0 = load i8* getelementptr inbounds (%struct.with_array* @array_with_zeroinit, i64 0, i32 2), align 4
19+
20+
tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* getelementptr inbounds (%struct.with_vector* @vector_with_zeroinit, i64 0, i32 0, i64 0), i8* getelementptr inbounds ({ <2 x i8>, i32, i8, [3 x i8] }* @main.obj_with_vector, i64 0, i32 0, i64 0), i64 12, i32 4, i1 false)
21+
%1 = load i8* getelementptr inbounds (%struct.with_vector* @vector_with_zeroinit, i64 0, i32 2), align 4
22+
%conv0 = sext i8 %0 to i32
23+
%conv1 = sext i8 %1 to i32
24+
%and = and i32 %conv0, %conv1
25+
ret i32 %and
26+
; CHECK-LABEL: define i32 @main(
27+
; CHECK: ret i32 1
28+
}
29+
30+
declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)

0 commit comments

Comments
 (0)