[PATCH] IR: Do not canonicalize constant GEPs into an out-of-bounds array access

[David Majnemer]

    - Correct some spelling.

Hi nicholas, chandlerc,

http://llvm-reviews.chandlerc.com/D2093

CHANGE SINCE LAST DIFF
  http://llvm-reviews.chandlerc.com/D2093?vs=5394&id=5395#toc

Files:
  lib/IR/ConstantFold.cpp
  test/Transforms/GVN/pr17732.ll

Index: lib/IR/ConstantFold.cpp
===================================================================
--- lib/IR/ConstantFold.cpp
+++ lib/IR/ConstantFold.cpp
@@ -1940,7 +1940,43 @@
            I != E; ++I)
         LastTy = *I;
 
-      if ((LastTy && isa<SequentialType>(LastTy)) || Idx0->isNullValue()) {
+      // We cannot combine indices if doing so would take us outside of an
+      // array or vector.  Doing otherwise could trick us if we evaluated such a
+      // GEP as part of a load.
+      //
+      // e.g. Consider if the original GEP was:
+      // i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,
+      //                    i32 0, i32 0, i64 0)
+      //
+      // If we then tried to offset it by '8' to get to the third element,
+      // an i8, we should *not* get:
+      // i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,
+      //                    i32 0, i32 0, i64 8)
+      //
+      // This GEP tries to index array element '8  which runs out-of-bounds.
+      // Subsequent evaluation would get confused and produce erroneous results.
+      //
+      // The following prohibits such a GEP from being formed by checking to see
+      // if the index is in-range with respect to an array or vector.
+      bool IsSequentialAccessInRange = false;
+      if (LastTy && isa<SequentialType>(LastTy)) {
+        uint64_t NumElements = 0;
+        if (ArrayType *ATy = dyn_cast<ArrayType>(LastTy))
+          NumElements = ATy->getNumElements();
+        else if (VectorType *VTy = dyn_cast<VectorType>(LastTy))
+          NumElements = VTy->getNumElements();
+
+        if (ConstantInt *CI = dyn_cast<ConstantInt>(Idx0)) {
+          int64_t Idx0Val = CI->getSExtValue();
+          if (NumElements > 0 && Idx0Val >= 0 &&
+              (uint64_t)Idx0Val < NumElements)
+            IsSequentialAccessInRange = true;
+        } else if (PointerType *PTy = dyn_cast<PointerType>(LastTy))
+          // Only handle pointers to sized types, not pointers to functions.
+          if (PTy->getElementType()->isSized())
+            IsSequentialAccessInRange = true;
+      }
+      if (IsSequentialAccessInRange || Idx0->isNullValue()) {
         SmallVector<Value*, 16> NewIndices;
         NewIndices.reserve(Idxs.size() + CE->getNumOperands());
         for (unsigned i = 1, e = CE->getNumOperands()-1; i != e; ++i)
Index: test/Transforms/GVN/pr17732.ll
===================================================================
--- /dev/null
+++ test/Transforms/GVN/pr17732.ll
@@ -0,0 +1,30 @@
+; RUN: opt -gvn -S -o - < %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"
+target triple = "x86_64-unknown-linux-gnu"
+
+%struct.with_array = type { [2 x i8], i32, i8 }
+%struct.with_vector = type { <2 x i8>, i32, i8 }
+
+ at 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
+ at array_with_zeroinit = common global %struct.with_array zeroinitializer, align 4
+
+ at 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
+ at vector_with_zeroinit = common global %struct.with_vector zeroinitializer, align 4
+
+define i32 @main() {
+entry:
+  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)
+  %0 = load i8* getelementptr inbounds (%struct.with_array* @array_with_zeroinit, i64 0, i32 2), align 4
+
+  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)
+  %1 = load i8* getelementptr inbounds (%struct.with_vector* @vector_with_zeroinit, i64 0, i32 2), align 4
+  %conv0 = sext i8 %0 to i32
+  %conv1 = sext i8 %1 to i32
+  %and = and i32 %conv0, %conv1
+  ret i32 %and
+; CHECK-LABEL: define i32 @main(
+; CHECK: ret i32 1
+}
+
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D2093.4.patch
Type: text/x-patch
Size: 4420 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131107/e92f3a19/attachment.bin>