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

[Evan Cheng]

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120424/9669d102/attachment.html>