From 9d13ff8790df2f91667df536360e7532259468fc Mon Sep 17 00:00:00 2001 From: Kazu Hirata Date: Fri, 14 Nov 2025 20:23:28 -0800 Subject: [PATCH] [ADT] Make DenseMapBase::moveFrom safer (NFC) Without this patch, DenseMapBase::moveFrom() moves buckets and leaves the moved-from object in a zombie state. This patch teaches moveFrom() to call kill() so that the move-from object is in a known good state. This brings moveFrom()'s behavior in line with standard C++ move semantics. kill() is implemented so that it takes the fast path in the destructor -- both destroyAll() and deallocateBuckets(). --- llvm/include/llvm/ADT/DenseMap.h | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h index 333bbcb9399ce..db59bd595bca7 100644 --- a/llvm/include/llvm/ADT/DenseMap.h +++ b/llvm/include/llvm/ADT/DenseMap.h @@ -441,8 +441,12 @@ class DenseMapBase : public DebugEpochBase { moveFromImpl(OldBuckets); } - // Move key/value from Other to *this. Other will be in a zombie state. - void moveFrom(DerivedT &Other) { moveFromImpl(Other.buckets()); } + // Move key/value from Other to *this. + // Other is left in a valid but empty state. + void moveFrom(DerivedT &Other) { + moveFromImpl(Other.buckets()); + Other.derived().kill(); + } void copyFrom(const DerivedT &other) { this->destroyAll(); @@ -830,6 +834,13 @@ class DenseMap : public DenseMapBase, } } + // Put the zombie instance in a known good state after a move. + void kill() { + deallocateBuckets(); + Buckets = nullptr; + NumBuckets = 0; + } + void grow(unsigned AtLeast) { unsigned OldNumBuckets = NumBuckets; BucketT *OldBuckets = Buckets; @@ -1107,6 +1118,13 @@ class SmallDenseMap this->BaseT::initEmpty(); } + // Put the zombie instance in a known good state after a move. + void kill() { + deallocateBuckets(); + Small = false; + new (getLargeRep()) LargeRep{nullptr, 0}; + } + void grow(unsigned AtLeast) { if (AtLeast > InlineBuckets) AtLeast = std::max(64, NextPowerOf2(AtLeast - 1)); @@ -1117,14 +1135,13 @@ class SmallDenseMap if (Tmp.Small) { // Use moveFrom in those rare cases where we stay in the small mode. This // can happen when we have many tombstones. + Small = true; this->BaseT::initEmpty(); this->moveFrom(Tmp); - Tmp.Small = false; - Tmp.getLargeRep()->NumBuckets = 0; } else { - deallocateBuckets(); Small = false; - NumTombstones = 0; + NumEntries = Tmp.NumEntries; + NumTombstones = Tmp.NumTombstones; *getLargeRep() = std::move(*Tmp.getLargeRep()); Tmp.getLargeRep()->NumBuckets = 0; }