[llvm] [StringMap] Replace tombstone deletion with TAOCP 6.4 Algorithm R (PR #202103)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 9 04:03:07 PDT 2026
https://github.com/MaskRay updated https://github.com/llvm/llvm-project/pull/202103
>From b1c6d90694257dd0d0b5d123798a997177dfa443 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Fri, 5 Jun 2026 23:46:15 -0700
Subject: [PATCH 1/4] [StringMap] Replace tombstone deletion with TAOCP 6.4
Algorithm R
StringMap uses quadratic probing with lazy deletion: an erased entry
becomes a tombstone, a third bucket state alongside empty and live that
every find/insert must inspect.
Switch to linear probing with Knuth TAOCP 6.4 Algorithm R deletion
, similar to DenseMap #200595.
erase now relocates the following entries to close the hole. StringMap
buckets are pointers to heap-allocated entries, so only the pointers
(and the parallel hash array) move. References and pointers to entries
remain valid, but iterators are invalidated.
---
llvm/docs/ProgrammersManual.rst | 2 +-
llvm/include/llvm/ADT/StringMap.h | 52 +++++------------
llvm/lib/Support/StringMap.cpp | 71 +++++++++---------------
llvm/unittests/ADT/StringMapTest.cpp | 41 ++++++++++++++
llvm/utils/gdb-scripts/prettyprinters.py | 3 +-
5 files changed, 82 insertions(+), 87 deletions(-)
diff --git a/llvm/docs/ProgrammersManual.rst b/llvm/docs/ProgrammersManual.rst
index ddb9c57ad4e2a..fb66f86567457 100644
--- a/llvm/docs/ProgrammersManual.rst
+++ b/llvm/docs/ProgrammersManual.rst
@@ -2383,7 +2383,7 @@ same allocation with the string data immediately after the element object.
This container guarantees the "``(char*)(&Value+1)``" points to the key string
for a value.
-The ``StringMap`` is very fast for several reasons: quadratic probing is very cache
+The ``StringMap`` is very fast for several reasons: linear probing is very cache
efficient for lookups, the hash value of strings in buckets is not recomputed
when looking up an element, ``StringMap`` rarely has to touch the memory for
unrelated objects when looking up a value (even when hash collisions happen),
diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h
index 7901365daa462..a68de8ce6139a 100644
--- a/llvm/include/llvm/ADT/StringMap.h
+++ b/llvm/include/llvm/ADT/StringMap.h
@@ -38,19 +38,16 @@ class StringMapImpl {
StringMapEntryBase **TheTable = nullptr;
unsigned NumBuckets = 0;
unsigned NumItems = 0;
- unsigned NumTombstones = 0;
unsigned ItemSize;
protected:
explicit StringMapImpl(unsigned itemSize) : ItemSize(itemSize) {}
StringMapImpl(StringMapImpl &&RHS)
: TheTable(RHS.TheTable), NumBuckets(RHS.NumBuckets),
- NumItems(RHS.NumItems), NumTombstones(RHS.NumTombstones),
- ItemSize(RHS.ItemSize) {
+ NumItems(RHS.NumItems), ItemSize(RHS.ItemSize) {
RHS.TheTable = nullptr;
RHS.NumBuckets = 0;
RHS.NumItems = 0;
- RHS.NumTombstones = 0;
}
LLVM_ABI StringMapImpl(unsigned InitSize, unsigned ItemSize);
@@ -94,14 +91,6 @@ class StringMapImpl {
}
public:
- static constexpr uintptr_t TombstoneIntVal =
- static_cast<uintptr_t>(-1)
- << PointerLikeTypeTraits<StringMapEntryBase *>::NumLowBitsAvailable;
-
- static StringMapEntryBase *getTombstoneVal() {
- return reinterpret_cast<StringMapEntryBase *>(TombstoneIntVal);
- }
-
[[nodiscard]] unsigned getNumBuckets() const { return NumBuckets; }
[[nodiscard]] unsigned getNumItems() const { return NumItems; }
@@ -119,7 +108,6 @@ class StringMapImpl {
std::swap(TheTable, Other.TheTable);
std::swap(NumBuckets, Other.NumBuckets);
std::swap(NumItems, Other.NumItems);
- std::swap(NumTombstones, Other.NumTombstones);
}
};
@@ -169,26 +157,19 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
*RHSHashTable = (unsigned *)(RHS.TheTable + NumBuckets + 1);
NumItems = RHS.NumItems;
- NumTombstones = RHS.NumTombstones;
+ // Copy the bucket layout verbatim. Because RHS was built with linear
+ // probing, preserving each entry's slot keeps the probe-sequence invariant
+ // intact without re-probing.
for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
StringMapEntryBase *Bucket = RHS.TheTable[I];
- if (!Bucket || Bucket == getTombstoneVal()) {
- TheTable[I] = Bucket;
+ if (!Bucket)
continue;
- }
TheTable[I] = MapEntryTy::create(
static_cast<MapEntryTy *>(Bucket)->getKey(), getAllocator(),
static_cast<MapEntryTy *>(Bucket)->getValue());
HashTable[I] = RHSHashTable[I];
}
-
- // Note that here we've copied everything from the RHS into this object,
- // tombstones included. We could, instead, have re-probed for each key to
- // instantiate this new object without any tombstone buckets. The
- // assumption here is that items are rarely deleted from most StringMaps,
- // and so tombstones are rare, so the cost of re-probing for all inputs is
- // not worthwhile.
}
StringMap &operator=(StringMap RHS) {
@@ -203,7 +184,7 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
// work not required in the destructor.
if (!empty()) {
for (StringMapEntryBase *Bucket : buckets()) {
- if (Bucket && Bucket != getTombstoneVal()) {
+ if (Bucket) {
static_cast<MapEntryTy *>(Bucket)->Destroy(getAllocator());
}
}
@@ -321,14 +302,12 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
bool insert(MapEntryTy *KeyValue) {
unsigned BucketNo = LookupBucketFor(KeyValue->getKey());
StringMapEntryBase *&Bucket = TheTable[BucketNo];
- if (Bucket && Bucket != getTombstoneVal())
+ if (Bucket)
return false; // Already exists in map.
- if (Bucket == getTombstoneVal())
- --NumTombstones;
Bucket = KeyValue;
++NumItems;
- assert(NumItems + NumTombstones <= NumBuckets);
+ assert(NumItems <= NumBuckets);
RehashTable();
return true;
@@ -388,15 +367,13 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
ArgsTy &&...Args) {
unsigned BucketNo = LookupBucketFor(Key, FullHashValue);
StringMapEntryBase *&Bucket = TheTable[BucketNo];
- if (Bucket && Bucket != getTombstoneVal())
+ if (Bucket)
return {iterator(TheTable + BucketNo), false}; // Already exists in map.
- if (Bucket == getTombstoneVal())
- --NumTombstones;
Bucket =
MapEntryTy::create(Key, getAllocator(), std::forward<ArgsTy>(Args)...);
++NumItems;
- assert(NumItems + NumTombstones <= NumBuckets);
+ assert(NumItems <= NumBuckets);
BucketNo = RehashTable(BucketNo);
return {iterator(TheTable + BucketNo), true};
@@ -407,17 +384,16 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
if (empty())
return;
- // Zap all values, resetting the keys back to non-present (not tombstone),
- // which is safe because we're removing all elements.
+ // Zap all values, resetting the keys back to non-present, which is safe
+ // because we're removing all elements.
for (StringMapEntryBase *&Bucket : buckets()) {
- if (Bucket && Bucket != getTombstoneVal()) {
+ if (Bucket) {
static_cast<MapEntryTy *>(Bucket)->Destroy(getAllocator());
}
Bucket = nullptr;
}
NumItems = 0;
- NumTombstones = 0;
}
/// remove - Remove the specified key/value pair from the map, but do not
@@ -495,7 +471,7 @@ template <typename ValueTy, bool IsConst> class StringMapIterBase {
private:
void AdvancePastEmptyBuckets() {
- while (*Ptr == nullptr || *Ptr == StringMapImpl::getTombstoneVal())
+ while (*Ptr == nullptr)
++Ptr;
}
};
diff --git a/llvm/lib/Support/StringMap.cpp b/llvm/lib/Support/StringMap.cpp
index 4aee30cd484e0..0a0c53fccbfa3 100644
--- a/llvm/lib/Support/StringMap.cpp
+++ b/llvm/lib/Support/StringMap.cpp
@@ -62,7 +62,6 @@ void StringMapImpl::init(unsigned InitSize) {
unsigned NewNumBuckets = InitSize ? InitSize : 16;
NumItems = 0;
- NumTombstones = 0;
TheTable = createTable(NewNumBuckets);
@@ -88,28 +87,15 @@ unsigned StringMapImpl::LookupBucketFor(StringRef Name,
unsigned BucketNo = FullHashValue & (NumBuckets - 1);
unsigned *HashTable = getHashTable(TheTable, NumBuckets);
- unsigned ProbeAmt = 1;
- int FirstTombstone = -1;
while (true) {
StringMapEntryBase *BucketItem = TheTable[BucketNo];
// If we found an empty bucket, this key isn't in the table yet, return it.
if (LLVM_LIKELY(!BucketItem)) {
- // If we found a tombstone, we want to reuse the tombstone instead of an
- // empty bucket. This reduces probing.
- if (FirstTombstone != -1) {
- HashTable[FirstTombstone] = FullHashValue;
- return FirstTombstone;
- }
-
HashTable[BucketNo] = FullHashValue;
return BucketNo;
}
- if (BucketItem == getTombstoneVal()) {
- // Skip over tombstones. However, remember the first one we see.
- if (FirstTombstone == -1)
- FirstTombstone = BucketNo;
- } else if (LLVM_LIKELY(HashTable[BucketNo] == FullHashValue)) {
+ if (LLVM_LIKELY(HashTable[BucketNo] == FullHashValue)) {
// If the full hash value matches, check deeply for a match. The common
// case here is that we are only looking at the buckets (for item info
// being non-null and for the full hash value) not at the items. This
@@ -125,11 +111,7 @@ unsigned StringMapImpl::LookupBucketFor(StringRef Name,
}
// Okay, we didn't find the item. Probe to the next bucket.
- BucketNo = (BucketNo + ProbeAmt) & (NumBuckets - 1);
-
- // Use quadratic probing, it has fewer clumping artifacts than linear
- // probing and has good cache behavior in the common case.
- ++ProbeAmt;
+ BucketNo = (BucketNo + 1) & (NumBuckets - 1);
}
}
@@ -147,16 +129,13 @@ int StringMapImpl::FindKey(StringRef Key, uint32_t FullHashValue) const {
unsigned BucketNo = FullHashValue & (NumBuckets - 1);
unsigned *HashTable = getHashTable(TheTable, NumBuckets);
- unsigned ProbeAmt = 1;
while (true) {
StringMapEntryBase *BucketItem = TheTable[BucketNo];
// If we found an empty bucket, this key isn't in the table yet, return.
if (LLVM_LIKELY(!BucketItem))
return -1;
- if (BucketItem == getTombstoneVal()) {
- // Ignore tombstones.
- } else if (LLVM_LIKELY(HashTable[BucketNo] == FullHashValue)) {
+ if (LLVM_LIKELY(HashTable[BucketNo] == FullHashValue)) {
// If the full hash value matches, check deeply for a match. The common
// case here is that we are only looking at the buckets (for item info
// being non-null and for the full hash value) not at the items. This
@@ -172,11 +151,7 @@ int StringMapImpl::FindKey(StringRef Key, uint32_t FullHashValue) const {
}
// Okay, we didn't find the item. Probe to the next bucket.
- BucketNo = (BucketNo + ProbeAmt) & (NumBuckets - 1);
-
- // Use quadratic probing, it has fewer clumping artifacts than linear
- // probing and has good cache behavior in the common case.
- ++ProbeAmt;
+ BucketNo = (BucketNo + 1) & (NumBuckets - 1);
}
}
@@ -197,10 +172,24 @@ StringMapEntryBase *StringMapImpl::RemoveKey(StringRef Key) {
return nullptr;
StringMapEntryBase *Result = TheTable[Bucket];
- TheTable[Bucket] = getTombstoneVal();
+
+ // Knuth TAOCP 6.4 Algorithm R: open a hole at the removed slot, then walk
+ // forward sliding each following entry whose probe path crosses the hole
+ // back into it. The scan stops at the next empty bucket, which is
+ // guaranteed to exist because the table is never full.
+ unsigned *HashTable = getHashTable(TheTable, NumBuckets);
+ unsigned Mask = NumBuckets - 1;
+ unsigned I = Bucket, J = I;
+ while ((J = (J + 1) & Mask), TheTable[J]) {
+ unsigned Ideal = HashTable[J];
+ if (((I - Ideal) & Mask) < ((J - Ideal) & Mask)) {
+ TheTable[I] = TheTable[J];
+ HashTable[I] = HashTable[J];
+ I = J;
+ }
+ }
+ TheTable[I] = nullptr;
--NumItems;
- ++NumTombstones;
- assert(NumItems + NumTombstones <= NumBuckets);
return Result;
}
@@ -209,14 +198,9 @@ StringMapEntryBase *StringMapImpl::RemoveKey(StringRef Key) {
/// the appropriate mod-of-hashtable-size.
unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
unsigned NewSize;
- // If the hash table is now more than 3/4 full, or if fewer than 1/8 of
- // the buckets are empty (meaning that many are filled with tombstones),
- // grow/rehash the table.
+ // If the hash table is now more than 3/4 full, grow the table.
if (LLVM_UNLIKELY(NumItems * 4 > NumBuckets * 3)) {
NewSize = NumBuckets * 2;
- } else if (LLVM_UNLIKELY(NumBuckets - (NumItems + NumTombstones) <=
- NumBuckets / 8)) {
- NewSize = NumBuckets;
} else {
return BucketNo;
}
@@ -230,16 +214,12 @@ unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
// the hash values available, so we don't have to rehash any strings.
for (unsigned I = 0, E = NumBuckets; I != E; ++I) {
StringMapEntryBase *Bucket = TheTable[I];
- if (Bucket && Bucket != getTombstoneVal()) {
+ if (Bucket) {
// If the bucket is not available, probe for a spot.
unsigned FullHash = HashTable[I];
unsigned NewBucket = FullHash & (NewSize - 1);
- if (NewTableArray[NewBucket]) {
- unsigned ProbeSize = 1;
- do {
- NewBucket = (NewBucket + ProbeSize++) & (NewSize - 1);
- } while (NewTableArray[NewBucket]);
- }
+ while (NewTableArray[NewBucket])
+ NewBucket = (NewBucket + 1) & (NewSize - 1);
// Finally found a slot. Fill it in.
NewTableArray[NewBucket] = Bucket;
@@ -253,6 +233,5 @@ unsigned StringMapImpl::RehashTable(unsigned BucketNo) {
TheTable = NewTableArray;
NumBuckets = NewSize;
- NumTombstones = 0;
return NewBucketNo;
}
diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp
index 1d92de4e92325..7b9ac1282ef2c 100644
--- a/llvm/unittests/ADT/StringMapTest.cpp
+++ b/llvm/unittests/ADT/StringMapTest.cpp
@@ -12,6 +12,7 @@
#include "llvm/Support/DataTypes.h"
#include "gtest/gtest.h"
#include <limits>
+#include <map>
#include <tuple>
using namespace llvm;
@@ -180,6 +181,46 @@ TEST_F(StringMapTest, SmallFullMapTest) {
EXPECT_EQ(5, Map.lookup("funf"));
}
+// Stress the backward-shift deletion (Knuth TAOCP 6.4 Algorithm R) used by
+// erase: interleave inserts and erases so that probe clusters form, shrink, and
+// straddle the wrap-around, then verify every surviving key is still findable
+// and every erased key is gone. A broken shift leaves keys stranded behind a
+// hole and would be caught here.
+TEST_F(StringMapTest, EraseStressTest) {
+ llvm::StringMap<unsigned> Map;
+ std::map<std::string, unsigned> Ref;
+
+ // Simple deterministic LCG so the sequence is reproducible across platforms.
+ uint64_t State = 0x1234567;
+ auto Next = [&] {
+ return (State = State * 6364136223846793005ULL + 1) >> 33;
+ };
+
+ for (unsigned Iter = 0; Iter != 20000; ++Iter) {
+ std::string Key = Twine(Next() % 500).str();
+ if (Next() & 1) {
+ Map[Key] = Iter;
+ Ref[Key] = Iter;
+ } else {
+ EXPECT_EQ(Map.erase(Key), Ref.erase(Key) != 0);
+ }
+
+ // Periodically cross-check the whole map against the reference.
+ if (Iter % 997 == 0) {
+ EXPECT_EQ(Map.size(), Ref.size());
+ for (const auto &KV : Ref) {
+ auto It = Map.find(KV.first);
+ ASSERT_NE(It, Map.end()) << "missing key " << KV.first;
+ EXPECT_EQ(It->second, KV.second);
+ }
+ }
+ }
+
+ EXPECT_EQ(Map.size(), Ref.size());
+ for (const auto &KV : Ref)
+ EXPECT_EQ(Map.lookup(KV.first), KV.second);
+}
+
TEST_F(StringMapTest, CopyCtorTest) {
llvm::StringMap<int> Map;
diff --git a/llvm/utils/gdb-scripts/prettyprinters.py b/llvm/utils/gdb-scripts/prettyprinters.py
index 69d5d06c04682..711f7ff0e74da 100644
--- a/llvm/utils/gdb-scripts/prettyprinters.py
+++ b/llvm/utils/gdb-scripts/prettyprinters.py
@@ -220,11 +220,10 @@ def children(self):
end = it + self.val["NumBuckets"]
value_ty = self.val.type.template_argument(0)
entry_base_ty = gdb.lookup_type("llvm::StringMapEntryBase")
- tombstone = gdb.parse_and_eval("llvm::StringMapImpl::TombstoneIntVal")
while it != end:
it_deref = it.dereference()
- if it_deref == 0 or it_deref == tombstone:
+ if it_deref == 0:
it = it + 1
continue
>From d076aa76c1e13474cf31db5866ab675d4d0317af Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Mon, 8 Jun 2026 21:10:27 -0700
Subject: [PATCH 2/4] doc
---
llvm/docs/ProgrammersManual.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/docs/ProgrammersManual.rst b/llvm/docs/ProgrammersManual.rst
index e70a1b96afec8..26a6c13edd094 100644
--- a/llvm/docs/ProgrammersManual.rst
+++ b/llvm/docs/ProgrammersManual.rst
@@ -2375,7 +2375,7 @@ long, expensive to copy, etc. ``StringMap`` is a specialized container designed
cope with these issues. It supports mapping an arbitrary range of bytes to an
arbitrary other object.
-The ``StringMap`` implementation uses a quadratically-probed hash table, where the
+The ``StringMap`` implementation uses a linear-probed hash table, where the
buckets store a pointer to the heap allocated entries (and some other stuff).
The entries in the map must be heap allocated because the strings are variable
length. The string data (key) and the element object (value) are stored in the
>From 32f38bf2f2ceca76ece67a0fc78b2fa1450d696a Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Mon, 8 Jun 2026 21:33:00 -0700
Subject: [PATCH 3/4] comment
---
llvm/include/llvm/ADT/StringMap.h | 4 ++--
llvm/lib/Support/StringMap.cpp | 9 +++------
2 files changed, 5 insertions(+), 8 deletions(-)
diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h
index 0474ee15512ee..5e6cea505f540 100644
--- a/llvm/include/llvm/ADT/StringMap.h
+++ b/llvm/include/llvm/ADT/StringMap.h
@@ -447,8 +447,8 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
continue;
}
Entry->Destroy(getAllocator());
- // erase relocates a following entry into this slot to close the hole, so
- // re-examine the same index rather than advancing past it.
+ // This may relocate a following entry into this slot to close the hole,
+ // so re-examine the same index rather than advancing past it.
removeBucket(I);
Removed = true;
}
diff --git a/llvm/lib/Support/StringMap.cpp b/llvm/lib/Support/StringMap.cpp
index 8f4b3ca6762b7..fe59d4485a422 100644
--- a/llvm/lib/Support/StringMap.cpp
+++ b/llvm/lib/Support/StringMap.cpp
@@ -164,13 +164,10 @@ void StringMapImpl::RemoveKey(StringMapEntryBase *V) {
assert(V == V2 && "Didn't find key?");
}
-/// RemoveKey - Remove the StringMapEntry for the specified key from the
-/// table, returning it. If the key is not in the table, this returns null.
+// Remove the StringMapEntry for the specified key from the table. Knuth
+// TAOCP 6.4 Algorithm R: walk forward sliding each following entry whose probe
+// path crosses the hole.
void StringMapImpl::removeBucket(unsigned Bucket) {
- // Knuth TAOCP 6.4 Algorithm R: open a hole at the removed slot, then walk
- // forward sliding each following entry whose probe path crosses the hole
- // back into it. The scan stops at the next empty bucket, which is
- // guaranteed to exist because the table is never full.
unsigned *HashTable = getHashTable(TheTable, NumBuckets);
unsigned Mask = NumBuckets - 1;
unsigned I = Bucket, J = I;
>From ac2621b75227e9e618e8f76085bf61c5456e4006 Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Mon, 8 Jun 2026 23:27:01 -0700
Subject: [PATCH 4/4] Fix StripSymbols.cpp; increment epoch for remove()
---
llvm/include/llvm/ADT/StringMap.h | 6 ++++--
llvm/lib/Transforms/IPO/StripSymbols.cpp | 17 ++++++++++-------
llvm/unittests/ADT/StringMapTest.cpp | 11 +++++++++++
3 files changed, 25 insertions(+), 9 deletions(-)
diff --git a/llvm/include/llvm/ADT/StringMap.h b/llvm/include/llvm/ADT/StringMap.h
index 5e6cea505f540..af96b397eb969 100644
--- a/llvm/include/llvm/ADT/StringMap.h
+++ b/llvm/include/llvm/ADT/StringMap.h
@@ -410,11 +410,13 @@ class LLVM_ALLOCATORHOLDER_EMPTYBASE StringMap
/// remove - Remove the specified key/value pair from the map, but do not
/// erase it. This aborts if the key is not in the map.
- void remove(MapEntryTy *KeyValue) { RemoveKey(KeyValue); }
+ void remove(MapEntryTy *KeyValue) {
+ incrementEpoch();
+ RemoveKey(KeyValue);
+ }
void erase(iterator I) {
MapEntryTy &V = *I;
- incrementEpoch();
remove(&V);
V.Destroy(getAllocator());
}
diff --git a/llvm/lib/Transforms/IPO/StripSymbols.cpp b/llvm/lib/Transforms/IPO/StripSymbols.cpp
index ec701b6d0037b..5e00ef468f9ef 100644
--- a/llvm/lib/Transforms/IPO/StripSymbols.cpp
+++ b/llvm/lib/Transforms/IPO/StripSymbols.cpp
@@ -22,6 +22,7 @@
#include "llvm/Transforms/IPO/StripSymbols.h"
#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/IR/Constants.h"
#include "llvm/IR/DebugInfo.h"
#include "llvm/IR/DerivedTypes.h"
@@ -76,15 +77,17 @@ static void RemoveDeadConstant(Constant *C) {
// Strip the symbol table of its names.
//
static void StripSymtab(ValueSymbolTable &ST, bool PreserveDbgInfo) {
- for (ValueSymbolTable::iterator VI = ST.begin(), VE = ST.end(); VI != VE; ) {
- Value *V = VI->getValue();
- ++VI;
- if (!isa<GlobalValue>(V) || cast<GlobalValue>(V)->hasLocalLinkage()) {
+ // Collect the values to rename first: setName("") removes the value from the
+ // symbol table, which invalidates iterators into it.
+ SmallVector<Value *, 0> ToStrip;
+ for (const ValueName &VN : ST) {
+ Value *V = VN.getValue();
+ if (!isa<GlobalValue>(V) || cast<GlobalValue>(V)->hasLocalLinkage())
if (!PreserveDbgInfo || !V->getName().starts_with("llvm.dbg"))
- // Set name to "", removing from symbol table!
- V->setName("");
- }
+ ToStrip.push_back(V);
}
+ for (Value *V : ToStrip)
+ V->setName("");
}
// Strip any named types of their names.
diff --git a/llvm/unittests/ADT/StringMapTest.cpp b/llvm/unittests/ADT/StringMapTest.cpp
index 83babad12a604..f9e20e6da925d 100644
--- a/llvm/unittests/ADT/StringMapTest.cpp
+++ b/llvm/unittests/ADT/StringMapTest.cpp
@@ -862,6 +862,17 @@ TEST(StringMapCustomTest, EraseInvalidatesIterators) {
EXPECT_DEATH((void)It->second, "invalid iterator access");
}
+TEST(StringMapCustomTest, RemoveInvalidatesIterators) {
+ StringMap<int> Map;
+ Map["a"] = 1;
+ Map["b"] = 2;
+ auto It = Map.find("a");
+ auto *Entry = &*Map.find("b");
+ Map.remove(Entry);
+ Entry->Destroy(Map.getAllocator());
+ EXPECT_DEATH((void)It->second, "invalid iterator access");
+}
+
TEST(StringMapCustomTest, ClearInvalidatesIterators) {
StringMap<int> Map;
Map["a"] = 1;
More information about the llvm-commits
mailing list