From dc587e627b57106fe9a526bb1b4fcf13efcb44ab Mon Sep 17 00:00:00 2001
From: Andrew Noyes <andrew@weaselab.dev>
Date: Tue, 28 May 2024 16:35:58 -0700
Subject: [PATCH] Fuzzer thinks FdbVersionedMap seems ok

---
 FdbVersionedMap.cpp | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/FdbVersionedMap.cpp b/FdbVersionedMap.cpp
index f275761..4bdc391 100644
--- a/FdbVersionedMap.cpp
+++ b/FdbVersionedMap.cpp
@@ -1135,6 +1135,9 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl {
   VersionedData versionedData;
   std::map<Version, StandaloneVerUpdateRef>
       mutationLog; // versions (durableVersion, version]
+  std::map<Version, std::vector<Arena>>
+      freeable; // for each version, arena's that must be held until that
+                // version is < oldestVersion
 
   StandaloneVerUpdateRef &addVersionToMutationLog(Version v) {
     // return existing version...
@@ -1148,6 +1151,24 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl {
     return u;
   }
 
+  void expandClear(Key &begin, Key &end, VersionedData const &data,
+                   Arena &arena) {
+    // Expand the clear
+    const auto &d = data.atLatest();
+
+    // If another clear overlaps the beginning of this one, engulf it
+    auto i = d.lastLess(begin);
+    if (i && i->isClearTo() && i->getEndKey() >= begin)
+      begin = copy(arena, i.key());
+
+    // If another clear overlaps the end of this one, engulf it; otherwise
+    // expand
+    i = d.lastLessOrEqual(end);
+    if (i && i->isClearTo() && i->getEndKey() >= end) {
+      end = copy(arena, i->getEndKey());
+    }
+  }
+
   void addMutations(const Mutation *mutations, int numMutations,
                     int64_t version) {
     versionedData.createNewVersion(version);
@@ -1164,8 +1185,6 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl {
                         ? copy(arena, Key{m.param2, m.param2Len})
                         : keyAfter(param1, arena);
 
-      verUpdateRef.mutations.push_back(
-          Mutation{param1.p, param2.p, param1.len, param2.len, m.type});
       if (m.type == weaselab::VersionedMap::Set) {
         // VersionedMap (data) is bookkeeping all empty ranges. If the key to be
         // set is new, it is supposed to be in a range what was empty. Break the
@@ -1201,16 +1220,21 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl {
         if (param2.len == 0) {
           param2 = keyAfter(param1, arena);
         }
+        expandClear(param1, param2, versionedData, arena);
         versionedData.erase(param1, param2);
         assert(param2 > param1);
         assert(
             !versionedData.isClearContaining(versionedData.atLatest(), param1));
         versionedData.insert(param1, ValueOrClearToRef::clearTo(param2));
       }
+
+      verUpdateRef.mutations.push_back(
+          Mutation{param1.p, param2.p, param1.len, param2.len, m.type});
     }
   }
 
   void setOldestVersion(int64_t oldestVersion) {
+    auto &freeVec = freeable[versionedData.getLatestVersion()];
     auto iter = mutationLog.begin();
     while (iter != mutationLog.end() && iter->first <= oldestVersion) {
       for (const auto &m : iter->second.mutations) {
@@ -1231,10 +1255,11 @@ struct __attribute__((__visibility__("hidden"))) VersionedMap::Impl {
           }
         }
       }
-      // TODO this is a heap-use-after-free
+      freeVec.push_back(std::move(iter->second.arena));
       iter = mutationLog.erase(iter);
     }
     versionedData.forgetVersionsBefore(oldestVersion);
+    freeable.erase(freeable.begin(), freeable.lower_bound(oldestVersion));
   }
 };
 
@@ -1319,8 +1344,6 @@ VersionedMap::Iterator::operator*() const {
   }
   result.version = impl->iter.insertVersion();
 
-  assert(result.param1[0] != 0);
-  assert(result.param2[0] != 0);
   return result;
 }