]> granicus.if.org Git - llvm/commitdiff
[PM] Use PoisoningVH correctly when merely deleting entries in a map
authorChandler Carruth <chandlerc@gmail.com>
Thu, 26 Jan 2017 08:31:54 +0000 (08:31 +0000)
committerChandler Carruth <chandlerc@gmail.com>
Thu, 26 Jan 2017 08:31:54 +0000 (08:31 +0000)
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.

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

lib/Analysis/LazyValueInfo.cpp

index b2a8e6f3ce5ad0beaf18347f839d5a1fdc3d34ae..dcf0ca20c58c6e92f6e67c762a547c1302ecc836 100644 (file)
@@ -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);
 }