Patch Review for PR17158

[Bill Wendling]

Hi Chandler et al,

This is a potential patch for PR17158. What seems to be happening is that GVN gets ahold of the file, it tries to determine if the initializer for x7 can be folded into the use. Because of SROA, it's doing some weird pointer things involving the structure padding. So for instance, the structure is this:

	{ { i32, i8, [3 x i8] }, i8, [3 x i8] }

It copies over the 'i32' and 'i8' from the inner structure. SROA then generates a memcpy from the [3 x i8] in the inner structure to the end of the outer structure, which sounds okay. However, GVN looks at the access to the outer 'i8' element, but because of SROA, that element is generated as the element of the inner [3 x i8] at offset 3. This is of course outside of the array. Because it's padding, the value it returns is undef. Thus it folds the value of '0' into the call to 'printf'.

Got that so far? ;-)

Here's a patch I have. It basically makes sure that, if the constant it's looking at is an array, the index into that array does not go outside of that array. If that happens, then there's something complex going on and the constant folding isn't safe anymore.

What do you think? Is this the correct solution?

-bw

Run the attached t.ll with "opt -sroa -instcombine -gvn", generate the .s file, and run it. It should print '1', but instead prints '0'.

diff --git a/lib/Analysis/ConstantFolding.cpp b/lib/Analysis/ConstantFolding.cpp
index e093631..527fa80 100644
--- a/lib/Analysis/ConstantFolding.cpp
+++ b/lib/Analysis/ConstantFolding.cpp
@@ -1145,7 +1145,11 @@ Constant *llvm::ConstantFoldLoadThroughGEPConstantExpr(Constant *C,
   // Loop over all of the operands, tracking down which value we are
   // addressing.
   for (unsigned i = 2, e = CE->getNumOperands(); i != e; ++i) {
-    C = C->getAggregateElement(CE->getOperand(i));
+    Constant *Offset = CE->getOperand(i);
+    if (const ArrayType *AT = dyn_cast<ArrayType>(C->getType()))
+      if (AT->getNumElements() <= cast<ConstantInt>(Offset)->getZExtValue())
+        return 0;
+    C = C->getAggregateElement(Offset);
     if (C == 0)
       return 0;
   }

-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.ll
Type: application/octet-stream
Size: 2925 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131106/f72ea040/attachment.obj>