[libcxx-commits] [PATCH] D147089: [libc++] Add assertions for potential OOB reads in std::sort

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

Thanks for working on this. Does it resolve https://github.com/llvm/llvm-project/issues/61663 ?

I like to see the CI green before approving, the issues seem to be introduced by this patch.



================
Comment at: libcxx/include/__algorithm/sort.h:298
         __j = __k;
+        _LIBCPP_ASSERT(__j != __leftmost, "Would read out of bounds, is your comparator a valid strict-weak ordering?");
       } while (__comp(__t, *--__k)); // No need for bounds check due to the assumption stated above.
----------------
I think the current message looks weird.


================
Comment at: libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp:28
+// If this is not satisfied, we have seen issues in the past where the std::sort implementation
+// would proceed to do OOB reads.
+
----------------



================
Comment at: libcxx/test/libcxx/algorithms/alg.sorting/assert.sort.invalid_comparator.pass.cpp:43
+#   include "bad_comparator_values.dat"
+;
+
----------------
Since we only do this in C++20 and later we could use a `inline constexpr std::string_view` in a header file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147089/new/

https://reviews.llvm.org/D147089