commit e9ca8d1ca225ef94fd20890b5440b22f490a410a
author Evan Brown <ezb@google.com> Thu Sep 05 13:15:11 2024 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Sep 05 13:16:03 2024 -0700
tree ac77a12c1e0393616a803928299d7f32262c7673
parent 043fe3c1d60de2261cfc817df96f9e042db12120

Distinguish the debug message for the case of self-move-assigned swiss tables.

PiperOrigin-RevId: 671484965
Change-Id: Ia1da7db0db1f776d48c74efaeab7252445208088

absl/container/internal/raw_hash_set.h

diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h
index f5463fb..03ec5b0 100644
--- a/absl/container/internal/raw_hash_set.h
+++ b/absl/container/internal/raw_hash_set.h
@@ -545,7 +545,10 @@
   kAboveMaxValidCapacity = ~size_t{} - 100,
   kReentrance,
   kDestroyed,
+
+  // These two must be last because we use `>= kMovedFrom` to mean moved-from.
   kMovedFrom,
+  kSelfMovedFrom,
 };
 
 // Returns a pointer to a control byte group that can be used by empty tables.
@@ -2911,7 +2914,7 @@
 
   ABSL_ATTRIBUTE_REINITIALIZES void clear() {
     if (SwisstableGenerationsEnabled() &&
-        capacity() == InvalidCapacity::kMovedFrom) {
+        capacity() >= InvalidCapacity::kMovedFrom) {
       common().set_capacity(DefaultCapacity());
     }
     AssertNotDebugCapacity();
@@ -3596,7 +3599,7 @@
 
   inline void destructor_impl() {
     if (SwisstableGenerationsEnabled() &&
-        capacity() == InvalidCapacity::kMovedFrom) {
+        capacity() >= InvalidCapacity::kMovedFrom) {
       return;
     }
     if (capacity() == 0) return;
@@ -3778,7 +3781,8 @@
     // than using NDEBUG) to avoid issues in which NDEBUG is enabled in some
     // translation units but not in others.
     if (SwisstableGenerationsEnabled()) {
-      that.common().set_capacity(InvalidCapacity::kMovedFrom);
+      that.common().set_capacity(this == &that ? InvalidCapacity::kSelfMovedFrom
+                                               : InvalidCapacity::kMovedFrom);
     }
     if (!SwisstableGenerationsEnabled() || capacity() == DefaultCapacity() ||
         capacity() > kAboveMaxValidCapacity) {
@@ -3908,7 +3912,12 @@
     assert(capacity() != InvalidCapacity::kDestroyed &&
            "Use of destroyed hash table.");
     if (SwisstableGenerationsEnabled() &&
-        ABSL_PREDICT_FALSE(capacity() == InvalidCapacity::kMovedFrom)) {
+        ABSL_PREDICT_FALSE(capacity() >= InvalidCapacity::kMovedFrom)) {
+      if (capacity() == InvalidCapacity::kSelfMovedFrom) {
+        // If this log triggers, then a hash table was move-assigned to itself
+        // and then used again later without being reinitialized.
+        ABSL_RAW_LOG(FATAL, "Use of self-move-assigned hash table.");
+      }
       ABSL_RAW_LOG(FATAL, "Use of moved-from hash table.");
     }
   }

absl/container/internal/raw_hash_set_test.cc

diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc
index f69fca3..79c00fd 100644
--- a/absl/container/internal/raw_hash_set_test.cc
+++ b/absl/container/internal/raw_hash_set_test.cc
@@ -2093,7 +2093,7 @@
   t = std::move(*&t);
   if (SwisstableGenerationsEnabled()) {
     // NOLINTNEXTLINE(bugprone-use-after-move)
-    EXPECT_DEATH_IF_SUPPORTED(t.contains("a"), "");
+    EXPECT_DEATH_IF_SUPPORTED(t.contains("a"), "self-move-assigned");
   }
   // As long as we don't crash, it's fine.
 }
@@ -3689,14 +3689,14 @@
     t1.insert(1);
     t2 = std::move(t1);
     // NOLINTNEXTLINE(bugprone-use-after-move)
-    EXPECT_DEATH_IF_SUPPORTED(t1.contains(1), "");
+    EXPECT_DEATH_IF_SUPPORTED(t1.contains(1), "moved-from");
   }
   {
     ABSL_ATTRIBUTE_UNUSED IntTable t1;
     t1.insert(1);
     ABSL_ATTRIBUTE_UNUSED IntTable t2(std::move(t1));
     // NOLINTNEXTLINE(bugprone-use-after-move)
-    EXPECT_DEATH_IF_SUPPORTED(t1.contains(1), "");
+    EXPECT_DEATH_IF_SUPPORTED(t1.contains(1), "moved-from");
     t1.clear();  // Clearing a moved-from table is allowed.
   }
 }