[llvm-commits] [llvm] r160589 - in /llvm/trunk: include/llvm/Transforms/Utils/Local.h lib/Analysis/MemoryBuiltins.cpp test/Instrumentation/BoundsChecking/simple.ll

Nuno Lopes nunoplopes at sapo.pt
Mon Jul 23 08:36:47 PDT 2012


Quoting Duncan Sands <baldrick at free.fr>:

> Hi Nuno,
>
>> baby steps toward fixing some problems with inbound GEPs that  
>> overflow, as discussed 2 months ago or so.
>> Make sure we do not emit index computations with NSW flags so that  
>> we dont get an undef value if the GEP overflows
>
> does this really help?  I mean, the optimizers may have exploited  
> nsw, undef and
> inbounds before this point to produce really weird bitcode, and may  
> exploit them
> afterwards, eg because after inlining they see that your computation  
> is fed by
> undef.  I'm worried that this appears to fix something, but doesn't  
> really.  I
> don't think this kind of issue can be fixed without some kind of compiler
> barrier like the llvm.identity intrinsic I mentioned a while ago.

It surely helps.
When we reach this point, the GEP may be undef (or become undef  
later), yes, but in that case the load/store was probably removed  
already. Theoretically this may not be truth, but it happens in  
practice.
Second,  we really don't want to generate the instructions that  
compute the GEP offset with the NSW flag, otherwise instcombine and/or  
a more aggressive range analysis can clean up the check. With the NSW  
flags, it could happen that the bounds check was removed but not the  
load/store.

So, this patch does the right thing, which is to compute the real  
offset with wrapping semantics.  I'm aware this is not the end of the  
problem, though, as you pointed out in our previous discussion.

Nuno


> Ciao, Duncan.
>
>>
>> Modified:
>>      llvm/trunk/include/llvm/Transforms/Utils/Local.h
>>      llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>>      llvm/trunk/test/Instrumentation/BoundsChecking/simple.ll
>>
>> Modified: llvm/trunk/include/llvm/Transforms/Utils/Local.h
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/Local.h?rev=160589&r1=160588&r2=160589&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Transforms/Utils/Local.h (original)
>> +++ llvm/trunk/include/llvm/Transforms/Utils/Local.h Fri Jul 20  
>> 18:07:40 2012
>> @@ -168,15 +168,18 @@
>>   /// EmitGEPOffset - Given a getelementptr  
>> instruction/constantexpr, emit the
>>   /// code necessary to compute the offset from the base pointer  
>> (without adding
>>   /// in the base pointer).  Return the result as a signed integer  
>> of intptr size.
>> +/// When NoAssumptions is true, no assumptions about index computation not
>> +/// overflowing is made.
>>   template<typename IRBuilderTy>
>> -Value *EmitGEPOffset(IRBuilderTy *Builder, const TargetData &TD,  
>> User *GEP) {
>> +Value *EmitGEPOffset(IRBuilderTy *Builder, const TargetData &TD, User *GEP,
>> +                     bool NoAssumptions = false) {
>>     gep_type_iterator GTI = gep_type_begin(GEP);
>>     Type *IntPtrTy = TD.getIntPtrType(GEP->getContext());
>>     Value *Result = Constant::getNullValue(IntPtrTy);
>>
>>     // If the GEP is inbounds, we know that none of the addressing  
>> operations will
>>     // overflow in an unsigned sense.
>> -  bool isInBounds = cast<GEPOperator>(GEP)->isInBounds();
>> +  bool isInBounds = cast<GEPOperator>(GEP)->isInBounds() && !NoAssumptions;
>>
>>     // Build a mask for high order bits.
>>     unsigned IntPtrWidth = TD.getPointerSizeInBits();
>>
>> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=160589&r1=160588&r2=160589&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
>> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Fri Jul 20 18:07:40 2012
>> @@ -645,7 +645,7 @@
>>     if (!bothKnown(PtrData))
>>       return unknown();
>>
>> -  Value *Offset = EmitGEPOffset(&Builder, *TD, &GEP);
>> +  Value *Offset = EmitGEPOffset(&Builder, *TD, &GEP,  
>> /*NoAssumptions=*/true);
>>     Offset = Builder.CreateAdd(PtrData.second, Offset);
>>     return std::make_pair(PtrData.first, Offset);
>>   }
>>
>> Modified: llvm/trunk/test/Instrumentation/BoundsChecking/simple.ll
>> URL:  
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Instrumentation/BoundsChecking/simple.ll?rev=160589&r1=160588&r2=160589&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Instrumentation/BoundsChecking/simple.ll (original)
>> +++ llvm/trunk/test/Instrumentation/BoundsChecking/simple.ll Fri  
>> Jul 20 18:07:40 2012
>> @@ -116,3 +116,13 @@
>>     %3 = load i8* %2, align 4
>>     ret void
>>   }
>> +
>> +; CHECK: @f12
>> +define i64 @f12(i64 %x, i64 %y) nounwind {
>> +  %1 = tail call i8* @calloc(i64 1, i64 %x)
>> +; CHECK: mul i64 %y, 8
>> +  %2 = bitcast i8* %1 to i64*
>> +  %3 = getelementptr inbounds i64* %2, i64 %y
>> +  %4 = load i64* %3, align 8
>> +  ret i64 %4
>> +}
>>



More information about the llvm-commits mailing list