[llvm-commits] [llvm] r172756 - in /llvm/trunk: include/llvm/Analysis/MemoryBuiltins.h lib/Analysis/MemoryBuiltins.cpp test/Transforms/InstCombine/objsize.ll
Bill Wendling
isanbard at gmail.com
Thu Jan 17 13:49:30 PST 2013
Yeah, I turned off basic AA and it worked. I looked at the code and couldn't find anything that looked like undefined behavior to me. I could be wrong, though...Perhaps one of the iterators is bad?
-bw
On Jan 17, 2013, at 1:46 PM, "Nuno Lopes" <nunoplopes at sapo.pt> wrote:
> That's interesting...
> Thanks for investigating and sorry for the breakage.
>
> Just guessing, I would say that this is a problem related with aliasing, since basic AA uses this code. Either that function has some undefined behavior that is now being exploited or there's a bug in basic AA.
> Unfortunately I won't have time to analyze this problem in the next couple of weeks.
>
> Nuno
>
>
> ----- Original Message -----
>> Author: void
>> Date: Thu Jan 17 15:28:46 2013
>> New Revision: 172756
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=172756&view=rev
>> Log:
>> Reverting r171325 & r172363. This was causing a mis-compile on the self-hosted LTO build bots.
>>
>> Okay, here's how to reproduce the problem:
>>
>> 1) Build a Release (or Release+Asserts) version of clang in the normal way.
>>
>> 2) Using the clang & clang++ binaries from (1), build a Release (or
>> Release+Asserts) version of the same sources, but this time enable LTO ---
>> specify the `-flto' flag on the command line.
>>
>> 3) Run the ARC migrator tests:
>>
>> $ arcmt-test --args -triple x86_64-apple-darwin10 -fsyntax-only -x objective-c++ ./src/tools/clang/test/ARCMT/cxx-rewrite.mm
>>
>> You'll see that the output isn't correct (the whitespace is off).
>>
>> The mis-compile is in the function `RewriteBuffer::RemoveText' in the
>> clang/lib/Rewrite/Core/Rewriter.cpp file. When that function and RewriteRope.cpp
>> are compiled with LTO and the `arcmt-test' executable is regenerated, you'll see
>> the error. When those files are not LTO'ed, then the output of the `arcmt-test'
>> is fine.
>>
>> It is *really* hard to get a testcase out of this. I'll file a PR with what I
>> have currently.
>>
>> --- Reverse-merging r172363 into '.':
>> U include/llvm/Analysis/MemoryBuiltins.h
>> U lib/Analysis/MemoryBuiltins.cpp
>>
>> --- Reverse-merging r171325 into '.':
>> U test/Transforms/InstCombine/objsize.ll
>> G include/llvm/Analysis/MemoryBuiltins.h
>> G lib/Analysis/MemoryBuiltins.cpp
>>
>>
>>
>> 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=172756&r1=172755&r2=172756&view=diff
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h (original)
>> +++ llvm/trunk/include/llvm/Analysis/MemoryBuiltins.h Thu Jan 17 15:28:46 2013
>> @@ -153,14 +153,12 @@
>> class ObjectSizeOffsetVisitor
>> : public InstVisitor<ObjectSizeOffsetVisitor, SizeOffsetType> {
>>
>> - typedef DenseMap<const Value*, SizeOffsetType> CacheMapTy;
>> -
>> const DataLayout *TD;
>> const TargetLibraryInfo *TLI;
>> bool RoundToAlign;
>> unsigned IntTyBits;
>> APInt Zero;
>> - CacheMapTy CacheMap;
>> + SmallPtrSet<Instruction *, 8> SeenInsts;
>>
>> APInt align(APInt Size, uint64_t Align);
>>
>>
>> Modified: llvm/trunk/lib/Analysis/MemoryBuiltins.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/MemoryBuiltins.cpp?rev=172756&r1=172755&r2=172756&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/MemoryBuiltins.cpp (original)
>> +++ llvm/trunk/lib/Analysis/MemoryBuiltins.cpp Thu Jan 17 15:28:46 2013
>> @@ -385,23 +385,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 (isa<Instruction>(V) || isa<GEPOperator>(V)) {
>> - // return cached value or insert unknown in cache if size of V was not
>> - // computed yet in order to avoid recursions in PHis
>> - std::pair<CacheMapTy::iterator, bool> CacheVal =
>> - CacheMap.insert(std::make_pair(V, unknown()));
>> - if (!CacheVal.second)
>> - return CacheVal.first->second;
>> -
>> - SizeOffsetType Result;
>> if (GEPOperator *GEP = dyn_cast<GEPOperator>(V))
>> - Result = visitGEPOperator(*GEP);
>> - else
>> - Result = visit(cast<Instruction>(*V));
>> - return CacheMap[V] = Result;
>> + return visitGEPOperator(*GEP);
>> + return visit(*I);
>> }
>> -
>> if (Argument *A = dyn_cast<Argument>(V))
>> return visitArgument(*A);
>> if (ConstantPointerNull *P = dyn_cast<ConstantPointerNull>(V))
>> @@ -415,6 +408,8 @@
>> 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
>> @@ -548,21 +543,9 @@
>> return unknown();
>> }
>>
>> -SizeOffsetType ObjectSizeOffsetVisitor::visitPHINode(PHINode &PHI) {
>> - if (PHI.getNumIncomingValues() == 0)
>> - return unknown();
>> -
>> - SizeOffsetType Ret = compute(PHI.getIncomingValue(0));
>> - if (!bothKnown(Ret))
>> - return unknown();
>> -
>> - // verify that all PHI incoming pointers have the same size and offset
>> - for (unsigned i = 1, e = PHI.getNumIncomingValues(); i != e; ++i) {
>> - SizeOffsetType EdgeData = compute(PHI.getIncomingValue(i));
>> - if (!bothKnown(EdgeData) || EdgeData != Ret)
>> - return unknown();
>> - }
>> - return Ret;
>> +SizeOffsetType ObjectSizeOffsetVisitor::visitPHINode(PHINode&) {
>> + // too complex to analyze statically.
>> + return unknown();
>> }
>>
>> SizeOffsetType ObjectSizeOffsetVisitor::visitSelectInst(SelectInst &I) {
>>
>> Modified: llvm/trunk/test/Transforms/InstCombine/objsize.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstCombine/objsize.ll?rev=172756&r1=172755&r2=172756&view=diff
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/InstCombine/objsize.ll (original)
>> +++ llvm/trunk/test/Transforms/InstCombine/objsize.ll Thu Jan 17 15:28:46 2013
>> @@ -256,131 +256,3 @@
>> return:
>> ret i32 7
>> }
>> -
>> -declare noalias i8* @valloc(i32) nounwind
>> -
>> -; CHECK: @test14
>> -; CHECK: ret i32 6
>> -define i32 @test14(i32 %a) nounwind {
>> - switch i32 %a, label %sw.default [
>> - i32 1, label %sw.bb
>> - i32 2, label %sw.bb1
>> - ]
>> -
>> -sw.bb:
>> - %call = tail call noalias i8* @malloc(i32 6) nounwind
>> - br label %sw.epilog
>> -
>> -sw.bb1:
>> - %call2 = tail call noalias i8* @calloc(i32 3, i32 2) nounwind
>> - br label %sw.epilog
>> -
>> -sw.default:
>> - %call3 = tail call noalias i8* @valloc(i32 6) nounwind
>> - br label %sw.epilog
>> -
>> -sw.epilog:
>> - %b.0 = phi i8* [ %call3, %sw.default ], [ %call2, %sw.bb1 ], [ %call, %sw.bb ]
>> - %1 = tail call i32 @llvm.objectsize.i32(i8* %b.0, i1 false)
>> - ret i32 %1
>> -}
>> -
>> -; CHECK: @test15
>> -; CHECK: llvm.objectsize
>> -define i32 @test15(i32 %a) nounwind {
>> - switch i32 %a, label %sw.default [
>> - i32 1, label %sw.bb
>> - i32 2, label %sw.bb1
>> - ]
>> -
>> -sw.bb:
>> - %call = tail call noalias i8* @malloc(i32 3) nounwind
>> - br label %sw.epilog
>> -
>> -sw.bb1:
>> - %call2 = tail call noalias i8* @calloc(i32 2, i32 1) nounwind
>> - br label %sw.epilog
>> -
>> -sw.default:
>> - %call3 = tail call noalias i8* @valloc(i32 3) nounwind
>> - br label %sw.epilog
>> -
>> -sw.epilog:
>> - %b.0 = phi i8* [ %call3, %sw.default ], [ %call2, %sw.bb1 ], [ %call, %sw.bb ]
>> - %1 = tail call i32 @llvm.objectsize.i32(i8* %b.0, i1 false)
>> - ret i32 %1
>> -}
>> -
>> -; CHECK: @test16
>> -; CHECK: llvm.objectsize
>> -define i32 @test16(i8* %a, i32 %n) nounwind {
>> - %b = alloca [5 x i8], align 1
>> - %c = alloca [5 x i8], align 1
>> - switch i32 %n, label %sw.default [
>> - i32 1, label %sw.bb
>> - i32 2, label %sw.bb1
>> - ]
>> -
>> -sw.bb:
>> - %bp = bitcast [5 x i8]* %b to i8*
>> - br label %sw.epilog
>> -
>> -sw.bb1:
>> - %cp = bitcast [5 x i8]* %c to i8*
>> - br label %sw.epilog
>> -
>> -sw.default:
>> - br label %sw.epilog
>> -
>> -sw.epilog:
>> - %phi = phi i8* [ %a, %sw.default ], [ %cp, %sw.bb1 ], [ %bp, %sw.bb ]
>> - %sz = call i32 @llvm.objectsize.i32(i8* %phi, i1 false)
>> - ret i32 %sz
>> -}
>> -
>> -; CHECK: @test17
>> -; CHECK: ret i32 5
>> -define i32 @test17(i32 %n) nounwind {
>> - %b = alloca [5 x i8], align 1
>> - %c = alloca [5 x i8], align 1
>> - %bp = bitcast [5 x i8]* %b to i8*
>> - switch i32 %n, label %sw.default [
>> - i32 1, label %sw.bb
>> - i32 2, label %sw.bb1
>> - ]
>> -
>> -sw.bb:
>> - br label %sw.epilog
>> -
>> -sw.bb1:
>> - %cp = bitcast [5 x i8]* %c to i8*
>> - br label %sw.epilog
>> -
>> -sw.default:
>> - br label %sw.epilog
>> -
>> -sw.epilog:
>> - %phi = phi i8* [ %bp, %sw.default ], [ %cp, %sw.bb1 ], [ %bp, %sw.bb ]
>> - %sz = call i32 @llvm.objectsize.i32(i8* %phi, i1 false)
>> - ret i32 %sz
>> -}
>> -
>> - at globalalias = alias internal [60 x i8]* @a
>> -
>> -; CHECK: @test18
>> -; CHECK-NEXT: ret i32 60
>> -define i32 @test18() {
>> - %bc = bitcast [60 x i8]* @globalalias to i8*
>> - %1 = call i32 @llvm.objectsize.i32(i8* %bc, i1 false)
>> - ret i32 %1
>> -}
>> -
>> - at globalalias2 = alias weak [60 x i8]* @a
>> -
>> -; CHECK: @test19
>> -; CHECK: llvm.objectsize
>> -define i32 @test19() {
>> - %bc = bitcast [60 x i8]* @globalalias2 to i8*
>> - %1 = call i32 @llvm.objectsize.i32(i8* %bc, i1 false)
>> - ret i32 %1
>> -}
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
More information about the llvm-commits
mailing list