From: Philip Reames <listmail@philipreames.com>
Date: Sat, 28 Feb 2015 00:47:50 +0000 (+0000)
Subject: [RewriteStatepointsForGC] Fix iterator invalidation bug
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=dfa22d308f6661cf739d41704929712486ab3086;p=oota-llvm.git

[RewriteStatepointsForGC] Fix iterator invalidation bug

Inserting into a DenseMap you're iterating over is not well defined.  This is unfortunate since this is well defined on a std::map.

"cleanup per llvm code style standards" bug #2





git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@230827 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
index bf5364a4df5..37c273c7fbe 100644
--- a/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
+++ b/lib/Transforms/Scalar/RewriteStatepointsForGC.cpp
@@ -684,12 +684,19 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
   states[def] = PhiState();
   // Recursively fill in all phis & selects reachable from the initial one
   // for which we don't already know a definite base value for
-  // PERF: Yes, this is as horribly inefficient as it looks.
+  // TODO: This should be rewritten with a worklist
   bool done = false;
   while (!done) {
     done = true;
+    // Since we're adding elements to 'states' as we run, we can't keep
+    // iterators into the set.
+    SmallVector<Value*, 16> Keys;
+    Keys.reserve(states.size());
     for (auto Pair : states) {
-      Value *v = Pair.first;
+      Value *V = Pair.first;
+      Keys.push_back(V);
+    }
+    for (Value *v : Keys) {
       assert(!isKnownBaseResult(v) && "why did it get added?");
       if (PHINode *phi = dyn_cast<PHINode>(v)) {
         assert(phi->getNumIncomingValues() > 0 &&
@@ -734,6 +741,7 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
   while (progress) {
     oldSize = states.size();
     progress = false;
+    // We're only changing keys in this loop, thus safe to keep iterators
     for (auto Pair : states) {
       MeetPhiStates calculateMeet(states);
       Value *v = Pair.first;
@@ -768,6 +776,7 @@ static Value *findBasePointer(Value *I, DefiningValueMapTy &cache,
   }
 
   // Insert Phis for all conflicts
+  // Only changing keys in 'states', thus safe to keep iterators
   for (auto Pair : states) {
     Instruction *v = cast<Instruction>(Pair.first);
     PhiState state = Pair.second;