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

[Bill Wendling]

Thanks! I committed it to the branch.

-bw

On Apr 24, 2012, at 4:26 PM, Evan Cheng wrote:

> 
> On Apr 24, 2012, at 11:48 AM, Douglas Gregor <dgregor at apple.com> wrote:
> 
>> 
>> 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?
> 
> I approve.
> 
> Evan
> 
>> 
>> 	- 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
>>> 
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits