Skip to content

Commit

Permalink
AK: Fix overflow and mixed-signedness issues in binary_search() (Sere…
Browse files Browse the repository at this point in the history
  • Loading branch information
tryfinally committed Aug 2, 2020
1 parent 2242f69 commit 615ba0f
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 8 deletions.
21 changes: 15 additions & 6 deletions AK/BinarySearch.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,24 @@ int integral_compare(const T& a, const T& b)
}

template<typename T, typename Compare>
T* binary_search(Span<T> haystack, const T& needle, Compare compare = integral_compare, int* nearby_index = nullptr)
T* binary_search(Span<T> haystack, const T& needle, Compare compare = integral_compare, size_t* nearby_index = nullptr)
{
int low = 0;
int high = haystack.size() - 1;
if (haystack.size() == 0) {
if (nearby_index)
*nearby_index = 0;
return nullptr;
}

size_t low = 0;
size_t high = haystack.size() - 1;
while (low <= high) {
int middle = (low + high) / 2;
size_t middle = low + ((high - low) / 2);
int comparison = compare(needle, haystack[middle]);
if (comparison < 0)
high = middle - 1;
if (middle != 0)
high = middle - 1;
else
break;
else if (comparison > 0)
low = middle + 1;
else {
Expand All @@ -58,7 +67,7 @@ T* binary_search(Span<T> haystack, const T& needle, Compare compare = integral_c
}

if (nearby_index)
*nearby_index = max(0, min(low, high));
*nearby_index = min(low, high);

return nullptr;
}
Expand Down
40 changes: 40 additions & 0 deletions AK/Tests/TestBinarySearch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@
#include <AK/TestSuite.h>

#include <AK/BinarySearch.h>
#include <AK/Span.h>
#include <cstring>
#include <new>

TEST_CASE(vector_ints)
{
Expand Down Expand Up @@ -106,4 +108,42 @@ TEST_CASE(no_elements)
EXPECT_EQ(test1, nullptr);
}

TEST_CASE(huge_char_array)
{
const size_t N = 2147483680;
Bytes span { new (std::nothrow) u8[N], N };
EXPECT(span.data() != nullptr);

for (size_t i = 0; i < span.size(); ++i)
span[i] = 'a';
size_t index = N - 1;
for (u8 c = 'z'; c > 'b'; --c)
span[index--] = c;

EXPECT_EQ(span[N - 1], 'z');

const u8 a = 'a';
auto where = binary_search(span, a, AK::integral_compare<u8>);
EXPECT(where != nullptr);
EXPECT_EQ(*where, 'a');

const u8 z = 'z';
where = binary_search(span, z, AK::integral_compare<u8>);
EXPECT(where != nullptr);
EXPECT_EQ(*where, 'z');

size_t near = 0;
const u8 tilde = '~';
where = binary_search(span, tilde, AK::integral_compare<u8>, &near);
EXPECT_EQ(where, nullptr);
EXPECT_EQ(near, N - 1);

const u8 b = 'b';
where = binary_search(span, b, AK::integral_compare<u8>, &near);
EXPECT_EQ(where, nullptr);
EXPECT_EQ(near, N - ('z' - b + 1));

delete[] span.data();
}

TEST_MAIN(BinarySearch)
2 changes: 1 addition & 1 deletion Kernel/VM/RangeAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ void RangeAllocator::deallocate(Range range)

ASSERT(!m_available_ranges.is_empty());

int nearby_index = 0;
size_t nearby_index = 0;
auto* existing_range = binary_search(
m_available_ranges.span(), range, [](auto& a, auto& b) {
return a.base().get() - b.end().get();
Expand Down
2 changes: 1 addition & 1 deletion Libraries/LibLine/XtermSuggestionDisplay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ bool XtermSuggestionDisplay::cleanup()
size_t XtermSuggestionDisplay::fit_to_page_boundary(size_t selection_index)
{
ASSERT(m_pages.size() > 0);
int index = 0;
size_t index = 0;

auto* match = binary_search(
m_pages.span(), { selection_index, selection_index }, [](auto& a, auto& b) -> int {
Expand Down

0 comments on commit 615ba0f

Please sign in to comment.