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

[Douglas Gregor]

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

> 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?

I think it's a good candidate for the 3.1 branch, but I can't approve it. Evan or Chris?

	- Doug

> -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/29bf983a/attachment.html>