Skip to content

Commit e68f118

Browse files
ezbrcopybara-github
authored andcommitted
Update AssertIsValidForComparison to have better comments for cases in which there are sanitizer crashes that we can't give a good assertion failure message for.
Also add a test for comparing an iterator from a moved from table. PiperOrigin-RevId: 911594422 Change-Id: I72ed53b5db81fb7f9656979d406ac7729eb20dc5
1 parent 9855fba commit e68f118

2 files changed

Lines changed: 30 additions & 5 deletions

File tree

absl/container/internal/raw_hash_set.h

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1434,13 +1434,25 @@ inline void AssertIsValidForComparison(const ctrl_t* ctrl,
14341434
GenerationType generation,
14351435
const GenerationType* generation_ptr) {
14361436
if (!SwisstableDebugEnabled()) return;
1437-
const bool ctrl_is_valid_for_comparison =
1438-
ctrl == nullptr || ctrl == DefaultIterControl() || IsFull(*ctrl);
1437+
const bool ctrl_is_valid_for_comparison = [ctrl]() {
1438+
if (ctrl == nullptr) return true;
1439+
if (ctrl == DefaultIterControl()) return true;
1440+
// Note: if the following line crashes, then it's likely that `ctrl` is from
1441+
// a backing array that has been deallocated. If you see a crash here, it
1442+
// likely means that you are comparing an invalid iterator from a table that
1443+
// has rehashed, moved, or been destroyed.
1444+
return IsFull(*ctrl);
1445+
}();
14391446
if (SwisstableGenerationsEnabled()) {
14401447
if (ABSL_PREDICT_FALSE(generation != *generation_ptr)) {
1441-
ABSL_RAW_LOG(FATAL,
1442-
"Invalid iterator comparison. The table could have rehashed "
1443-
"or moved since this iterator was initialized.");
1448+
// Note: in the case of a rehash, we would expect to see a sanitizer crash
1449+
// above when `ctrl` is dereferenced so this assertion will only catch
1450+
// moved table cases, unless we're using a custom allocator that does not
1451+
// deallocate the old backing array (e.g. an arena allocator).
1452+
ABSL_RAW_LOG(
1453+
FATAL,
1454+
"Invalid iterator comparison. The table was likely moved (or "
1455+
"possibly rehashed) since this iterator was initialized.");
14441456
}
14451457
if (ABSL_PREDICT_FALSE(!ctrl_is_valid_for_comparison)) {
14461458
ABSL_RAW_LOG(

absl/container/internal/raw_hash_set_test.cc

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2976,6 +2976,19 @@ TYPED_TEST(SooTest, IteratorInvalidAssertsEqualityOperatorRehash) {
29762976
EXPECT_DEATH_IF_SUPPORTED(void(iter == t.begin()), InvalidIteratorMatcher());
29772977
}
29782978

2979+
TYPED_TEST(SooTest, IteratorInvalidAssertsEqualityOperatorMovedFrom) {
2980+
if (!SwisstableGenerationsEnabled())
2981+
GTEST_SKIP() << "Generations not enabled.";
2982+
2983+
TypeParam t;
2984+
for (int i = 0; i < 10; ++i) t.insert(i);
2985+
auto iter = t.begin();
2986+
2987+
TypeParam t2 = std::move(t);
2988+
2989+
EXPECT_DEATH_IF_SUPPORTED(void(iter == t2.begin()), InvalidIteratorMatcher());
2990+
}
2991+
29792992
#if defined(ABSL_INTERNAL_HASHTABLEZ_SAMPLE)
29802993
template <typename T>
29812994
class RawHashSamplerTest : public testing::Test {};

0 commit comments

Comments
 (0)