[llvm-commits] [llvm] r155466 - in /llvm/trunk: lib/Analysis/ConstantFolding.cpp test/Transforms/GlobalOpt/constantfold-initializers.ll

[Chandler Carruth]

Hey Bill & Doug (cc-ed because it impacts only Clang / C++11 thus far),

This commit (despite going to LLVM) fixes a crash-on-valid in C++11 mode.
We hit this pretty early during stress testing of C++11 codegen, open
source packages like aspell trigger it.

Is this a good candidate for back-porting to the 3.1 branch?

-Chandler

On Tue, Apr 24, 2012 at 11:42 AM, Chandler Carruth <chandlerc at gmail.com>wrote:

> Author: chandlerc
> Date: Tue Apr 24 13:42:47 2012
> New Revision: 155466
>
> URL: http://llvm.org/viewvc/llvm-project?rev=155466&view=rev
> Log:
> Fix a crash on valid (if UB) bitcode that is produced for some global
> constants in C++11 mode. I have no idea why it required such particular
> circumstances to get here, the code seems clearly to rely upon unchecked
> assumptions.
>
> Specifically, when we decide to form an index into a struct type, we may
> have gone through (at least one) zero-length array indexing round, which
> would have left the offset un-adjusted, and thus not necessarily valid
> for use when indexing the struct type.
>
> This is just an canonicalization step, so the correct thing is to refuse
> to canonicalize nonsensical GEPs of this form. Implemented, and test
> case added.
>
> Fixes PR12642. Pair debugged and coded with Richard Smith. =] I credit
> him with most of the debugging, and preventing me from writing the wrong
> code.
>
> Modified:
>    llvm/trunk/lib/Analysis/ConstantFolding.cpp
>    llvm/trunk/test/Transforms/GlobalOpt/constantfold-initializers.ll
>
> Modified: llvm/trunk/lib/Analysis/ConstantFolding.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ConstantFolding.cpp?rev=155466&r1=155465&r2=155466&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ConstantFolding.cpp (original)
> +++ llvm/trunk/lib/Analysis/ConstantFolding.cpp Tue Apr 24 13:42:47 2012
> @@ -681,6 +681,7 @@
>   // This makes it easy to determine if the getelementptr is "inbounds".
>   // Also, this helps GlobalOpt do SROA on GlobalVariables.
>   Type *Ty = Ptr->getType();
> +  assert(Ty->isPointerTy() && "Forming regular GEP of non-pointer type");
>   SmallVector<Constant*, 32> NewIdxs;
>   do {
>     if (SequentialType *ATy = dyn_cast<SequentialType>(Ty)) {
> @@ -711,10 +712,17 @@
>       }
>       Ty = ATy->getElementType();
>     } else if (StructType *STy = dyn_cast<StructType>(Ty)) {
> -      // Determine which field of the struct the offset points into. The
> -      // getZExtValue is at least as safe as the StructLayout API because
> we
> -      // know the offset is within the struct at this point.
> +      // If we end up with an offset that isn't valid for this struct
> type, we
> +      // can't re-form this GEP in a regular form, so bail out. The
> pointer
> +      // operand likely went through casts that are necessary to make the
> GEP
> +      // sensible.
>       const StructLayout &SL = *TD->getStructLayout(STy);
> +      if (Offset.uge(SL.getSizeInBytes()))
> +        break;
> +
> +      // Determine which field of the struct the offset points into. The
> +      // getZExtValue is fine as we've already ensured that the offset is
> +      // within the range representable by the StructLayout API.
>       unsigned ElIdx =
> SL.getElementContainingOffset(Offset.getZExtValue());
>
> NewIdxs.push_back(ConstantInt::get(Type::getInt32Ty(Ty->getContext()),
>                                          ElIdx));
>
> Modified: llvm/trunk/test/Transforms/GlobalOpt/constantfold-initializers.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/GlobalOpt/constantfold-initializers.ll?rev=155466&r1=155465&r2=155466&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/GlobalOpt/constantfold-initializers.ll
> (original)
> +++ llvm/trunk/test/Transforms/GlobalOpt/constantfold-initializers.ll Tue
> Apr 24 13:42:47 2012
> @@ -12,6 +12,11 @@
>  @xs = global [2 x i32] zeroinitializer, align 4
>  ; CHECK: @xs = global [2 x i32] [i32 1, i32 1]
>
> +; PR12642
> +%PR12642.struct = type { i8 }
> + at PR12642.s = global <{}> zeroinitializer, align 1
> + at PR12642.p = constant %PR12642.struct* bitcast (i8* getelementptr (i8*
> bitcast (<{}>* @PR12642.s to i8*), i64 1) to %PR12642.struct*), align 8
> +
>  define internal void @test1() {
>  entry:
>   store i32 1, i32* getelementptr inbounds ([2 x i32]* @xs, i64 0, i64 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/20120424/686b77c2/attachment.html>