AK: Avoid hashing CircularBuffer data multiple times

Hashing the same data multiple times would result in a trivial loop in
the hash lookup table.

AK/CircularBuffer.cpp

@@ -305,21 +305,21 @@ ErrorOr<SearchableCircularBuffer> SearchableCircularBuffer::create_initialized(B
 ErrorOr<Bytes> SearchableCircularBuffer::read(Bytes bytes)
 {
     auto read_bytes_span = CircularBuffer::read(bytes);
-    TRY(hash_last_bytes(read_bytes_span.size()));
+    TRY(hash_new_bytes());
     return read_bytes_span;
 }
 
 ErrorOr<void> SearchableCircularBuffer::discard(size_t discarded_bytes)
 {
     TRY(CircularBuffer::discard(discarded_bytes));
-    TRY(hash_last_bytes(discarded_bytes));
+    TRY(hash_new_bytes());
     return {};
 }
 
 ErrorOr<size_t> SearchableCircularBuffer::flush_to_stream(Stream& stream)
 {
     auto flushed_byte_count = TRY(CircularBuffer::flush_to_stream(stream));
-    TRY(hash_last_bytes(flushed_byte_count));
+    TRY(hash_new_bytes());
     return flushed_byte_count;
 }
 
@@ -534,21 +534,20 @@ ErrorOr<void> SearchableCircularBuffer::insert_location_hash(ReadonlyBytes value
     return {};
 }
 
-ErrorOr<void> SearchableCircularBuffer::hash_last_bytes(size_t count)
+ErrorOr<void> SearchableCircularBuffer::hash_new_bytes()
 {
     // Stop early if we don't have enough data overall to hash a full chunk.
     if (search_limit() < HASH_CHUNK_SIZE)
         return {};
 
-    auto remaining_recalculations = count;
-    while (remaining_recalculations > 0) {
-        // Note: We offset everything by HASH_CHUNK_SIZE because we have up to HASH_CHUNK_SIZE - 1 bytes that we couldn't hash before (as we had missing data).
-        //       The number of recalculations stays the same, since we now have up to HASH_CHUNK_SIZE - 1 bytes that we can't hash now.
-        auto recalculation_span = next_search_span(min(remaining_recalculations + HASH_CHUNK_SIZE - 1, search_limit()));
+    auto last_hash_head = (capacity() + m_reading_head - HASH_CHUNK_SIZE) % capacity();
+
+    while (m_hash_head <= last_hash_head) {
+        auto recalculation_span = m_buffer.span().slice(m_hash_head, (capacity() + m_reading_head - m_hash_head) % capacity());
 
         // If the span is smaller than a hash chunk, we need to manually craft some consecutive data to do the hashing.
         if (recalculation_span.size() < HASH_CHUNK_SIZE) {
-            auto auxiliary_span = next_seekback_span(remaining_recalculations);
+            auto auxiliary_span = m_buffer.span().slice((m_hash_head + recalculation_span.size()) % capacity());
 
             // Ensure that our math is correct and that both spans are "adjacent".
             VERIFY(recalculation_span.data() + recalculation_span.size() == m_buffer.data() + m_buffer.size());
@@ -566,7 +565,7 @@ ErrorOr<void> SearchableCircularBuffer::hash_last_bytes(size_t count)
                 TRY(insert_location_hash(temporary_hash_chunk, recalculation_span.data() - m_buffer.data()));
 
                 recalculation_span = recalculation_span.slice(1);
-                remaining_recalculations--;
+                m_hash_head++;
             }
 
             continue;
@@ -576,7 +575,7 @@ ErrorOr<void> SearchableCircularBuffer::hash_last_bytes(size_t count)
             auto value = recalculation_span.slice(i, HASH_CHUNK_SIZE);
             auto raw_offset = value.data() - m_buffer.data();
             TRY(insert_location_hash(value, raw_offset));
-            remaining_recalculations--;
+            m_hash_head++;
         }
     }
 

AK/CircularBuffer.h

@@ -95,11 +95,12 @@ private:
 
     SearchableCircularBuffer(ByteBuffer);
 
+    size_t m_hash_head {};
     HashMap<unsigned, size_t> m_hash_location_map;
     HashMap<size_t, size_t> m_location_chain_map;
 
     ErrorOr<void> insert_location_hash(ReadonlyBytes value, size_t raw_offset);
-    ErrorOr<void> hash_last_bytes(size_t count);
+    ErrorOr<void> hash_new_bytes();
 };
 
 }

Tests/AK/TestCircularBuffer.cpp

@@ -427,6 +427,34 @@ TEST_CASE(find_copy_in_seekback)
     }
 }
 
+TEST_CASE(find_copy_in_seekback_small_haystack)
+{
+    // This covers multiple individual bugs, depending on the broken implementation this may
+    // loop infinitely or run out of memory bounds.
+
+    Array<u8, 3> const haystack {
+        0x41, 0x4b, 0x2f
+    };
+    Array<u8, 4> const needle {
+        0x41, 0x4b, 0x2f, 0x47
+    };
+
+    // Set up the buffer for testing.
+    auto buffer = MUST(SearchableCircularBuffer::create_empty(haystack.size() + needle.size()));
+    auto written_haystack_bytes = buffer.write(haystack);
+    VERIFY(written_haystack_bytes == haystack.size());
+    MUST(buffer.discard(haystack.size()));
+    auto written_needle_bytes = buffer.write(needle);
+    VERIFY(written_needle_bytes == needle.size());
+
+    {
+        auto match = buffer.find_copy_in_seekback(273, 2);
+        EXPECT(match.has_value());
+        EXPECT_EQ(match.value().distance, 3ul);
+        EXPECT_EQ(match.value().length, 3ul);
+    }
+}
+
 BENCHMARK_CASE(looping_copy_from_seekback)
 {
     auto circular_buffer = MUST(CircularBuffer::create_empty(16 * MiB));