[llvm] r265605 - [LoopUnroll] Fix the way we update DT after complete unrolling.
Michael Zolotukhin via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 6 16:47:19 PDT 2016
> On Apr 6, 2016, at 4:39 PM, Mike Aizatsky <aizatsky at google.com> wrote:
>
> Michael,
>
> It seems that this change has introduced buffer overrun:
>
> http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11708 <http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11708>
>
> Can you take a look?
Looking into it, thanks!
Michael
>
> =================================================================
> ==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 <mailto: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 <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 <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 <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 <mailto:llvm-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <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/dbead193/attachment.html>
More information about the llvm-commits
mailing list