[llvm] r293160 - [PM] Use PoisoningVH correctly when merely deleting entries in a map

[Chandler Carruth via llvm-commits]

Author: chandlerc
Date: Thu Jan 26 02:31:54 2017
New Revision: 293160

URL: http://llvm.org/viewvc/llvm-project?rev=293160&view=rev
Log:
[PM] Use PoisoningVH correctly when merely deleting entries in a map
with it.

This code was dereferencing the PoisoningVH which isn't allowed once it
is poisoned. But the code itself really doesn't need to access the
pointer, it is just doing the safe stuff of clearing out data structures
keyed on the pointer value.

Change the code to use iterators to erase directly from a DenseMap. This
is also substantially more efficient as it avoids lots of hashing and
lookups to do the erasure. DenseMap supports iterating behind the
iteration which is fairly easy to implement.

Sadly, I don't have a test case here. I'm not even close and I don't
know that I ever will be. The issue is that several of the tricky
aspects of fixing this only show up when you cause the stack's
SmallVector to be in *EXACTLY* the right location. I only ever got
a reproduction for those with Clang, and only with *exactly* the right
command line flags. Any adjustment, even to seemingly unrelated flags,
would make partial and half-way solutions magically start to "work". In
good news, all of this was caught with the LLVM test suite. Also, there
is no *specific* code here that is untested, just that the old pattern
of code won't immediately fail on any test case I've managed to
contrive.

Modified:
    llvm/trunk/lib/Analysis/LazyValueInfo.cpp

Modified: llvm/trunk/lib/Analysis/LazyValueInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/LazyValueInfo.cpp?rev=293160&r1=293159&r2=293160&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/LazyValueInfo.cpp (original)
+++ llvm/trunk/lib/Analysis/LazyValueInfo.cpp Thu Jan 26 02:31:54 2017
@@ -459,15 +459,15 @@ namespace {
 }
 
 void LazyValueInfoCache::eraseValue(Value *V) {
-  SmallVector<AssertingVH<BasicBlock>, 4> ToErase;
-  for (auto &I : OverDefinedCache) {
-    SmallPtrSetImpl<Value *> &ValueSet = I.second;
+  for (auto I = OverDefinedCache.begin(), E = OverDefinedCache.end(); I != E;) {
+    // Copy and increment the iterator immediately so we can erase behind
+    // ourselves.
+    auto Iter = I++;
+    SmallPtrSetImpl<Value *> &ValueSet = Iter->second;
     ValueSet.erase(V);
     if (ValueSet.empty())
-      ToErase.push_back(&*I.first);
+      OverDefinedCache.erase(Iter);
   }
-  for (auto &BB : ToErase)
-    OverDefinedCache.erase(&*BB);
 
   ValueCache.erase(V);
 }