[ADT] Make DenseMapBase::moveFrom safer (NFC)

[kazutakahirata]

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().

[llvmbot]

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

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().

---

Full diff: https://github.com/llvm/llvm-project/pull/168180.diff

1 Files Affected:

• (modified) llvm/include/llvm/ADT/DenseMap.h (+23-6)

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<DenseMap<KeyT, ValueT, KeyInfoT, BucketT>,
     }
   }
 
+  // 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<unsigned>(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;
     }