<div dir="ltr">I believe that it is impossible to hit the case, we can just return true for pointer types.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 20, 2015 at 10:39 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">(adding the new mailing list...<div><div class="h5"><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">David Blaikie</b> <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span><br>Date: Thu, Aug 20, 2015 at 10:35 PM<br>Subject: Re: [llvm] r194220 - IR: Do not canonicalize constant GEPs into an out-of-bounds array access<br>To: David Majnemer <<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>><br>Cc: "<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>" <<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a>><br><br><br><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Thu, Nov 7, 2013 at 2:15 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: majnemer<br>
Date: Thu Nov 7 16:15:53 2013<br>
New Revision: 194220<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=194220&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=194220&view=rev</a><br>
Log:<br>
IR: Do not canonicalize constant GEPs into an out-of-bounds array access<br>
<br>
Summary:<br>
Consider a GEP of:<br>
i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c, i32 0, i32 0, i64 0)<br>
<br>
If we proceeded to GEP the aforementioned object by 8, would form a GEP of:<br>
i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c, i32 0, i32 0, i64 8)<br>
<br>
Note that we would go through the first array member, causing an<br>
out-of-bounds accesses. This is problematic because we might get fooled<br>
if we are trying to evaluate loads using this GEP, for example, based<br>
off of an object with a constant initializer where the array is zero.<br>
<br>
This fixes PR17732.<br>
<br>
Reviewers: nicholas, chandlerc, void<br>
<br>
Reviewed By: void<br>
<br>
CC: llvm-commits, echristo, void, aemerson<br>
<br>
Differential Revision: <a href="http://llvm-reviews.chandlerc.com/D2093" rel="noreferrer" target="_blank">http://llvm-reviews.chandlerc.com/D2093</a><br>
<br>
Added:<br>
llvm/trunk/test/Transforms/GVN/pr17732.ll<br>
Modified:<br>
llvm/trunk/lib/IR/ConstantFold.cpp<br>
<br>
Modified: llvm/trunk/lib/IR/ConstantFold.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=194220&r1=194219&r2=194220&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/ConstantFold.cpp?rev=194220&r1=194219&r2=194220&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/IR/ConstantFold.cpp (original)<br>
+++ llvm/trunk/lib/IR/ConstantFold.cpp Thu Nov 7 16:15:53 2013<br>
@@ -1940,7 +1940,43 @@ static Constant *ConstantFoldGetElementP<br>
I != E; ++I)<br>
LastTy = *I;<br>
<br>
- if ((LastTy && isa<SequentialType>(LastTy)) || Idx0->isNullValue()) {<br>
+ // We cannot combine indices if doing so would take us outside of an<br>
+ // array or vector. Doing otherwise could trick us if we evaluated such a<br>
+ // GEP as part of a load.<br>
+ //<br>
+ // e.g. Consider if the original GEP was:<br>
+ // i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,<br>
+ // i32 0, i32 0, i64 0)<br>
+ //<br>
+ // If we then tried to offset it by '8' to get to the third element,<br>
+ // an i8, we should *not* get:<br>
+ // i8* getelementptr ({ [2 x i8], i32, i8, [3 x i8] }* @main.c,<br>
+ // i32 0, i32 0, i64 8)<br>
+ //<br>
+ // This GEP tries to index array element '8 which runs out-of-bounds.<br>
+ // Subsequent evaluation would get confused and produce erroneous results.<br>
+ //<br>
+ // The following prohibits such a GEP from being formed by checking to see<br>
+ // if the index is in-range with respect to an array or vector.<br>
+ bool IsSequentialAccessInRange = false;<br>
+ if (LastTy && isa<SequentialType>(LastTy)) {<br>
+ uint64_t NumElements = 0;<br>
+ if (ArrayType *ATy = dyn_cast<ArrayType>(LastTy))<br>
+ NumElements = ATy->getNumElements();<br>
+ else if (VectorType *VTy = dyn_cast<VectorType>(LastTy))<br>
+ NumElements = VTy->getNumElements();<br>
+<br>
+ if (ConstantInt *CI = dyn_cast<ConstantInt>(Idx0)) {<br>
+ int64_t Idx0Val = CI->getSExtValue();<br>
+ if (NumElements > 0 && Idx0Val >= 0 &&<br>
+ (uint64_t)Idx0Val < NumElements)<br>
+ IsSequentialAccessInRange = true;<br>
+ } else if (PointerType *PTy = dyn_cast<PointerType>(LastTy))<br>
+ // Only handle pointers to sized types, not pointers to functions.<br>
+ if (PTy->getElementType()->isSized())<br></blockquote></div></div><div><br>While going about my business removing access to pointee types, I came across this code. Removing this conditional ^ doesn't result in any tests failing. Any idea if it's necessary/useful and could/should be tested? (& if it's necessary, how to implement it in a world involving opaque pointer types)<br> </div><div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ IsSequentialAccessInRange = true;<br>
+ }<br>
+ if (IsSequentialAccessInRange || Idx0->isNullValue()) {<br>
SmallVector<Value*, 16> NewIndices;<br>
NewIndices.reserve(Idxs.size() + CE->getNumOperands());<br>
for (unsigned i = 1, e = CE->getNumOperands()-1; i != e; ++i)<br>
<br>
Added: llvm/trunk/test/Transforms/GVN/pr17732.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/pr17732.ll?rev=194220&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GVN/pr17732.ll?rev=194220&view=auto</a><br>
==============================================================================<br>
--- llvm/trunk/test/Transforms/GVN/pr17732.ll (added)<br>
+++ llvm/trunk/test/Transforms/GVN/pr17732.ll Thu Nov 7 16:15:53 2013<br>
@@ -0,0 +1,30 @@<br>
+; RUN: opt -gvn -S -o - < %s | FileCheck %s<br>
+<br>
+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"<br>
+target triple = "x86_64-unknown-linux-gnu"<br>
+<br>
+%struct.with_array = type { [2 x i8], i32, i8 }<br>
+%struct.with_vector = type { <2 x i8>, i32, i8 }<br>
+<br>
+@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<br>
+@array_with_zeroinit = common global %struct.with_array zeroinitializer, align 4<br>
+<br>
+@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<br>
+@vector_with_zeroinit = common global %struct.with_vector zeroinitializer, align 4<br>
+<br>
+define i32 @main() {<br>
+entry:<br>
+ 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)<br>
+ %0 = load i8* getelementptr inbounds (%struct.with_array* @array_with_zeroinit, i64 0, i32 2), align 4<br>
+<br>
+ 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)<br>
+ %1 = load i8* getelementptr inbounds (%struct.with_vector* @vector_with_zeroinit, i64 0, i32 2), align 4<br>
+ %conv0 = sext i8 %0 to i32<br>
+ %conv1 = sext i8 %1 to i32<br>
+ %and = and i32 %conv0, %conv1<br>
+ ret i32 %and<br>
+; CHECK-LABEL: define i32 @main(<br>
+; CHECK: ret i32 1<br>
+}<br>
+<br>
+declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture readonly, i64, i32, i1)<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div><br></div></div>
</div><br></div></div></div>
</blockquote></div><br></div>