diff --git a/AK/CircularBuffer.cpp b/AK/CircularBuffer.cpp index 43b08412685..810aaca07a2 100644 --- a/AK/CircularBuffer.cpp +++ b/AK/CircularBuffer.cpp @@ -305,21 +305,21 @@ ErrorOr SearchableCircularBuffer::create_initialized(B ErrorOr 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 SearchableCircularBuffer::discard(size_t discarded_bytes) { TRY(CircularBuffer::discard(discarded_bytes)); - TRY(hash_last_bytes(discarded_bytes)); + TRY(hash_new_bytes()); return {}; } ErrorOr 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 SearchableCircularBuffer::insert_location_hash(ReadonlyBytes value return {}; } -ErrorOr SearchableCircularBuffer::hash_last_bytes(size_t count) +ErrorOr 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 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 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++; } } diff --git a/AK/CircularBuffer.h b/AK/CircularBuffer.h index 0323b1afcc6..1bcf6d5e1bf 100644 --- a/AK/CircularBuffer.h +++ b/AK/CircularBuffer.h @@ -95,11 +95,12 @@ private: SearchableCircularBuffer(ByteBuffer); + size_t m_hash_head {}; HashMap m_hash_location_map; HashMap m_location_chain_map; ErrorOr insert_location_hash(ReadonlyBytes value, size_t raw_offset); - ErrorOr hash_last_bytes(size_t count); + ErrorOr hash_new_bytes(); }; } diff --git a/Tests/AK/TestCircularBuffer.cpp b/Tests/AK/TestCircularBuffer.cpp index ec563b3c51e..7c218fd0be4 100644 --- a/Tests/AK/TestCircularBuffer.cpp +++ b/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 const haystack { + 0x41, 0x4b, 0x2f + }; + Array 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));