[llvm-commits] [llvm] r162120 - in /llvm/trunk: include/llvm/Analysis/MemoryBuiltins.h lib/Analysis/MemoryBuiltins.cpp test/Transforms/InstCombine/objsize.ll
Eli Friedman
eli.friedman at gmail.com
Fri Aug 17 14:14:44 PDT 2012
On Fri, Aug 17, 2012 at 12:47 PM, Benjamin Kramer <benny.kra at gmail.com> wrote:
>
> On 17.08.2012, at 21:39, Eli Friedman <eli.friedman at gmail.com> wrote:
>
>> On Fri, Aug 17, 2012 at 12:26 PM, Benjamin Kramer
>> <benny.kra at googlemail.com> wrote:
>>> Author: d0k
>>> Date: Fri Aug 17 14:26:41 2012
>>> New Revision: 162120
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=162120&view=rev
>>> Log:
>>> MemoryBuiltins: Properly guard ObjectSizeOffsetVisitor against cycles in the IR.
>>>
>>> The previous fix only checked for simple cycles, use a set to catch longer
>>> cycles too.
>>>
>>> Drop the broken check from the ObjectSizeOffsetEvaluator. The BoundsChecking
>>> pass doesn't have to deal with invalid IR like InstCombine does.
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h
>>> llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>>> llvm/trunk/test/Transforms/InstCombine/objsize.ll
>>>
>>> Modified: llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h?rev=162120&r1=162119&r2=162120&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h (original)
>>> +++ llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h Fri Aug 17 14:26:41 2012
>>> @@ -146,6 +146,7 @@
>>> bool RoundToAlign;
>>> unsigned IntTyBits;
>>> APInt Zero;
>>> + SmallPtrSet<Instruction *, 8> SeenInsts;
>>>
>>> APInt align(APInt Size, uint64_t Align);
>>>
>>> @@ -203,7 +204,6 @@
>>> const TargetData *TD;
>>> LLVMContext &Context;
>>> BuilderTy Builder;
>>> - ObjectSizeOffsetVisitor Visitor;
>>> IntegerType *IntTy;
>>> Value *Zero;
>>> CacheMapTy CacheMap;
>>>
>>> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=162120&r1=162119&r2=162120&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
>>> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Fri Aug 17 14:26:41 2012
>>> @@ -358,11 +358,16 @@
>>>
>>> SizeOffsetType ObjectSizeOffsetVisitor::compute(Value *V) {
>>> V = V->stripPointerCasts();
>>> + if (Instruction *I = dyn_cast<Instruction>(V)) {
>>> + // If we have already seen this instruction, bail out. Cycles can happen in
>>> + // unreachable code after constant propagation.
>>> + if (!SeenInsts.insert(I))
>>> + return unknown();
>>>
>>> - if (GEPOperator *GEP = dyn_cast<GEPOperator>(V))
>>> - return visitGEPOperator(*GEP);
>>> - if (Instruction *I = dyn_cast<Instruction>(V))
>>> + if (GEPOperator *GEP = dyn_cast<GEPOperator>(V))
>>> + return visitGEPOperator(*GEP);
>>> return visit(*I);
>>> + }
>>> if (Argument *A = dyn_cast<Argument>(V))
>>> return visitArgument(*A);
>>> if (ConstantPointerNull *P = dyn_cast<ConstantPointerNull>(V))
>>> @@ -371,9 +376,12 @@
>>> return visitGlobalVariable(*GV);
>>> if (UndefValue *UV = dyn_cast<UndefValue>(V))
>>> return visitUndefValue(*UV);
>>> - if (ConstantExpr *CE = dyn_cast<ConstantExpr>(V))
>>> + if (ConstantExpr *CE = dyn_cast<ConstantExpr>(V)) {
>>> if (CE->getOpcode() == Instruction::IntToPtr)
>>> return unknown(); // clueless
>>> + if (CE->getOpcode() == Instruction::GetElementPtr)
>>> + return visitGEPOperator(cast<GEPOperator>(*CE));
>>> + }
>>>
>>> DEBUG(dbgs() << "ObjectSizeOffsetVisitor::compute() unhandled value: " << *V
>>> << '\n');
>>> @@ -473,10 +481,6 @@
>>> }
>>>
>>> SizeOffsetType ObjectSizeOffsetVisitor::visitGEPOperator(GEPOperator &GEP) {
>>> - // Ignore self-referencing GEPs, they can occur in unreachable code.
>>> - if (&GEP == GEP.getPointerOperand())
>>> - return unknown();
>>> -
>>> SizeOffsetType PtrData = compute(GEP.getPointerOperand());
>>> if (!bothKnown(PtrData) || !GEP.hasAllConstantIndices())
>>> return unknown();
>>> @@ -510,10 +514,6 @@
>>> }
>>>
>>> SizeOffsetType ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I) {
>>> - // ignore malformed self-looping selects
>>> - if (I.getTrueValue() == &I || I.getFalseValue() == &I)
>>> - return unknown();
>>> -
>>> SizeOffsetType TrueSide = compute(I.getTrueValue());
>>> SizeOffsetType FalseSide = compute(I.getFalseValue());
>>> if (bothKnown(TrueSide) && bothKnown(FalseSide) && TrueSide == FalseSide)
>>> @@ -533,8 +533,7 @@
>>>
>>> ObjectSizeOffsetEvaluator::ObjectSizeOffsetEvaluator(const TargetData *TD,
>>> LLVMContext &Context)
>>> -: TD(TD), Context(Context), Builder(Context, TargetFolder(TD)),
>>> -Visitor(TD, Context) {
>>> +: TD(TD), Context(Context), Builder(Context, TargetFolder(TD)) {
>>> IntTy = TD->getIntPtrType(Context);
>>> Zero = ConstantInt::get(IntTy, 0);
>>> }
>>> @@ -559,6 +558,7 @@
>>> }
>>>
>>> SizeOffsetEvalType ObjectSizeOffsetEvaluator::compute_(Value *V) {
>>> + ObjectSizeOffsetVisitor Visitor(TD, Context);
>>> SizeOffsetType Const = Visitor.compute(V);
>>> if (Visitor.bothKnown(Const))
>>> return std::make_pair(ConstantInt::get(Context, Const.first),
>>> @@ -719,10 +719,6 @@
>>> }
>>>
>>> SizeOffsetEvalType ObjectSizeOffsetEvaluator::visitSelectInst(SelectInst &I) {
>>> - // ignore malformed self-looping selects
>>> - if (I.getTrueValue() == &I || I.getFalseValue() == &I)
>>> - return unknown();
>>> -
>>> SizeOffsetEvalType TrueSide = compute_(I.getTrueValue());
>>> SizeOffsetEvalType FalseSide = compute_(I.getFalseValue());
>>>
>>>
>>> Modified: llvm/trunk/test/Transforms/InstCombine/objsize.ll
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/objsize.ll?rev=162120&r1=162119&r2=162120&view=diff
>>> ==============================================================================
>>> --- llvm/trunk/test/Transforms/InstCombine/objsize.ll (original)
>>> +++ llvm/trunk/test/Transforms/InstCombine/objsize.ll Fri Aug 17 14:26:41 2012
>>> @@ -247,7 +247,8 @@
>>>
>>> ; technically reachable, but this malformed IR may appear as a result of constant propagation
>>> xpto:
>>> - %gep = getelementptr i8* %gep, i32 1
>>> + %gep2 = getelementptr i8* %gep, i32 1
>>> + %gep = getelementptr i8* %gep2, i32 1
>>> %o = call i32 @llvm.objectsize.i32(i8* %gep, i1 true)
>>> ; CHECK: ret i32 undef
>>> ret i32 %o
>>
>> Err, wait a sec: this testcase isn't valid! The verifier should catch it.
>
> The verifier only runs after the specified pass, instcombine removes dead code so it doesn't fire. It's really hard to find a test case for this without writing invalid IR, and if you have a reduced one it tends to be fragile :(
Ah.
Random idea: would it be possible to avoid this whole mess by making
sure instcombine never visits unreachable instructions?
-Eli
More information about the llvm-commits
mailing list