diff --git a/absl/container/internal/raw_hash_set.h b/absl/container/internal/raw_hash_set.h index 52d24a1a..191b1c9b 100644 --- a/absl/container/internal/raw_hash_set.h +++ b/absl/container/internal/raw_hash_set.h @@ -1379,6 +1379,26 @@ constexpr bool SwisstableDebugEnabled() { #endif } +// Dereferences `ptr`. The function is named in order to provide a helpful error +// message when users see crashing stack traces. Note that this function is not +// guaranteed to crash when `ptr` is invalid if sanitizer mode is not enabled. +template +T CrashIfIteratorIsInvalid(const T* ptr) { + // If the following line(s) crash, then it's likely that `ptr` is from a + // backing array that has been deallocated. If you see a crash here, it likely + // means that you are comparing an invalid iterator from a table that has + // rehashed, moved, or been destroyed. In such cases, it is often helpful to + // reproduce the issue with --config=asan and (assuming there's a crash here) + // examine the corresponding deallocation stack trace. + T ret = *ptr; + // Force a read with inline asm to make sure that a crash happens here, rather + // than later when the value is used. +#ifdef __clang__ + asm("" : "+r"(ret)); +#endif + return ret; +} + inline void AssertIsFull(const ctrl_t* ctrl, GenerationType generation, const GenerationType* generation_ptr, const char* operation) { @@ -1396,20 +1416,21 @@ inline void AssertIsFull(const ctrl_t* ctrl, GenerationType generation, operation); } if (SwisstableGenerationsEnabled()) { - if (ABSL_PREDICT_FALSE(generation != *generation_ptr)) { + if (ABSL_PREDICT_FALSE(generation != + CrashIfIteratorIsInvalid(generation_ptr))) { ABSL_RAW_LOG(FATAL, "%s called on invalid iterator. The table could have " "rehashed or moved since this iterator was initialized.", operation); } - if (ABSL_PREDICT_FALSE(!IsFull(*ctrl))) { + if (ABSL_PREDICT_FALSE(!IsFull(CrashIfIteratorIsInvalid(ctrl)))) { ABSL_RAW_LOG( FATAL, "%s called on invalid iterator. The element was likely erased.", operation); } } else { - if (ABSL_PREDICT_FALSE(!IsFull(*ctrl))) { + if (ABSL_PREDICT_FALSE(!IsFull(CrashIfIteratorIsInvalid(ctrl)))) { ABSL_RAW_LOG( FATAL, "%s called on invalid iterator. The element might have been erased " @@ -1425,20 +1446,15 @@ inline void AssertIsValidForComparison(const ctrl_t* ctrl, GenerationType generation, const GenerationType* generation_ptr) { if (!SwisstableDebugEnabled()) return; - const bool ctrl_is_valid_for_comparison = [ctrl]() { - if (ctrl == nullptr) return true; - if (ctrl == DefaultIterControl()) return true; - // Note: if the following line crashes, then it's likely that `ctrl` is from - // a backing array that has been deallocated. If you see a crash here, it - // likely means that you are comparing an invalid iterator from a table that - // has rehashed, moved, or been destroyed. - return IsFull(*ctrl); - }(); + const bool ctrl_is_valid_for_comparison = + ctrl == nullptr || ctrl == DefaultIterControl() || + IsFull(CrashIfIteratorIsInvalid(ctrl)); if (SwisstableGenerationsEnabled()) { - if (ABSL_PREDICT_FALSE(generation != *generation_ptr)) { + if (ABSL_PREDICT_FALSE(generation != + CrashIfIteratorIsInvalid(generation_ptr))) { // Note: in the case of a rehash, we would expect to see a sanitizer crash - // above when `ctrl` is dereferenced so this assertion will only catch - // moved table cases, unless we're using a custom allocator that does not + // in CrashIfIteratorIsInvalid so this assertion will only catch moved + // table cases, unless we're using a custom allocator that does not // deallocate the old backing array (e.g. an arena allocator). ABSL_RAW_LOG( FATAL, diff --git a/absl/container/internal/raw_hash_set_test.cc b/absl/container/internal/raw_hash_set_test.cc index cf9c7e2e..3f81f26e 100644 --- a/absl/container/internal/raw_hash_set_test.cc +++ b/absl/container/internal/raw_hash_set_test.cc @@ -2922,12 +2922,11 @@ TEST(TableDeathTest, InvalidIteratorAssertsSoo) { // the control is static constant. } -// Invalid iterator use can trigger use-after-free in asan/hwasan, -// use-of-uninitialized-value in msan, or invalidated iterator assertions. +// Invalid iterator use can trigger crashes or invalidated iterator assertions. testing::Matcher InvalidIteratorMatcher() { return AnyOf(HasSubstr("invalidated iterator"), HasSubstr("Invalid iterator"), - HasSubstr("invalid iterator"), HasSubstr("use-after-free"), - HasSubstr("use-of-uninitialized-value")); + HasSubstr("invalid iterator"), + HasSubstr("CrashIfIteratorIsInvalid")); } TYPED_TEST(SooTest, IteratorInvalidAssertsEqualityOperator) {