[llvm] 2cf3c03 - [DenseMap] Don't capture the BucketEnd pointer before an operation that might change the number of buckets.

[Craig Topper via llvm-commits]

Author: Craig Topper
Date: 2020-04-21T00:36:34-07:00
New Revision: 2cf3c033f3a1d4c2c36a94a80b7ba6bcba1e8711

URL: https://github.com/llvm/llvm-project/commit/2cf3c033f3a1d4c2c36a94a80b7ba6bcba1e8711
DIFF: https://github.com/llvm/llvm-project/commit/2cf3c033f3a1d4c2c36a94a80b7ba6bcba1e8711.diff

LOG: [DenseMap] Don't capture the BucketEnd pointer before an operation that might change the number of buckets.

This code was added in 887efa51c1e0e43ca684ed78b92dbc3a0720881b to
fix reverse iteration.

The call to InsertIntoBucket/InsertIntoBucketWithLookup can change
the number of buckets which will invalidate the BucketEnd. So
don't cache it and calculate it when creating the iterator.

Added: 
    

Modified: 
    llvm/include/llvm/ADT/DenseMap.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/DenseMap.h b/llvm/include/llvm/ADT/DenseMap.h
index ff9dab15291f..d68f90ef68db 100644
--- a/llvm/include/llvm/ADT/DenseMap.h
+++ b/llvm/include/llvm/ADT/DenseMap.h
@@ -220,16 +220,22 @@ class DenseMapBase : public DebugEpochBase {
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(KeyT &&Key, Ts &&... Args) {
     BucketT *TheBucket;
-    BucketT *EndBucket =
-        shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();
     if (LookupBucketFor(Key, TheBucket))
-      return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
+      return std::make_pair(makeIterator(TheBucket,
+                                         shouldReverseIterate<KeyT>()
+                                             ? getBuckets()
+                                             : getBucketsEnd(),
+                                         *this, true),
                             false); // Already in map.
 
     // Otherwise, insert the new element.
     TheBucket =
         InsertIntoBucket(TheBucket, std::move(Key), std::forward<Ts>(Args)...);
-    return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
+    return std::make_pair(makeIterator(TheBucket,
+                                       shouldReverseIterate<KeyT>()
+                                           ? getBuckets()
+                                           : getBucketsEnd(),
+                                       *this, true),
                           true);
   }
 
@@ -239,15 +245,21 @@ class DenseMapBase : public DebugEpochBase {
   template <typename... Ts>
   std::pair<iterator, bool> try_emplace(const KeyT &Key, Ts &&... Args) {
     BucketT *TheBucket;
-    BucketT *EndBucket =
-        shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();
     if (LookupBucketFor(Key, TheBucket))
-      return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
+      return std::make_pair(makeIterator(TheBucket,
+                                         shouldReverseIterate<KeyT>()
+                                             ? getBuckets()
+                                             : getBucketsEnd(),
+                                         *this, true),
                             false); // Already in map.
 
     // Otherwise, insert the new element.
     TheBucket = InsertIntoBucket(TheBucket, Key, std::forward<Ts>(Args)...);
-    return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
+    return std::make_pair(makeIterator(TheBucket,
+                                       shouldReverseIterate<KeyT>()
+                                           ? getBuckets()
+                                           : getBucketsEnd(),
+                                       *this, true),
                           true);
   }
 
@@ -260,16 +272,22 @@ class DenseMapBase : public DebugEpochBase {
   std::pair<iterator, bool> insert_as(std::pair<KeyT, ValueT> &&KV,
                                       const LookupKeyT &Val) {
     BucketT *TheBucket;
-    BucketT *EndBucket =
-        shouldReverseIterate<KeyT>() ? getBuckets() : getBucketsEnd();
     if (LookupBucketFor(Val, TheBucket))
-      return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
+      return std::make_pair(makeIterator(TheBucket,
+                                         shouldReverseIterate<KeyT>()
+                                             ? getBuckets()
+                                             : getBucketsEnd(),
+                                         *this, true),
                             false); // Already in map.
 
     // Otherwise, insert the new element.
     TheBucket = InsertIntoBucketWithLookup(TheBucket, std::move(KV.first),
                                            std::move(KV.second), Val);
-    return std::make_pair(makeIterator(TheBucket, EndBucket, *this, true),
+    return std::make_pair(makeIterator(TheBucket,
+                                       shouldReverseIterate<KeyT>()
+                                           ? getBuckets()
+                                           : getBucketsEnd(),
+                                       *this, true),
                           true);
   }