From 2629ef6e41d804ec1db826c4d1aff4e41d3c0ae4 Mon Sep 17 00:00:00 2001
From: Chandler Carruth <chandlerc@gmail.com>
Date: Thu, 1 May 2014 10:41:51 +0000
Subject: [PATCH] [LCG] Fix a bad bug in the new fancy iterator scheme I added
 to support removal. We can't just blindly increment (or decrement) the
 adapted iterator when the value is null because doing so can walk past the
 end (or beginning) and keep inspecting the value. The fix I've implemented is
 to restrict this further to a forward iterator and add an end iterator to the
 members (replacing a member that had become dead when I switched to the
 adaptor base!) and using that to stop the iteration.

I'm not entirely pleased with this solution. I feel like forward
iteration is too restrictive. I wasn't even happy about bidirectional
iteration. It also makes the iterator objects larger and the iteration
loops more complex. However, I also don't really like the other
alternative that seems obvious: a sentinel node. I'm still hoping to
come up with a more elegant solution here, but this at least fixes the
MSan and Valgrind errors on this code.

llvm-svn: 207743
---
 llvm/include/llvm/Analysis/LazyCallGraph.h | 33 ++++++++++------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/llvm/include/llvm/Analysis/LazyCallGraph.h b/llvm/include/llvm/Analysis/LazyCallGraph.h
index 2b391e047668..e5dd5cc2caa0 100644
--- a/llvm/include/llvm/Analysis/LazyCallGraph.h
+++ b/llvm/include/llvm/Analysis/LazyCallGraph.h
@@ -115,17 +115,18 @@ public:
   /// the graph.
   class iterator
       : public iterator_adaptor_base<iterator, NodeVectorImplT::iterator,
-                                     std::bidirectional_iterator_tag, Node> {
+                                     std::forward_iterator_tag, Node> {
     friend class LazyCallGraph;
     friend class LazyCallGraph::Node;
 
     LazyCallGraph *G;
-    NodeVectorImplT::iterator NI;
+    NodeVectorImplT::iterator E;
 
     // Build the iterator for a specific position in a node list.
-    iterator(LazyCallGraph &G, NodeVectorImplT::iterator NI)
-        : iterator_adaptor_base(NI), G(&G) {
-      while (I->isNull())
+    iterator(LazyCallGraph &G, NodeVectorImplT::iterator NI,
+             NodeVectorImplT::iterator E)
+        : iterator_adaptor_base(NI), G(&G), E(E) {
+      while (I != E && I->isNull())
         ++I;
     }
 
@@ -136,15 +137,7 @@ public:
     iterator &operator++() {
       do {
         ++I;
-      } while (I->isNull());
-      return *this;
-    }
-
-    using iterator_adaptor_base::operator--;
-    iterator &operator--() {
-      do {
-        --I;
-      } while (I->isNull());
+      } while (I != E && I->isNull());
       return *this;
     }
 
@@ -199,8 +192,10 @@ public:
       return F;
     };
 
-    iterator begin() const { return iterator(*G, Callees.begin()); }
-    iterator end() const { return iterator(*G, Callees.end()); }
+    iterator begin() const {
+      return iterator(*G, Callees.begin(), Callees.end());
+    }
+    iterator end() const { return iterator(*G, Callees.end(), Callees.end()); }
 
     /// Equality is defined as address equality.
     bool operator==(const Node &N) const { return this == &N; }
@@ -358,8 +353,10 @@ public:
   LazyCallGraph(LazyCallGraph &&G);
   LazyCallGraph &operator=(LazyCallGraph &&RHS);
 
-  iterator begin() { return iterator(*this, EntryNodes.begin()); }
-  iterator end() { return iterator(*this, EntryNodes.end()); }
+  iterator begin() {
+    return iterator(*this, EntryNodes.begin(), EntryNodes.end());
+  }
+  iterator end() { return iterator(*this, EntryNodes.end(), EntryNodes.end()); }
 
   postorder_scc_iterator postorder_scc_begin() {
     return postorder_scc_iterator(*this);