Adds bounds-checking to the second range of absl container algorithms (#811)
The APIs for the two-range `absl::c_mismatch`, `absl::c_swap_ranges`,
and `absl::c_transform` are misleading as they do not check the bounds
of the second range against the first one.
This commit cleans up ensures that buggy calls are not exploitable;
non-buggy calls are unaffected.
This is consistent with both C++14's two-range `std::` equivalents and
C++20's `std::ranges::` equivalents.
http://wg21.link/mismatch
http://wg21.link/alg.swap
http://wg21.link/alg.transform
diff --git a/absl/algorithm/container.h b/absl/algorithm/container.h
index d72532d..a2f936c 100644
--- a/absl/algorithm/container.h
+++ b/absl/algorithm/container.h
@@ -340,24 +340,45 @@
// c_mismatch()
//
// Container-based version of the <algorithm> `std::mismatch()` function to
-// return the first element where two ordered containers differ.
+// return the first element where two ordered containers differ. Applies `==` to
+// the first N elements of `c1` and `c2`, where N = min(size(c1), size(c2)).
template <typename C1, typename C2>
container_algorithm_internal::ContainerIterPairType<C1, C2>
c_mismatch(C1& c1, C2& c2) {
- return std::mismatch(container_algorithm_internal::c_begin(c1),
- container_algorithm_internal::c_end(c1),
- container_algorithm_internal::c_begin(c2));
+ auto first1 = container_algorithm_internal::c_begin(c1);
+ auto last1 = container_algorithm_internal::c_end(c1);
+ auto first2 = container_algorithm_internal::c_begin(c2);
+ auto last2 = container_algorithm_internal::c_end(c2);
+
+ for (; first1 != last1 && first2 != last2; ++first1, (void)++first2) {
+ // Negates equality because Cpp17EqualityComparable doesn't require clients
+ // to overload both `operator==` and `operator!=`.
+ if (!(*first1 == *first2)) {
+ break;
+ }
+ }
+
+ return std::make_pair(first1, first2);
}
// Overload of c_mismatch() for using a predicate evaluation other than `==` as
-// the function's test condition.
+// the function's test condition. Applies `pred`to the first N elements of `c1`
+// and `c2`, where N = min(size(c1), size(c2)).
template <typename C1, typename C2, typename BinaryPredicate>
container_algorithm_internal::ContainerIterPairType<C1, C2>
-c_mismatch(C1& c1, C2& c2, BinaryPredicate&& pred) {
- return std::mismatch(container_algorithm_internal::c_begin(c1),
- container_algorithm_internal::c_end(c1),
- container_algorithm_internal::c_begin(c2),
- std::forward<BinaryPredicate>(pred));
+c_mismatch(C1& c1, C2& c2, BinaryPredicate pred) {
+ auto first1 = container_algorithm_internal::c_begin(c1);
+ auto last1 = container_algorithm_internal::c_end(c1);
+ auto first2 = container_algorithm_internal::c_begin(c2);
+ auto last2 = container_algorithm_internal::c_end(c2);
+
+ for (; first1 != last1 && first2 != last2; ++first1, (void)++first2) {
+ if (!pred(*first1, *first2)) {
+ break;
+ }
+ }
+
+ return std::make_pair(first1, first2);
}
// c_equal()
@@ -539,12 +560,20 @@
// c_swap_ranges()
//
// Container-based version of the <algorithm> `std::swap_ranges()` function to
-// swap a container's elements with another container's elements.
+// swap a container's elements with another container's elements. Swaps the
+// first N elements of `c1` and `c2`, where N = min(size(c1), size(c2)).
template <typename C1, typename C2>
container_algorithm_internal::ContainerIter<C2> c_swap_ranges(C1& c1, C2& c2) {
- return std::swap_ranges(container_algorithm_internal::c_begin(c1),
- container_algorithm_internal::c_end(c1),
- container_algorithm_internal::c_begin(c2));
+ auto first1 = container_algorithm_internal::c_begin(c1);
+ auto last1 = container_algorithm_internal::c_end(c1);
+ auto first2 = container_algorithm_internal::c_begin(c2);
+ auto last2 = container_algorithm_internal::c_end(c2);
+
+ using std::swap;
+ for (; first1 != last1 && first2 != last2; ++first1, (void)++first2) {
+ swap(*first1, *first2);
+ }
+ return first2;
}
// c_transform()
@@ -562,16 +591,23 @@
}
// Overload of c_transform() for performing a transformation using a binary
-// predicate.
+// predicate. Applies `binary_op` to the first N elements of `c1` and `c2`,
+// where N = min(size(c1), size(c2)).
template <typename InputSequence1, typename InputSequence2,
typename OutputIterator, typename BinaryOp>
OutputIterator c_transform(const InputSequence1& input1,
const InputSequence2& input2, OutputIterator output,
BinaryOp&& binary_op) {
- return std::transform(container_algorithm_internal::c_begin(input1),
- container_algorithm_internal::c_end(input1),
- container_algorithm_internal::c_begin(input2), output,
- std::forward<BinaryOp>(binary_op));
+ auto first1 = container_algorithm_internal::c_begin(input1);
+ auto last1 = container_algorithm_internal::c_end(input1);
+ auto first2 = container_algorithm_internal::c_begin(input2);
+ auto last2 = container_algorithm_internal::c_end(input2);
+ for (; first1 != last1 && first2 != last2;
+ ++first1, (void)++first2, ++output) {
+ *output = binary_op(*first1, *first2);
+ }
+
+ return output;
}
// c_replace()
diff --git a/absl/algorithm/container_test.cc b/absl/algorithm/container_test.cc
index 0a4abe9..605afc8 100644
--- a/absl/algorithm/container_test.cc
+++ b/absl/algorithm/container_test.cc
@@ -57,9 +57,7 @@
};
struct AccumulateCalls {
- void operator()(int value) {
- calls.push_back(value);
- }
+ void operator()(int value) { calls.push_back(value); }
std::vector<int> calls;
};
@@ -68,7 +66,6 @@
bool Equals(int v1, int v2) { return v1 == v2; }
bool IsOdd(int x) { return x % 2 != 0; }
-
TEST_F(NonMutatingTest, Distance) {
EXPECT_EQ(container_.size(), absl::c_distance(container_));
EXPECT_EQ(sequence_.size(), absl::c_distance(sequence_));
@@ -151,13 +148,90 @@
}
TEST_F(NonMutatingTest, Mismatch) {
- absl::c_mismatch(container_, sequence_);
- absl::c_mismatch(sequence_, container_);
+ // Testing necessary as absl::c_mismatch executes logic.
+ {
+ auto result = absl::c_mismatch(vector_, sequence_);
+ EXPECT_EQ(result.first, vector_.end());
+ EXPECT_EQ(result.second, sequence_.end());
+ }
+ {
+ auto result = absl::c_mismatch(sequence_, vector_);
+ EXPECT_EQ(result.first, sequence_.end());
+ EXPECT_EQ(result.second, vector_.end());
+ }
+
+ sequence_.back() = 5;
+ {
+ auto result = absl::c_mismatch(vector_, sequence_);
+ EXPECT_EQ(result.first, std::prev(vector_.end()));
+ EXPECT_EQ(result.second, std::prev(sequence_.end()));
+ }
+ {
+ auto result = absl::c_mismatch(sequence_, vector_);
+ EXPECT_EQ(result.first, std::prev(sequence_.end()));
+ EXPECT_EQ(result.second, std::prev(vector_.end()));
+ }
+
+ sequence_.pop_back();
+ {
+ auto result = absl::c_mismatch(vector_, sequence_);
+ EXPECT_EQ(result.first, std::prev(vector_.end()));
+ EXPECT_EQ(result.second, sequence_.end());
+ }
+ {
+ auto result = absl::c_mismatch(sequence_, vector_);
+ EXPECT_EQ(result.first, sequence_.end());
+ EXPECT_EQ(result.second, std::prev(vector_.end()));
+ }
+ {
+ struct NoNotEquals {
+ constexpr bool operator==(NoNotEquals) const { return true; }
+ constexpr bool operator!=(NoNotEquals) const = delete;
+ };
+ std::vector<NoNotEquals> first;
+ std::list<NoNotEquals> second;
+
+ // Check this still compiles.
+ absl::c_mismatch(first, second);
+ }
}
TEST_F(NonMutatingTest, MismatchWithPredicate) {
- absl::c_mismatch(container_, sequence_, BinPredicate);
- absl::c_mismatch(sequence_, container_, BinPredicate);
+ // Testing necessary as absl::c_mismatch executes logic.
+ {
+ auto result = absl::c_mismatch(vector_, sequence_, BinPredicate);
+ EXPECT_EQ(result.first, vector_.begin());
+ EXPECT_EQ(result.second, sequence_.begin());
+ }
+ {
+ auto result = absl::c_mismatch(sequence_, vector_, BinPredicate);
+ EXPECT_EQ(result.first, sequence_.begin());
+ EXPECT_EQ(result.second, vector_.begin());
+ }
+
+ sequence_.front() = 0;
+ {
+ auto result = absl::c_mismatch(vector_, sequence_, BinPredicate);
+ EXPECT_EQ(result.first, vector_.begin());
+ EXPECT_EQ(result.second, sequence_.begin());
+ }
+ {
+ auto result = absl::c_mismatch(sequence_, vector_, BinPredicate);
+ EXPECT_EQ(result.first, std::next(sequence_.begin()));
+ EXPECT_EQ(result.second, std::next(vector_.begin()));
+ }
+
+ sequence_.clear();
+ {
+ auto result = absl::c_mismatch(vector_, sequence_, BinPredicate);
+ EXPECT_EQ(result.first, vector_.begin());
+ EXPECT_EQ(result.second, sequence_.end());
+ }
+ {
+ auto result = absl::c_mismatch(sequence_, vector_, BinPredicate);
+ EXPECT_EQ(result.first, sequence_.end());
+ EXPECT_EQ(result.second, vector_.begin());
+ }
}
TEST_F(NonMutatingTest, Equal) {
@@ -519,11 +593,9 @@
TEST_F(SortingTest, NthElement) {
std::vector<int> unsorted = {2, 4, 1, 3};
absl::c_nth_element(unsorted, unsorted.begin() + 2);
- EXPECT_THAT(unsorted,
- ElementsAre(Lt(3), Lt(3), 3, Gt(3)));
+ EXPECT_THAT(unsorted, ElementsAre(Lt(3), Lt(3), 3, Gt(3)));
absl::c_nth_element(unsorted, unsorted.begin() + 2, std::greater<int>());
- EXPECT_THAT(unsorted,
- ElementsAre(Gt(2), Gt(2), 2, Lt(2)));
+ EXPECT_THAT(unsorted, ElementsAre(Gt(2), Gt(2), 2, Lt(2)));
}
TEST(MutatingTest, IsPartitioned) {
@@ -676,6 +748,15 @@
absl::c_swap_ranges(odds, evens);
EXPECT_THAT(odds, ElementsAre(1, 3, 5));
EXPECT_THAT(evens, ElementsAre(2, 4, 6));
+
+ odds.pop_back();
+ absl::c_swap_ranges(odds, evens);
+ EXPECT_THAT(odds, ElementsAre(2, 4));
+ EXPECT_THAT(evens, ElementsAre(1, 3, 6));
+
+ absl::c_swap_ranges(evens, odds);
+ EXPECT_THAT(odds, ElementsAre(1, 3));
+ EXPECT_THAT(evens, ElementsAre(2, 4, 6));
}
TEST_F(NonMutatingTest, Transform) {
@@ -690,6 +771,20 @@
EXPECT_EQ(std::vector<int>({1, 5, 4}), z);
*end = 7;
EXPECT_EQ(std::vector<int>({1, 5, 4, 7}), z);
+
+ z.clear();
+ y.pop_back();
+ end = absl::c_transform(x, y, std::back_inserter(z), std::plus<int>());
+ EXPECT_EQ(std::vector<int>({1, 5}), z);
+ *end = 7;
+ EXPECT_EQ(std::vector<int>({1, 5, 7}), z);
+
+ z.clear();
+ std::swap(x, y);
+ end = absl::c_transform(x, y, std::back_inserter(z), std::plus<int>());
+ EXPECT_EQ(std::vector<int>({1, 5}), z);
+ *end = 7;
+ EXPECT_EQ(std::vector<int>({1, 5, 7}), z);
}
TEST(MutatingTest, Replace) {
@@ -755,10 +850,9 @@
TEST(MutatingTest, StableSort) {
std::vector<Element> test_vector = {{1, 1}, {2, 1}, {2, 0}, {1, 0}, {2, 2}};
absl::c_stable_sort(test_vector);
- EXPECT_THAT(
- test_vector,
- ElementsAre(IsElement(1, 1), IsElement(1, 0), IsElement(2, 1),
- IsElement(2, 0), IsElement(2, 2)));
+ EXPECT_THAT(test_vector,
+ ElementsAre(IsElement(1, 1), IsElement(1, 0), IsElement(2, 1),
+ IsElement(2, 0), IsElement(2, 2)));
}
TEST(MutatingTest, StableSortWithPredicate) {
@@ -766,10 +860,9 @@
absl::c_stable_sort(test_vector, [](const Element& e1, const Element& e2) {
return e2 < e1;
});
- EXPECT_THAT(
- test_vector,
- ElementsAre(IsElement(2, 1), IsElement(2, 0), IsElement(2, 2),
- IsElement(1, 1), IsElement(1, 0)));
+ EXPECT_THAT(test_vector,
+ ElementsAre(IsElement(2, 1), IsElement(2, 0), IsElement(2, 2),
+ IsElement(1, 1), IsElement(1, 0)));
}
TEST(MutatingTest, ReplaceCopyIf) {
