[llvm] r265605 - [LoopUnroll] Fix the way we update DT after complete unrolling.
Mike Aizatsky via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 6 16:39:44 PDT 2016
Michael,
It seems that this change has introduced buffer overrun:
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11708
Can you take a look?
=================================================================
==16604==ERROR: AddressSanitizer: container-overflow on address
0x6030000075b0 at pc 0x000004056540 bp 0x7ffe0b808250 sp 0x7ffe0b808248
READ of size 8 at 0x6030000075b0 thread T0
#0 0x405653f in llvm::UnrollLoop(llvm::Loop*, unsigned int, unsigned
int, bool, bool, unsigned int, llvm::LoopInfo*, llvm::ScalarEvolution*,
llvm::DominatorTree*, llvm::AssumptionCache*, bool)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Transforms/Utils/LoopUnroll.cpp:566:31
#1 0x3bf36ab in tryToUnrollLoop(llvm::Loop*, llvm::DominatorTree&,
llvm::LoopInfo*, llvm::ScalarEvolution*, llvm::TargetTransformInfo const&,
llvm::AssumptionCache&, bool, llvm::Optional<unsigned int>,
llvm::Optional<unsigned int>, llvm::Optional<bool>, llvm::Optional<bool>)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp:761:8
#2 0x3bed313 in (anonymous
namespace)::LoopUnroll::runOnLoop(llvm::Loop*, llvm::LPPassManager&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Transforms/Scalar/LoopUnrollPass.cpp:803:12
#3 0x26dd7ca in llvm::LPPassManager::runOnFunction(llvm::Function&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Analysis/LoopPass.cpp:198:20
#4 0x333671c in llvm::FPPassManager::runOnFunction(llvm::Function&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1550:23
#5 0x3336cb5 in llvm::FPPassManager::runOnModule(llvm::Module&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1571:16
#6 0x3337be2 in runOnModule
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1627:23
#7 0x3337be2 in llvm::legacy::PassManagerImpl::run(llvm::Module&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1730
#8 0x7c6d61 in main
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:636:3
#9 0x7f5dc4bd3ec4 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
#10 0x6b3669 in _start
(/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/opt+0x6b3669)
0x6030000075b0 is located 16 bytes inside of 32-byte region
[0x6030000075a0,0x6030000075c0)
allocated by thread T0 here:
#0 0x789ed0 in operator new(unsigned long)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/projects/compiler-rt/lib/asan/asan_new_delete.cc:62
#1 0x327c46e in __allocate
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/new:168:10
#2 0x327c46e in allocate
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/memory:1736
#3 0x327c46e in allocate
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/memory:1488
#4 0x327c46e in __split_buffer
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__split_buffer:311
#5 0x327c46e in void
std::__1::vector<llvm::DomTreeNodeBase<llvm::BasicBlock>*,
std::__1::allocator<llvm::DomTreeNodeBase<llvm::BasicBlock>*>
>::__push_back_slow_path<llvm::DomTreeNodeBase<llvm::BasicBlock>*>(llvm::DomTreeNodeBase<llvm::BasicBlock>*&&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/vector:1573
#6 0x32707f3 in push_back
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/vector:1614:9
#7 0x32707f3 in addChild
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/GenericDomTree.h:99
#8 0x32707f3 in void llvm::Calculate<llvm::Function,
llvm::BasicBlock*>(llvm::DominatorTreeBase<llvm::GraphTraits<llvm::BasicBlock*>::NodeType>&,
llvm::Function&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/GenericDomTreeConstruction.h:276
#9 0x274a998 in void
llvm::DominatorTreeBase<llvm::BasicBlock>::recalculate<llvm::Function>(llvm::Function&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/GenericDomTree.h:729:7
#10 0x327c06c in
llvm::DominatorTreeWrapperPass::runOnFunction(llvm::Function&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/Dominators.cpp:344:3
#11 0x333671c in llvm::FPPassManager::runOnFunction(llvm::Function&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1550:23
#12 0x3336cb5 in llvm::FPPassManager::runOnModule(llvm::Module&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1571:16
#13 0x3337be2 in runOnModule
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1627:23
#14 0x3337be2 in llvm::legacy::PassManagerImpl::run(llvm::Module&)
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1730
#15 0x7c6d61 in main
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:636:3
#16 0x7f5dc4bd3ec4 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)
SUMMARY: AddressSanitizer: container-overflow
/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/Transforms/Utils/LoopUnroll.cpp:566:31
in llvm::UnrollLoop(llvm::Loop*, unsigned int, unsigned int, bool, bool,
unsigned int, llvm::LoopInfo*, llvm::ScalarEvolution*,
llvm::DominatorTree*, llvm::AssumptionCache*, bool)
Shadow bytes around the buggy address:
On Wed, Apr 6, 2016 at 2:52 PM Michael Zolotukhin via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Author: mzolotukhin
> Date: Wed Apr 6 16:47:12 2016
> New Revision: 265605
>
> URL: http://llvm.org/viewvc/llvm-project?rev=265605&view=rev
> Log:
> [LoopUnroll] Fix the way we update DT after complete unrolling.
>
> Updating dominators for exit-blocks of the unrolled loops is not enough,
> as shown in PR27157. The proper way is to update dominators for all
> dominance-children of original loop blocks.
>
> Added:
> llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll
> Modified:
> llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp
>
> Modified: llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp?rev=265605&r1=265604&r2=265605&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp (original)
> +++ llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp Wed Apr 6 16:47:12 2016
> @@ -262,6 +262,7 @@ bool llvm::UnrollLoop(Loop *L, unsigned
> bool CompletelyUnroll = Count == TripCount;
> SmallVector<BasicBlock *, 4> ExitBlocks;
> L->getExitBlocks(ExitBlocks);
> + std::vector<BasicBlock*> OriginalLoopBlocks = L->getBlocks();
>
> // Go through all exits of L and see if there are any phi-nodes there.
> We just
> // conservatively assume that they're inserted to preserve LCSSA form,
> which
> @@ -551,20 +552,24 @@ bool llvm::UnrollLoop(Loop *L, unsigned
> Term->eraseFromParent();
> }
> }
> - // Update dominators of loop exit blocks.
> - // Immediate dominator of an exit block might change, because we add
> more
> + // Update dominators of blocks we might reach through exits.
> + // Immediate dominator of such block might change, because we add more
> // routes which can lead to the exit: we can now reach it from the
> copied
> - // iterations too. Thus, the new idom of the exit block will be the
> nearest
> + // iterations too. Thus, the new idom of the block will be the nearest
> // common dominator of the previous idom and common dominator of all
> copies of
> - // the exiting block. This is equivalent to the nearest common
> dominator of
> + // the previous idom. This is equivalent to the nearest common
> dominator of
> // the previous idom and the first latch, which dominates all copies of
> the
> - // exiting block.
> + // previous idom.
> if (DT && Count > 1) {
> - for (auto Exit : ExitBlocks) {
> - BasicBlock *PrevIDom = DT->getNode(Exit)->getIDom()->getBlock();
> - BasicBlock *NewIDom =
> - DT->findNearestCommonDominator(PrevIDom, Latches[0]);
> - DT->changeImmediateDominator(Exit, NewIDom);
> + for (auto *BB : OriginalLoopBlocks) {
> + auto *BBDomNode = DT->getNode(BB);
> + for (auto *ChildDomNode : BBDomNode->getChildren()) {
> + auto *ChildBB = ChildDomNode->getBlock();
> + if (L->contains(ChildBB))
> + continue;
> + BasicBlock *NewIDom = DT->findNearestCommonDominator(BB,
> Latches[0]);
> + DT->changeImmediateDominator(ChildBB, NewIDom);
> + }
> }
> }
>
>
> Added: llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll?rev=265605&view=auto
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll (added)
> +++ llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll Wed Apr 6 16:47:12
> 2016
> @@ -0,0 +1,53 @@
> +; RUN: opt -loop-unroll -debug-only=loop-unroll -disable-output < %s
> +; REQUIRES: asserts
> +; Compile this test with debug flag on to verify domtree right after loop
> unrolling.
> +target datalayout =
> "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"
> +
> +; PR27157
> +define void @foo() {
> +entry:
> + br label %loop_header
> +loop_header:
> + %iv = phi i64 [ 0, %entry ], [ %iv_next, %loop_latch ]
> + br i1 undef, label %loop_latch, label %loop_exiting_bb1
> +loop_exiting_bb1:
> + br i1 false, label %loop_exiting_bb2, label %exit1.loopexit
> +loop_exiting_bb2:
> + br i1 false, label %loop_latch, label %bb
> +bb:
> + br label %exit1
> +loop_latch:
> + %iv_next = add nuw nsw i64 %iv, 1
> + %cmp = icmp ne i64 %iv_next, 2
> + br i1 %cmp, label %loop_header, label %exit2
> +exit1.loopexit:
> + br label %exit1
> +exit1:
> + ret void
> +exit2:
> + ret void
> +}
> +
> +define void @foo2() {
> +entry:
> + br label %loop.header
> +loop.header:
> + %iv = phi i32 [ 0, %entry ], [ %iv.inc, %latch ]
> + %iv.inc = add i32 %iv, 1
> + br i1 undef, label %diamond, label %latch
> +diamond:
> + br i1 undef, label %left, label %right
> +left:
> + br i1 undef, label %exit, label %merge
> +right:
> + br i1 undef, label %exit, label %merge
> +merge:
> + br label %latch
> +latch:
> + %end.cond = icmp eq i32 %iv, 1
> + br i1 %end.cond, label %exit1, label %loop.header
> +exit:
> + ret void
> +exit1:
> + ret void
> +}
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
--
Mike
Sent from phone
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160406/5ae38a34/attachment.html>
More information about the llvm-commits
mailing list