diff --git a/libcxx/include/__algorithm/comp_ref_type.h b/libcxx/include/__algorithm/comp_ref_type.h index 15f4a535a30bf..aa9350c38caa9 100644 --- a/libcxx/include/__algorithm/comp_ref_type.h +++ b/libcxx/include/__algorithm/comp_ref_type.h @@ -44,7 +44,7 @@ struct __debug_less { _LIBCPP_CONSTEXPR_SINCE_CXX14 inline _LIBCPP_HIDE_FROM_ABI decltype((void)std::declval<_Compare&>()( std::declval<_LHS&>(), std::declval<_RHS&>())) __do_compare_assert(int, _LHS& __l, _RHS& __r) { - _LIBCPP_ASSERT_UNCATEGORIZED(!__comp_(__l, __r), "Comparator does not induce a strict weak ordering"); + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(!__comp_(__l, __r), "Comparator does not induce a strict weak ordering"); (void)__l; (void)__r; } @@ -53,8 +53,7 @@ struct __debug_less { _LIBCPP_CONSTEXPR_SINCE_CXX14 inline _LIBCPP_HIDE_FROM_ABI void __do_compare_assert(long, _LHS&, _RHS&) {} }; -// Pass the comparator by lvalue reference. Or in debug mode, using a -// debugging wrapper that stores a reference. +// Pass the comparator by lvalue reference. Or in the debug mode, using a debugging wrapper that stores a reference. #if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG template using __comp_ref_type = __debug_less<_Comp>; diff --git a/libcxx/include/__algorithm/nth_element.h b/libcxx/include/__algorithm/nth_element.h index a059705125951..37ddfbdacf044 100644 --- a/libcxx/include/__algorithm/nth_element.h +++ b/libcxx/include/__algorithm/nth_element.h @@ -114,12 +114,12 @@ __nth_element( while (true) { while (!__comp(*__first, *__i)) { ++__i; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __i != __last, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __j != __first, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__j; @@ -150,13 +150,13 @@ __nth_element( // __m still guards upward moving __i while (__comp(*__i, *__m)) { ++__i; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __i != __last, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } // It is now known that a guard exists for downward moving __j do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __j != __first, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__j; diff --git a/libcxx/include/__algorithm/sort.h b/libcxx/include/__algorithm/sort.h index ac47489af0aac..451133a2d193d 100644 --- a/libcxx/include/__algorithm/sort.h +++ b/libcxx/include/__algorithm/sort.h @@ -325,7 +325,7 @@ __insertion_sort_unguarded(_RandomAccessIterator const __first, _RandomAccessIte do { *__j = _Ops::__iter_move(__k); __j = __k; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __k != __leftmost, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } while (__comp(__t, *--__k)); // No need for bounds check due to the assumption stated above. @@ -544,7 +544,7 @@ __bitset_partition(_RandomAccessIterator __first, _RandomAccessIterator __last, // Not guarded since we know the last element is greater than the pivot. do { ++__first; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __first != __end, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } while (!__comp(__pivot, *__first)); @@ -557,7 +557,7 @@ __bitset_partition(_RandomAccessIterator __first, _RandomAccessIterator __last, // It will be always guarded because __introsort will do the median-of-three // before calling this. do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __last != __begin, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__last; @@ -635,7 +635,7 @@ __partition_with_equals_on_right(_RandomAccessIterator __first, _RandomAccessIte // this. do { ++__first; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __first != __end, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } while (__comp(*__first, __pivot)); @@ -647,7 +647,7 @@ __partition_with_equals_on_right(_RandomAccessIterator __first, _RandomAccessIte } else { // Guarded. do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __last != __begin, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__last; @@ -665,12 +665,12 @@ __partition_with_equals_on_right(_RandomAccessIterator __first, _RandomAccessIte _Ops::iter_swap(__first, __last); do { ++__first; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __first != __end, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } while (__comp(*__first, __pivot)); do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __last != __begin, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__last; @@ -702,7 +702,7 @@ __partition_with_equals_on_left(_RandomAccessIterator __first, _RandomAccessIter // Guarded. do { ++__first; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __first != __end, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } while (!__comp(__pivot, *__first)); @@ -715,7 +715,7 @@ __partition_with_equals_on_left(_RandomAccessIterator __first, _RandomAccessIter // It will be always guarded because __introsort will do the // median-of-three before calling this. do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __last != __begin, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__last; @@ -725,12 +725,12 @@ __partition_with_equals_on_left(_RandomAccessIterator __first, _RandomAccessIter _Ops::iter_swap(__first, __last); do { ++__first; - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __first != __end, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); } while (!__comp(__pivot, *__first)); do { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_VALID_ELEMENT_ACCESS( __last != __begin, "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); --__last; diff --git a/libcxx/include/__algorithm/three_way_comp_ref_type.h b/libcxx/include/__algorithm/three_way_comp_ref_type.h index 8fd15c5d66f2f..70c5818976f07 100644 --- a/libcxx/include/__algorithm/three_way_comp_ref_type.h +++ b/libcxx/include/__algorithm/three_way_comp_ref_type.h @@ -50,14 +50,14 @@ struct __debug_three_way_comp { __expected = _Order::greater; if (__o == _Order::greater) __expected = _Order::less; - _LIBCPP_ASSERT_UNCATEGORIZED(__comp_(__l, __r) == __expected, "Comparator does not induce a strict weak ordering"); + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT( + __comp_(__l, __r) == __expected, "Comparator does not induce a strict weak ordering"); (void)__l; (void)__r; } }; -// Pass the comparator by lvalue reference. Or in debug mode, using a -// debugging wrapper that stores a reference. +// Pass the comparator by lvalue reference. Or in the debug mode, using a debugging wrapper that stores a reference. # if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG template using __three_way_comp_ref_type = __debug_three_way_comp<_Comp>; diff --git a/libcxx/include/__config b/libcxx/include/__config index f0275d0256d27..eab37e7576bdc 100644 --- a/libcxx/include/__config +++ b/libcxx/include/__config @@ -313,6 +313,10 @@ // - `_LIBCPP_ASSERT_PEDANTIC` -- checks prerequisites which are imposed by the Standard, but violating which happens to // be benign in our implementation. // +// - `_LIBCPP_ASSERT_SEMANTIC_REQUIREMENT` -- checks that the given argument satisfies the semantic requirements imposed +// by the Standard. Typically, there is no simple way to completely prove that a semantic requirement is satisfied; +// thus, this would often be a heuristic check and it might be quite expensive. +// // - `_LIBCPP_ASSERT_INTERNAL` -- checks that internal invariants of the library hold. These assertions don't depend on // user input. // @@ -359,6 +363,7 @@ _LIBCPP_HARDENING_MODE_DEBUG # define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression) +# define _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression) @@ -378,6 +383,7 @@ _LIBCPP_HARDENING_MODE_DEBUG # define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message) # define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message) // Disabled checks. +# define _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression) // Debug hardening mode checks. @@ -394,6 +400,7 @@ _LIBCPP_HARDENING_MODE_DEBUG # define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSERT(expression, message) # define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSERT(expression, message) # define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSERT(expression, message) +# define _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(expression, message) _LIBCPP_ASSERT(expression, message) # define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSERT(expression, message) # define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSERT(expression, message) @@ -411,6 +418,7 @@ _LIBCPP_HARDENING_MODE_DEBUG # define _LIBCPP_ASSERT_COMPATIBLE_ALLOCATOR(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_ARGUMENT_WITHIN_DOMAIN(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_PEDANTIC(expression, message) _LIBCPP_ASSUME(expression) +# define _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_INTERNAL(expression, message) _LIBCPP_ASSUME(expression) # define _LIBCPP_ASSERT_UNCATEGORIZED(expression, message) _LIBCPP_ASSUME(expression) diff --git a/libcxx/include/__debug_utils/strict_weak_ordering_check.h b/libcxx/include/__debug_utils/strict_weak_ordering_check.h index 99200ad65eac9..3a9d887284164 100644 --- a/libcxx/include/__debug_utils/strict_weak_ordering_check.h +++ b/libcxx/include/__debug_utils/strict_weak_ordering_check.h @@ -26,12 +26,12 @@ _LIBCPP_BEGIN_NAMESPACE_STD template _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX14 void __check_strict_weak_ordering_sorted(_RandomAccessIterator __first, _RandomAccessIterator __last, _Comp& __comp) { -#ifdef _LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK +#if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG using __diff_t = __iter_diff_t<_RandomAccessIterator>; using _Comp_ref = __comp_ref_type<_Comp>; if (!__libcpp_is_constant_evaluated()) { // Check if the range is actually sorted. - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT( (std::is_sorted<_RandomAccessIterator, _Comp_ref>(__first, __last, _Comp_ref(__comp))), "The range is not sorted after the sort, your comparator is not a valid strict-weak ordering"); // Limit the number of elements we need to check. @@ -46,18 +46,18 @@ __check_strict_weak_ordering_sorted(_RandomAccessIterator __first, _RandomAccess // Check that the elements from __p to __q are equal between each other. for (__diff_t __b = __p; __b < __q; ++__b) { for (__diff_t __a = __p; __a <= __b; ++__a) { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT( !__comp(*(__first + __a), *(__first + __b)), "Your comparator is not a valid strict-weak ordering"); - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT( !__comp(*(__first + __b), *(__first + __a)), "Your comparator is not a valid strict-weak ordering"); } } // Check that elements between __p and __q are less than between __q and __size. for (__diff_t __a = __p; __a < __q; ++__a) { for (__diff_t __b = __q; __b < __size; ++__b) { - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT( __comp(*(__first + __a), *(__first + __b)), "Your comparator is not a valid strict-weak ordering"); - _LIBCPP_ASSERT_UNCATEGORIZED( + _LIBCPP_ASSERT_SEMANTIC_REQUIREMENT( !__comp(*(__first + __b), *(__first + __a)), "Your comparator is not a valid strict-weak ordering"); } } @@ -69,7 +69,7 @@ __check_strict_weak_ordering_sorted(_RandomAccessIterator __first, _RandomAccess (void)__first; (void)__last; (void)__comp; -#endif +#endif // _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG } _LIBCPP_END_NAMESPACE_STD diff --git a/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp deleted file mode 100644 index f90e2a3a1a71f..0000000000000 --- a/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp +++ /dev/null @@ -1,255 +0,0 @@ -//===----------------------------------------------------------------------===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -// REQUIRES: stdlib=libc++ - -// REQUIRES: has-unix-headers -// UNSUPPORTED: c++03, c++11, c++14, c++17 -// UNSUPPORTED: !libcpp-hardening-mode=debug -// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing -// ADDITIONAL_COMPILE_FLAGS: -D_LIBCPP_DEBUG_STRICT_WEAK_ORDERING_CHECK -// When the debug mode is enabled, this test fails because we actually catch on the fly that the comparator is not -// a strict-weak ordering before we catch that we'd dereference out-of-bounds inside std::sort, which leads to different -// errors than the ones tested below. -// XFAIL: libcpp-hardening-mode=debug - -// This test uses a specific combination of an invalid comparator and sequence of values to -// ensure that our sorting functions do not go out-of-bounds and satisfy strict weak ordering in that case. -// Instead, we should fail loud with an assertion. The specific issue we're looking for here is when the comparator -// does not satisfy the strict weak ordering: -// -// Irreflexivity: comp(a, a) is false -// Antisymmetry: comp(a, b) implies that !comp(b, a) -// Transitivity: comp(a, b), comp(b, c) imply comp(a, c) -// Transitivity of equivalence: !comp(a, b), !comp(b, a), !comp(b, c), !comp(c, b) imply !comp(a, c), !comp(c, a) -// -// If this is not satisfied, we have seen issues in the past where the std::sort implementation -// would proceed to do OOB reads. (rdar://106897934). -// Other algorithms like std::stable_sort, std::sort_heap do not go out of bounds but can produce -// incorrect results, we also want to assert on that. -// Sometimes std::sort does not go out of bounds as well, for example, right now if transitivity -// of equivalence is not met, std::sort can only produce incorrect result but would not fail. - -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "bad_comparator_values.h" -#include "check_assertion.h" - -class ComparisonResults { -public: - explicit ComparisonResults(std::string_view data) { - for (auto line : std::views::split(data, '\n') | std::views::filter([](auto const& line) { return !line.empty(); })) { - auto values = std::views::split(line, ' '); - auto it = values.begin(); - std::size_t left = std::stol(std::string((*it).data(), (*it).size())); - it = std::next(it); - std::size_t right = std::stol(std::string((*it).data(), (*it).size())); - it = std::next(it); - bool result = static_cast(std::stol(std::string((*it).data(), (*it).size()))); - comparison_results[left][right] = result; - } - } - - bool compare(size_t* left, size_t* right) const { - assert(left != nullptr && right != nullptr && "something is wrong with the test"); - assert(comparison_results.contains(*left) && comparison_results.at(*left).contains(*right) && "malformed input data?"); - return comparison_results.at(*left).at(*right); - } - - size_t size() const { return comparison_results.size(); } -private: - std::map> comparison_results; // terrible for performance, but really convenient -}; - -void check_oob_sort_read() { - ComparisonResults comparison_results(SORT_DATA); - std::vector> elements; - std::set valid_ptrs; - for (std::size_t i = 0; i != comparison_results.size(); ++i) { - elements.push_back(std::make_unique(i)); - valid_ptrs.insert(elements.back().get()); - } - - auto checked_predicate = [&](size_t* left, size_t* right) { - // If the pointers passed to the comparator are not in the set of pointers we - // set up above, then we're being passed garbage values from the algorithm - // because we're reading OOB. - assert(valid_ptrs.contains(left)); - assert(valid_ptrs.contains(right)); - return comparison_results.compare(left, right); - }; - - // Check the classic sorting algorithms - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::sort(copy.begin(), copy.end(), checked_predicate), "Would read out of bounds"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::stable_sort(copy.begin(), copy.end(), checked_predicate), "not a valid strict-weak ordering"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - std::make_heap(copy.begin(), copy.end(), checked_predicate); // doesn't go OOB even with invalid comparator - TEST_LIBCPP_ASSERT_FAILURE(std::sort_heap(copy.begin(), copy.end(), checked_predicate), "not a valid strict-weak ordering"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::partial_sort(copy.begin(), copy.end(), copy.end(), checked_predicate), "not a valid strict-weak ordering"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - std::vector results(copy.size(), nullptr); - TEST_LIBCPP_ASSERT_FAILURE(std::partial_sort_copy(copy.begin(), copy.end(), results.begin(), results.end(), checked_predicate), "not a valid strict-weak ordering"); - } - - // Check the Ranges sorting algorithms - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::sort(copy, checked_predicate), "Would read out of bounds"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::stable_sort(copy, checked_predicate), "not a valid strict-weak ordering"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - std::ranges::make_heap(copy, checked_predicate); // doesn't go OOB even with invalid comparator - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::sort_heap(copy, checked_predicate), "not a valid strict-weak ordering"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::partial_sort(copy, copy.end(), checked_predicate), "not a valid strict-weak ordering"); - } - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - std::vector results(copy.size(), nullptr); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::partial_sort_copy(copy, results, checked_predicate), "not a valid strict-weak ordering"); - } -} - -void check_oob_nth_element_read() { - ComparisonResults results(NTH_ELEMENT_DATA); - std::vector> elements; - std::set valid_ptrs; - for (std::size_t i = 0; i != results.size(); ++i) { - elements.push_back(std::make_unique(i)); - valid_ptrs.insert(elements.back().get()); - } - - auto checked_predicate = [&](size_t* left, size_t* right) { - // If the pointers passed to the comparator are not in the set of pointers we - // set up above, then we're being passed garbage values from the algorithm - // because we're reading OOB. - assert(valid_ptrs.contains(left)); - assert(valid_ptrs.contains(right)); - return results.compare(left, right); - }; - - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::nth_element(copy.begin(), copy.begin(), copy.end(), checked_predicate), "Would read out of bounds"); - } - - { - std::vector copy; - for (auto const& e : elements) - copy.push_back(e.get()); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::nth_element(copy, copy.begin(), checked_predicate), "Would read out of bounds"); - } -} - -struct FloatContainer { - float value; - bool operator<(const FloatContainer& other) const { - return value < other.value; - } -}; - -// Nans in floats do not satisfy strict weak ordering by breaking transitivity of equivalence. -std::vector generate_float_data() { - std::vector floats(50); - for (int i = 0; i < 50; ++i) { - floats[i].value = static_cast(i); - } - floats.push_back(FloatContainer{std::numeric_limits::quiet_NaN()}); - std::shuffle(floats.begin(), floats.end(), std::default_random_engine()); - return floats; -} - -void check_nan_floats() { - auto floats = generate_float_data(); - TEST_LIBCPP_ASSERT_FAILURE(std::sort(floats.begin(), floats.end()), "not a valid strict-weak ordering"); - floats = generate_float_data(); - TEST_LIBCPP_ASSERT_FAILURE(std::stable_sort(floats.begin(), floats.end()), "not a valid strict-weak ordering"); - floats = generate_float_data(); - std::make_heap(floats.begin(), floats.end()); - TEST_LIBCPP_ASSERT_FAILURE(std::sort_heap(floats.begin(), floats.end()), "not a valid strict-weak ordering"); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::sort(generate_float_data(), std::less()), "not a valid strict-weak ordering"); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::stable_sort(generate_float_data(), std::less()), "not a valid strict-weak ordering"); - floats = generate_float_data(); - std::ranges::make_heap(floats, std::less()); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::sort_heap(floats, std::less()), "not a valid strict-weak ordering"); -} - -void check_irreflexive() { - std::vector v(1); - TEST_LIBCPP_ASSERT_FAILURE(std::sort(v.begin(), v.end(), std::greater_equal()), "not a valid strict-weak ordering"); - TEST_LIBCPP_ASSERT_FAILURE(std::stable_sort(v.begin(), v.end(), std::greater_equal()), "not a valid strict-weak ordering"); - std::make_heap(v.begin(), v.end(), std::greater_equal()); - TEST_LIBCPP_ASSERT_FAILURE(std::sort_heap(v.begin(), v.end(), std::greater_equal()), "not a valid strict-weak ordering"); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::sort(v, std::greater_equal()), "not a valid strict-weak ordering"); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::stable_sort(v, std::greater_equal()), "not a valid strict-weak ordering"); - std::ranges::make_heap(v, std::greater_equal()); - TEST_LIBCPP_ASSERT_FAILURE(std::ranges::sort_heap(v, std::greater_equal()), "not a valid strict-weak ordering"); -} - -int main(int, char**) { - - check_oob_sort_read(); - - check_oob_nth_element_read(); - - check_nan_floats(); - - check_irreflexive(); - - return 0; -} diff --git a/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/assert.sort.invalid_comparator.oob.pass.cpp b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/assert.sort.invalid_comparator.oob.pass.cpp new file mode 100644 index 0000000000000..6ddee1b2aabe0 --- /dev/null +++ b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/assert.sort.invalid_comparator.oob.pass.cpp @@ -0,0 +1,72 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// REQUIRES: has-unix-headers +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// In the debug mode, the comparator validations will notice that it doesn't satisfy strict weak ordering before the +// algorithm actually runs and goes out of bounds, so the test will terminate before the tested assertions are +// triggered. +// UNSUPPORTED: libcpp-hardening-mode=none, libcpp-hardening-mode=debug +// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing + +#include +#include +#include +#include +#include +#include +#include + +#include "bad_comparator_values.h" +#include "check_assertion.h" +#include "invalid_comparator_utilities.h" + +void check_oob_sort_read() { + SortingFixture fixture(SORT_DATA); + + // Check the classic sorting algorithms + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::sort(copy.begin(), copy.end(), fixture.checked_predicate()), + "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); + } + + // Check the Ranges sorting algorithms + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort(copy, fixture.checked_predicate()), + "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); + } +} + +void check_oob_nth_element_read() { + SortingFixture fixture(NTH_ELEMENT_DATA); + + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::nth_element(copy.begin(), copy.begin(), copy.end(), fixture.checked_predicate()), + "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); + } + + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::nth_element(copy, copy.begin(), fixture.checked_predicate()), + "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); + } +} + +int main(int, char**) { + check_oob_sort_read(); + check_oob_nth_element_read(); + + return 0; +} diff --git a/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/assert.sort.invalid_comparator.pass.cpp b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/assert.sort.invalid_comparator.pass.cpp new file mode 100644 index 0000000000000..92671617eb032 --- /dev/null +++ b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/assert.sort.invalid_comparator.pass.cpp @@ -0,0 +1,195 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +// REQUIRES: has-unix-headers +// UNSUPPORTED: c++03, c++11, c++14, c++17 +// REQUIRES: libcpp-hardening-mode=debug +// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing + +// This test uses a specific combination of an invalid comparator and sequence of values to +// ensure that our sorting functions do not go out-of-bounds and satisfy strict weak ordering in that case. +// Instead, we should fail loud with an assertion. The specific issue we're looking for here is when the comparator +// does not satisfy the strict weak ordering: +// +// Irreflexivity: comp(a, a) is false +// Antisymmetry: comp(a, b) implies that !comp(b, a) +// Transitivity: comp(a, b), comp(b, c) imply comp(a, c) +// Transitivity of equivalence: !comp(a, b), !comp(b, a), !comp(b, c), !comp(c, b) imply !comp(a, c), !comp(c, a) +// +// If this is not satisfied, we have seen issues in the past where the std::sort implementation +// would proceed to do OOB reads. (rdar://106897934). +// Other algorithms like std::stable_sort, std::sort_heap do not go out of bounds but can produce +// incorrect results, we also want to assert on that. +// Sometimes std::sort does not go out of bounds as well, for example, right now if transitivity +// of equivalence is not met, std::sort can only produce incorrect result but would not fail. + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "bad_comparator_values.h" +#include "check_assertion.h" +#include "invalid_comparator_utilities.h" + +void check_oob_sort_read() { + SortingFixture fixture(SORT_DATA); + + // Check the classic sorting algorithms + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::sort(copy.begin(), copy.end(), fixture.checked_predicate()), + "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); + } + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::stable_sort(copy.begin(), copy.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::make_heap(copy.begin(), copy.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::sort_heap(copy.begin(), copy.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::partial_sort(copy.begin(), copy.end(), copy.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + { + std::vector copy = fixture.create_elements(); + std::vector results(copy.size(), nullptr); + TEST_LIBCPP_ASSERT_FAILURE( + std::partial_sort_copy(copy.begin(), copy.end(), results.begin(), results.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + + // Check the Ranges sorting algorithms + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort(copy, fixture.checked_predicate()), + "Would read out of bounds, does your comparator satisfy the strict-weak ordering requirement?"); + } + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::ranges::stable_sort(copy, fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::make_heap(copy, fixture.checked_predicate()), "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort_heap(copy, fixture.checked_predicate()), "Comparator does not induce a strict weak ordering"); + } + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::ranges::partial_sort(copy, copy.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + { + std::vector copy = fixture.create_elements(); + std::vector results(copy.size(), nullptr); + TEST_LIBCPP_ASSERT_FAILURE(std::ranges::partial_sort_copy(copy, results, fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } +} + +void check_oob_nth_element_read() { + SortingFixture fixture(NTH_ELEMENT_DATA); + + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::nth_element(copy.begin(), copy.begin(), copy.end(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } + + { + std::vector copy = fixture.create_elements(); + TEST_LIBCPP_ASSERT_FAILURE(std::ranges::nth_element(copy, copy.begin(), fixture.checked_predicate()), + "Comparator does not induce a strict weak ordering"); + } +} + +struct FloatContainer { + float value; + bool operator<(const FloatContainer& other) const { return value < other.value; } +}; + +// Nans in floats do not satisfy strict weak ordering by breaking transitivity of equivalence. +std::vector generate_float_data() { + std::vector floats(50); + for (int i = 0; i < 50; ++i) { + floats[i].value = static_cast(i); + } + floats.push_back(FloatContainer{std::numeric_limits::quiet_NaN()}); + std::shuffle(floats.begin(), floats.end(), std::default_random_engine()); + return floats; +} + +void check_nan_floats() { + auto floats = generate_float_data(); + TEST_LIBCPP_ASSERT_FAILURE( + std::sort(floats.begin(), floats.end()), "Your comparator is not a valid strict-weak ordering"); + floats = generate_float_data(); + TEST_LIBCPP_ASSERT_FAILURE( + std::stable_sort(floats.begin(), floats.end()), "Your comparator is not a valid strict-weak ordering"); + floats = generate_float_data(); + std::make_heap(floats.begin(), floats.end()); + TEST_LIBCPP_ASSERT_FAILURE( + std::sort_heap(floats.begin(), floats.end()), "Your comparator is not a valid strict-weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort(generate_float_data(), std::less()), "Your comparator is not a valid strict-weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::ranges::stable_sort(generate_float_data(), std::less()), + "Your comparator is not a valid strict-weak ordering"); + floats = generate_float_data(); + std::ranges::make_heap(floats, std::less()); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort_heap(floats, std::less()), "Your comparator is not a valid strict-weak ordering"); +} + +void check_irreflexive() { + std::vector v(1); + TEST_LIBCPP_ASSERT_FAILURE( + std::sort(v.begin(), v.end(), std::greater_equal()), "Your comparator is not a valid strict-weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE(std::stable_sort(v.begin(), v.end(), std::greater_equal()), + "Your comparator is not a valid strict-weak ordering"); + std::make_heap(v.begin(), v.end(), std::greater_equal()); + TEST_LIBCPP_ASSERT_FAILURE(std::sort_heap(v.begin(), v.end(), std::greater_equal()), + "Comparator does not induce a strict weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort(v, std::greater_equal()), "Your comparator is not a valid strict-weak ordering"); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::stable_sort(v, std::greater_equal()), "Your comparator is not a valid strict-weak ordering"); + std::ranges::make_heap(v, std::greater_equal()); + TEST_LIBCPP_ASSERT_FAILURE( + std::ranges::sort_heap(v, std::greater_equal()), "Comparator does not induce a strict weak ordering"); +} + +int main(int, char**) { + check_oob_sort_read(); + + check_oob_nth_element_read(); + + check_nan_floats(); + + check_irreflexive(); + + return 0; +} diff --git a/libcxx/test/libcxx/algorithms/alg.sorting/bad_comparator_values.h b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/bad_comparator_values.h similarity index 98% rename from libcxx/test/libcxx/algorithms/alg.sorting/bad_comparator_values.h rename to libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/bad_comparator_values.h index c0ffd16cd4ac4..1a5910cfcd937 100644 --- a/libcxx/test/libcxx/algorithms/alg.sorting/bad_comparator_values.h +++ b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/bad_comparator_values.h @@ -6,8 +6,8 @@ // //===----------------------------------------------------------------------===// -#ifndef TEST_LIBCXX_ALGORITHMS_ALG_SORTING_BAD_COMPARATOR_VALUES_H -#define TEST_LIBCXX_ALGORITHMS_ALG_SORTING_BAD_COMPARATOR_VALUES_H +#ifndef TEST_LIBCXX_ALGORITHMS_ALG_SORTING_ASSERT_SORT_INVALID_COMPARATOR_BAD_COMPARATOR_VALUES_H +#define TEST_LIBCXX_ALGORITHMS_ALG_SORTING_ASSERT_SORT_INVALID_COMPARATOR_BAD_COMPARATOR_VALUES_H #include @@ -3562,4 +3562,4 @@ inline constexpr std::string_view SORT_DATA = R"( 58 58 0 )"; -#endif // TEST_LIBCXX_ALGORITHMS_ALG_SORTING_BAD_COMPARATOR_VALUES_H +#endif // TEST_LIBCXX_ALGORITHMS_ALG_SORTING_ASSERT_SORT_INVALID_COMPARATOR_BAD_COMPARATOR_VALUES_H diff --git a/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/invalid_comparator_utilities.h b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/invalid_comparator_utilities.h new file mode 100644 index 0000000000000..f5e602577cae2 --- /dev/null +++ b/libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator/invalid_comparator_utilities.h @@ -0,0 +1,85 @@ +//===----------------------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef TEST_LIBCXX_ALGORITHMS_ALG_SORTING_ASSERT_SORT_INVALID_COMPARATOR_INVALID_COMPARATOR_UTILITIES_H +#define TEST_LIBCXX_ALGORITHMS_ALG_SORTING_ASSERT_SORT_INVALID_COMPARATOR_INVALID_COMPARATOR_UTILITIES_H + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +class ComparisonResults { +public: + explicit ComparisonResults(std::string_view data) { + for (auto line : + std::views::split(data, '\n') | std::views::filter([](auto const& line) { return !line.empty(); })) { + auto values = std::views::split(line, ' '); + auto it = values.begin(); + std::size_t left = std::stol(std::string((*it).data(), (*it).size())); + it = std::next(it); + std::size_t right = std::stol(std::string((*it).data(), (*it).size())); + it = std::next(it); + bool result = static_cast(std::stol(std::string((*it).data(), (*it).size()))); + comparison_results[left][right] = result; + } + } + + bool compare(size_t* left, size_t* right) const { + assert(left != nullptr && right != nullptr && "something is wrong with the test"); + assert(comparison_results.contains(*left) && comparison_results.at(*left).contains(*right) && + "malformed input data?"); + return comparison_results.at(*left).at(*right); + } + + size_t size() const { return comparison_results.size(); } + +private: + std::map> + comparison_results; // terrible for performance, but really convenient +}; + +class SortingFixture { +public: + explicit SortingFixture(std::string_view data) : comparison_results_(data) { + for (std::size_t i = 0; i != comparison_results_.size(); ++i) { + elements_.push_back(std::make_unique(i)); + valid_ptrs_.insert(elements_.back().get()); + } + } + + std::vector create_elements() { + std::vector copy; + for (auto const& e : elements_) + copy.push_back(e.get()); + return copy; + } + + auto checked_predicate() { + return [this](size_t* left, size_t* right) { + // If the pointers passed to the comparator are not in the set of pointers we + // set up above, then we're being passed garbage values from the algorithm + // because we're reading OOB. + assert(valid_ptrs_.contains(left)); + assert(valid_ptrs_.contains(right)); + return comparison_results_.compare(left, right); + }; + } + +private: + ComparisonResults comparison_results_; + std::vector> elements_; + std::set valid_ptrs_; +}; + +#endif // TEST_LIBCXX_ALGORITHMS_ALG_SORTING_ASSERT_SORT_INVALID_COMPARATOR_INVALID_COMPARATOR_UTILITIES_H