Skip to content

Commit

Permalink
Distinguish the debug message for the case of self-move-assigned swis…
Browse files Browse the repository at this point in the history
…s tables.

PiperOrigin-RevId: 671484965
Change-Id: Ia1da7db0db1f776d48c74efaeab7252445208088
  • Loading branch information
ezbr authored and copybara-github committed Sep 5, 2024
1 parent 043fe3c commit e9ca8d1
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
17 changes: 13 additions & 4 deletions absl/container/internal/raw_hash_set.h
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,10 @@ enum InvalidCapacity : size_t {
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.
Expand Down Expand Up @@ -2911,7 +2914,7 @@ class raw_hash_set {

ABSL_ATTRIBUTE_REINITIALIZES void clear() {
if (SwisstableGenerationsEnabled() &&
capacity() == InvalidCapacity::kMovedFrom) {
capacity() >= InvalidCapacity::kMovedFrom) {
common().set_capacity(DefaultCapacity());
}
AssertNotDebugCapacity();
Expand Down Expand Up @@ -3596,7 +3599,7 @@ class raw_hash_set {

inline void destructor_impl() {
if (SwisstableGenerationsEnabled() &&
capacity() == InvalidCapacity::kMovedFrom) {
capacity() >= InvalidCapacity::kMovedFrom) {
return;
}
if (capacity() == 0) return;
Expand Down Expand Up @@ -3778,7 +3781,8 @@ class raw_hash_set {
// 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) {
Expand Down Expand Up @@ -3908,7 +3912,12 @@ class raw_hash_set {
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.");
}
}
Expand Down
6 changes: 3 additions & 3 deletions absl/container/internal/raw_hash_set_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2093,7 +2093,7 @@ TEST(Table, MoveSelfAssign) {
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.
}
Expand Down Expand Up @@ -3689,14 +3689,14 @@ TEST(Table, MovedFromCallsFail) {
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.
}
}
Expand Down

0 comments on commit e9ca8d1

Please sign in to comment.