Patch Review for PR17158

[David Majnemer]

This looks awfully familiar to a bug that my patch fixes.  I tried a
similar approach earlier in http://llvm-reviews.chandlerc.com/D2060 but
instead settled on http://llvm-reviews.chandlerc.com/D2093

I have a strong feeling that my second patch will fix your bug (and
potentially others).

 --
David Majnemer


On Wed, Nov 6, 2013 at 4:07 PM, Bill Wendling <isanbard at gmail.com> wrote:

> 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;
>    }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131106/28d36d87/attachment.html>