[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

1 files changed, 15 insertions, 18 deletions

diff --git a/llvm/include/llvm/Analysis/LazyCallGraph.h b/llvm/include/llvm/Analysis/LazyCallGraph.h
index 2b391e04766..e5dd5cc2caa 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);