XLinfini/sousa-gecko
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 2070249: Prevent unsigned values in binary search. r=mfbt-reviewers,glandium

Browse Source 
It is relatively easy for unsigned values to sneak into a binary search,
in which case `result < 0` will always return false. Statically
asserting that the result is not unsigned prevents this.

Technically this condition is a little bit loose, in that things like
`bool` still slip through. There are probably some nasty ways of getting
a very precise type assertion using concepts or something, but unsigned
seems like the biggest and most obvious footgun.

Differential Revision: https://phabricator.services.mozilla.com/D324362
This commit is contained in:
Ben Visness
2026-09-09 16:29:37 +00:00
committed by bvisness@mozilla.com
parent 8c2245fa4a
commit f16e730a7e
1 changed files with 13 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
mfbt/BinarySearch.h
+13 -3
View File
@@ -6,6 +6,7 @@
#define mozilla_BinarySearch_h
#include <cstddef>
#include <type_traits>
#include <utility>
#include "mozilla/Assertions.h"
@@ -32,8 +33,8 @@ namespace mozilla {
*
* The BinarySearchIf() version behaves similarly, but takes |aComparator|, a
* functor to compare the values with, instead of a value to find.
* That functor should take one argument - the value to compare - and return an
* |int| with the comparison result:
* That functor should take one argument - the value to compare - and return a
* signed integer with the comparison result:
*
* * 0, if the argument is equal to,
* * less than 0, if the argument is greater than,
@@ -57,7 +58,8 @@ namespace mozilla {
*
* size_t match;
* if (BinarySearchIf(sortedInts, 0, sortedInts.length(), Comparator(13),
* &match)) { printf("found 13 at %lu\n", match);
* &match)) {
* printf("found 13 at %lu\n", match);
* }
*
*/
@@ -76,6 +78,8 @@ bool BinarySearchIf(const Container& aContainer, size_t aBegin, size_t aEnd,
// Allow any intermediate type so long as it provides a suitable ordering
// relation.
const auto result = aCompare(aContainer[middle]);
static_assert(!std::is_unsigned_v<decltype(result)>,
"comparator result must allow negative values");
if (result == 0) {
*aMatchOrInsertionPoint = middle;
@@ -174,6 +178,8 @@ size_t LowerBound(const Container& aContainer, size_t aBegin, size_t aEnd,
// Allow any intermediate type so long as it provides a suitable ordering
// relation.
const auto result = aCompare(aContainer[middle]);
static_assert(!std::is_unsigned_v<decltype(result)>,
"comparator result must allow negative values");
// The range returning from LowerBound does include elements
// equivalent to the given value i.e. aCompare(element) == 0
@@ -200,6 +206,8 @@ size_t UpperBound(const Container& aContainer, size_t aBegin, size_t aEnd,
// Allow any intermediate type so long as it provides a suitable ordering
// relation.
const auto result = aCompare(aContainer[middle]);
static_assert(!std::is_unsigned_v<decltype(result)>,
"comparator result must allow negative values");
// The range returning from UpperBound does NOT include elements
// equivalent to the given value i.e. aCompare(element) == 0
@@ -226,6 +234,8 @@ std::pair<size_t, size_t> EqualRange(const Container& aContainer, size_t aBegin,
// Allow any intermediate type so long as it provides a suitable ordering
// relation.
const auto result = aCompare(aContainer[middle]);
static_assert(!std::is_unsigned_v<decltype(result)>,
"comparator result must allow negative values");
if (result < 0) {
high = middle;