AK: Fix overflow and mixed-signedness issues in binary_search() (#2961)

4 files changed, 57 insertions, 8 deletions

diff --git a/AK/BinarySearch.h b/AK/BinarySearch.h
index 81adefb82e..be16f57f7e 100644
--- a/AK/BinarySearch.h
+++ b/AK/BinarySearch.h
@@ -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 {
@@ -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;
 }
diff --git a/AK/Tests/TestBinarySearch.cpp b/AK/Tests/TestBinarySearch.cpp
index 6308833715..a0800b4f61 100644
--- a/AK/Tests/TestBinarySearch.cpp
+++ b/AK/Tests/TestBinarySearch.cpp
@@ -27,7 +27,9 @@
 #include <AK/TestSuite.h>
 
 #include <AK/BinarySearch.h>
+#include <AK/Span.h>
 #include <cstring>
+#include <new>
 
 TEST_CASE(vector_ints)
 {
@@ -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)
diff --git a/Kernel/VM/RangeAllocator.cpp b/Kernel/VM/RangeAllocator.cpp
index 0ea151d07c..3f79f46db8 100644
--- a/Kernel/VM/RangeAllocator.cpp
+++ b/Kernel/VM/RangeAllocator.cpp
@@ -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();
diff --git a/Libraries/LibLine/XtermSuggestionDisplay.cpp b/Libraries/LibLine/XtermSuggestionDisplay.cpp
index 9a56bd784b..4fac92b56f 100644
--- a/Libraries/LibLine/XtermSuggestionDisplay.cpp
+++ b/Libraries/LibLine/XtermSuggestionDisplay.cpp
@@ -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 {