Move dereferences that crash when the iterator is invalid into the new CrashIfIteratorIsInvalid function in order to give more helpful stack traces.

PiperOrigin-RevId: 915656842
Change-Id: I8542ce08c37d44f35a85e666eaf7eccd46132aef

Author: ezbr

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 <typename T>
+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,

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<const std::string&> 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) {