From 41421df02b9009976b037b08fa199d578b40ce58 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Thu, 26 Jan 2017 08:31:54 +0000 Subject: [PATCH] [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. llvm-svn: 293160 --- llvm/lib/Analysis/LazyValueInfo.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp index b2a8e6f3ce5a..dcf0ca20c58c 100644 --- a/llvm/lib/Analysis/LazyValueInfo.cpp +++ b/llvm/lib/Analysis/LazyValueInfo.cpp @@ -459,15 +459,15 @@ namespace { } void LazyValueInfoCache::eraseValue(Value *V) { - SmallVector, 4> ToErase; - for (auto &I : OverDefinedCache) { - SmallPtrSetImpl &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 &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); }