<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Apr 6, 2016, at 4:39 PM, Mike Aizatsky <<a href="mailto:aizatsky@google.com" class="">aizatsky@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">Michael,<div class=""><br class=""></div><div class="">It seems that this change has introduced buffer overrun:</div><div class=""><br class=""></div><div class=""><a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11708" class="">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/11708</a><br class=""></div><div class=""><br class=""></div><div class=""><div class="">Can you take a look?</div></div></div></div></blockquote>Hopefully, r265625 fixes it.</div><div><br class=""></div><div>Michael</div><div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><div class=""><br class=""></div><div class="">=================================================================</div><div class="">==16604==ERROR: AddressSanitizer: container-overflow on address 0x6030000075b0 at pc 0x000004056540 bp 0x7ffe0b808250 sp 0x7ffe0b808248</div><div class="">READ of size 8 at 0x6030000075b0 thread T0</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #6 0x3337be2 in runOnModule /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1627:23</div><div class=""> #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</div><div class=""> #8 0x7c6d61 in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:636:3</div><div class=""> #9 0x7f5dc4bd3ec4 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)</div><div class=""> #10 0x6b3669 in _start (/mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/opt+0x6b3669)</div><div class=""><br class=""></div><div class="">0x6030000075b0 is located 16 bytes inside of 32-byte region [0x6030000075a0,0x6030000075c0)</div><div class="">allocated by thread T0 here:</div><div class=""> #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</div><div class=""> #1 0x327c46e in __allocate /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/new:168:10</div><div class=""> #2 0x327c46e in allocate /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/memory:1736</div><div class=""> #3 0x327c46e in allocate /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/memory:1488</div><div class=""> #4 0x327c46e in __split_buffer /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/__split_buffer:311</div><div class=""> #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</div><div class=""> #6 0x32707f3 in push_back /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/libcxx_build_asan/include/c++/v1/vector:1614:9</div><div class=""> #7 0x32707f3 in addChild /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/include/llvm/Support/GenericDomTree.h:99</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #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</div><div class=""> #13 0x3337be2 in runOnModule /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/lib/IR/LegacyPassManager.cpp:1627:23</div><div class=""> #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</div><div class=""> #15 0x7c6d61 in main /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/opt/opt.cpp:636:3</div><div class=""> #16 0x7f5dc4bd3ec4 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21ec4)</div><div class=""><br class=""></div><div class="">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)</div><div class="">Shadow bytes around the buggy address:</div></div><div class=""><br class=""></div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Apr 6, 2016 at 2:52 PM Michael Zolotukhin via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: mzolotukhin<br class="">
Date: Wed Apr 6 16:47:12 2016<br class="">
New Revision: 265605<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=265605&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=265605&view=rev</a><br class="">
Log:<br class="">
[LoopUnroll] Fix the way we update DT after complete unrolling.<br class="">
<br class="">
Updating dominators for exit-blocks of the unrolled loops is not enough,<br class="">
as shown in PR27157. The proper way is to update dominators for all<br class="">
dominance-children of original loop blocks.<br class="">
<br class="">
Added:<br class="">
llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll<br class="">
Modified:<br class="">
llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp<br class="">
<br class="">
Modified: llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp?rev=265605&r1=265604&r2=265605&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp?rev=265605&r1=265604&r2=265605&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp (original)<br class="">
+++ llvm/trunk/lib/Transforms/Utils/LoopUnroll.cpp Wed Apr 6 16:47:12 2016<br class="">
@@ -262,6 +262,7 @@ bool llvm::UnrollLoop(Loop *L, unsigned<br class="">
bool CompletelyUnroll = Count == TripCount;<br class="">
SmallVector<BasicBlock *, 4> ExitBlocks;<br class="">
L->getExitBlocks(ExitBlocks);<br class="">
+ std::vector<BasicBlock*> OriginalLoopBlocks = L->getBlocks();<br class="">
<br class="">
// Go through all exits of L and see if there are any phi-nodes there. We just<br class="">
// conservatively assume that they're inserted to preserve LCSSA form, which<br class="">
@@ -551,20 +552,24 @@ bool llvm::UnrollLoop(Loop *L, unsigned<br class="">
Term->eraseFromParent();<br class="">
}<br class="">
}<br class="">
- // Update dominators of loop exit blocks.<br class="">
- // Immediate dominator of an exit block might change, because we add more<br class="">
+ // Update dominators of blocks we might reach through exits.<br class="">
+ // Immediate dominator of such block might change, because we add more<br class="">
// routes which can lead to the exit: we can now reach it from the copied<br class="">
- // iterations too. Thus, the new idom of the exit block will be the nearest<br class="">
+ // iterations too. Thus, the new idom of the block will be the nearest<br class="">
// common dominator of the previous idom and common dominator of all copies of<br class="">
- // the exiting block. This is equivalent to the nearest common dominator of<br class="">
+ // the previous idom. This is equivalent to the nearest common dominator of<br class="">
// the previous idom and the first latch, which dominates all copies of the<br class="">
- // exiting block.<br class="">
+ // previous idom.<br class="">
if (DT && Count > 1) {<br class="">
- for (auto Exit : ExitBlocks) {<br class="">
- BasicBlock *PrevIDom = DT->getNode(Exit)->getIDom()->getBlock();<br class="">
- BasicBlock *NewIDom =<br class="">
- DT->findNearestCommonDominator(PrevIDom, Latches[0]);<br class="">
- DT->changeImmediateDominator(Exit, NewIDom);<br class="">
+ for (auto *BB : OriginalLoopBlocks) {<br class="">
+ auto *BBDomNode = DT->getNode(BB);<br class="">
+ for (auto *ChildDomNode : BBDomNode->getChildren()) {<br class="">
+ auto *ChildBB = ChildDomNode->getBlock();<br class="">
+ if (L->contains(ChildBB))<br class="">
+ continue;<br class="">
+ BasicBlock *NewIDom = DT->findNearestCommonDominator(BB, Latches[0]);<br class="">
+ DT->changeImmediateDominator(ChildBB, NewIDom);<br class="">
+ }<br class="">
}<br class="">
}<br class="">
<br class="">
<br class="">
Added: llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll?rev=265605&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll?rev=265605&view=auto</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll (added)<br class="">
+++ llvm/trunk/test/Transforms/LoopUnroll/pr27157.ll Wed Apr 6 16:47:12 2016<br class="">
@@ -0,0 +1,53 @@<br class="">
+; RUN: opt -loop-unroll -debug-only=loop-unroll -disable-output < %s<br class="">
+; REQUIRES: asserts<br class="">
+; Compile this test with debug flag on to verify domtree right after loop unrolling.<br class="">
+target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"<br class="">
+<br class="">
+; PR27157<br class="">
+define void @foo() {<br class="">
+entry:<br class="">
+ br label %loop_header<br class="">
+loop_header:<br class="">
+ %iv = phi i64 [ 0, %entry ], [ %iv_next, %loop_latch ]<br class="">
+ br i1 undef, label %loop_latch, label %loop_exiting_bb1<br class="">
+loop_exiting_bb1:<br class="">
+ br i1 false, label %loop_exiting_bb2, label %exit1.loopexit<br class="">
+loop_exiting_bb2:<br class="">
+ br i1 false, label %loop_latch, label %bb<br class="">
+bb:<br class="">
+ br label %exit1<br class="">
+loop_latch:<br class="">
+ %iv_next = add nuw nsw i64 %iv, 1<br class="">
+ %cmp = icmp ne i64 %iv_next, 2<br class="">
+ br i1 %cmp, label %loop_header, label %exit2<br class="">
+exit1.loopexit:<br class="">
+ br label %exit1<br class="">
+exit1:<br class="">
+ ret void<br class="">
+exit2:<br class="">
+ ret void<br class="">
+}<br class="">
+<br class="">
+define void @foo2() {<br class="">
+entry:<br class="">
+ br label %loop.header<br class="">
+loop.header:<br class="">
+ %iv = phi i32 [ 0, %entry ], [ %iv.inc, %latch ]<br class="">
+ %iv.inc = add i32 %iv, 1<br class="">
+ br i1 undef, label %diamond, label %latch<br class="">
+diamond:<br class="">
+ br i1 undef, label %left, label %right<br class="">
+left:<br class="">
+ br i1 undef, label %exit, label %merge<br class="">
+right:<br class="">
+ br i1 undef, label %exit, label %merge<br class="">
+merge:<br class="">
+ br label %latch<br class="">
+latch:<br class="">
+ %end.cond = icmp eq i32 %iv, 1<br class="">
+ br i1 %end.cond, label %exit1, label %loop.header<br class="">
+exit:<br class="">
+ ret void<br class="">
+exit1:<br class="">
+ ret void<br class="">
+}<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div><div dir="ltr" class="">-- <br class=""></div>Mike<br class="">Sent from phone
</div></blockquote></div><br class=""></body></html>